Keep ESP-NOW TX seen keys instead of duplicate packet snapshots

src/helpers/SimpleMeshTables.h

@@ -53,6 +53,15 @@ inline bool routeHashPrefixesOverlap(const uint8_t* a, uint8_t a_len,
 
 class SimpleMeshTables : public mesh::MeshTables {
 public:
+  // A queued transport can retire an unsent admission after its Packet pool
+  // entry is gone. Keep the complete hash and its separate ACK-cache domain.
+  struct SeenKey {
+    uint8_t hash[MAX_HASH_SIZE] = {};
+    bool dedicated_ack = false;
+  };
+  static_assert(sizeof(SeenKey) == MAX_HASH_SIZE + 1,
+                "A seen key must not retain packet data or alignment padding");
+
   // Recent rows belong to the mesh loop, including CLI requests marshaled from
   // network tasks. They must not be read or changed concurrently by an ISR or
   // worker. Byte access avoids unaligned uint32_t loads in the compact rows;
@@ -411,26 +420,32 @@ public:
     }
   }
 
-  void clear(const mesh::Packet* packet) override {
-    if (isDedicatedAckPacket(packet)) {
-      uint8_t hash[MAX_HASH_SIZE];
-      packet->calculatePacketHash(hash);
-      clearAckHash(hash);
+  SeenKey seenKey(const mesh::Packet& packet) const {
+    SeenKey key;
+    packet.calculatePacketHash(key.hash);
+    key.dedicated_ack = isDedicatedAckPacket(&packet);
+    return key;
+  }
+
+  void clearSeenKey(const SeenKey& key) {
+    if (key.dedicated_ack) {
+      clearAckHash(key.hash);
       return;
     }
 
-    uint8_t hash[MAX_HASH_SIZE];
-    packet->calculatePacketHash(hash);
-
     uint8_t* sp = _hashes;
     for (int i = 0; i < MAX_PACKET_HASHES; i++, sp += MAX_HASH_SIZE) {
-      if (memcmp(hash, sp, MAX_HASH_SIZE) == 0) {
+      if (memcmp(key.hash, sp, MAX_HASH_SIZE) == 0) {
         memset(sp, 0, MAX_HASH_SIZE);
         break;
       }
     }
   }
 
+  void clear(const mesh::Packet* packet) override {
+    clearSeenKey(seenKey(*packet));
+  }
+
   uint32_t getNumDirectDups() const { return _direct_dups; }
   uint32_t getNumFloodDups() const { return _flood_dups; }
 

src/helpers/bridges/ESPNowBridge.cpp

@@ -304,7 +304,8 @@ void ESPNowBridge::end() {
   uint8_t tx_index = _tx_tail;
   for (uint8_t i = 0; i < _tx_count; ++i) {
     const QueuedTransmit& queued_tx = _tx_queue[tx_index];
-    if (!queued_tx.started && !queued_tx.diagnostic) _seen_packets.clear(&queued_tx.packet);
+    if (!queued_tx.started && !queued_tx.diagnostic)
+      _seen_packets.clearSeenKey(queued_tx.seen_key);
     tx_index = static_cast<uint8_t>((tx_index + 1) % TX_QUEUE_DEPTH);
   }
   portENTER_CRITICAL(&_tx_mux);
@@ -385,12 +386,16 @@ bool ESPNowBridge::queueTransmitFrames(
     }
   }
 
+  // Hash the caller-owned snapshot before entering the callback-shared queue
+  // lock. SHA work never extends an ESP-NOW critical section.
+  SimpleMeshTables::SeenKey seen_key;
+  if (!diagnostic) seen_key = _seen_packets.seenKey(packet);
   bool queued = false;
   portENTER_CRITICAL(&_tx_mux);
   if (_tx_count < TX_QUEUE_DEPTH) {
     QueuedTransmit& queued_tx = _tx_queue[_tx_head];
     queued_tx.frames = frames;
-    if (!diagnostic) queued_tx.packet = packet;
+    queued_tx.seen_key = seen_key;
     queued_tx.next_frame = 0;
     queued_tx.started = false;
     queued_tx.start_retries = 0;
@@ -417,7 +422,7 @@ void ESPNowBridge::pumpTransmitQueue() {
   bool completed = false;
   int completed_status = ESP_NOW_SEND_SUCCESS;
   bool clear_unsent_seen = false;
-  mesh::Packet unsent_packet;
+  SimpleMeshTables::SeenKey unsent_key;
   uint32_t dropped = 0;
   const uint32_t now = millis();
   bool timed_out = false;
@@ -527,7 +532,7 @@ void ESPNowBridge::pumpTransmitQueue() {
         _tx_retry_at = now + TX_RETRY_DELAY_MS;
       } else {
         if (!queued_tx.started && !queued_tx.diagnostic) {
-          unsent_packet = queued_tx.packet;
+          unsent_key = queued_tx.seen_key;
           clear_unsent_seen = true;
         }
         _tx_tail = static_cast<uint8_t>((_tx_tail + 1) % TX_QUEUE_DEPTH);
@@ -539,7 +544,7 @@ void ESPNowBridge::pumpTransmitQueue() {
     portEXIT_CRITICAL(&_tx_mux);
     // Admission marks the packet to suppress duplicate queue entries. Restore
     // the prior behavior when even its first esp_now_send() cannot start.
-    if (clear_unsent_seen) _seen_packets.clear(&unsent_packet);
+    if (clear_unsent_seen) _seen_packets.clearSeenKey(unsent_key);
     BRIDGE_DEBUG_PRINTLN("TX failed to start, error=%d\n", (int)result);
   } else {
     portENTER_CRITICAL(&_tx_mux);

src/helpers/bridges/ESPNowBridge.h

@@ -104,12 +104,15 @@ private:
   // instead of transmitting an orphan fragment.
   struct QueuedTransmit {
     mesh::espnow::ESPNowRawFrames frames;
-    mesh::Packet packet;
+    SimpleMeshTables::SeenKey seen_key;
     uint8_t next_frame;
     bool started;
     uint8_t start_retries;
     bool diagnostic;
   };
+  static_assert(sizeof(QueuedTransmit) <= sizeof(mesh::espnow::ESPNowRawFrames) +
+                    sizeof(SimpleMeshTables::SeenKey) + 8,
+                "ESP-NOW TX must retain encoded frames without a second packet copy");
   QueuedTransmit _tx_queue[TX_QUEUE_DEPTH];
   portMUX_TYPE _tx_mux = portMUX_INITIALIZER_UNLOCKED;
   volatile uint8_t _tx_head;

test/fixtures/espnow_wifi_handoff.cpp

@@ -168,10 +168,10 @@ public:
   int _tx_mux=0, _tx_head=0, _tx_tail=0, _tx_count=0, _tx_waiting=0, _tx_callback_done=0;
   int _tx_callback_status=0, _tx_dropped=0, _tx_dropped_reported=0;
   mesh::espnow::ESPNowRawReassembler _raw_reassembler;
-  struct QueuedTransmit { bool started=false, diagnostic=false; int packet=0; };
+  struct QueuedTransmit { bool started=false, diagnostic=false; int seen_key=0; };
   static constexpr int TX_QUEUE_DEPTH=2;
   QueuedTransmit _tx_queue[TX_QUEUE_DEPTH];
-  struct { void clear(const int*) {} } _seen_packets;
+  struct { void clearSeenKey(const int&) {} } _seen_packets;
   static void recv_cb() {}
   static void send_cb() {}
   uint32_t _tx_started_at = 0, _tx_retry_at = 0;

test/test_bridge_recovery.py

@@ -91,11 +91,15 @@ struct NodePrefs {
  char bridge_secret[32]="fleet-test";
 };
 struct Seen {
+ struct SeenKey {uint8_t hash[8]={};bool dedicated_ack=false;};
  std::vector<uint8_t> ids;
+ SeenKey seenKey(const mesh::Packet& p)const{SeenKey key;key.hash[0]=p.data[0];return key;}
+ void clearSeenKey(const SeenKey& key){ids.erase(std::remove(ids.begin(),ids.end(),key.hash[0]),ids.end());}
  bool wasSeen(const mesh::Packet* p){return std::find(ids.begin(),ids.end(),p->data[0])!=ids.end();}
  void markSeen(const mesh::Packet* p){if(!wasSeen(p))ids.push_back(p->data[0]);}
  void clear(const mesh::Packet* p){ids.erase(std::remove(ids.begin(),ids.end(),p->data[0]),ids.end());}
 };
+using SimpleMeshTables=Seen;
 class BridgeBase {
 public:
 @INGRESS_HOOK@
@@ -196,6 +200,71 @@ class BridgeRecoveryTests(unittest.TestCase):
                     result = subprocess.run([str(exe)], capture_output=True, text=True)
                     self.assertEqual(result.returncode, 0, result.stderr)
 
+    def test_seen_keys_keep_all_six_frames_and_survive_caller_packet_reuse(self):
+        self.run_fixture(r'''
+int main(){
+ Backend backend;backend.wifi=true;mesh::wireless::control().begin(backend);
+ sdk_initialized=sdk_started=true;sdk_mode=WIFI_STA;
+ NodePrefs prefs;mesh::PacketManager mgr;mesh::RTCClock rtc;
+ ESPNowBridge bridge(&prefs,&mgr,&rtc);bridge.begin();
+ std::array<mesh::Packet,6> originals;
+ std::array<mesh::Packet,6> reusable;
+ for(unsigned i=0;i<6;++i){
+  originals[i]=packet(uint8_t(0x11+i),255);reusable[i]=originals[i];
+  bridge.sendPacket(&reusable[i]);
+  assert(bridge._tx_count==int(i+1));
+  assert(bridge._tx_queue[i].seen_key.hash[0]==originals[i].data[0]);
+  memset(reusable[i].data,0x90+i,sizeof reusable[i].data);
+ }
+ auto overflow=packet(0x27);bridge.sendPacket(&overflow);
+ assert(bridge._tx_count==6&&!bridge._seen_packets.wasSeen(&overflow));
+ bridge.end();
+ assert(bridge._seen_packets.ids.empty()&&send_calls==0); // exact original admissions retired
+ bridge.begin();
+ for(auto& p:originals)bridge.sendPacket(&p);
+ bridge.loop();
+ for(unsigned i=0;i<6;++i){
+  mesh::espnow::ESPNowRawFrames expected;
+  assert(mesh::espnow::encodeEspNowRawFrames(originals[i].data,255,expected));
+  assert(expected.count==2&&sent[2*i]==std::vector<uint8_t>(expected.data[0],expected.data[0]+expected.lengths[0]));
+  complete(bridge);
+  assert(sent[2*i+1]==std::vector<uint8_t>(expected.data[1],expected.data[1]+expected.lengths[1]));
+  complete(bridge);
+ }
+ assert(bridge._tx_count==0&&sent.size()==12);
+ bridge.end();
+ for(auto& p:originals)assert(bridge._seen_packets.wasSeen(&p)); // completed entries stay seen
+}
+''', versions=(40400,50200,50500))
+
+    def test_seen_cleanup_retries_and_timeout_distinguish_never_sent_entries(self):
+        self.run_fixture(r'''
+int main(){
+ Backend backend;backend.wifi=true;mesh::wireless::control().begin(backend);
+ sdk_initialized=sdk_started=true;sdk_mode=WIFI_STA;
+ NodePrefs prefs;mesh::PacketManager mgr;mesh::RTCClock rtc;
+ ESPNowBridge bridge(&prefs,&mgr,&rtc);bridge.begin();
+ auto original=packet(0x15,255),caller=original;
+ bridge.sendPacket(&caller);memset(caller.data,0x95,sizeof caller.data);
+ send_result=ESP_ERR_ESPNOW_NO_MEM;
+ for(unsigned i=0;i<4;++i){bridge.loop();now_ms+=20;}
+ assert(bridge._tx_count==0&&!bridge._seen_packets.wasSeen(&original));
+ // Once a first fragment was accepted, an immediate sibling rejection cannot
+ // clear the logical packet's admission, even if the caller reused its pool entry.
+ send_result=ESP_OK;bridge.sendPacket(&original);bridge.loop();
+ assert(bridge._tx_count==1&&bridge._tx_queue[bridge._tx_tail].started);
+ send_result=ESP_ERR_INVALID_ARG;complete(bridge);
+ assert(bridge._tx_count==0&&bridge._seen_packets.wasSeen(&original));
+ auto active=packet(0x21,255),waiting=packet(0x25,255);
+ send_result=ESP_OK;bridge.sendPacket(&active);bridge.sendPacket(&waiting);bridge.loop();
+ assert(bridge._tx_count==2&&bridge._tx_waiting);
+ now_ms+=1000;bridge.loop(); // lost callback tears down before restarting the session
+ assert(bridge._initialized&&bridge._tx_count==0);
+ assert(bridge._seen_packets.wasSeen(&active)&&!bridge._seen_packets.wasSeen(&waiting));
+ bridge.end();
+}
+''', versions=(40400,50200,50500))
+
     def test_ota_identical_retries_cross_espnow_and_uart_without_permanent_dedup(self):
         self.run_fixture(r'''
 int main(){

test/test_espnow_radio_lifecycle.py

@@ -225,10 +225,10 @@ public:
  int _tx_callback_status=0,_tx_dropped=0,_tx_dropped_reported=0;
  uint32_t _tx_started_at=0,_tx_retry_at=0;bool _sdk_teardown_failed=false;
  mesh::espnow::ESPNowRawReassembler _raw_reassembler;
- struct QueuedTransmit {bool started=false,diagnostic=false;int packet=0;};
+ struct QueuedTransmit {bool started=false,diagnostic=false;int seen_key=0;};
  static constexpr int TX_QUEUE_DEPTH=2;
  QueuedTransmit _tx_queue[TX_QUEUE_DEPTH];
- struct {void clear(const int*){}} _seen_packets;
+ struct {void clearSeenKey(const int&){}} _seen_packets;
  static void recv_cb(){}static void send_cb(){}
  void begin();void end();
 };

test/test_mesh_tables/test_simple_mesh_tables.cpp

@@ -124,6 +124,64 @@ static Packet makeMultipartAckPacket(uint32_t crc, uint8_t remaining = 1, bool d
     return p;
 }
 
+TEST(SimpleMeshTables, SeenKeyRetainsEveryHashByteAndAckCacheDomain) {
+    SimpleMeshTables table;
+    Packet ordinary = makeFloodPacket(0x43);
+    Packet plain_ack = makeAckPacket(0x10203040);
+    Packet multipart_ack = makeMultipartAckPacket(0x10203040);
+    for (Packet* packet : {&ordinary, &plain_ack, &multipart_ack}) {
+        table.markSeen(packet);
+        auto key = table.seenKey(*packet);
+        uint8_t expected[MAX_HASH_SIZE];
+        packet->calculatePacketHash(expected);
+        EXPECT_EQ(0, memcmp(expected, key.hash, sizeof(expected)));
+        EXPECT_EQ(packet != &ordinary, key.dedicated_ack);
+        auto wrong_domain = key;
+        wrong_domain.dedicated_ack = !wrong_domain.dedicated_ack;
+        table.clearSeenKey(wrong_domain);
+        EXPECT_TRUE(table.wasSeen(packet));
+        table.clearSeenKey(key);
+        EXPECT_FALSE(table.wasSeen(packet));
+    }
+}
+
+TEST(SimpleMeshTables, SeenKeyClassifiesShortAndNonAckMultipartAsOrdinary) {
+    SimpleMeshTables table;
+    Packet short_ack = makeAckPacket(0x10203040);
+    short_ack.payload_len = 3;
+    Packet short_multipart = makeMultipartAckPacket(0x10203040);
+    short_multipart.payload_len = 4;
+    Packet other_multipart = makeMultipartAckPacket(0x10203040);
+    other_multipart.payload[0] = PAYLOAD_TYPE_RAW_CUSTOM;
+    for (Packet* packet : {&short_ack, &short_multipart, &other_multipart}) {
+        auto key = table.seenKey(*packet);
+        EXPECT_FALSE(key.dedicated_ack);
+        table.markSeen(packet);
+        EXPECT_TRUE(table.wasSeen(packet));
+        table.clearSeenKey(key);
+        EXPECT_FALSE(table.wasSeen(packet));
+    }
+}
+
+TEST(SimpleMeshTables, SeenKeySurvivesPacketReuseAndTracePathChanges) {
+    SimpleMeshTables table;
+    Packet original = makeFloodPacket(0x51);
+    original.header = ROUTE_TYPE_FLOOD | (PAYLOAD_TYPE_TRACE << PH_TYPE_SHIFT);
+    original.path_len = 3;
+    Packet reused = original;
+    table.markSeen(&original);
+    const auto key = table.seenKey(reused);
+    reused.path_len = 4; // TRACE's hash also includes its current path length.
+    table.markSeen(&reused);
+    EXPECT_TRUE(table.wasSeen(&original));
+    EXPECT_TRUE(table.wasSeen(&reused));
+    table.clearSeenKey(key);
+    EXPECT_FALSE(table.wasSeen(&original));
+    EXPECT_TRUE(table.wasSeen(&reused));
+    table.clear(&reused); // Packet callers retain the same public operation.
+    EXPECT_FALSE(table.wasSeen(&reused));
+}
+
 // -- wasSeen: pure query -------------------------------------------------------
 
 TEST(SimpleMeshTables, WasSeen_ReturnsFalseForUnseen) {

test/test_wifi_ap_lr_safety.py

@@ -21,10 +21,10 @@ public:
  mesh::bridge::diag::Stats _stats;
  std::atomic<bool> _debug_dirty{false};
  mesh::espnow::ESPNowRawReassembler _raw_reassembler;
- struct QueuedTransmit {bool started=false,diagnostic=false;int packet=0;};
+ struct QueuedTransmit {bool started=false,diagnostic=false;int seen_key=0;};
  static constexpr int TX_QUEUE_DEPTH=2;
  QueuedTransmit _tx_queue[TX_QUEUE_DEPTH];
- struct {void clear(const int*){}} _seen_packets;
+ struct {void clearSeenKey(const int&){}} _seen_packets;
  static void recv_cb(){}static void send_cb(){}
  void begin();void end();
  void recordError(mesh::bridge::diag::Stage,int32_t);void emitDebug();