Refactor bridge packet handling to use common magic number and size constants

Author: jbrazio

src/helpers/bridges/BridgeBase.h

@@ -21,6 +21,26 @@ class BridgeBase : public AbstractBridge {
 public:
   virtual ~BridgeBase() = default;
 
+  /**
+   * @brief Common magic number used by all bridge implementations for packet identification
+   * 
+   * This magic number is placed at the beginning of bridge packets to identify
+   * them as mesh bridge packets and provide frame synchronization.
+   */
+  static constexpr uint16_t BRIDGE_PACKET_MAGIC = 0xC03E;
+
+  /**
+   * @brief Common field sizes used by bridge implementations
+   * 
+   * These constants define the size of common packet fields used across bridges.
+   * BRIDGE_MAGIC_SIZE is used by all bridges for packet identification.
+   * BRIDGE_LENGTH_SIZE is used by bridges that need explicit length fields (like RS232).
+   * BRIDGE_CHECKSUM_SIZE is used by all bridges for Fletcher-16 checksums.
+   */
+  static constexpr uint16_t BRIDGE_MAGIC_SIZE = sizeof(BRIDGE_PACKET_MAGIC);
+  static constexpr uint16_t BRIDGE_LENGTH_SIZE = sizeof(uint16_t);
+  static constexpr uint16_t BRIDGE_CHECKSUM_SIZE = sizeof(uint16_t);
+
 protected:
   /** Packet manager for allocating and queuing mesh packets */
   mesh::PacketManager *_mgr;

src/helpers/bridges/ESPNowBridge.cpp

@@ -66,7 +66,7 @@ void ESPNowBridge::xorCrypt(uint8_t *data, size_t len) {
 
 void ESPNowBridge::onDataRecv(const uint8_t *mac, const uint8_t *data, int32_t len) {
   // Ignore packets that are too small to contain header + checksum
-  if (len < (MAGIC_HEADER_SIZE + CHECKSUM_SIZE)) {
+  if (len < (BRIDGE_MAGIC_SIZE + BRIDGE_CHECKSUM_SIZE)) {
 #if MESH_PACKET_LOGGING
     Serial.printf("%s: ESPNOW BRIDGE: RX packet too small, len=%d\n", getLogDateTime(), len);
 #endif
@@ -83,7 +83,7 @@ void ESPNowBridge::onDataRecv(const uint8_t *mac, const uint8_t *data, int32_t l
 
   // Check packet header magic
   uint16_t received_magic = (data[0] << 8) | data[1];
-  if (received_magic != ESPNOW_HEADER_MAGIC) {
+  if (received_magic != BRIDGE_PACKET_MAGIC) {
 #if MESH_PACKET_LOGGING
     Serial.printf("%s: ESPNOW BRIDGE: RX invalid magic 0x%04X\n", getLogDateTime(), received_magic);
 #endif
@@ -92,17 +92,17 @@ void ESPNowBridge::onDataRecv(const uint8_t *mac, const uint8_t *data, int32_t l
 
   // Make a copy we can decrypt
   uint8_t decrypted[MAX_ESPNOW_PACKET_SIZE];
-  const size_t encryptedDataLen = len - MAGIC_HEADER_SIZE;
-  memcpy(decrypted, data + MAGIC_HEADER_SIZE, encryptedDataLen);
+  const size_t encryptedDataLen = len - BRIDGE_MAGIC_SIZE;
+  memcpy(decrypted, data + BRIDGE_MAGIC_SIZE, encryptedDataLen);
 
   // Try to decrypt (checksum + payload)
   xorCrypt(decrypted, encryptedDataLen);
 
   // Validate checksum
   uint16_t received_checksum = (decrypted[0] << 8) | decrypted[1];
-  const size_t payloadLen = encryptedDataLen - CHECKSUM_SIZE;
+  const size_t payloadLen = encryptedDataLen - BRIDGE_CHECKSUM_SIZE;
 
-  if (!validateChecksum(decrypted + CHECKSUM_SIZE, payloadLen, received_checksum)) {
+  if (!validateChecksum(decrypted + BRIDGE_CHECKSUM_SIZE, payloadLen, received_checksum)) {
     // Failed to decrypt - likely from a different network
 #if MESH_PACKET_LOGGING
     Serial.printf("%s: ESPNOW BRIDGE: RX checksum mismatch, rcv=0x%04X\n", getLogDateTime(),
@@ -119,7 +119,7 @@ void ESPNowBridge::onDataRecv(const uint8_t *mac, const uint8_t *data, int32_t l
   mesh::Packet *pkt = _instance->_mgr->allocNew();
   if (!pkt) return;
 
-  if (pkt->readFrom(decrypted + CHECKSUM_SIZE, payloadLen)) {
+  if (pkt->readFrom(decrypted + BRIDGE_CHECKSUM_SIZE, payloadLen)) {
     _instance->onPacketReceived(pkt);
   } else {
     _instance->_mgr->free(pkt);
@@ -161,11 +161,11 @@ void ESPNowBridge::onPacketTransmitted(mesh::Packet *packet) {
     uint8_t buffer[MAX_ESPNOW_PACKET_SIZE];
 
     // Write magic header (2 bytes)
-    buffer[0] = (ESPNOW_HEADER_MAGIC >> 8) & 0xFF;
-    buffer[1] = ESPNOW_HEADER_MAGIC & 0xFF;
+    buffer[0] = (BRIDGE_PACKET_MAGIC >> 8) & 0xFF;
+    buffer[1] = BRIDGE_PACKET_MAGIC & 0xFF;
 
     // Write packet payload starting after magic header and checksum
-    const size_t packetOffset = MAGIC_HEADER_SIZE + CHECKSUM_SIZE;
+    const size_t packetOffset = BRIDGE_MAGIC_SIZE + BRIDGE_CHECKSUM_SIZE;
     memcpy(buffer + packetOffset, sizingBuffer, meshPacketLen);
 
     // Calculate and add checksum (only of the payload)
@@ -174,10 +174,10 @@ void ESPNowBridge::onPacketTransmitted(mesh::Packet *packet) {
     buffer[3] = checksum & 0xFF;        // Low byte
 
     // Encrypt payload and checksum (not including magic header)
-    xorCrypt(buffer + MAGIC_HEADER_SIZE, meshPacketLen + CHECKSUM_SIZE);
+    xorCrypt(buffer + BRIDGE_MAGIC_SIZE, meshPacketLen + BRIDGE_CHECKSUM_SIZE);
 
     // Total packet size: magic header + checksum + payload
-    const size_t totalPacketSize = MAGIC_HEADER_SIZE + CHECKSUM_SIZE + meshPacketLen;
+    const size_t totalPacketSize = BRIDGE_MAGIC_SIZE + BRIDGE_CHECKSUM_SIZE + meshPacketLen;
 
     // Broadcast using ESP-NOW
     uint8_t broadcastAddress[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF };

src/helpers/bridges/ESPNowBridge.h

@@ -65,14 +65,7 @@ class ESPNowBridge : public BridgeBase {
   /**
    * Size constants for packet parsing
    */
-  static const size_t MAGIC_HEADER_SIZE = 2;
-  static const size_t CHECKSUM_SIZE = 2;
-  static const size_t MAX_PAYLOAD_SIZE = MAX_ESPNOW_PACKET_SIZE - (MAGIC_HEADER_SIZE + CHECKSUM_SIZE);
-
-  /**
-   * Magic bytes to identify ESPNowBridge packets
-   */
-  static const uint16_t ESPNOW_HEADER_MAGIC = 0xC03E;
+  static const size_t MAX_PAYLOAD_SIZE = MAX_ESPNOW_PACKET_SIZE - (BRIDGE_MAGIC_SIZE + BRIDGE_CHECKSUM_SIZE);
 
   /** Buffer for receiving ESP-NOW packets */
   uint8_t _rx_buffer[MAX_ESPNOW_PACKET_SIZE];

src/helpers/bridges/RS232Bridge.cpp

@@ -52,8 +52,8 @@ void RS232Bridge::onPacketTransmitted(mesh::Packet *packet) {
     }
 
     // Build packet header
-    buffer[0] = (SERIAL_PKT_MAGIC >> 8) & 0xFF; // Magic high byte
-    buffer[1] = SERIAL_PKT_MAGIC & 0xFF;        // Magic low byte
+    buffer[0] = (BRIDGE_PACKET_MAGIC >> 8) & 0xFF; // Magic high byte
+    buffer[1] = BRIDGE_PACKET_MAGIC & 0xFF;        // Magic low byte
     buffer[2] = (len >> 8) & 0xFF;              // Length high byte
     buffer[3] = len & 0xFF;                     // Length low byte
 
@@ -77,14 +77,14 @@ void RS232Bridge::loop() {
 
     if (_rx_buffer_pos < 2) {
       // Waiting for magic word
-      if ((_rx_buffer_pos == 0 && b == ((SERIAL_PKT_MAGIC >> 8) & 0xFF)) ||
-          (_rx_buffer_pos == 1 && b == (SERIAL_PKT_MAGIC & 0xFF))) {
+      if ((_rx_buffer_pos == 0 && b == ((BRIDGE_PACKET_MAGIC >> 8) & 0xFF)) ||
+          (_rx_buffer_pos == 1 && b == (BRIDGE_PACKET_MAGIC & 0xFF))) {
         _rx_buffer[_rx_buffer_pos++] = b;
       } else {
         // Invalid magic byte, reset and start over
         _rx_buffer_pos = 0;
         // Check if this byte could be the start of a new magic word
-        if (b == ((SERIAL_PKT_MAGIC >> 8) & 0xFF)) {
+        if (b == ((BRIDGE_PACKET_MAGIC >> 8) & 0xFF)) {
           _rx_buffer[_rx_buffer_pos++] = b;
         }
       }

src/helpers/bridges/RS232Bridge.h

@@ -113,21 +113,11 @@ class RS232Bridge : public BridgeBase {
    * Total overhead: 6 bytes
    */
 
-  /** Magic number to identify start of RS232Bridge packets */
-  static constexpr uint16_t SERIAL_PKT_MAGIC = 0xC03E;
-
-  /**
-   * Size constants for packet parsing and construction
-   */
-  static constexpr uint16_t MAGIC_HEADER_SIZE = 2;
-  static constexpr uint16_t LENGTH_FIELD_SIZE = 2;
-  static constexpr uint16_t CHECKSUM_SIZE = 2;
-
   /**
    * @brief The total overhead of the serial protocol in bytes.
    * Includes: MAGIC_WORD (2) + LENGTH (2) + CHECKSUM (2) = 6 bytes
    */
-  static constexpr uint16_t SERIAL_OVERHEAD = MAGIC_HEADER_SIZE + LENGTH_FIELD_SIZE + CHECKSUM_SIZE;
+  static constexpr uint16_t SERIAL_OVERHEAD = BRIDGE_MAGIC_SIZE + BRIDGE_LENGTH_SIZE + BRIDGE_CHECKSUM_SIZE;
 
   /**
    * @brief The maximum size of a complete packet on the serial line.

variants/lilygo_tlora_v2_1/platformio.ini

@@ -198,7 +198,7 @@ build_flags =
   -D MAX_NEIGHBOURS=8
   -D WITH_ESPNOW_BRIDGE=1
   -D WITH_ESPNOW_BRIDGE_SECRET='"shared-secret"'
-;  -D MESH_PACKET_LOGGING=1
+  -D MESH_PACKET_LOGGING=1
 ;  -D MESH_DEBUG=1
 ;  -D CORE_DEBUG_LEVEL=3
 lib_deps =