Fix computation of CRC for Write requests (non-zero value2)

src/modules/protocol.cpp

@@ -259,7 +259,15 @@ DecodeStatus Protocol::DecodeResponse(uint8_t c) {
 }
 
 uint8_t Protocol::EncodeResponseCmdAR(const RequestMsg &msg, ResponseMsgParamCodes ar, uint8_t *txbuff) {
-    const ResponseMsg rsp(msg, ar, 0); // this needs some cleanup @@TODO - check assembly how bad is it
+    // BEWARE:
+    // ResponseMsg rsp(RequestMsg(msg.code, msg.value), ar, 0);
+    // ... is NOT the same as:
+    // ResponseMsg rsp(msg, ar, 0);
+    // ... because of the usually unused parameter value2 (which only comes non-zero in write requests).
+    // It took me a few hours to find out why the CRC from the MMU never matched all the other sides (unit tests and the MK3S)
+    // It is because this was the only place where the original request kept its value2 non-zero.
+    // In the response, we must make sure value2 is actually zero unless being sent along with it (which is not right now)
+    const ResponseMsg rsp(RequestMsg(msg.code, msg.value), 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;

tests/unit/application/test_application.cpp

@@ -2,6 +2,7 @@
 #include "application.h"
 #include <stdint.h>
 #include "../modules/stubs/stub_serial.h"
+#include "modules/protocol.h"
 
 TEST_CASE("application::ReportVersion", "[application]") {
     modules::serial::ClearRX();
@@ -11,3 +12,22 @@ TEST_CASE("application::ReportVersion", "[application]") {
 
     REQUIRE(modules::serial::tx == "S0 A2*37\n");
 }
+
+TEST_CASE("application::WriteRegister", "[application]") {
+    modules::serial::ClearRX();
+    modules::serial::ClearTX();
+
+    application.ReportWriteRegister(mp::RequestMsg(mp::RequestMsgCodes::Write, 0x1b, 2));
+
+    REQUIRE(modules::serial::tx == "W1b A*c2\n");
+
+    {
+        modules::protocol::Protocol p;
+        // verify decoding of the message - it seems we are having issues with it on the MK3 side for unknown reason
+        // The MMU sends: W1c A*f0\n for some very strange reason, even though the unit tests compute a different CRC with the same piece of code!
+        // Found it, see explanation in Protocol::EncodeResponseCmdAR...
+        for (uint8_t i = 0; i < modules::serial::tx.size(); ++i) {
+            CHECK(p.DecodeResponse(modules::serial::tx[i]) != modules::protocol::DecodeStatus::Error);
+        }
+    }
+}