From 1d25c87c57ddca51eb0abbad272b5d45951c64e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Br=C3=A1zio?= Date: Mon, 8 Sep 2025 11:15:28 +0100 Subject: [PATCH] Refactor bridge packet handling to use common magic number and size constants --- src/helpers/bridges/BridgeBase.h | 20 +++++++++++++++++++ src/helpers/bridges/ESPNowBridge.cpp | 24 +++++++++++------------ src/helpers/bridges/ESPNowBridge.h | 9 +-------- src/helpers/bridges/RS232Bridge.cpp | 10 +++++----- src/helpers/bridges/RS232Bridge.h | 12 +----------- variants/lilygo_tlora_v2_1/platformio.ini | 2 +- 6 files changed, 40 insertions(+), 37 deletions(-) diff --git a/src/helpers/bridges/BridgeBase.h b/src/helpers/bridges/BridgeBase.h index 2aff23069..c1764ae3a 100644 --- a/src/helpers/bridges/BridgeBase.h +++ b/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; diff --git a/src/helpers/bridges/ESPNowBridge.cpp b/src/helpers/bridges/ESPNowBridge.cpp index baf683caa..a02f804e3 100644 --- a/src/helpers/bridges/ESPNowBridge.cpp +++ b/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 }; diff --git a/src/helpers/bridges/ESPNowBridge.h b/src/helpers/bridges/ESPNowBridge.h index d9962c72f..b43f1744f 100644 --- a/src/helpers/bridges/ESPNowBridge.h +++ b/src/helpers/bridges/ESPNowBridge.h @@ -65,14 +65,7 @@ private: /** * 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]; diff --git a/src/helpers/bridges/RS232Bridge.cpp b/src/helpers/bridges/RS232Bridge.cpp index 39a0968ac..b209a6da9 100644 --- a/src/helpers/bridges/RS232Bridge.cpp +++ b/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; } } diff --git a/src/helpers/bridges/RS232Bridge.h b/src/helpers/bridges/RS232Bridge.h index 32fad17fc..3b09de759 100644 --- a/src/helpers/bridges/RS232Bridge.h +++ b/src/helpers/bridges/RS232Bridge.h @@ -113,21 +113,11 @@ private: * 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. diff --git a/variants/lilygo_tlora_v2_1/platformio.ini b/variants/lilygo_tlora_v2_1/platformio.ini index aa957fba1..d9ff17008 100644 --- a/variants/lilygo_tlora_v2_1/platformio.ini +++ b/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 =