From dd99f82324b3852f198d3cf0a5b31605b7437562 Mon Sep 17 00:00:00 2001 From: Paul Hollinsky Date: Thu, 18 Oct 2018 14:06:58 -0400 Subject: [PATCH] Allow the decoder to fail --- communication/communication.cpp | 12 +++--- communication/decoder.cpp | 41 +++++++++++---------- communication/include/decoder.h | 2 +- communication/multichannelcommunication.cpp | 5 ++- device/neovifire2/include/neovifire2eth.h | 5 ++- device/radgalaxy/include/radgalaxy.h | 5 ++- device/radstar2/include/radstar2eth.h | 5 ++- 7 files changed, 45 insertions(+), 30 deletions(-) diff --git a/communication/communication.cpp b/communication/communication.cpp index 4ccda4c..4b525e4 100644 --- a/communication/communication.cpp +++ b/communication/communication.cpp @@ -54,7 +54,7 @@ bool Communication::sendCommand(Command cmd, std::vector arguments) { std::vector packet; if(!encoder->encode(packet, cmd, arguments)) return false; - + return sendPacket(packet); } @@ -126,10 +126,12 @@ void Communication::readTask() { if(impl->readWait(readBytes)) { if(packetizer->input(readBytes)) { for(auto& packet : packetizer->output()) { - auto msg = decoder->decodePacket(packet); - //std::cout << "Got packet for " << msg->network << ", calling " << messageCallbacks.size() << " callbacks" << std::endl; - for(auto& cb : messageCallbacks) { // We might have closed while reading or processing - if(!closing) { + std::shared_ptr msg; + if(!decoder->decode(msg, packet)) + continue; // TODO Report an error to the user, we failed to decode this packet + + for(auto& cb : messageCallbacks) { + if(!closing) { // We might have closed while reading or processing cb.second.callIfMatch(msg); } } diff --git a/communication/decoder.cpp b/communication/decoder.cpp index c45d78e..0c4401a 100644 --- a/communication/decoder.cpp +++ b/communication/decoder.cpp @@ -15,11 +15,11 @@ uint64_t Decoder::GetUInt64FromLEBytes(uint8_t* bytes) { return ret; } -std::shared_ptr Decoder::decodePacket(const std::shared_ptr& packet) { +bool Decoder::decode(std::shared_ptr& result, const std::shared_ptr& packet) { switch(packet->network.getType()) { case Network::Type::CAN: { if(packet->data.size() < 24) - break; // We would read garbage when interpereting the data + return false; HardwareCANPacket* data = (HardwareCANPacket*)packet->data.data(); auto msg = std::make_shared(); @@ -76,9 +76,7 @@ std::shared_ptr Decoder::decodePacket(const std::shared_ptr& pa length = 64; break; default: - length = 0; - // TODO Flag an error - break; + return false; } } @@ -97,13 +95,14 @@ std::shared_ptr Decoder::decodePacket(const std::shared_ptr& pa } } - return msg; + result = msg; + return true; } case Network::Type::Internal: { switch(packet->network.getNetID()) { case Network::NetID::Reset_Status: { if(packet->data.size() < sizeof(HardwareResetStatusPacket)) - break; + return false; HardwareResetStatusPacket* data = (HardwareResetStatusPacket*)packet->data.data(); auto msg = std::make_shared(); @@ -125,10 +124,11 @@ std::shared_ptr Decoder::decodePacket(const std::shared_ptr& pa msg->cmTooBig = data->status.cm_too_big; msg->hidUsbState = data->status.hidUsbState; msg->fpgaUsbState = data->status.fpgaUsbState; - return msg; + result = msg; + return true; } default: - break; + return false; } } default: @@ -147,17 +147,17 @@ std::shared_ptr Decoder::decodePacket(const std::shared_ptr& pa msg->hasPCBSerial = packet->data.size() >= 31; if(msg->hasPCBSerial) memcpy(msg->pcbSerial, packet->data.data() + 15, sizeof(msg->pcbSerial)); - return msg; + result = msg; + return true; } - break; default: auto msg = std::make_shared(); msg->network = packet->network; msg->command = Command(packet->data[0]); msg->data.insert(msg->data.begin(), packet->data.begin() + 1, packet->data.end()); - return msg; + result = msg; + return true; } - break; } case Network::NetID::RED_OLDFORMAT: { /* So-called "old format" messages are a "new style, long format" wrapper around the old short messages. @@ -169,18 +169,19 @@ std::shared_ptr Decoder::decodePacket(const std::shared_ptr& pa */ uint16_t length = packet->data[0] | (packet->data[1] << 8); packet->network = Network(packet->data[2] & 0xF); - //std::cout << "Got an old format packet, decoding against " << packet->network << std::endl; packet->data.erase(packet->data.begin(), packet->data.begin() + 3); if(packet->data.size() != length) packet->data.resize(length); - return decodePacket(packet); + return decode(result, packet); } } - break; } - auto msg = std::make_shared(); - msg->network = packet->network; - msg->data = packet->data; - return msg; + return false; + + // auto msg = std::make_shared(); + // msg->network = packet->network; + // msg->data = packet->data; + // result = msg; + // return true; } \ No newline at end of file diff --git a/communication/include/decoder.h b/communication/include/decoder.h index 7ea4e3d..45105b2 100644 --- a/communication/include/decoder.h +++ b/communication/include/decoder.h @@ -16,7 +16,7 @@ namespace icsneo { class Decoder { public: static uint64_t GetUInt64FromLEBytes(uint8_t* bytes); - std::shared_ptr decodePacket(const std::shared_ptr& message); + bool decode(std::shared_ptr& result, const std::shared_ptr& packet); private: typedef uint16_t icscm_bitfield; diff --git a/communication/multichannelcommunication.cpp b/communication/multichannelcommunication.cpp index 00c5cf0..6bdf8a3 100644 --- a/communication/multichannelcommunication.cpp +++ b/communication/multichannelcommunication.cpp @@ -102,7 +102,10 @@ void MultiChannelCommunication::readTask() { if(packetizer->input(payloadBytes)) { for(auto& packet : packetizer->output()) { - auto msg = decoder->decodePacket(packet); + std::shared_ptr msg; + if(!decoder->decode(msg, packet)) + continue; // TODO Report an error to the user, we failed to decode this packet + for(auto& cb : messageCallbacks) { // We might have closed while reading or processing if(!closing) { cb.second.callIfMatch(msg); diff --git a/device/neovifire2/include/neovifire2eth.h b/device/neovifire2/include/neovifire2eth.h index 7fbf918..7c3301b 100644 --- a/device/neovifire2/include/neovifire2eth.h +++ b/device/neovifire2/include/neovifire2eth.h @@ -31,7 +31,10 @@ public: for(auto& payload : foundDev.discoveryPackets) packetizer->input(payload); for(auto& packet : packetizer->output()) { - auto msg = decoder->decodePacket(packet); + std::shared_ptr msg; + if(!decoder->decode(msg, packet)) + continue; // We failed to decode this packet + if(!msg || msg->network.getNetID() != Network::NetID::Main51) continue; // Not a message we care about auto sn = std::dynamic_pointer_cast(msg); diff --git a/device/radgalaxy/include/radgalaxy.h b/device/radgalaxy/include/radgalaxy.h index 3c39f64..9b70b6b 100644 --- a/device/radgalaxy/include/radgalaxy.h +++ b/device/radgalaxy/include/radgalaxy.h @@ -42,7 +42,10 @@ public: for(auto& payload : foundDev.discoveryPackets) packetizer->input(payload); for(auto& packet : packetizer->output()) { - auto msg = decoder->decodePacket(packet); + std::shared_ptr msg; + if(!decoder->decode(msg, packet)) + continue; // We failed to decode this packet + if(!msg || msg->network.getNetID() != Network::NetID::Main51) continue; // Not a message we care about auto sn = std::dynamic_pointer_cast(msg); diff --git a/device/radstar2/include/radstar2eth.h b/device/radstar2/include/radstar2eth.h index 2f709e1..1bcdc76 100644 --- a/device/radstar2/include/radstar2eth.h +++ b/device/radstar2/include/radstar2eth.h @@ -35,7 +35,10 @@ public: for(auto& payload : foundDev.discoveryPackets) packetizer->input(payload); for(auto& packet : packetizer->output()) { - auto msg = decoder->decodePacket(packet); + std::shared_ptr msg; + if(!decoder->decode(msg, packet)) + continue; // We failed to decode this packet + if(!msg || msg->network.getNetID() != Network::NetID::Main51) continue; // Not a message we care about auto sn = std::dynamic_pointer_cast(msg);