Added error checking and removed some redundancy from device isOpen()

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<uint8_t>& bytes) {
 	// This is here so that other communication types (like multichannel) can override it
 	return rawWrite(bytes);

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;

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<uint8_t>& bytes) { return impl->write(bytes); }
@@ -62,8 +63,6 @@ protected:
 	std::atomic<bool> closing{false};
 
 private:
-	bool isOpen = false;
-
 	std::thread readTaskThread;
 	void readTask();
 };

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();
 

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;
 }

platform/posix/stm32.cpp

@@ -191,6 +191,10 @@ std::vector<neodevice_t> 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() {

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();

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;