diff --git a/src/modules/CMakeLists.txt b/src/modules/CMakeLists.txt index 0a89bcb..c3ca5e7 100644 --- a/src/modules/CMakeLists.txt +++ b/src/modules/CMakeLists.txt @@ -1,6 +1,6 @@ target_sources( firmware - PRIVATE protocol.cpp + PRIVATE crc.cpp buttons.cpp debouncer.cpp finda.cpp @@ -11,6 +11,7 @@ target_sources( motion.cpp movable_base.cpp permanent_storage.cpp + protocol.cpp pulley.cpp pulse_gen.cpp selector.cpp diff --git a/src/modules/crc.cpp b/src/modules/crc.cpp new file mode 100644 index 0000000..ed35d9c --- /dev/null +++ b/src/modules/crc.cpp @@ -0,0 +1,22 @@ +/// @file +#include "crc.h" + +#ifdef __AVR__ +#include +#endif + +namespace modules { +namespace crc { + +#ifdef __AVR__ +uint8_t CRC8::CCITT_update(uint8_t crc, uint8_t b) { + return _crc8_ccitt_update(crc, b); +} +#else +uint8_t CRC8::CCITT_update(uint8_t crc, uint8_t b) { + return CCITT_updateCX(crc, b); +} +#endif + +} // namespace crc +} // namespace modules diff --git a/src/modules/crc.h b/src/modules/crc.h new file mode 100644 index 0000000..cc8f06d --- /dev/null +++ b/src/modules/crc.h @@ -0,0 +1,43 @@ +/// @file +#pragma once +#include + +namespace modules { + +/// Contains all the necessary functions for computation of CRC +namespace crc { + +class CRC8 { +public: + /// Compute/update CRC8 CCIIT from 8bits. + /// Details: https://www.nongnu.org/avr-libc/user-manual/group__util__crc.html + static uint8_t CCITT_update(uint8_t crc, uint8_t b); + + static constexpr uint8_t CCITT_updateCX(uint8_t crc, uint8_t b) { + uint8_t data = crc ^ b; + for (uint8_t i = 0; i < 8; i++) { + if ((data & 0x80U) != 0) { + data <<= 1U; + data ^= 0x07U; + } else { + data <<= 1U; + } + } + return data; + } + + /// Compute/update CRC8 CCIIT from 16bits (convenience wrapper) + static constexpr uint8_t CCITT_updateW(uint8_t crc, uint16_t w) { + union U { + uint8_t b[2]; + uint16_t w; + explicit constexpr inline U(uint16_t w) + : w(w) {} + } u(w); + return CCITT_updateCX(CCITT_updateCX(crc, u.b[0]), u.b[1]); + } +}; + +} // namespace crc + +} // namespace modules diff --git a/src/modules/protocol.cpp b/src/modules/protocol.cpp index bd8e3d1..13fe598 100644 --- a/src/modules/protocol.cpp +++ b/src/modules/protocol.cpp @@ -9,7 +9,7 @@ // any of the running operation statuses: OID: [T|L|U|E|C|W|K][0-4] // P[0-9] : command being processed i.e. operation running, may contain a state number // E[0-9][0-9] : error 1-9 while doing a tool change -// F : operation finished - will be repeated to "Q" messages until a new command is issued +// F[0-9] : operation finished - will be repeated to "Q" messages until a new command is issued namespace modules { namespace protocol { @@ -49,6 +49,7 @@ DecodeStatus Protocol::DecodeRequest(uint8_t c) { requestMsg.code = (RequestMsgCodes)c; requestMsg.value = 0; requestMsg.value2 = 0; + requestMsg.crc8 = 0; rqState = (c == 'W') ? RequestStates::Address : RequestStates::Value; // prepare special automaton path for Write commands return DecodeStatus::NeedMoreData; default: @@ -61,9 +62,9 @@ DecodeStatus Protocol::DecodeRequest(uint8_t c) { requestMsg.value <<= 4U; requestMsg.value |= Char2Nibble(c); return DecodeStatus::NeedMoreData; - } else if (IsNewLine(c)) { - rqState = RequestStates::Code; - return DecodeStatus::MessageCompleted; + } else if (IsCRCSeparator(c)) { + rqState = RequestStates::CRC; + return DecodeStatus::NeedMoreData; } else { requestMsg.code = RequestMsgCodes::unknown; rqState = RequestStates::Error; @@ -87,14 +88,31 @@ DecodeStatus Protocol::DecodeRequest(uint8_t c) { requestMsg.value2 <<= 4U; requestMsg.value2 |= Char2Nibble(c); return DecodeStatus::NeedMoreData; - } else if (IsNewLine(c)) { - rqState = RequestStates::Code; - return DecodeStatus::MessageCompleted; + } else if (IsCRCSeparator(c)) { + rqState = RequestStates::CRC; + return DecodeStatus::NeedMoreData; } else { requestMsg.code = RequestMsgCodes::unknown; rqState = RequestStates::Error; return DecodeStatus::Error; } + case RequestStates::CRC: + if (IsHexDigit(c)) { + requestMsg.crc8 <<= 4U; + requestMsg.crc8 |= Char2Nibble(c); + return DecodeStatus::NeedMoreData; + } else if (IsNewLine(c)) { + // check CRC at this spot + if (requestMsg.crc8 != requestMsg.ComputeCRC8()) { + // CRC mismatch + requestMsg.code = RequestMsgCodes::unknown; + rqState = RequestStates::Error; + return DecodeStatus::Error; + } else { + rqState = RequestStates::Code; + return DecodeStatus::MessageCompleted; + } + } default: //case error: if (IsNewLine(c)) { rqState = RequestStates::Code; @@ -108,26 +126,24 @@ DecodeStatus Protocol::DecodeRequest(uint8_t c) { } uint8_t Protocol::EncodeRequest(const RequestMsg &msg, uint8_t *txbuff) { - uint8_t i = 1; txbuff[0] = (uint8_t)msg.code; - uint8_t v = msg.value >> 4; - if (v != 0) { // skip the first '0' if any - txbuff[i] = Nibble2Char(v); - ++i; - } - v = msg.value & 0xf; - txbuff[i] = Nibble2Char(v); - ++i; + uint8_t i = 1 + UInt8ToHex(msg.value, txbuff + 1); + + i += AppendCRC(msg.CRC(), txbuff + i); + txbuff[i] = '\n'; ++i; return i; - static_assert(4 <= MaxRequestSize(), "Request message length exceeded the maximum size, increase the magic constant in MaxRequestSize()"); + static_assert(7 <= MaxRequestSize(), "Request message length exceeded the maximum size, increase the magic constant in MaxRequestSize()"); } -uint8_t Protocol::EncodeWriteRequest(const RequestMsg &msg, uint16_t value2, uint8_t *txbuff) { +uint8_t Protocol::EncodeWriteRequest(uint8_t address, uint16_t value, uint8_t *txbuff) { + const RequestMsg msg(RequestMsgCodes::Write, address, value); uint8_t i = BeginEncodeRequest(msg, txbuff); // dump the value - i += Value2Hex(value2, txbuff + i); + i += UInt16ToHex(value, txbuff + i); + + i += AppendCRC(msg.CRC(), txbuff + i); txbuff[i] = '\n'; ++i; @@ -156,6 +172,8 @@ DecodeStatus Protocol::DecodeResponse(uint8_t c) { case 'R': responseMsg.request.code = (RequestMsgCodes)c; responseMsg.request.value = 0; + responseMsg.request.value2 = 0; + responseMsg.request.crc8 = 0; rspState = ResponseStates::RequestValue; return DecodeStatus::NeedMoreData; case 0x0a: @@ -200,9 +218,30 @@ DecodeStatus Protocol::DecodeResponse(uint8_t c) { responseMsg.paramValue <<= 4U; responseMsg.paramValue += Char2Nibble(c); return DecodeStatus::NeedMoreData; + } else if (IsCRCSeparator(c)) { + rspState = ResponseStates::CRC; + return DecodeStatus::NeedMoreData; + } else { + responseMsg.paramCode = ResponseMsgParamCodes::unknown; + rspState = ResponseStates::Error; + return DecodeStatus::Error; + } + case ResponseStates::CRC: + if (IsHexDigit(c)) { + responseMsg.request.crc8 <<= 4U; + responseMsg.request.crc8 += Char2Nibble(c); + return DecodeStatus::NeedMoreData; } else if (IsNewLine(c)) { - rspState = ResponseStates::RequestCode; - return DecodeStatus::MessageCompleted; + // check CRC at this spot + if (responseMsg.request.crc8 != responseMsg.ComputeCRC8()) { + // CRC mismatch + responseMsg.paramCode = ResponseMsgParamCodes::unknown; + rspState = ResponseStates::Error; + return DecodeStatus::Error; + } else { + rspState = ResponseStates::RequestCode; + return DecodeStatus::MessageCompleted; + } } else { responseMsg.paramCode = ResponseMsgParamCodes::unknown; rspState = ResponseStates::Error; @@ -220,65 +259,71 @@ DecodeStatus Protocol::DecodeResponse(uint8_t c) { } uint8_t Protocol::EncodeResponseCmdAR(const RequestMsg &msg, ResponseMsgParamCodes ar, uint8_t *txbuff) { - uint8_t i = BeginEncodeRequest(msg, txbuff); + const ResponseMsg rsp(msg, ar, 0); // this needs some cleanup @@TODO - check assembly how bad is it + uint8_t i = BeginEncodeRequest(rsp.request, txbuff); txbuff[i] = (uint8_t)ar; ++i; + i += AppendCRC(rsp.CRC(), txbuff + i); txbuff[i] = '\n'; ++i; return i; } uint8_t Protocol::EncodeResponseReadFINDA(const RequestMsg &msg, uint8_t findaValue, uint8_t *txbuff) { - // txbuff[0] = (uint8_t)msg.code; - // txbuff[1] = msg.value + '0'; - // txbuff[2] = ' '; - // txbuff[3] = (uint8_t)ResponseMsgParamCodes::Accepted; - // txbuff[4] = findaValue + '0'; - // txbuff[5] = '\n'; - // return 6; return EncodeResponseRead(msg, true, findaValue, txbuff); } uint8_t Protocol::EncodeResponseVersion(const RequestMsg &msg, uint16_t value, uint8_t *txbuff) { - // txbuff[0] = (uint8_t)msg.code; - // txbuff[1] = msg.value + '0'; - // txbuff[2] = ' '; - // txbuff[3] = (uint8_t)ResponseMsgParamCodes::Accepted; - // uint8_t *dst = txbuff + 4; - // dst += Value2Hex(value, dst); - // *dst = '\n'; - // return dst - txbuff + 1; return EncodeResponseRead(msg, true, value, txbuff); } uint8_t Protocol::EncodeResponseQueryOperation(const RequestMsg &msg, ResponseCommandStatus rcs, uint8_t *txbuff) { - txbuff[0] = (uint8_t)msg.code; - txbuff[1] = msg.value + '0'; - txbuff[2] = ' '; - txbuff[3] = (uint8_t)rcs.code; - uint8_t *dst = txbuff + 4; - dst += Value2Hex(rcs.value, dst); - *dst = '\n'; - return dst - txbuff + 1; + const ResponseMsg rsp(msg, rcs.code, rcs.value); + uint8_t i = BeginEncodeRequest(msg, txbuff); + txbuff[i] = (uint8_t)rsp.paramCode; + ++i; + i += UInt16ToHex(rsp.paramValue, txbuff + i); + i += AppendCRC(rsp.CRC(), txbuff + i); + txbuff[i] = '\n'; + return i + 1; } uint8_t Protocol::EncodeResponseRead(const RequestMsg &msg, bool accepted, uint16_t value2, uint8_t *txbuff) { + const ResponseMsg rsp(msg, + accepted ? ResponseMsgParamCodes::Accepted : ResponseMsgParamCodes::Rejected, + accepted ? value2 : 0 // be careful about this value for CRC computation - rejected status doesn't have any meaningful value which could be reconstructed from the textual form of the message + ); uint8_t i = BeginEncodeRequest(msg, txbuff); - if (accepted) { - txbuff[i] = (uint8_t)ResponseMsgParamCodes::Accepted; - ++i; - // dump the value - i += Value2Hex(value2, txbuff + i); - } else { - txbuff[i] = (uint8_t)ResponseMsgParamCodes::Rejected; - ++i; - } - txbuff[i] = '\n'; + txbuff[i] = (uint8_t)rsp.paramCode; ++i; - return i; + if (accepted) { + // dump the value + i += UInt16ToHex(value2, txbuff + i); + } + i += AppendCRC(rsp.CRC(), txbuff + i); + txbuff[i] = '\n'; + return i + 1; } -uint8_t Protocol::Value2Hex(uint16_t value, uint8_t *dst) { +uint8_t Protocol::UInt8ToHex(uint8_t value, uint8_t *dst) { + if (value == 0) { + *dst = '0'; + return 1; + } + + uint8_t v = value >> 4U; + uint8_t charsOut = 1; + if (v != 0) { // skip the first '0' if any + *dst = Nibble2Char(v); + ++dst; + charsOut = 2; + } + v = value & 0xfU; + *dst = Nibble2Char(v); + return charsOut; +} + +uint8_t Protocol::UInt16ToHex(uint16_t value, uint8_t *dst) { constexpr uint16_t topNibbleMask = 0xf000; if (value == 0) { *dst = '0'; @@ -299,20 +344,19 @@ uint8_t Protocol::Value2Hex(uint16_t value, uint8_t *dst) { return charsOut; } -uint8_t Protocol::BeginEncodeRequest(const RequestMsg &msg, uint8_t *txbuff) { - uint8_t i = 1; - txbuff[0] = (uint8_t)msg.code; - uint8_t v = msg.value >> 4U; - if (v != 0) { // skip the first '0' if any - txbuff[i] = Nibble2Char(v); - ++i; - } - v = msg.value & 0xfU; - txbuff[i] = Nibble2Char(v); - ++i; - txbuff[i] = ' '; +uint8_t Protocol::BeginEncodeRequest(const RequestMsg &msg, uint8_t *dst) { + dst[0] = (uint8_t)msg.code; + + uint8_t i = 1 + UInt8ToHex(msg.value, dst + 1); + + dst[i] = ' '; return i + 1; } +uint8_t Protocol::AppendCRC(uint8_t crc, uint8_t *dst) { + dst[0] = '*'; // reprap-style separator of CRC + return 1 + UInt8ToHex(crc, dst + 1); +} + } // namespace protocol } // namespace modules diff --git a/src/modules/protocol.h b/src/modules/protocol.h index fa9a9ec..e157ecd 100644 --- a/src/modules/protocol.h +++ b/src/modules/protocol.h @@ -1,6 +1,7 @@ /// @file protocol.h #pragma once #include +#include "crc.h" namespace modules { @@ -48,12 +49,40 @@ struct RequestMsg { uint8_t value; ///< value of the request message or address of variable to read/write uint16_t value2; ///< in case or write messages - value to be written into the register + /// CRC8 check - please note we abuse this byte for CRC of ResponseMsgs as well. + /// The crc8 byte itself is not added into the CRC computation (obviously ;) ) + /// Beware - adding any members of this data structure may need changing the way CRC is being computed! + uint8_t crc8; + + constexpr uint8_t ComputeCRC8() const { + uint8_t crc = 0; + crc = modules::crc::CRC8::CCITT_updateCX(0, (uint8_t)code); + crc = modules::crc::CRC8::CCITT_updateCX(crc, value); + crc = modules::crc::CRC8::CCITT_updateCX(crc, value2); + return crc; + } + /// @param code of the request message /// @param value of the request message inline constexpr RequestMsg(RequestMsgCodes code, uint8_t value) : code(code) , value(value) - , value2(0) {} + , value2(0) + , crc8(ComputeCRC8()) { + } + + /// Intended for write requests + /// @param code of the request message ('W') + /// @param address of the register + /// @param value to write into the register + inline constexpr RequestMsg(RequestMsgCodes code, uint8_t address, uint16_t value) + : code(code) + , value(address) + , value2(value) + , crc8(ComputeCRC8()) { + } + + constexpr uint8_t CRC() const { return crc8; } }; /// A response message - responses are being sent from the MMU into the printer as a response to a request message. @@ -62,13 +91,24 @@ struct ResponseMsg { ResponseMsgParamCodes paramCode; ///< code of the parameter uint16_t paramValue; ///< value of the parameter + constexpr uint8_t ComputeCRC8() const { + uint8_t crc = request.ComputeCRC8(); + crc = modules::crc::CRC8::CCITT_updateCX(crc, (uint8_t)paramCode); + crc = modules::crc::CRC8::CCITT_updateW(crc, paramValue); + return crc; + } + /// @param request the source request message this response is a reply to /// @param paramCode code of the parameter /// @param paramValue value of the parameter inline constexpr ResponseMsg(RequestMsg request, ResponseMsgParamCodes paramCode, uint16_t paramValue) : request(request) , paramCode(paramCode) - , paramValue(paramValue) {} + , paramValue(paramValue) { + this->request.crc8 = ComputeCRC8(); + } + + constexpr uint8_t CRC() const { return request.crc8; } }; /// Combined commandStatus and its value into one data structure (optimization purposes) @@ -116,11 +156,15 @@ public: /// Encodes Write request message msg into txbuff memory /// It is expected the txbuff is large enough to fit the message /// @returns number of bytes written into txbuff - static uint8_t EncodeWriteRequest(const RequestMsg &msg, uint16_t value2, uint8_t *txbuff); + static uint8_t EncodeWriteRequest(uint8_t address, uint16_t value, uint8_t *txbuff); /// @returns the maximum byte length necessary to encode a request message /// Beneficial in case of pre-allocating a buffer for enconding a RequestMsg. - static constexpr uint8_t MaxRequestSize() { return 4; } + static constexpr uint8_t MaxRequestSize() { return 13; } + + /// @returns the maximum byte length necessary to encode a response message + /// Beneficial in case of pre-allocating a buffer for enconding a ResponseMsg. + static constexpr uint8_t MaxResponseSize() { return 14; } /// Encode generic response Command Accepted or Rejected /// @param msg source request message for this response @@ -183,6 +227,7 @@ private: Value, ///< expecting code value Address, ///< expecting address for Write command WriteValue, ///< value to be written (Write command) + CRC, ///< CRC Error ///< automaton in error state }; @@ -194,6 +239,7 @@ private: RequestValue, ///< expecting code value ParamCode, ///< expecting param code ParamValue, ///< expecting param value + CRC, ///< expecting CRC value Error ///< automaton in error state }; @@ -206,6 +252,9 @@ private: static constexpr bool IsDigit(uint8_t c) { return c >= '0' && c <= '9'; } + static constexpr bool IsCRCSeparator(uint8_t c) { + return c == '*'; + } static constexpr bool IsHexDigit(uint8_t c) { return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f'); } @@ -260,9 +309,14 @@ private: } /// @returns number of characters written - static uint8_t Value2Hex(uint16_t value, uint8_t *dst); + static uint8_t UInt8ToHex(uint8_t value, uint8_t *dst); - static uint8_t BeginEncodeRequest(const RequestMsg &msg, uint8_t *txbuff); + /// @returns number of characters written + static uint8_t UInt16ToHex(uint16_t value, uint8_t *dst); + + static uint8_t BeginEncodeRequest(const RequestMsg &msg, uint8_t *dst); + + static uint8_t AppendCRC(uint8_t crc, uint8_t *dst); }; } // namespace protocol diff --git a/tests/unit/modules/protocol/test_protocol.cpp b/tests/unit/modules/protocol/test_protocol.cpp index 8e079ee..62d2608 100644 --- a/tests/unit/modules/protocol/test_protocol.cpp +++ b/tests/unit/modules/protocol/test_protocol.cpp @@ -4,7 +4,8 @@ #include "protocol.h" #include -using Catch::Matchers::Equals; +// some safe margin for the buffer +using TXBuff = std::array; TEST_CASE("protocol::Char2Nibble2Char", "[protocol]") { uint8_t character, value; @@ -37,7 +38,7 @@ TEST_CASE("protocol::Value2Hex", "[protocol]") { for (uint32_t v = 0; v < 0xffff; ++v) { constexpr size_t buffSize = 5; uint8_t tmp[buffSize]; - uint8_t chars = mp::Protocol::Value2Hex((uint16_t)v, tmp); + uint8_t chars = mp::Protocol::UInt16ToHex((uint16_t)v, tmp); if (v < 0x10) { REQUIRE(chars == 1); } else if (v < 0x100) { @@ -52,43 +53,75 @@ TEST_CASE("protocol::Value2Hex", "[protocol]") { } } +std::string MakeCRC(uint8_t crc) { + char tmp[5]; // *ffn are 4 bytes + \0 at the end + snprintf(tmp, 5, "*%x\n", (unsigned)crc); + return std::string(tmp); +} + +std::string MakeCRC(const std::string_view src) { + // this code basically needs parsing of the input text and compute the CRC from the parsed data + REQUIRE(src.size() > 1); + // code + uint8_t crc = modules::crc::CRC8::CCITT_updateCX(0, src[0]); + // scan hex value + uint16_t rqValue = std::stoul(src.data() + 1, nullptr, 16); + crc = modules::crc::CRC8::CCITT_updateW(crc, rqValue); + if (!src[2]) + return MakeCRC(crc); // eof + // [2] is a space + REQUIRE(src.size() > 3); + crc = modules::crc::CRC8::CCITT_updateCX(crc, src[3]); // param code + if (!src[4]) + return MakeCRC(crc); // eof + REQUIRE(src.size() > 4); + uint16_t paramValue = std::stoul(src.data() + 4, nullptr, 16); + crc = modules::crc::CRC8::CCITT_updateW(crc, paramValue); + return MakeCRC(crc); +} + TEST_CASE("protocol::EncodeRequests", "[protocol]") { mp::RequestMsgCodes code; uint8_t value; - std::tie(code, value) = GENERATE( - std::make_tuple(mp::RequestMsgCodes::Button, 0), - std::make_tuple(mp::RequestMsgCodes::Button, 1), - std::make_tuple(mp::RequestMsgCodes::Button, 2), - std::make_tuple(mp::RequestMsgCodes::Cut, 0), - std::make_tuple(mp::RequestMsgCodes::Eject, 0), - std::make_tuple(mp::RequestMsgCodes::Finda, 0), - std::make_tuple(mp::RequestMsgCodes::Home, 0), - std::make_tuple(mp::RequestMsgCodes::Load, 0), - std::make_tuple(mp::RequestMsgCodes::Load, 1), - std::make_tuple(mp::RequestMsgCodes::Load, 2), - std::make_tuple(mp::RequestMsgCodes::Load, 3), - std::make_tuple(mp::RequestMsgCodes::Load, 4), - std::make_tuple(mp::RequestMsgCodes::Mode, 0), - std::make_tuple(mp::RequestMsgCodes::Mode, 1), - std::make_tuple(mp::RequestMsgCodes::Query, 0), - std::make_tuple(mp::RequestMsgCodes::Reset, 0), - std::make_tuple(mp::RequestMsgCodes::Tool, 0), - std::make_tuple(mp::RequestMsgCodes::Tool, 1), - std::make_tuple(mp::RequestMsgCodes::Tool, 2), - std::make_tuple(mp::RequestMsgCodes::Tool, 3), - std::make_tuple(mp::RequestMsgCodes::Tool, 4), - std::make_tuple(mp::RequestMsgCodes::Unload, 0), - std::make_tuple(mp::RequestMsgCodes::Version, 0), - std::make_tuple(mp::RequestMsgCodes::Version, 1), - std::make_tuple(mp::RequestMsgCodes::Version, 2), - std::make_tuple(mp::RequestMsgCodes::unknown, 0)); + std::string str; + std::tie(code, value, str) = GENERATE( + std::make_tuple(mp::RequestMsgCodes::Button, 0, "B0"), + std::make_tuple(mp::RequestMsgCodes::Button, 1, "B1"), + std::make_tuple(mp::RequestMsgCodes::Button, 2, "B2"), + std::make_tuple(mp::RequestMsgCodes::Cut, 0, "K0"), + std::make_tuple(mp::RequestMsgCodes::Eject, 0, "E0"), + std::make_tuple(mp::RequestMsgCodes::Finda, 0, "P0"), + std::make_tuple(mp::RequestMsgCodes::Home, 0, "H0"), + std::make_tuple(mp::RequestMsgCodes::Load, 0, "L0"), + std::make_tuple(mp::RequestMsgCodes::Load, 1, "L1"), + std::make_tuple(mp::RequestMsgCodes::Load, 2, "L2"), + std::make_tuple(mp::RequestMsgCodes::Load, 3, "L3"), + std::make_tuple(mp::RequestMsgCodes::Load, 4, "L4"), + std::make_tuple(mp::RequestMsgCodes::Mode, 0, "M0"), + std::make_tuple(mp::RequestMsgCodes::Mode, 1, "M1"), + std::make_tuple(mp::RequestMsgCodes::Query, 0, "Q0"), + std::make_tuple(mp::RequestMsgCodes::Reset, 0, "X0"), + std::make_tuple(mp::RequestMsgCodes::Tool, 0, "T0"), + std::make_tuple(mp::RequestMsgCodes::Tool, 1, "T1"), + std::make_tuple(mp::RequestMsgCodes::Tool, 2, "T2"), + std::make_tuple(mp::RequestMsgCodes::Tool, 3, "T3"), + std::make_tuple(mp::RequestMsgCodes::Tool, 4, "T4"), + std::make_tuple(mp::RequestMsgCodes::Unload, 0, "U0"), + std::make_tuple(mp::RequestMsgCodes::Version, 0, "S0"), + std::make_tuple(mp::RequestMsgCodes::Version, 1, "S1"), + std::make_tuple(mp::RequestMsgCodes::Version, 2, "S2") + /*std::make_tuple(mp::RequestMsgCodes::unknown, 0, "\00")*/); - std::array txbuff; + TXBuff txbuff; + mp::RequestMsg msg(code, value); + str.append(MakeCRC(msg.CRC())); - CHECK(mp::Protocol::EncodeRequest(mp::RequestMsg(code, value), txbuff.data()) == 3); - CHECK(txbuff[0] == (uint8_t)code); - CHECK(txbuff[1] == value + '0'); - CHECK(txbuff[2] == '\n'); + uint8_t packetSize = mp::Protocol::EncodeRequest(msg, txbuff.data()); + REQUIRE(packetSize < mp::Protocol::MaxRequestSize()); + CHECK(packetSize <= 6); + for (uint8_t i = 0; i < packetSize; ++i) { + CHECK(str[i] == txbuff[i]); + } } TEST_CASE("protocol::EncodeResponseCmdAR", "[protocol]") { @@ -143,57 +176,67 @@ TEST_CASE("protocol::EncodeResponseCmdAR", "[protocol]") { auto responseStatus = GENERATE(mp::ResponseMsgParamCodes::Accepted, mp::ResponseMsgParamCodes::Rejected, mp::ResponseMsgParamCodes::Button); - std::array txbuff; + TXBuff txbuff; + mp::ResponseMsg rsp(requestMsg, responseStatus, 0); uint8_t msglen = mp::Protocol::EncodeResponseCmdAR(requestMsg, responseStatus, txbuff.data()); + REQUIRE(msglen < mp::Protocol::MaxResponseSize()); + std::string crc = MakeCRC(rsp.CRC()); if (requestMsg.value < 10) { - CHECK(msglen == 5); + CHECK(msglen == 4 + crc.size()); CHECK(txbuff[0] == (uint8_t)requestMsg.code); CHECK(txbuff[1] == requestMsg.value + '0'); CHECK(txbuff[2] == ' '); CHECK(txbuff[3] == (uint8_t)responseStatus); - CHECK(txbuff[4] == '\n'); + CHECK(txbuff[4] == '*'); + CHECK(txbuff[msglen - 1] == '\n'); } else if (requestMsg.value < 16) { - CHECK(msglen == 5); + CHECK(msglen == 4 + crc.size()); CHECK(txbuff[0] == (uint8_t)requestMsg.code); CHECK(txbuff[1] == requestMsg.value - 10 + 'a'); CHECK(txbuff[2] == ' '); CHECK(txbuff[3] == (uint8_t)responseStatus); - CHECK(txbuff[4] == '\n'); + CHECK(txbuff[4] == '*'); + CHECK(txbuff[msglen - 1] == '\n'); } else if (requestMsg.value < 0x1a) { - CHECK(msglen == 6); + CHECK(msglen == 5 + crc.size()); CHECK(txbuff[0] == (uint8_t)requestMsg.code); CHECK(txbuff[1] == (requestMsg.value >> 4U) + '0'); CHECK(txbuff[2] == (requestMsg.value & 0xfU) + '0'); CHECK(txbuff[3] == ' '); CHECK(txbuff[4] == (uint8_t)responseStatus); - CHECK(txbuff[5] == '\n'); + CHECK(txbuff[5] == '*'); + CHECK(txbuff[msglen - 1] == '\n'); } else { - CHECK(msglen == 6); + CHECK(msglen == 5 + crc.size()); CHECK(txbuff[0] == (uint8_t)requestMsg.code); CHECK(txbuff[1] == (requestMsg.value >> 4U) - 10 + 'a'); CHECK(txbuff[2] == (requestMsg.value & 0xfU) - 10 + 'a'); CHECK(txbuff[3] == ' '); CHECK(txbuff[4] == (uint8_t)responseStatus); - CHECK(txbuff[5] == '\n'); + CHECK(txbuff[5] == '*'); + CHECK(txbuff[msglen - 1] == '\n'); } } TEST_CASE("protocol::EncodeResponseReadFINDA", "[protocol]") { auto requestMsg = mp::RequestMsg(mp::RequestMsgCodes::Finda, 0); - uint8_t findaStatus = GENERATE(0, 1); - std::array txbuff; + TXBuff txbuff; + mp::ResponseMsg rsp(requestMsg, mp::ResponseMsgParamCodes::Accepted, findaStatus); + std::string crc = MakeCRC(rsp.CRC()); uint8_t msglen = mp::Protocol::EncodeResponseReadFINDA(requestMsg, findaStatus, txbuff.data()); + REQUIRE(msglen < mp::Protocol::MaxResponseSize()); - CHECK(msglen == 6); + CHECK(msglen == 5 + crc.size()); CHECK(txbuff[0] == (uint8_t)requestMsg.code); CHECK(txbuff[1] == requestMsg.value + '0'); CHECK(txbuff[2] == ' '); CHECK(txbuff[3] == (uint8_t)mp::ResponseMsgParamCodes::Accepted); CHECK(txbuff[4] == findaStatus + '0'); - CHECK(txbuff[5] == '\n'); + CHECK(txbuff[5] == '*'); + CHECK(txbuff[msglen - 1] == '\n'); } TEST_CASE("protocol::EncodeResponseVersion", "[protocol]") { @@ -201,19 +244,22 @@ TEST_CASE("protocol::EncodeResponseVersion", "[protocol]") { auto requestMsg = mp::RequestMsg(mp::RequestMsgCodes::Version, versionQueryType); for (uint32_t version = 0; version < 0xffff; ++version) { - std::array txbuff; + TXBuff txbuff; + mp::ResponseMsg rsp(requestMsg, mp::ResponseMsgParamCodes::Accepted, (uint16_t)version); uint8_t msglen = mp::Protocol::EncodeResponseVersion(requestMsg, (uint16_t)version, txbuff.data()); + REQUIRE(msglen < mp::Protocol::MaxResponseSize()); + std::string crc = MakeCRC(rsp.CRC()); - CHECK(msglen <= 9); CHECK(txbuff[0] == (uint8_t)requestMsg.code); CHECK(txbuff[1] == requestMsg.value + '0'); CHECK(txbuff[2] == ' '); CHECK(txbuff[3] == (uint8_t)mp::ResponseMsgParamCodes::Accepted); - char chk[6]; - int chars = snprintf(chk, 6, "%x\n", version); - REQUIRE(chars < 6); + char chk[10]; + int chars = snprintf(chk, sizeof(chk), "%x", version); + REQUIRE(chars < 10); std::string chks(chk, chk + chars); + chks.append(crc); std::string vers((const char *)(&txbuff[4]), (const char *)(&txbuff[msglen])); REQUIRE(chks == vers); @@ -298,11 +344,13 @@ TEST_CASE("protocol::EncodeResponseQueryOperation", "[protocol]") { ErrorCode::TMC_OVER_TEMPERATURE_WARN, ErrorCode::TMC_OVER_TEMPERATURE_ERROR); - std::array txbuff; + TXBuff txbuff; uint16_t encodedParamValue = responseStatus == mp::ResponseMsgParamCodes::Error ? (uint16_t)error : (uint16_t)value; + mp::ResponseMsg rsp(requestMsg, mp::ResponseMsgParamCodes(responseStatus), encodedParamValue); uint8_t msglen = mp::Protocol::EncodeResponseQueryOperation(requestMsg, mp::ResponseCommandStatus(responseStatus, encodedParamValue), txbuff.data()); + REQUIRE(msglen < mp::Protocol::MaxResponseSize()); CHECK(msglen <= txbuff.size()); CHECK(txbuff[0] == (uint8_t)requestMsg.code); @@ -311,33 +359,36 @@ TEST_CASE("protocol::EncodeResponseQueryOperation", "[protocol]") { CHECK(txbuff[3] == (uint8_t)responseStatus); char chk[6]; - int chars = snprintf(chk, 6, "%x\n", encodedParamValue); + int chars = snprintf(chk, 6, "%x", encodedParamValue); REQUIRE(chars < 6); std::string chks(chk, chk + chars); + chks.append(MakeCRC(rsp.ComputeCRC8())); std::string txs((const char *)(&txbuff[4]), (const char *)(&txbuff[msglen])); - REQUIRE(chk == txs); + REQUIRE(chks == txs); CHECK(txbuff[msglen - 1] == '\n'); } TEST_CASE("protocol::DecodeRequest", "[protocol]") { mp::Protocol p; - const char *rxbuff = GENERATE( - "B0\n", "B1\n", "B2\n", - "E0\n", "E1\n", "E2\n", "E3\n", "E4\n", - "H0\n", "H1\n", "H2\n", - "F0\n", "F1\n", - "f0\n", "f1\n", - "K0\n", - "L0\n", "L1\n", "L2\n", "L3\n", "L4\n", - "M0\n", "M1\n", - "P0\n", - "Q0\n", - "S0\n", "S1\n", "S2\n", "S3\n", - "T0\n", "T1\n", "T2\n", "T3\n", - "U0\n", - "X0\n"); + std::string rxbuff = GENERATE( + "B0", "B1", "B2", + "E0", "E1", "E2", "E3", "E4", + "H0", "H1", "H2", + "F0", "F1", + "f0", "f1", + "K0", + "L0", "L1", "L2", "L3", "L4", + "M0", "M1", + "P0", + "Q0", + "S0", "S1", "S2", "S3", + "T0", "T1", "T2", "T3", "T4", + "U0", + "X0"); - const char *pc = rxbuff; + rxbuff.append(MakeCRC(rxbuff.c_str())); + + const char *pc = rxbuff.c_str(); for (;;) { uint8_t c = *pc++; if (c == 0) { @@ -355,15 +406,16 @@ TEST_CASE("protocol::DecodeRequest", "[protocol]") { const mp::RequestMsg &rq = p.GetRequestMsg(); CHECK((uint8_t)rq.code == rxbuff[0]); CHECK(rq.value == rxbuff[1] - '0'); + // CRC is checked implicitly - the protocol would not return MessageCompleted if CRC mismatched } TEST_CASE("protocol::DecodeResponseReadFinda", "[protocol]") { mp::Protocol p; - const char *rxbuff = GENERATE( - "P0 A0\n", - "P0 A1\n"); - - const char *pc = rxbuff; + std::string rxbuff = GENERATE( + "P0 A0", + "P0 A1"); + rxbuff.append(MakeCRC(rxbuff.c_str())); + const char *pc = rxbuff.c_str(); for (;;) { uint8_t c = *pc++; if (c == 0) { @@ -383,6 +435,7 @@ TEST_CASE("protocol::DecodeResponseReadFinda", "[protocol]") { CHECK(rsp.request.value == rxbuff[1] - '0'); CHECK((uint8_t)rsp.paramCode == rxbuff[3]); CHECK((uint8_t)rsp.paramValue == rxbuff[4] - '0'); + // CRC is checked implicitly - the protocol would not return MessageCompleted if CRC mismatched } TEST_CASE("protocol::DecodeResponseQueryOperation", "[protocol][.]") { @@ -401,7 +454,8 @@ TEST_CASE("protocol::DecodeResponseQueryOperation", "[protocol][.]") { std::string rxbuff(cmdReference); rxbuff += ' '; rxbuff += status; - rxbuff += '\n'; + + rxbuff.append(MakeCRC(rxbuff.c_str())); const char *pc = rxbuff.c_str(); for (;;) { @@ -467,6 +521,16 @@ TEST_CASE("protocol::DecodeRequestErrors", "[protocol]") { CHECK(p.GetRequestMsg().code == mp::RequestMsgCodes::unknown); CHECK(p.DecodeRequest('\n') == mp::DecodeStatus::MessageCompleted); CHECK(p.GetRequestMsg().code == mp::RequestMsgCodes::unknown); + + const char _B0_bad_crc[] = "B0*aa\n"; + CHECK(p.DecodeRequest(_B0_bad_crc[0]) == mp::DecodeStatus::NeedMoreData); + CHECK(p.GetRequestMsg().code == mp::RequestMsgCodes::Button); + CHECK(p.DecodeRequest(_B0_bad_crc[1]) == mp::DecodeStatus::NeedMoreData); + CHECK(p.DecodeRequest(_B0_bad_crc[2]) == mp::DecodeStatus::NeedMoreData); + CHECK(p.DecodeRequest(_B0_bad_crc[3]) == mp::DecodeStatus::NeedMoreData); + CHECK(p.DecodeRequest(_B0_bad_crc[4]) == mp::DecodeStatus::NeedMoreData); + CHECK(p.DecodeRequest(_B0_bad_crc[5]) == mp::DecodeStatus::Error); // bad crc + CHECK(p.GetRequestMsg().code == mp::RequestMsgCodes::unknown); } TEST_CASE("protocol::DecodeResponseErrors", "[protocol]") { @@ -497,15 +561,15 @@ TEST_CASE("protocol::DecodeResponseErrors", "[protocol]") { TEST_CASE("protocol::WriteRequest", "[protocol]") { // write requests need special handling - std::array txbuff; + TXBuff txbuff; mp::Protocol p; for (uint8_t address = 0; address < 255; ++address) { for (uint32_t value2 = 2; value2 < 0x10000; ++value2) { - mp::RequestMsg msg(mp::RequestMsgCodes::Write, address); - uint8_t bytes = mp::Protocol::EncodeWriteRequest(msg, (uint16_t)value2, txbuff.data()); + uint8_t msglen = mp::Protocol::EncodeWriteRequest(address, (uint16_t)value2, txbuff.data()); + REQUIRE(msglen < mp::Protocol::MaxRequestSize()); p.ResetRequestDecoder(); - for (uint8_t i = 0; i < bytes; ++i) { + for (uint8_t i = 0; i < msglen; ++i) { p.DecodeRequest(txbuff[i]); } @@ -517,14 +581,15 @@ TEST_CASE("protocol::WriteRequest", "[protocol]") { } TEST_CASE("protocol::ReadRequest", "[protocol]") { - std::array txbuff; + TXBuff txbuff; mp::Protocol p; for (uint16_t address = 0; address <= 255; ++address) { mp::RequestMsg msg(mp::RequestMsgCodes::Read, (uint8_t)address); - uint8_t bytes = mp::Protocol::EncodeRequest(msg, txbuff.data()); + uint8_t msglen = mp::Protocol::EncodeRequest(msg, txbuff.data()); + REQUIRE(msglen < mp::Protocol::MaxRequestSize()); p.ResetRequestDecoder(); - for (uint8_t i = 0; i < bytes; ++i) { + for (uint8_t i = 0; i < msglen; ++i) { p.DecodeRequest(txbuff[i]); } @@ -534,16 +599,17 @@ TEST_CASE("protocol::ReadRequest", "[protocol]") { } TEST_CASE("protocol::ReadResponse", "[protocol]") { - std::array txbuff; + TXBuff txbuff; mp::Protocol p; for (uint8_t address = 0; address < 255; ++address) { for (uint32_t value2 = 2; value2 < 0x10000; ++value2) { for (uint8_t ar = 0; ar <= 1; ++ar) { mp::RequestMsg msg(mp::RequestMsgCodes::Read, address); - uint8_t bytes = mp::Protocol::EncodeResponseRead(msg, ar != 0, value2, txbuff.data()); + uint8_t msglen = mp::Protocol::EncodeResponseRead(msg, ar != 0, value2, txbuff.data()); + REQUIRE(msglen < mp::Protocol::MaxResponseSize()); p.ResetResponseDecoder(); - for (uint8_t i = 0; i < bytes; ++i) { + for (uint8_t i = 0; i < msglen; ++i) { p.DecodeResponse(txbuff[i]); }