From b4735f4224d46b84ec9602703699499332006d35 Mon Sep 17 00:00:00 2001
From: GUVWAF <thijs@havinga.eu>
Date: Tue, 27 Dec 2022 14:32:07 +0100
Subject: [PATCH] Don't resend ACK every time you receive the packet

---
 src/mesh/FloodingRouter.cpp |  2 +-
 src/mesh/FloodingRouter.h   |  2 +-
 src/mesh/MeshModule.cpp     |  2 +-
 src/mesh/ReliableRouter.cpp | 18 +++++-------------
 src/mesh/ReliableRouter.h   |  2 +-
 src/mesh/Router.h           |  2 +-
 6 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/src/mesh/FloodingRouter.cpp b/src/mesh/FloodingRouter.cpp
index 92df575b1..e21820305 100644
--- a/src/mesh/FloodingRouter.cpp
+++ b/src/mesh/FloodingRouter.cpp
@@ -17,7 +17,7 @@ ErrorCode FloodingRouter::send(MeshPacket *p)
     return Router::send(p);
 }
 
-bool FloodingRouter::shouldFilterReceived(MeshPacket *p)
+bool FloodingRouter::shouldFilterReceived(const MeshPacket *p)
 {
     if (wasSeenRecently(p)) { // Note: this will also add a recent packet record
         printPacket("Ignoring incoming msg, because we've already seen it", p);
diff --git a/src/mesh/FloodingRouter.h b/src/mesh/FloodingRouter.h
index 7e6271fc0..01b51c9a2 100644
--- a/src/mesh/FloodingRouter.h
+++ b/src/mesh/FloodingRouter.h
@@ -51,7 +51,7 @@ class FloodingRouter : public Router, protected PacketHistory
      * Called immedately on receiption, before any further processing.
      * @return true to abandon the packet
      */
-    virtual bool shouldFilterReceived(MeshPacket *p) override;
+    virtual bool shouldFilterReceived(const MeshPacket *p) override;
 
     /**
      * Look for broadcasts we need to rebroadcast
diff --git a/src/mesh/MeshModule.cpp b/src/mesh/MeshModule.cpp
index 02289f863..8019cda1e 100644
--- a/src/mesh/MeshModule.cpp
+++ b/src/mesh/MeshModule.cpp
@@ -48,7 +48,7 @@ MeshPacket *MeshModule::allocAckNak(Routing_Error err, NodeNum to, PacketId idFr
 
     p->priority = MeshPacket_Priority_ACK;
 
-    p->hop_limit = config.lora.hop_limit; // Assume just immediate neighbors for now
+    p->hop_limit = config.lora.hop_limit; // Flood ACK back to original sender
     p->to = to;
     p->decoded.request_id = idFrom;
     p->channel = chIndex;
diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp
index a60c37ebc..ba886ed71 100644
--- a/src/mesh/ReliableRouter.cpp
+++ b/src/mesh/ReliableRouter.cpp
@@ -27,7 +27,7 @@ ErrorCode ReliableRouter::send(MeshPacket *p)
     return FloodingRouter::send(p);
 }
 
-bool ReliableRouter::shouldFilterReceived(MeshPacket *p)
+bool ReliableRouter::shouldFilterReceived(const MeshPacket *p)
 {
     // Note: do not use getFrom() here, because we want to ignore messages sent from phone
     if (p->from == getNodeNum()) {
@@ -54,18 +54,10 @@ bool ReliableRouter::shouldFilterReceived(MeshPacket *p)
         }
     }
 
-
-    //bool isAck = ((c && c->error_reason == Routing_Error_NONE)); // consider only ROUTING_APP message without error as ACK
-    /* send acks for repeated packets that want acks and are destined for us
-    * this way if an ACK is dropped and a packet is resent we'll ACK the resent packet
-    * make sure wasSeenRecently _doesn't_ update
-    * finding the channel requires decoding the packet. */
-    if (p->want_ack && (p->to == getNodeNum()) && wasSeenRecently(p, false) && !MeshModule::currentReply) {
-        if (perhapsDecode(p)) {
-            sendAckNak(Routing_Error_NONE, getFrom(p), p->id, p->channel);
-            DEBUG_MSG("acking a repeated want_ack packet\n");
-        }
-    } else if (wasSeenRecently(p, false) && p->hop_limit == HOP_RELIABLE && !MeshModule::currentReply && p->to != nodeDB.getNodeNum()) {
+    /* Resend implicit ACKs for repeated packets (assuming original packet was sent with HOP_RELIABLE)
+    * this way if an implicit ACK is dropped and a packet is resent we'll rebroadcast again
+    * Not used for real ACKs, as you might receive a packet multiple times due to flooding. */
+    if (wasSeenRecently(p, false) && p->hop_limit == HOP_RELIABLE && !MeshModule::currentReply && p->to != nodeDB.getNodeNum()) {
         // retransmission on broadcast has hop_limit still equal to HOP_RELIABLE
         DEBUG_MSG("Resending implicit ack for a repeated floodmsg\n");
         MeshPacket *tosend = packetPool.allocCopy(*p);
diff --git a/src/mesh/ReliableRouter.h b/src/mesh/ReliableRouter.h
index ff304cdd7..65f486e5b 100644
--- a/src/mesh/ReliableRouter.h
+++ b/src/mesh/ReliableRouter.h
@@ -96,7 +96,7 @@ class ReliableRouter : public FloodingRouter
     /**
      * We hook this method so we can see packets before FloodingRouter says they should be discarded
      */
-    virtual bool shouldFilterReceived(MeshPacket *p) override;
+    virtual bool shouldFilterReceived(const MeshPacket *p) override;
 
     /**
      * Add p to the list of packets to retransmit occasionally.  We will free it once we stop retransmitting.
diff --git a/src/mesh/Router.h b/src/mesh/Router.h
index 3f079d92c..f7748bb2b 100644
--- a/src/mesh/Router.h
+++ b/src/mesh/Router.h
@@ -90,7 +90,7 @@ class Router : protected concurrency::OSThread
      * Called immedately on receiption, before any further processing.
      * @return true to abandon the packet
      */
-    virtual bool shouldFilterReceived(MeshPacket *p) { return false; }
+    virtual bool shouldFilterReceived(const MeshPacket *p) { return false; }
 
     /**
      * Every (non duplicate) packet this node receives will be passed through this method.  This allows subclasses to