Formatting and correct NUM_RETRANSMISSIONS

src/mesh/NextHopRouter.cpp

@@ -122,6 +122,7 @@ PendingPacket *NextHopRouter::findPendingPacket(GlobalPacketId key)
     } else
         return NULL;
 }
+
 /**
  * Stop any retransmissions we are doing of the specified node/packet ID pair
  */
@@ -138,7 +139,7 @@ bool NextHopRouter::stopRetransmission(GlobalPacketId key)
         auto p = old->packet;
         /* Only when we already transmitted a packet via LoRa, we will cancel the packet in the Tx queue
           to avoid canceling a transmission if it was ACKed super fast via MQTT */
-        if (old->numRetransmissions < NUM_RETRANSMISSIONS - 1) {
+        if (old->numRetransmissions < NUM_RELIABLE_RETX - 1) {
             // remove the 'original' (identified by originator and packet->id) from the txqueue and free it
             cancelSending(getFrom(p), p->id);
             // now free the pooled copy for retransmission too

src/mesh/NextHopRouter.h

@@ -49,13 +49,14 @@ class GlobalPacketIdHashFunction
 };
 
 /*
-  Router which only relays if it is the next hop for a packet.
+  Router for direct messages, which only relays if it is the next hop for a packet. The next hop is set by the current
-  The next hop is set by the relay node of a packet, which bases this on information from either the NeighborInfoModule, or a
+  relayer of a packet, which bases this on information from a previous successful delivery to the destination via flooding.
-  previous successful delivery via flooding. It is only used for DMs and not used for broadcasts. Using the NeighborInfoModule, it
+  Namely, in the PacketHistory, we keep track of (up to 3) relayers of a packet. When the ACK is delivered back to us via a node
-  can derive the next hop of neighbors and that of neighbors of neighbors. For others, it has no information in the beginning,
+  that also relayed the original packet, we use that node as next hop for the destination from then on. This makes sure that only
-  which results into falling back to the FloodingRouter. Upon successful delivery via flooding, it updates the next hop of the
+  when there’s a two-way connection, we assign a next hop. Both the ReliableRouter and NextHopRouter will do retransmissions (the
-  recipient to the node that last relayed the ACK to us. When the ReliableRouter is doing retransmissions, at the last retry, it
+  NextHopRouter only 1 time). For the final retry, if no one actually relayed the packet, it will reset the next hop in order to
-  will reset the next hop, in order to fall back to the FloodingRouter.
+  fall back to the FloodingRouter again. Note that thus also intermediate hops will do a single retransmission if the intended
+  next-hop didn’t relay, in order to fix changes in the middle of the route.
 */
 class NextHopRouter : public FloodingRouter
 {
@@ -85,7 +86,10 @@ class NextHopRouter : public FloodingRouter
         return min(d, r);
     }
 
-    constexpr static uint8_t NUM_RETRANSMISSIONS = 2;
+    // The number of retransmissions intermediate nodes will do (actually 1 less than this)
+    constexpr static uint8_t NUM_INTERMEDIATE_RETX = 2;
+    // The number of retransmissions the original sender will do
+    constexpr static uint8_t NUM_RELIABLE_RETX = 3;
 
   protected:
     /**
@@ -115,7 +119,7 @@ class NextHopRouter : public FloodingRouter
     /**
      * Add p to the list of packets to retransmit occasionally.  We will free it once we stop retransmitting.
      */
-    PendingPacket *startRetransmission(meshtastic_MeshPacket *p, uint8_t numReTx = NUM_RETRANSMISSIONS);
+    PendingPacket *startRetransmission(meshtastic_MeshPacket *p, uint8_t numReTx = NUM_INTERMEDIATE_RETX);
 
     /**
      * Stop any retransmissions we are doing of the specified node/packet ID pair

src/mesh/PacketHistory.cpp

@@ -49,13 +49,13 @@ bool PacketHistory::wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpd
     }
 
     if (seenRecently) {
-        // If it was seen with a next-hop not set to us and now it's NO_NEXT_HOP_PREFERENCE, relayer relayed already before, it's
+        // If it was seen with a next-hop not set to us and now it's NO_NEXT_HOP_PREFERENCE, and the relayer relayed already
-        // a fallback to flooding. If we didn't already relay and the next-hop neither, consider it unseen because we might need
+        // before, it's a fallback to flooding. If we didn't already relay and the next-hop neither, consider it unseen because we
-        // to handle it now
+        // might need to handle it now
         uint8_t ourRelayID = nodeDB->getLastByteOfNodeNum(nodeDB->getNodeNum());
-        if (found->sender != nodeDB->getNodeNum() && found->next_hop != NO_NEXT_HOP_PREFERENCE &&
+        if (found->sender != nodeDB->getNodeNum() && found->next_hop != NO_NEXT_HOP_PREFERENCE && found->next_hop != ourRelayID &&
-            p->next_hop == NO_NEXT_HOP_PREFERENCE && found->next_hop != ourRelayID && wasRelayer(p->relay_node, found) &&
+            p->next_hop == NO_NEXT_HOP_PREFERENCE && wasRelayer(p->relay_node, found) && !wasRelayer(ourRelayID, found) &&
-            !wasRelayer(ourRelayID, found) && !wasRelayer(found->next_hop, found)) {
+            !wasRelayer(found->next_hop, found)) {
             LOG_INFO("Fallback to flooding, consider unseen relay_node=0x%x", p->relay_node);
             seenRecently = false;
         }

src/mesh/ReliableRouter.cpp

@@ -23,7 +23,7 @@ ErrorCode ReliableRouter::send(meshtastic_MeshPacket *p)
         }
 
         auto copy = packetPool.allocCopy(*p);
-        startRetransmission(copy, this->NUM_RETRANSMISSIONS);
+        startRetransmission(copy, NUM_RELIABLE_RETX);
     }
 
     /* If we have pending retransmissions, add the airtime of this packet to it, because during that time we cannot receive an

src/mesh/ReliableRouter.h

@@ -31,6 +31,4 @@ class ReliableRouter : public NextHopRouter
      * We hook this method so we can see packets before FloodingRouter says they should be discarded
      */
     virtual bool shouldFilterReceived(const meshtastic_MeshPacket *p) override;
-
-    constexpr static uint8_t NUM_RETRANSMISSIONS = 3;
 };