From f4b1678535796ec1f45ce501d5b894f7ce5ee70b Mon Sep 17 00:00:00 2001
From: geeksville <kevinh@geeksville.com>
Date: Mon, 25 May 2020 11:55:42 -0700
Subject: [PATCH] my DSR changes broke acks for flood routing also.  Fix #146

---
 src/mesh/FloodingRouter.cpp |  5 +++--
 src/mesh/ReliableRouter.cpp | 37 ++++++++++++++++++++++---------------
 src/mesh/ReliableRouter.h   |  5 +++++
 3 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/src/mesh/FloodingRouter.cpp b/src/mesh/FloodingRouter.cpp
index 0b7aea64..ffd652bf 100644
--- a/src/mesh/FloodingRouter.cpp
+++ b/src/mesh/FloodingRouter.cpp
@@ -19,8 +19,9 @@ ErrorCode FloodingRouter::send(MeshPacket *p)
 
 bool FloodingRouter::shouldFilterReceived(const MeshPacket *p)
 {
-    if (wasSeenRecently(p)) {
-        DEBUG_MSG("Ignoring incoming msg, because we've already seen it\n");
+    if (wasSeenRecently(p)) { // Note: this will also add a recent packet record
+        DEBUG_MSG("Ignoring incoming msg, because we've already seen it: fr=0x%x,to=0x%x,id=%d,hop_limit=%d\n", p->from, p->to,
+                  p->id, p->hop_limit);
         return true;
     }
 
diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp
index 7056e36e..603768a0 100644
--- a/src/mesh/ReliableRouter.cpp
+++ b/src/mesh/ReliableRouter.cpp
@@ -24,6 +24,23 @@ ErrorCode ReliableRouter::send(MeshPacket *p)
     return FloodingRouter::send(p);
 }
 
+bool ReliableRouter::shouldFilterReceived(const MeshPacket *p)
+{
+    if (p->to == NODENUM_BROADCAST && p->from == getNodeNum()) {
+        DEBUG_MSG("Received someone rebroadcasting for us fr=0x%x,to=0x%x,id=%d\n", p->from, p->to, p->id);
+
+        // We are seeing someone rebroadcast one of our broadcast attempts.
+        // If this is the first time we saw this, cancel any retransmissions we have queued up and generate an internal ack for
+        // the original sending process.
+        if (stopRetransmission(p->from, p->id)) {
+            DEBUG_MSG("Someone is retransmitting for us, generate implicit ack\n");
+            sendAckNak(true, p->from, p->id);
+        }
+    }
+
+    return FloodingRouter::shouldFilterReceived(p);
+}
+
 /**
  * If we receive a want_ack packet (do not check for wasSeenRecently), send back an ack (this might generate multiple ack sends in
  * case the our first ack gets lost)
@@ -40,18 +57,8 @@ void ReliableRouter::sniffReceived(const MeshPacket *p)
 {
     NodeNum ourNode = getNodeNum();
 
-    if (p->from == ourNode && p->to == NODENUM_BROADCAST) {
-        DEBUG_MSG("Received someone rebroadcasting for us fr=0x%x,to=0x%x,id=%d\n", p->from, p->to, p->id);
-
-        // We are seeing someone rebroadcast one of our broadcast attempts.
-        // If this is the first time we saw this, cancel any retransmissions we have queued up and generate an internal ack for
-        // the original sending process.
-        if (stopRetransmission(p->from, p->id)) {
-            DEBUG_MSG("Someone is retransmitting for us, generate implicit ack\n");
-            sendAckNak(true, p->from, p->id);
-        }
-    } else if (p->to == ourNode) { // ignore ack/nak/want_ack packets that are not address to us (we only handle 0 hop reliability
-                                   // - not DSR routing)
+    if (p->to == ourNode) { // ignore ack/nak/want_ack packets that are not address to us (we only handle 0 hop reliability
+                            // - not DSR routing)
         if (p->want_ack) {
             sendAckNak(true, p->from, p->id);
         }
@@ -61,8 +68,8 @@ void ReliableRouter::sniffReceived(const MeshPacket *p)
         PacketId ackId = p->decoded.which_ack == SubPacket_success_id_tag ? p->decoded.ack.success_id : 0;
         PacketId nakId = p->decoded.which_ack == SubPacket_fail_id_tag ? p->decoded.ack.fail_id : 0;
 
-        // we are careful to only read wasSeenRecently - not update it (to not mess up broadcasts)
-        if ((ackId || nakId) && !wasSeenRecently(p, false)) {
+        // We intentionally don't check wasSeenRecently, because it is harmless to delete non existent retransmission records
+        if (ackId || nakId) {
             if (ackId) {
                 DEBUG_MSG("Received a ack=%d, stopping retransmissions\n", ackId);
                 stopRetransmission(p->to, ackId);
@@ -169,7 +176,7 @@ void ReliableRouter::doRetransmissions()
                 sendAckNak(false, p.packet->from, p.packet->id);
                 // Note: we don't stop retransmission here, instead the Nak packet gets processed in sniffReceived - which
                 // allows the DSR version to still be able to look at the PendingPacket
-                // stopRetransmission(it->first);
+                stopRetransmission(it->first);
             } else {
                 DEBUG_MSG("Sending reliable retransmission fr=0x%x,to=0x%x,id=%d, tries left=%d\n", p.packet->from, p.packet->to,
                           p.packet->id, p.numRetransmissions);
diff --git a/src/mesh/ReliableRouter.h b/src/mesh/ReliableRouter.h
index b9617686..d984ea11 100644
--- a/src/mesh/ReliableRouter.h
+++ b/src/mesh/ReliableRouter.h
@@ -98,6 +98,11 @@ class ReliableRouter : public FloodingRouter
     PendingPacket *findPendingPacket(NodeNum from, PacketId id) { return findPendingPacket(GlobalPacketId(from, id)); }
     PendingPacket *findPendingPacket(GlobalPacketId p);
 
+    /**
+     * We hook this method so we can see packets before FloodingRouter says they should be discarded
+     */
+    virtual bool shouldFilterReceived(const MeshPacket *p);
+
   private:
     /**
      * Send an ack or a nak packet back towards whoever sent idFrom