From b7288edd9a721aa82cd950a519123e6cec43c63f Mon Sep 17 00:00:00 2001 From: EricLiu2000 Date: Thu, 13 Jun 2019 14:29:23 -0400 Subject: [PATCH] Finished adding error checking --- api/icsneoc/icsneoc.cpp | 124 +++++++++---------------------- api/icsneocpp/error.cpp | 4 + communication/decoder.cpp | 19 +++-- communication/icommunication.cpp | 12 +-- device/device.cpp | 4 +- include/icsneo/api/error.h | 1 + 6 files changed, 65 insertions(+), 99 deletions(-) diff --git a/api/icsneoc/icsneoc.cpp b/api/icsneoc/icsneoc.cpp index 3aebd46..a1edc7d 100644 --- a/api/icsneoc/icsneoc.cpp +++ b/api/icsneoc/icsneoc.cpp @@ -87,6 +87,11 @@ uint32_t icsneo_serialStringToNum(const char* str) { } bool icsneo_isValidNeoDevice(const neodevice_t* device) { + // return false on nullptr + if(!device) { + ErrorManager::GetInstance().add(APIError::RequiredParameterNull); + return false; + } // If this neodevice_t was returned by a previous search, it will no longer be valid (as the underlying icsneo::Device is freed) for(auto& dev : connectedDevices) { if(dev.get() == device->device) @@ -96,14 +101,14 @@ bool icsneo_isValidNeoDevice(const neodevice_t* device) { if(dev.get() == device->device) return true; } + + ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); return false; } bool icsneo_openDevice(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } if(!device->device->open()) return false; @@ -123,10 +128,8 @@ bool icsneo_openDevice(const neodevice_t* device) { } bool icsneo_closeDevice(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } if(!device->device->close()) return false; @@ -144,65 +147,51 @@ bool icsneo_closeDevice(const neodevice_t* device) { } bool icsneo_isOpen(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } return device->device->isOpen(); } bool icsneo_goOnline(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } return device->device->goOnline(); } bool icsneo_goOffline(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } return device->device->goOffline(); } bool icsneo_isOnline(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } return device->device->isOnline(); } bool icsneo_enableMessagePolling(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } device->device->enableMessagePolling(); return true; } bool icsneo_disableMessagePolling(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } return device->device->disableMessagePolling(); } bool icsneo_getMessages(const neodevice_t* device, neomessage_t* messages, size_t* items, uint64_t timeout) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } if(items == nullptr) { ErrorManager::GetInstance().add(APIError::RequiredParameterNull); @@ -233,19 +222,15 @@ bool icsneo_getMessages(const neodevice_t* device, neomessage_t* messages, size_ } size_t icsneo_getPollingMessageLimit(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return 0; - } return device->device->getPollingMessageLimit(); } bool icsneo_setPollingMessageLimit(const neodevice_t* device, size_t newLimit) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } device->device->setPollingMessageLimit(newLimit); return true; @@ -258,10 +243,8 @@ bool icsneo_getProductName(const neodevice_t* device, char* str, size_t* maxLeng return false; } - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } std::string output = device->device->getType().toString(); @@ -303,55 +286,43 @@ bool icsneo_getProductNameForType(devicetype_t type, char* str, size_t* maxLengt } bool icsneo_settingsRefresh(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } return device->device->settings->refresh(); } bool icsneo_settingsApply(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } return device->device->settings->apply(); } bool icsneo_settingsApplyTemporary(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } return device->device->settings->apply(true); } bool icsneo_settingsApplyDefaults(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } return device->device->settings->applyDefaults(); } bool icsneo_settingsApplyDefaultsTemporary(const neodevice_t* device) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } return device->device->settings->applyDefaults(true); } size_t icsneo_settingsReadStructure(const neodevice_t* device, void* structure, size_t structureSize) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return 0; - } size_t readSize = device->device->settings->getSize(); if(structure == nullptr) // Structure size request @@ -379,10 +350,8 @@ size_t icsneo_settingsReadStructure(const neodevice_t* device, void* structure, // Not exported static bool icsneo_settingsWriteStructure(const neodevice_t* device, const void* structure, size_t structureSize) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } if(structure == nullptr) { ErrorManager::GetInstance().add(APIError::RequiredParameterNull); @@ -418,46 +387,36 @@ bool icsneo_settingsApplyStructureTemporary(const neodevice_t* device, const voi } int64_t icsneo_getBaudrate(const neodevice_t* device, uint16_t netid) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return -1; - } return device->device->settings->getBaudrateFor(netid); } bool icsneo_setBaudrate(const neodevice_t* device, uint16_t netid, int64_t newBaudrate) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } return device->device->settings->setBaudrateFor(netid, newBaudrate); } int64_t icsneo_getFDBaudrate(const neodevice_t* device, uint16_t netid) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return -1; - } return device->device->settings->getFDBaudrateFor(netid); } bool icsneo_setFDBaudrate(const neodevice_t* device, uint16_t netid, int64_t newBaudrate) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } return device->device->settings->setFDBaudrateFor(netid, newBaudrate); } bool icsneo_transmit(const neodevice_t* device, const neomessage_t* message) { - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } return device->device->transmit(CreateMessageFromNeoMessage(message)); } @@ -479,10 +438,8 @@ bool icsneo_describeDevice(const neodevice_t* device, char* str, size_t* maxLeng return false; } - if(!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } std::string output = device->device->describe(); @@ -519,10 +476,8 @@ bool icsneo_getErrors(neoerror_t* errors, size_t* size) { } bool icsneo_getDeviceErrors(const neodevice_t* device, neoerror_t* errors, size_t* size) { - if(device != nullptr && !icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return false; - } if(size == nullptr) { ErrorManager::GetInstance().add(APIError::RequiredParameterNull); @@ -563,10 +518,8 @@ void icsneo_discardAllErrors(void) { } void icsneo_discardDeviceErrors(const neodevice_t* device) { - if(device != nullptr && !icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); + if(!icsneo_isValidNeoDevice(device)) return; - } if(device == nullptr) icsneo::DiscardErrors(nullptr); // Discard errors not associated with a device @@ -608,14 +561,11 @@ bool icsneo_getSupportedDevices(devicetype_t* devices, size_t* count) { return true; } -extern bool DLLExport icsneo_getTimestampResolution(const neodevice_t* device, uint16_t* resolution) -{ - if (!icsneo_isValidNeoDevice(device)) { - ErrorManager::GetInstance().add(APIError::InvalidNeoDevice); +extern bool DLLExport icsneo_getTimestampResolution(const neodevice_t* device, uint16_t* resolution) { + if(!icsneo_isValidNeoDevice(device)) return false; - } - if (resolution == nullptr) { + if(resolution == nullptr) { ErrorManager::GetInstance().add(APIError::RequiredParameterNull); return false; } diff --git a/api/icsneocpp/error.cpp b/api/icsneocpp/error.cpp index e4ab5ef..2139e3e 100644 --- a/api/icsneocpp/error.cpp +++ b/api/icsneocpp/error.cpp @@ -88,6 +88,7 @@ static constexpr const char* ERROR_PACKET_CHECKSUM_ERROR = "There was a checksum static constexpr const char* ERROR_TRANSMIT_BUFFER_FULL = "The transmit buffer is full and the device is set to non-blocking."; 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_PACKET_DECODING = "The packet could not be decoded."; 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."; @@ -183,6 +184,8 @@ const char* APIError::DescriptionForType(ErrorType type) { return ERROR_PCAP_COULD_NOT_START; case PCAPCouldNotFindDevices: return ERROR_PCAP_COULD_NOT_FIND_DEVICES; + case PacketDecodingError: + return ERROR_PACKET_DECODING; // Other Errors case TooManyErrors: @@ -248,6 +251,7 @@ APIError::Severity APIError::SeverityForType(ErrorType type) { case DriverFailedToClose: case PacketChecksumError: case TransmitBufferFull: + case PacketDecodingError: // Other Errors case TooManyErrors: case Unknown: diff --git a/communication/decoder.cpp b/communication/decoder.cpp index 3bb8edd..05aa600 100644 --- a/communication/decoder.cpp +++ b/communication/decoder.cpp @@ -22,8 +22,10 @@ bool Decoder::decode(std::shared_ptr& result, const std::shared_ptrnetwork.getType()) { case Network::Type::Ethernet: result = HardwareEthernetPacket::DecodeToMessage(packet->data); - if(!result) + if(!result) { + err(APIError::PacketDecodingError); return false; // A nullptr was returned, the packet was not long enough to decode + } // Timestamps are in (resolution) ns increments since 1/1/2007 GMT 00:00:00.0000 // The resolution depends on the device @@ -33,13 +35,16 @@ bool Decoder::decode(std::shared_ptr& result, const std::shared_ptrdata.size() < 24) + if(packet->data.size() < 24) { + err(APIError::PacketDecodingError); return false; + } result = HardwareCANPacket::DecodeToMessage(packet->data); - if(!result) + if(!result) { + err(APIError::PacketDecodingError); return false; // A nullptr was returned, the packet was malformed - + } // Timestamps are in (resolution) ns increments since 1/1/2007 GMT 00:00:00.0000 // The resolution depends on the device result->timestamp *= timestampResolution; @@ -49,8 +54,10 @@ bool Decoder::decode(std::shared_ptr& result, const std::shared_ptrnetwork.getNetID()) { case Network::NetID::Reset_Status: { - if(packet->data.size() < sizeof(HardwareResetStatusPacket)) + if(packet->data.size() < sizeof(HardwareResetStatusPacket)) { + err(APIError::PacketDecodingError); return false; + } HardwareResetStatusPacket* data = (HardwareResetStatusPacket*)packet->data.data(); auto msg = std::make_shared(); @@ -111,7 +118,7 @@ bool Decoder::decode(std::shared_ptr& result, const std::shared_ptr& bytes, std::chrono::millisec bytes.resize(actuallyRead); - return actuallyRead > 0; + bool ret = actuallyRead > 0; + if(!ret) + err(APIError::FailedToRead); + return ret; } bool ICommunication::write(const std::vector& bytes) { @@ -46,9 +49,8 @@ bool ICommunication::write(const std::vector& bytes) { if(writeBlocks) { std::unique_lock lk(writeMutex); - if(writeQueue.size_approx() > writeQueueSize) { + if(writeQueue.size_approx() > writeQueueSize) writeCV.wait(lk); - } } else { if(writeQueue.size_approx() > writeQueueSize) { err(APIError::TransmitBufferFull); @@ -57,8 +59,8 @@ bool ICommunication::write(const std::vector& bytes) { } bool ret = writeQueue.enqueue(WriteOperation(bytes)); - if(!ret) { + if(!ret) err(APIError::Unknown); - } + return ret; } \ No newline at end of file diff --git a/device/device.cpp b/device/device.cpp index ae26b33..c16bffe 100644 --- a/device/device.cpp +++ b/device/device.cpp @@ -204,7 +204,9 @@ bool Device::close() { internalHandlerCallbackID = 0; - goOffline(); + if(isOnline()) + goOffline(); + return com->close(); } diff --git a/include/icsneo/api/error.h b/include/icsneo/api/error.h index b7aaf1d..077b985 100644 --- a/include/icsneo/api/error.h +++ b/include/icsneo/api/error.h @@ -79,6 +79,7 @@ public: TransmitBufferFull = 0x3005, PCAPCouldNotStart = 0x3102, PCAPCouldNotFindDevices = 0x3103, + PacketDecodingError = 0x3104, TooManyErrors = 0xFFFFFFFE, Unknown = 0xFFFFFFFF