From 92d98f8bd5c2986c0976f801627b5e212cebb03c Mon Sep 17 00:00:00 2001 From: Paul Hollinsky Date: Tue, 30 Oct 2018 15:02:01 -0400 Subject: [PATCH] Remove all debugging printouts to stdout --- api/icsneocpp/error.cpp | 30 +++++++++++++++++++ communication/multichannelcommunication.cpp | 3 +- communication/packetizer.cpp | 2 +- include/icsneo/api/error.h | 8 +++++ .../message/filter/main51messagefilter.h | 9 +----- include/icsneo/device/device.h | 2 +- include/icsneo/platform/posix/ftdi.h | 4 ++- include/icsneo/platform/posix/stm32.h | 4 ++- include/icsneo/platform/windows/ftdi.h | 2 +- include/icsneo/platform/windows/pcap.h | 3 +- include/icsneo/platform/windows/stm32.h | 2 +- include/icsneo/platform/windows/vcp.h | 4 ++- platform/posix/ftdi.cpp | 2 +- platform/posix/stm32.cpp | 5 ++-- platform/windows/pcap.cpp | 16 +++++----- platform/windows/vcp.cpp | 29 +++++++----------- 16 files changed, 79 insertions(+), 46 deletions(-) diff --git a/api/icsneocpp/error.cpp b/api/icsneocpp/error.cpp index c9584bf..cb870f2 100644 --- a/api/icsneocpp/error.cpp +++ b/api/icsneocpp/error.cpp @@ -58,6 +58,14 @@ static constexpr const char* ERROR_SETTINGS_LENGTH = "The settings length is inc static constexpr const char* ERROR_SETTINGS_CHECKSUM = "The settings checksum is incorrect, attempting to set defaults may remedy this issue."; static constexpr const char* ERROR_SETTINGS_NOT_AVAILABLE = "Settings are not available for this device."; +// Transport Errors +static constexpr const char* ERROR_FAILED_TO_READ = "A read operation failed."; +static constexpr const char* ERROR_FAILED_TO_WRITE = "A write operation failed."; +static constexpr const char* ERROR_DRIVER_FAILED_TO_OPEN = "The device driver encountered a low-level error while opening the device."; +static constexpr const char* ERROR_PACKET_CHECKSUM_ERROR = "There was a checksum error while decoding a packet. The packet was dropped."; +static constexpr const char* ERROR_PCAP_COULD_NOT_START = "The PCAP driver could not be started. Ethernet devices will not be found."; +static constexpr const char* ERROR_PCAP_COULD_NOT_FIND_DEVICES = "The PCAP driver failed to find devices. Ethernet devices will not be found."; + static constexpr const char* ERROR_TOO_MANY_ERRORS = "Too many errors have occurred. The list has been truncated."; static constexpr const char* ERROR_UNKNOWN = "An unknown internal error occurred."; static constexpr const char* ERROR_INVALID = "An invalid internal error occurred."; @@ -92,6 +100,20 @@ const char* APIError::DescriptionForType(ErrorType type) { return ERROR_SETTINGS_CHECKSUM; case SettingsNotAvailable: return ERROR_SETTINGS_NOT_AVAILABLE; + + // Transport Errors + case FailedToRead: + return ERROR_FAILED_TO_READ; + case FailedToWrite: + return ERROR_FAILED_TO_WRITE; + case DriverFailedToOpen: + return ERROR_DRIVER_FAILED_TO_OPEN; + case PacketChecksumError: + return ERROR_PACKET_CHECKSUM_ERROR; + case PCAPCouldNotStart: + return ERROR_PCAP_COULD_NOT_START; + case PCAPCouldNotFindDevices: + return ERROR_PCAP_COULD_NOT_FIND_DEVICES; // Other Errors case TooManyErrors: @@ -109,6 +131,9 @@ APIError::Severity APIError::SeverityForType(ErrorType type) { case OutputTruncated: // Device Warnings case PollingMessageOverflow: + // Transport Warnings + case PCAPCouldNotStart: + case PCAPCouldNotFindDevices: return Severity::Warning; // API Errors @@ -124,6 +149,11 @@ APIError::Severity APIError::SeverityForType(ErrorType type) { case SettingsLengthError: case SettingsChecksumError: case SettingsNotAvailable: + // Transport Errors + case FailedToRead: + case FailedToWrite: + case DriverFailedToOpen: + case PacketChecksumError: // Other Errors case TooManyErrors: case Unknown: diff --git a/communication/multichannelcommunication.cpp b/communication/multichannelcommunication.cpp index c159dae..276880d 100644 --- a/communication/multichannelcommunication.cpp +++ b/communication/multichannelcommunication.cpp @@ -45,7 +45,8 @@ void MultiChannelCommunication::readTask() { currentCommandType = (CommandType)usbReadFifo[0]; if(!CommandTypeIsValid(currentCommandType)) { - std::cout << "cnv" << std::hex << (int)currentCommandType << ' ' << std::dec; + // TODO Flag error? Device to host bytes discarded + //std::cout << "cnv" << std::hex << (int)currentCommandType << ' ' << std::dec; usbReadFifo.pop_front(); continue; } diff --git a/communication/packetizer.cpp b/communication/packetizer.cpp index f20bb26..d839db8 100644 --- a/communication/packetizer.cpp +++ b/communication/packetizer.cpp @@ -111,7 +111,7 @@ bool Packetizer::input(const std::vector& inputBytes) { bytes.pop_front(); } else { if(gotGoodPackets) // Don't complain unless we've already gotten a good packet, in case we started in the middle of a stream - std::cout << "Dropping packet due to bad checksum" << std::endl; + err(APIError::PacketChecksumError); bytes.pop_front(); // Drop the first byte so it doesn't get picked up again } diff --git a/include/icsneo/api/error.h b/include/icsneo/api/error.h index ec1b90f..b103e2b 100644 --- a/include/icsneo/api/error.h +++ b/include/icsneo/api/error.h @@ -50,6 +50,14 @@ public: SettingsChecksumError = 0x2006, SettingsNotAvailable = 0x2007, + // Transport Errors + FailedToRead = 0x3000, + FailedToWrite = 0x3001, + DriverFailedToOpen = 0x3002, + PacketChecksumError = 0x3003, + PCAPCouldNotStart = 0x3102, + PCAPCouldNotFindDevices = 0x3103, + TooManyErrors = 0xFFFFFFFE, Unknown = 0xFFFFFFFF }; diff --git a/include/icsneo/communication/message/filter/main51messagefilter.h b/include/icsneo/communication/message/filter/main51messagefilter.h index 2466029..9435245 100644 --- a/include/icsneo/communication/message/filter/main51messagefilter.h +++ b/include/icsneo/communication/message/filter/main51messagefilter.h @@ -21,14 +21,7 @@ public: return false; } const auto main51Message = std::dynamic_pointer_cast(message); - if(!main51Message) - std::cout << "could not upcast " << message->network << std::endl; - if(main51Message == nullptr || !matchCommand(main51Message->command)) { - if(main51Message) - std::cout << "Could not match command " << (int)(command) << " to " << (int)(main51Message->command) << std::endl; - return false; - } - return true; + return main51Message && matchCommand(main51Message->command); } private: diff --git a/include/icsneo/device/device.h b/include/icsneo/device/device.h index 97395e0..f7aa05f 100644 --- a/include/icsneo/device/device.h +++ b/include/icsneo/device/device.h @@ -102,7 +102,7 @@ protected: } template - std::unique_ptr makeTransport() { return std::unique_ptr(new Transport(getWritableNeoDevice())); } + std::unique_ptr makeTransport() { return std::unique_ptr(new Transport(err, getWritableNeoDevice())); } virtual void setupTransport(ICommunication* transport) {} virtual std::shared_ptr makePacketizer() { return std::make_shared(err); } diff --git a/include/icsneo/platform/posix/ftdi.h b/include/icsneo/platform/posix/ftdi.h index 7ea478b..ead692f 100644 --- a/include/icsneo/platform/posix/ftdi.h +++ b/include/icsneo/platform/posix/ftdi.h @@ -9,6 +9,7 @@ #include "icsneo/device/neodevice.h" #include "icsneo/communication/icommunication.h" #include "icsneo/third-party/concurrentqueue/blockingconcurrentqueue.h" +#include "icsneo/api/errormanager.h" namespace icsneo { @@ -18,7 +19,7 @@ public: static std::vector FindByProduct(int product); static bool IsHandleValid(neodevice_handle_t handle); - FTDI(neodevice_t& forDevice); + FTDI(device_errorhandler_t err, neodevice_t& forDevice); ~FTDI() { close(); } bool open(); bool close(); @@ -43,6 +44,7 @@ private: bool openable; // Set to false in the constructor if the object has not been found in searchResultDevices neodevice_t& device; + device_errorhandler_t err; FTDIDevice ftdiDevice; }; diff --git a/include/icsneo/platform/posix/stm32.h b/include/icsneo/platform/posix/stm32.h index e3bd52e..f58eb0f 100644 --- a/include/icsneo/platform/posix/stm32.h +++ b/include/icsneo/platform/posix/stm32.h @@ -3,6 +3,7 @@ #include "icsneo/communication/icommunication.h" #include "icsneo/device/neodevice.h" +#include "icsneo/api/errormanager.h" #include #include @@ -10,7 +11,7 @@ namespace icsneo { class STM32 : public ICommunication { public: - STM32(neodevice_t& forDevice) : device(forDevice) {} + STM32(device_errorhandler_t err, neodevice_t& forDevice) : device(forDevice), err(err) {} static std::vector FindByProduct(int product); bool open(); @@ -19,6 +20,7 @@ public: private: neodevice_t& device; + device_errorhandler_t err; int fd = -1; static constexpr neodevice_handle_t HANDLE_OFFSET = 10; diff --git a/include/icsneo/platform/windows/ftdi.h b/include/icsneo/platform/windows/ftdi.h index 4cb50cd..860a845 100644 --- a/include/icsneo/platform/windows/ftdi.h +++ b/include/icsneo/platform/windows/ftdi.h @@ -7,7 +7,7 @@ namespace icsneo { class FTDI : public VCP { public: - FTDI(neodevice_t& forDevice) : VCP(forDevice) {} + FTDI(device_errorhandler_t err, neodevice_t& forDevice) : VCP(err, forDevice) {} static std::vector FindByProduct(int product) { return VCP::FindByProduct(product, L"serenum"); } }; diff --git a/include/icsneo/platform/windows/pcap.h b/include/icsneo/platform/windows/pcap.h index 424a912..1d9bca8 100644 --- a/include/icsneo/platform/windows/pcap.h +++ b/include/icsneo/platform/windows/pcap.h @@ -4,6 +4,7 @@ #include "icsneo/platform/windows/internal/pcapdll.h" #include "icsneo/device/neodevice.h" #include "icsneo/communication/icommunication.h" +#include "icsneo/api/errormanager.h" #include namespace icsneo { @@ -20,7 +21,7 @@ public: static std::string GetEthDevSerialFromMacAddress(uint8_t product, uint16_t macSerial); static bool IsHandleValid(neodevice_handle_t handle); - PCAP(neodevice_t& forDevice); + PCAP(device_errorhandler_t err, neodevice_t& forDevice); bool open(); bool isOpen(); bool close(); diff --git a/include/icsneo/platform/windows/stm32.h b/include/icsneo/platform/windows/stm32.h index 01cc8ee..0b0dc18 100644 --- a/include/icsneo/platform/windows/stm32.h +++ b/include/icsneo/platform/windows/stm32.h @@ -7,7 +7,7 @@ namespace icsneo { class STM32 : public VCP { public: - STM32(neodevice_t& forDevice) : VCP(forDevice) {} + STM32(device_errorhandler_t err, neodevice_t& forDevice) : VCP(err, forDevice) {} static std::vector FindByProduct(int product) { return VCP::FindByProduct(product, L"usbser"); } }; diff --git a/include/icsneo/platform/windows/vcp.h b/include/icsneo/platform/windows/vcp.h index 6f13f56..dd8baa8 100644 --- a/include/icsneo/platform/windows/vcp.h +++ b/include/icsneo/platform/windows/vcp.h @@ -9,6 +9,7 @@ #include #include "icsneo/device/neodevice.h" #include "icsneo/communication/icommunication.h" +#include "icsneo/api/errormanager.h" namespace icsneo { @@ -19,7 +20,7 @@ public: static bool IsHandleValid(neodevice_handle_t handle); typedef void(*fn_boolCallback)(bool success); - VCP(neodevice_t& forDevice) : device(forDevice) { + VCP(device_errorhandler_t err, neodevice_t& forDevice) : device(forDevice), err(err) { overlappedRead.hEvent = INVALID_HANDLE_VALUE; overlappedWrite.hEvent = INVALID_HANDLE_VALUE; overlappedWait.hEvent = INVALID_HANDLE_VALUE; @@ -34,6 +35,7 @@ private: bool open(bool fromAsync); bool opening = false; neodevice_t& device; + device_errorhandler_t err; HANDLE handle = INVALID_HANDLE_VALUE; OVERLAPPED overlappedRead = {}; OVERLAPPED overlappedWrite = {}; diff --git a/platform/posix/ftdi.cpp b/platform/posix/ftdi.cpp index 7cba067..df86cd7 100644 --- a/platform/posix/ftdi.cpp +++ b/platform/posix/ftdi.cpp @@ -58,7 +58,7 @@ bool FTDI::GetDeviceForHandle(neodevice_handle_t handle, FTDIDevice& device) { return false; } -FTDI::FTDI(neodevice_t& forDevice) : device(forDevice) { +FTDI::FTDI(device_errorhandler_t err, neodevice_t& forDevice) : device(forDevice), err(err) { openable = GetDeviceForHandle(forDevice.handle, ftdiDevice); } diff --git a/platform/posix/stm32.cpp b/platform/posix/stm32.cpp index d64e8b3..7fca33c 100644 --- a/platform/posix/stm32.cpp +++ b/platform/posix/stm32.cpp @@ -195,7 +195,8 @@ bool STM32::open() { ss << "/dev/ttyACM" << (int)(device.handle - HANDLE_OFFSET); fd = ::open(ss.str().c_str(), O_RDWR | O_NOCTTY | O_SYNC); if(!isOpen()) { - std::cout << "Open of " << ss.str().c_str() << " failed with " << strerror(errno) << ' '; + //std::cout << "Open of " << ss.str().c_str() << " failed with " << strerror(errno) << ' '; + err(APIError::DriverFailedToOpen); return false; } @@ -276,6 +277,6 @@ void STM32::writeTask() { const ssize_t writeSize = (ssize_t)writeOp.bytes.size(); ssize_t actualWritten = ::write(fd, writeOp.bytes.data(), writeSize); if(actualWritten != writeSize) - std::cout << "Failure to write " << writeSize << " bytes, wrote " << actualWritten << std::endl; + err(APIError::FailedToWrite); } } \ No newline at end of file diff --git a/platform/windows/pcap.cpp b/platform/windows/pcap.cpp index 18b2fb1..803af8a 100644 --- a/platform/windows/pcap.cpp +++ b/platform/windows/pcap.cpp @@ -20,7 +20,7 @@ std::vector PCAP::FindAll() { std::vector foundDevices; PCAPDLL pcap; if(!pcap.ok()) { - std::cout << "PCAP not okay" << std::endl; + ErrorManager::GetInstance().add(APIError::PCAPCouldNotStart); return std::vector(); } @@ -38,7 +38,7 @@ std::vector PCAP::FindAll() { } if(!success) { - std::cout << "PCAP FindAllDevs_Ex not okay " << errbuf << std::endl; + ErrorManager::GetInstance().add(APIError::PCAPCouldNotFindDevices); return std::vector(); } @@ -55,13 +55,13 @@ std::vector PCAP::FindAll() { // Now we're going to ask Win32 for the information as well ULONG size = 0; if(GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, nullptr, nullptr, &size) != ERROR_BUFFER_OVERFLOW) { - std::cout << "GetAdaptersAddresses size query not okay" << std::endl; + ErrorManager::GetInstance().add(APIError::PCAPCouldNotFindDevices); return std::vector(); } std::vector adapterAddressBuffer; adapterAddressBuffer.resize(size); if(GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, nullptr, (IP_ADAPTER_ADDRESSES*)adapterAddressBuffer.data(), &size) != ERROR_SUCCESS) { - std::cout << "GetAdaptersAddresses not okay" << std::endl; + ErrorManager::GetInstance().add(APIError::PCAPCouldNotFindDevices); return std::vector(); } @@ -124,7 +124,7 @@ std::vector PCAP::FindAll() { const uint8_t* data; auto res = pcap.next_ex(interface.fp, &header, &data); if(res < 0) { - std::cout << "pcapnextex failed with " << res << std::endl; + //std::cout << "pcapnextex failed with " << res << std::endl; break; } if(res == 0) @@ -177,7 +177,7 @@ bool PCAP::IsHandleValid(neodevice_handle_t handle) { return (netifIndex < knownInterfaces.size()); } -PCAP::PCAP(neodevice_t& forDevice) : device(forDevice) { +PCAP::PCAP(device_errorhandler_t err, neodevice_t& forDevice) : device(forDevice), err(err) { if(IsHandleValid(device.handle)) { interface = knownInterfaces[(device.handle >> 24) & 0xFF]; interface.fp = nullptr; // We're going to open our own connection to the interface. This should already be nullptr but just in case. @@ -206,7 +206,7 @@ bool PCAP::open() { // Open the interface interface.fp = pcap.open(interface.nameFromWinPCAP.c_str(), 100, PCAP_OPENFLAG_PROMISCUOUS | PCAP_OPENFLAG_MAX_RESPONSIVENESS, 1, nullptr, errbuf); if(interface.fp == nullptr) { - std::cout << "Open device " << device.serial << " failed with " << errbuf << std::endl; + err(APIError::DriverFailedToOpen); return false; } @@ -241,7 +241,7 @@ void PCAP::readTask() { while(!closing) { auto readBytes = pcap.next_ex(interface.fp, &header, &data); if(readBytes < 0) { - std::cout << "pcapnextex failed in read task with " << readBytes << std::endl; + err(APIError::FailedToRead); break; } if(readBytes == 0) diff --git a/platform/windows/vcp.cpp b/platform/windows/vcp.cpp index 6406b11..6ebc279 100644 --- a/platform/windows/vcp.cpp +++ b/platform/windows/vcp.cpp @@ -249,28 +249,22 @@ void VCP::readTask() { case LAUNCH: { COMSTAT comStatus; unsigned long errorCodes; - if(!ClearCommError(handle, &errorCodes, &comStatus)) - std::cout << "Error clearing com err" << std::endl; + ClearCommError(handle, &errorCodes, &comStatus); bytesRead = 0; if(ReadFile(handle, readbuf, READ_BUFFER_SIZE, nullptr, &overlappedRead)) { if(GetOverlappedResult(handle, &overlappedRead, &bytesRead, FALSE)) { if(bytesRead) readQueue.enqueue_bulk(readbuf, bytesRead); - } else { - std::cout <<"Readfile succeeded but not enqueued " << GetLastError() << std::endl; } continue; } - auto err = GetLastError(); - if(err == ERROR_SUCCESS) - std::cout << "Error was success?" << std::endl; - - if(err == ERROR_IO_PENDING) + auto lastError = GetLastError(); + if(lastError == ERROR_IO_PENDING) state = WAIT; - else - std::cout << "ReadFile failed " << err << std::endl; + else if(lastError != ERROR_SUCCESS) + err(APIError::FailedToRead); } break; case WAIT: { @@ -281,11 +275,11 @@ void VCP::readTask() { readQueue.enqueue_bulk(readbuf, bytesRead); state = LAUNCH; } else - std::cout << "ReadFile deferred failed " << err << std::endl; + err(APIError::FailedToRead); } if(ret == WAIT_ABANDONED) { state = LAUNCH; - std::cout << "Readfile abandoned" << std::endl; + err(APIError::FailedToRead); } } } @@ -311,20 +305,19 @@ void VCP::writeTask() { state = WAIT; } else - std::cout << "Writefile failed " << err << std::endl; + err(APIError::FailedToWrite); } break; case WAIT: { auto ret = WaitForSingleObject(overlappedWrite.hEvent, 50); if(ret == WAIT_OBJECT_0) { - if(!GetOverlappedResult(handle, &overlappedWrite, &bytesWritten, FALSE)) { - std::cout << "Writefile deferred failed " << GetLastError() << std::endl; - } + if(!GetOverlappedResult(handle, &overlappedWrite, &bytesWritten, FALSE)) + err(APIError::FailedToWrite); state = LAUNCH; } if(ret == WAIT_ABANDONED) { - std::cout << "Writefile deferred abandoned" << std::endl; + err(APIError::FailedToWrite); state = LAUNCH; } }