From 57b05968a67ff385ad6b6353d7744aa20da7763c Mon Sep 17 00:00:00 2001
From: Balazs Racz <balazs.racz@gmail.com>
Date: Sat, 10 Jul 2021 13:07:45 +0200
Subject: [PATCH] Adds checksum verification to the DccDecode class. (#550)

* Adds code to verify the XOR and the CRC8 checksum.

* Up max payload.
Fix length related CRC checksum.

* Prints the correct expected DLC in case of an error.
---
 src/dcc/DccDebug.cxx |  6 +++++-
 src/dcc/Receiver.hxx | 51 +++++++++++++++++++++++++++++++++++---------
 src/dcc/packet.h     |  6 +++---
 3 files changed, 49 insertions(+), 14 deletions(-)

diff --git a/src/dcc/DccDebug.cxx b/src/dcc/DccDebug.cxx
index 7564b4372..e0b1ccbc0 100644
--- a/src/dcc/DccDebug.cxx
+++ b/src/dcc/DccDebug.cxx
@@ -250,12 +250,16 @@ string packet_to_string(const DCCPacket &pkt, bool bin_payload)
     }
     else
     {
-        options += StringPrintf(" [bad dlc, exp %u, actual %u]", ofs, pkt.dlc);
+        options += StringPrintf(" [bad dlc, exp %u, actual %u]", ofs+1, pkt.dlc);
         while (ofs < pkt.dlc)
         {
             options += StringPrintf(" 0x%02x", pkt.payload[ofs++]);
         }
     }
+    if (pkt.packet_header.csum_error)
+    {
+        options += " [csum err]";
+    }
     return options;
 }
 
diff --git a/src/dcc/Receiver.hxx b/src/dcc/Receiver.hxx
index 667b8efa9..9ee2407fc 100644
--- a/src/dcc/Receiver.hxx
+++ b/src/dcc/Receiver.hxx
@@ -43,6 +43,7 @@
 #include "freertos/can_ioctl.h"
 #include "freertos_drivers/common/SimpleLog.hxx"
 #include "dcc/packet.h"
+#include "utils/Crc.hxx"
 
 // If defined, collects samples of timing and state into a ring buffer.
 //#define DCC_DECODER_DEBUG
@@ -67,7 +68,7 @@ public:
     }
 
     /// Internal states of the decoding state machine.
-    enum State
+    enum State : uint8_t
     {
         UNKNOWN,             // 0
         DCC_PREAMBLE,        // 1
@@ -134,10 +135,9 @@ public:
                 }
                 if (timings_[MM_PREAMBLE].match(value) && pkt_)
                 {
-                    parseCount_ = 1 << 2;
-                    pkt_->dlc = 0;
-                    pkt_->payload[0] = 0;
+                    clear_packet();
                     pkt_->packet_header.is_marklin = 1;
+                    parseCount_ = 1 << 2;
                     parseState_ = MM_DATA;
                     havePacket_ = true;
                 }
@@ -163,12 +163,12 @@ public:
                 {
                     parseState_ = DCC_DATA;
                     parseCount_ = 1 << 7;
+                    xorState_ = 0;
+                    crcState_.init();
                     if (pkt_)
                     {
-                        havePacket_ = true;
                         clear_packet();
-                        pkt_->dlc = 0;
-                        pkt_->payload[0] = 0;
+                        havePacket_ = true;
                         pkt_->packet_header.skip_ec = 1;
                     }
                     else
@@ -212,6 +212,16 @@ public:
                         // end of packet 1 bit.
                         if (havePacket_)
                         {
+                            if (checkCRC_ && (pkt_->dlc > 6) &&
+                                !crcState_.check_ok())
+                            {
+                                pkt_->packet_header.csum_error = 1;
+                            }
+                            xorState_ ^= pkt_->payload[pkt_->dlc];
+                            if (xorState_)
+                            {
+                                pkt_->packet_header.csum_error = 1;
+                            }
                             pkt_->dlc++;
                         }
                         parseState_ = DCC_MAYBE_CUTOUT;
@@ -235,6 +245,17 @@ public:
                         // end of byte zero bit. Packet is not finished yet.
                         if (havePacket_)
                         {
+                            xorState_ ^= pkt_->payload[pkt_->dlc];
+                            if ((pkt_->dlc == 0) &&
+                                (pkt_->payload[0] == 254 ||
+                                    pkt_->payload[0] == 253))
+                            {
+                                checkCRC_ = true;
+                            }
+                            if (checkCRC_)
+                            {
+                                crcState_.update16(pkt_->payload[pkt_->dlc]);
+                            }
                             pkt_->dlc++;
                             if (pkt_->dlc >= DCC_PACKET_MAX_PAYLOAD)
                             {
@@ -345,11 +366,17 @@ public:
 
 private:
     /// Counter that works through bit patterns.
-    uint32_t parseCount_ = 0;
+    uint8_t parseCount_ = 0;
+    /// True if we have storage in the right time for the current packet.
+    uint8_t havePacket_ : 1;
+    /// True if we need to check CRC
+    uint8_t checkCRC_ : 1;
+    /// Checksum state for XOR checksum.
+    uint8_t xorState_;
+    /// Checksum state for CRC8 checksum.
+    Crc8DallasMaxim crcState_;
     /// State machine for parsing.
     State parseState_ = UNKNOWN;
-    /// True if we have storage in the right time for the current packet.
-    bool havePacket_;
     /// Storage for the current packet.
     DCCPacket* pkt_ = nullptr;
 
@@ -359,6 +386,10 @@ private:
         pkt_->header_raw_data = 0;
         pkt_->dlc = 0;
         pkt_->feedback_key = 0;
+        pkt_->payload[0] = 0;
+        checkCRC_ = 0;
+        xorState_ = 0;
+        crcState_.init();
     }
 
     /// Represents the timing of a half-wave of the digital track signal.
diff --git a/src/dcc/packet.h b/src/dcc/packet.h
index fa1bc5d36..66775034d 100644
--- a/src/dcc/packet.h
+++ b/src/dcc/packet.h
@@ -42,7 +42,7 @@ extern "C" {
 #endif
 
 /** Maximum number of payload bytes. */
-#define DCC_PACKET_MAX_PAYLOAD (6)
+#define DCC_PACKET_MAX_PAYLOAD (32)
 /** Send this speed step to emergency-stop the locomotive. */
 #define DCC_PACKET_EMERGENCY_STOP (0xFFFF)
 /** Send this speed step to switch direction of the locomotive. Only used
@@ -72,8 +72,8 @@ typedef struct dcc_packet
         /// The packet will be sent 1 + rept_count times to the wire. default:
         /// 0.
         uint8_t rept_count : 2;
-        /// reserved for future use.
-        uint8_t reserved : 1;
+        /// 1 if there was a checksum error in this packet.
+        uint8_t csum_error : 1;
     };
 
     /// Specifies the meaning of the command byte for meta-commands to send.