Added error checking to functions called from Communication

api/icsneocpp/error.cpp

@@ -65,11 +65,15 @@ static constexpr const char* ERROR_CANFD_SETTINGS_NOT_AVAILABLE = "CANFD setting
 static constexpr const char* ERROR_LSFTCAN_SETTINGS_NOT_AVAILABLE = "LSFTCAN settings are not available for this device.";
 static constexpr const char* ERROR_SWCAN_SETTINGS_NOT_AVAILABLE = "SWCAN settings are not available for this device.";
 static constexpr const char* ERROR_BAUDRATE_NOT_FOUND = "The baudrate was not found.";
-static constexpr const char* ERROR_BAD_NETWORK_TYPE = "The network type was not found.";
+static constexpr const char* ERROR_UNEXPECTED_NETWORK_TYPE = "The network type was not found.";
 static constexpr const char* ERROR_DEVICE_FIRMWARE_OUT_OF_DATE = "The device firmware is out of date. New API functionality may not be supported.";
 static constexpr const char* ERROR_SETTINGS_STRUCTURE_MISMATCH = "Unexpected settings structure for this device.";
 static constexpr const char* ERROR_SETTINGS_STRUCTURE_TRUNCATED = "Settings structure is longer than the device supports and will be truncated.";
 static constexpr const char* ERROR_NO_DEVICE_RESPONSE = "Expected a response from the device but none were found.";
+static constexpr const char* ERROR_MESSAGE_FORMATTING = "The message was not properly formed.";
+static constexpr const char* ERROR_MESSAGE_DATA_OVER_CAPACITY = "The message has too much data for the protocol.";
+static constexpr const char* ERROR_CANFD_NOT_SUPPORTED = "This device does not support CANFD.";
+static constexpr const char* ERROR_RTR_NOT_SUPPORTED = "RTR is not supported with CANFD.";
 
 // Transport Errors
 static constexpr const char* ERROR_FAILED_TO_READ = "A read operation failed.";
