From 965679c370886b88dff338ee03b16ca726283841 Mon Sep 17 00:00:00 2001 From: EricLiu2000 Date: Mon, 10 Jun 2019 16:48:47 -0400 Subject: [PATCH] Added error checking and removed some redundancy from device isOpen() --- communication/communication.cpp | 18 +++++++++++------ device/device.cpp | 11 ++-------- include/icsneo/communication/communication.h | 3 +-- include/icsneo/device/device.h | 2 +- platform/posix/ftdi.cpp | 21 ++++++++++++++------ platform/posix/stm32.cpp | 17 ++++++++++++++-- platform/windows/pcap.cpp | 16 +++++++++++---- platform/windows/vcp.cpp | 20 ++++++++++++++----- 8 files changed, 73 insertions(+), 35 deletions(-) diff --git a/communication/communication.cpp b/communication/communication.cpp index c973bfb..b81ef5e 100644 --- a/communication/communication.cpp +++ b/communication/communication.cpp @@ -18,11 +18,12 @@ using namespace icsneo; int Communication::messageCallbackIDCounter = 1; bool Communication::open() { - if(isOpen) - return true; - + if(isOpen()) { + err(APIError::DeviceCurrentlyOpen); + return false; + } + spawnThreads(); - isOpen = true; return impl->open(); } @@ -38,15 +39,20 @@ void Communication::joinThreads() { } bool Communication::close() { - if(!isOpen) + if(!isOpen()) { + err(APIError::DeviceCurrentlyClosed); return false; + } - isOpen = false; joinThreads(); return impl->close(); } +bool Communication::isOpen() { + return impl->isOpen(); +} + bool Communication::sendPacket(std::vector& bytes) { // This is here so that other communication types (like multichannel) can override it return rawWrite(bytes); diff --git a/device/device.cpp b/device/device.cpp index 1459706..6ee5c82 100644 --- a/device/device.cpp +++ b/device/device.cpp @@ -135,17 +135,14 @@ void Device::enforcePollingMessageLimit() { } bool Device::open() { - if(opened) { - return false; - } - if(!com) { err(APIError::Unknown); return false; } - if(!com->open()) + if(!com->open()) { return false; + } auto serial = com->getSerialNumberSync(); int i = 0; @@ -179,10 +176,6 @@ bool Device::open() { } bool Device::close() { - if(!opened) { - return false; - } - if(!com) { err(APIError::Unknown); return false; diff --git a/include/icsneo/communication/communication.h b/include/icsneo/communication/communication.h index 47d0ec7..583dbb8 100644 --- a/include/icsneo/communication/communication.h +++ b/include/icsneo/communication/communication.h @@ -32,6 +32,7 @@ public: bool open(); bool close(); + bool isOpen(); virtual void spawnThreads(); virtual void joinThreads(); bool rawWrite(const std::vector& bytes) { return impl->write(bytes); } @@ -62,8 +63,6 @@ protected: std::atomic closing{false}; private: - bool isOpen = false; - std::thread readTaskThread; void readTask(); }; diff --git a/include/icsneo/device/device.h b/include/icsneo/device/device.h index 2871968..f9b2dcd 100644 --- a/include/icsneo/device/device.h +++ b/include/icsneo/device/device.h @@ -44,7 +44,7 @@ public: virtual bool open(); virtual bool close(); virtual bool isOnline() const { return online; } - virtual bool isOpen() const { return opened; } + virtual bool isOpen() const { return com->isOpen(); } virtual bool goOnline(); virtual bool goOffline(); diff --git a/platform/posix/ftdi.cpp b/platform/posix/ftdi.cpp index 8e2358e..b381391 100644 --- a/platform/posix/ftdi.cpp +++ b/platform/posix/ftdi.cpp @@ -44,9 +44,11 @@ FTDI::FTDI(const device_errorhandler_t& err, neodevice_t& forDevice) : ICommunic } bool FTDI::open() { - if(isOpen()) + if(isOpen()) { + err(APIError::DeviceCurrentlyOpen); return false; - + } + if(!openable) { err(APIError::InvalidNeoDevice); return false; @@ -74,9 +76,11 @@ bool FTDI::open() { } bool FTDI::close() { - if(!isOpen()) + if(!isOpen()) { + err(APIError::DeviceCurrentlyClosed); return false; - + } + closing = true; if(readThread.joinable()) @@ -86,7 +90,9 @@ bool FTDI::close() { writeThread.join(); bool ret = ftdi.closeDevice(); - + if(ret != 0) + err(APIError::DriverFailedToClose); + uint8_t flush; WriteOperation flushop; while(readQueue.try_dequeue(flush)) {} @@ -156,12 +162,15 @@ int FTDI::FTDIContext::openDevice(int pid, const char* serial) { bool FTDI::FTDIContext::closeDevice() { if(context == nullptr) return false; + + if(!deviceOpen) return true; int ret = ftdi_usb_close(context); - if(ret != 0) + if(ret != 0) return false; + deviceOpen = false; return true; } diff --git a/platform/posix/stm32.cpp b/platform/posix/stm32.cpp index b8ffc87..4cae1e9 100644 --- a/platform/posix/stm32.cpp +++ b/platform/posix/stm32.cpp @@ -191,6 +191,10 @@ std::vector STM32::FindByProduct(int product) { } bool STM32::open() { + if(isOpen()) { + err(APIError::DeviceCurrentlyOpen); + return false; + } std::stringstream ss; ss << "/dev/ttyACM" << (int)(device.handle - HANDLE_OFFSET); fd = ::open(ss.str().c_str(), O_RDWR | O_NOCTTY | O_SYNC); @@ -205,6 +209,7 @@ bool STM32::open() { if(tcgetattr(fd, &tty) != 0) { close(); + err(APIError::DriverFailedToOpen); return false; } @@ -227,6 +232,7 @@ bool STM32::open() { if(tcsetattr(fd, TCSAFLUSH, &tty) != 0) { // Flushes input and output buffers as well as setting settings close(); + err(APIError::DriverFailedToOpen); return false; } @@ -247,8 +253,10 @@ bool STM32::isOpen() { } bool STM32::close() { - if(!isOpen()) + if(!isOpen()) { + err(APIError::DeviceCurrentlyClosed); return false; + } closing = true; @@ -268,7 +276,12 @@ bool STM32::close() { while (readQueue.try_dequeue(flush)) {} while (writeQueue.try_dequeue(flushop)) {} - return ret == 0; + if(ret == 0) { + return true; + } else { + err(APIError::DriverFailedToClose); + return false; + } } void STM32::readTask() { diff --git a/platform/windows/pcap.cpp b/platform/windows/pcap.cpp index 5cc0a4e..6daf26c 100644 --- a/platform/windows/pcap.cpp +++ b/platform/windows/pcap.cpp @@ -194,14 +194,20 @@ PCAP::PCAP(const device_errorhandler_t& err, neodevice_t& forDevice) : ICommunic } bool PCAP::open() { - if(!openable) + if(!openable) { + err(APIError::InvalidNeoDevice); return false; + } - if(!pcap.ok()) + if(!pcap.ok()) { + err(APIError::DriverFailedToOpen); return false; + } - if(isOpen()) + if(isOpen()) { + err(APIError::DeviceCurrentlyOpen); return false; + } // Open the interface interface.fp = pcap.open(interface.nameFromWinPCAP.c_str(), 100, PCAP_OPENFLAG_PROMISCUOUS | PCAP_OPENFLAG_MAX_RESPONSIVENESS, 1, nullptr, errbuf); @@ -222,8 +228,10 @@ bool PCAP::isOpen() { } bool PCAP::close() { - if(!isOpen()) + if(!isOpen()) { + err(APIError::DeviceCurrentlyClosed); return false; + } closing = true; // Signal the threads that we are closing readThread.join(); diff --git a/platform/windows/vcp.cpp b/platform/windows/vcp.cpp index 3c75cdf..d8a10ad 100644 --- a/platform/windows/vcp.cpp +++ b/platform/windows/vcp.cpp @@ -183,8 +183,10 @@ bool VCP::IsHandleValid(neodevice_handle_t handle) { } bool VCP::open(bool fromAsync) { - if(isOpen() || (!fromAsync && opening)) + if(isOpen() || (!fromAsync && opening)) { + err(APIError::DeviceCurrentlyOpen); return false; + } if(!IsHandleValid(device.handle)) { err(APIError::DriverFailedToOpen); @@ -216,6 +218,7 @@ bool VCP::open(bool fromAsync) { COMMTIMEOUTS timeouts; if(!GetCommTimeouts(handle, &timeouts)) { close(); + err(APIError::DriverFailedToOpen); return false; } @@ -288,9 +291,11 @@ void VCP::openAsync(fn_boolCallback callback) { } bool VCP::close() { - if(!isOpen()) + if(!isOpen()) { + err(APIError::DeviceCurrentlyClosed); return false; - + } + closing = true; // Signal the threads that we are closing for(auto& t : threads) t->join(); // Wait for the threads to close @@ -298,9 +303,11 @@ bool VCP::close() { writeThread.join(); closing = false; - if(!CloseHandle(handle)) + if(!CloseHandle(handle)) { + err(APIError::DriverFailedToClose); return false; - + } + handle = INVALID_HANDLE_VALUE; bool ret = true; // If one of the events fails closing, we probably still want to try and close the others @@ -325,6 +332,9 @@ bool VCP::close() { while(readQueue.try_dequeue(flush)) {} while(writeQueue.try_dequeue(flushop)) {} + if(!ret) + err(APIError::DriverFailedToClose); + // TODO Set up some sort of shared memory, free which COM port we had open so we can try to open it again return ret;