@@ -131,6 +135,8 @@ const char* APIError::DescriptionForType(ErrorType type) {
 			return ERROR_SWCAN_SETTINGS_NOT_AVAILABLE;
 		case BaudrateNotFound:
 			return ERROR_BAUDRATE_NOT_FOUND;
+		case UnexpectedNetworkType:
+			return ERROR_UNEXPECTED_NETWORK_TYPE;
 		case DeviceFirmwareOutOfDate:
 			return ERROR_DEVICE_FIRMWARE_OUT_OF_DATE;
 		case SettingsStructureMismatch:
@@ -139,6 +145,14 @@ const char* APIError::DescriptionForType(ErrorType type) {
 			return ERROR_SETTINGS_STRUCTURE_TRUNCATED;
 		case NoDeviceResponse:
 			return ERROR_NO_DEVICE_RESPONSE;
+		case MessageFormattingError:
+			return ERROR_MESSAGE_FORMATTING;
+		case MessageDataOverCapacity:
+			return ERROR_MESSAGE_DATA_OVER_CAPACITY;
+		case CANFDNotSupported:
+			return ERROR_CANFD_NOT_SUPPORTED;
+		case RTRNotSupported:
+			return ERROR_RTR_NOT_SUPPORTED;
 
 		// Transport Errors
 		case FailedToRead:
@@ -203,8 +217,14 @@ APIError::Severity APIError::SeverityForType(ErrorType type) {
 		case LSFTCANSettingsNotAvailable:
 		case SWCANSettingsNotAvailable:
 		case BaudrateNotFound:
+		case UnexpectedNetworkType:
 		case SettingsStructureMismatch:
 		case NoDeviceResponse:
+		case MessageFormattingError:
+		case MessageDataOverCapacity:
+		case CANFDNotSupported:
+		case RTRNotSupported:
+
 		// Transport Errors
 		case FailedToRead:
 		case FailedToWrite:

communication/communication.cpp

@@ -73,11 +73,15 @@ bool Communication::getSettingsSync(std::vector<uint8_t>& data, std::chrono::mil
 		return false;
 
 	std::shared_ptr<ReadSettingsMessage> gsmsg = std::dynamic_pointer_cast<ReadSettingsMessage>(msg);
-	if(!gsmsg)
+	if(!gsmsg) {
+		err(APIError::Unknown);
 		return false;
+	}
 
-	if(gsmsg->response != ReadSettingsMessage::Response::OK)
+	if(gsmsg->response != ReadSettingsMessage::Response::OK) {
+		err(APIError::Unknown);
 		return false;
+	}
 
 	data = std::move(msg->data);
 	return true;
@@ -108,6 +112,7 @@ bool Communication::removeMessageCallback(int id) {
 		messageCallbacks.erase(id);
 		return true;
 	} catch(...) {
+		err(APIError::Unknown);
 		return false;
 	}
 }

communication/encoder.cpp

@@ -13,11 +13,13 @@ bool Encoder::encode(std::vector<uint8_t>& result, const std::shared_ptr<Message
 	switch(message->network.getType()) {
 		case Network::Type::Ethernet: {
 			auto ethmsg = std::dynamic_pointer_cast<EthernetMessage>(message);
-			if(!ethmsg)
+			if(!ethmsg) {
+				err(APIError::MessageFormattingError);
 				return false; // The message was not a properly formed EthernetMessage
+			}
 
 			useResultAsBuffer = true;
-			if(!HardwareEthernetPacket::EncodeFromMessage(*ethmsg, result))
+			if(!HardwareEthernetPacket::EncodeFromMessage(*ethmsg, result, err))
 				return false;
 
 			break;
@@ -26,14 +28,18 @@ bool Encoder::encode(std::vector<uint8_t>& result, const std::shared_ptr<Message
 		case Network::Type::SWCAN:
 		case Network::Type::LSFTCAN: {
 			auto canmsg = std::dynamic_pointer_cast<CANMessage>(message);
-			if(!canmsg)
+			if(!canmsg) {
+				err(APIError::MessageFormattingError);
 				return false; // The message was not a properly formed CANMessage
+			}
 
-			if(!supportCANFD && canmsg->isCANFD)
+			if(!supportCANFD && canmsg->isCANFD) {
+				err(APIError::CANFDNotSupported);
 				return false; // This device does not support CAN FD
+			}
 			
 			useResultAsBuffer = true;
-			if(!HardwareCANPacket::EncodeFromMessage(*canmsg, result))
+			if(!HardwareCANPacket::EncodeFromMessage(*canmsg, result, err))
 				return false; // The CANMessage was malformed
 
 			break;
@@ -69,6 +75,7 @@ bool Encoder::encode(std::vector<uint8_t>& result, const std::shared_ptr<Message
 					break;
 				}
 				default:
+					err(APIError::UnexpectedNetworkType);
 					return false;
 			}
 	}

communication/icommunication.cpp

@@ -50,5 +50,10 @@ bool ICommunication::write(const std::vector<uint8_t>& bytes) {
 			return false;
 		}
 	}
-	return writeQueue.enqueue(WriteOperation(bytes));
+
+	bool ret = writeQueue.enqueue(WriteOperation(bytes));
+	if(!ret) {
+		err(APIError::Unknown);
+	}
+	return ret;
 }

communication/packet/canpacket.cpp

@@ -79,13 +79,17 @@ std::shared_ptr<CANMessage> HardwareCANPacket::DecodeToMessage(const std::vector
 	return msg;
 }
 
-bool HardwareCANPacket::EncodeFromMessage(const CANMessage& message, std::vector<uint8_t>& result) {
+bool HardwareCANPacket::EncodeFromMessage(const CANMessage& message, std::vector<uint8_t>& result, const device_errorhandler_t& err) {
-	if(message.isCANFD && message.isRemote)
+	if(message.isCANFD && message.isRemote) {
+		err(APIError::RTRNotSupported);
 		return false; // RTR frames can not be used with CAN FD
+	}
 
 	const size_t dataSize = message.data.size();
-	if(dataSize > 64 || (dataSize > 8 && !message.isCANFD))
+	if(dataSize > 64 || (dataSize > 8 && !message.isCANFD)) {
+		err(APIError::MessageDataOverCapacity);
 		return false; // Too much data for the protocol
+	}
 
 	uint8_t lengthNibble = uint8_t(message.data.size());
 	uint8_t paddingBytes = 0;
@@ -162,6 +166,7 @@ bool HardwareCANPacket::EncodeFromMessage(const CANMessage& message, std::vector
 				lengthNibble = 0xF;
 				break;
 			default:
+				err(APIError::Unknown);
 				return false; // CAN FD frame may have had an incorrect byte count
 		}
 	}
@@ -176,8 +181,10 @@ bool HardwareCANPacket::EncodeFromMessage(const CANMessage& message, std::vector
 
 	// Next 2-4 bytes are ArbID
 	if(message.isExtended) {
-		if(message.arbid >= 0x20000000) // Extended messages use 29-bit arb IDs
+		if(message.arbid >= 0x20000000) {// Extended messages use 29-bit arb IDs
+			err(APIError::MessageFormattingError);
 			return false;
+		}
 
 		result.insert(result.end(), {
 			(uint8_t)(message.arbid >> 21),
@@ -186,8 +193,10 @@ bool HardwareCANPacket::EncodeFromMessage(const CANMessage& message, std::vector
 			(uint8_t)message.arbid
 		});
 	} else {
-		if(message.arbid >= 0x800) // Standard messages use 11-bit arb IDs
+		if(message.arbid >= 0x800) {// Standard messages use 11-bit arb IDs
+			err(APIError::MessageFormattingError);
 			return false;
+		}
 
 		result.insert(result.end(), {
 			(uint8_t)(message.arbid >> 3),

communication/packet/ethernetpacket.cpp

@@ -53,7 +53,7 @@ std::shared_ptr<EthernetMessage> HardwareEthernetPacket::DecodeToMessage(const s
 	return messagePtr;
 }
 
-bool HardwareEthernetPacket::EncodeFromMessage(const EthernetMessage& message, std::vector<uint8_t>& bytestream) {
+bool HardwareEthernetPacket::EncodeFromMessage(const EthernetMessage& message, std::vector<uint8_t>& bytestream, const device_errorhandler_t& err) {
 	const size_t unpaddedSize = message.data.size();
 	size_t paddedSize = unpaddedSize;
 

device/device.cpp

@@ -171,7 +171,6 @@ bool Device::open() {
 		handleInternalMessage(message);
 	}));
 
-	opened = true;
 	return true;
 }
 
@@ -185,8 +184,7 @@ bool Device::close() {
 		com->removeMessageCallback(internalHandlerCallbackID);
 
 	goOffline();
-	opened = !com->close();
+	return com->close();
-	return !opened;
 }
 
 bool Device::goOnline() {

device/idevicesettings.cpp

@@ -347,7 +347,7 @@ int64_t IDeviceSettings::getBaudrateFor(Network net) const {
 			return baudrate;
 		}
 		default:
-			err(APIError::BadNetworkType);
+			err(APIError::UnexpectedNetworkType);
 			return -1;
 	}
 }
@@ -426,7 +426,7 @@ bool IDeviceSettings::setBaudrateFor(Network net, int64_t baudrate) {
 			return true;
 		}
 		default:
-			err(APIError::BadNetworkType);
+			err(APIError::UnexpectedNetworkType);
 			return false;
 	}
 }
@@ -458,7 +458,7 @@ int64_t IDeviceSettings::getFDBaudrateFor(Network net) const {
 			return baudrate;
 		}
 		default:
-			err(APIError::BadNetworkType);
+			err(APIError::UnexpectedNetworkType);
 			return -1;
 	}
 }
@@ -496,7 +496,7 @@ bool IDeviceSettings::setFDBaudrateFor(Network net, int64_t baudrate) {
 			return true;
 		}
 		default:
-			err(APIError::BadNetworkType);
+			err(APIError::UnexpectedNetworkType);
 			return false;
 	}
 }

include/icsneo/api/error.h

@@ -56,11 +56,15 @@ public:
 		LSFTCANSettingsNotAvailable = 0x2011,
 		SWCANSettingsNotAvailable = 0x2012,
 		BaudrateNotFound = 0x2013,
-		BadNetworkType = 0x2014,
+		UnexpectedNetworkType = 0x2014,
 		DeviceFirmwareOutOfDate = 0x2015,
 		SettingsStructureMismatch = 0x2016,
 		SettingsStructureTruncated = 0x2017,
 		NoDeviceResponse = 0x2018,
+		MessageFormattingError = 0x2019,
+		MessageDataOverCapacity = 0x2020,
+		CANFDNotSupported = 0x2021,
+		RTRNotSupported = 0x2022,
 
 		// Transport Errors
 		FailedToRead = 0x3000,

include/icsneo/communication/packet/canpacket.h

@@ -2,6 +2,7 @@
 #define __CANPACKET_H__
 
 #include "icsneo/communication/message/canmessage.h"
+#include "icsneo/api/errormanager.h"
 #include <cstdint>
 #include <memory>
 
@@ -11,7 +12,7 @@ typedef uint16_t icscm_bitfield;
 
 struct HardwareCANPacket {
 	static std::shared_ptr<CANMessage> DecodeToMessage(const std::vector<uint8_t>& bytestream);
-	static bool EncodeFromMessage(const CANMessage& message, std::vector<uint8_t>& bytestream);
+	static bool EncodeFromMessage(const CANMessage& message, std::vector<uint8_t>& bytestream, const device_errorhandler_t& err);
 
 	struct {
 		icscm_bitfield IDE : 1;

include/icsneo/communication/packet/ethernetpacket.h

@@ -2,6 +2,7 @@
 #define __ETHERNETPACKET_H__
 
 #include "icsneo/communication/message/ethernetmessage.h"
+#include "icsneo/api/errormanager.h"
 #include <cstdint>
 #include <memory>
 
@@ -11,7 +12,7 @@ typedef uint16_t icscm_bitfield;
 
 struct HardwareEthernetPacket {
 	static std::shared_ptr<EthernetMessage> DecodeToMessage(const std::vector<uint8_t>& bytestream);
-	static bool EncodeFromMessage(const EthernetMessage& message, std::vector<uint8_t>& bytestream);
+	static bool EncodeFromMessage(const EthernetMessage& message, std::vector<uint8_t>& bytestream, const device_errorhandler_t& err);
 
 	struct {
 		icscm_bitfield FCS_AVAIL : 1;

include/icsneo/device/device.h

@@ -84,7 +84,6 @@ public:
 protected:
 	uint16_t productId = 0;
 	bool online = false;
-	bool opened = false;
 	int messagePollingCallbackID = 0;
 	int internalHandlerCallbackID = 0;
 	device_errorhandler_t err;