diff --git a/src/mesh/FloodingRouter.cpp b/src/mesh/FloodingRouter.cpp index 925735903..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); @@ -34,7 +34,8 @@ void FloodingRouter::sniffReceived(const MeshPacket *p, const Routing *c) // do not flood direct message that is ACKed DEBUG_MSG("Receiving an ACK not for me, but don't need to rebroadcast this direct message anymore.\n"); Router::cancelSending(p->to, p->decoded.request_id); // cancel rebroadcast for this DM - } else if ((p->to != getNodeNum()) && (p->hop_limit > 0) && (getFrom(p) != getNodeNum())) { + } + if ((p->to != getNodeNum()) && (p->hop_limit > 0) && (getFrom(p) != getNodeNum())) { if (p->id != 0) { if (config.device.role != Config_DeviceConfig_Role_CLIENT_MUTE) { MeshPacket *tosend = packetPool.allocCopy(*p); // keep a copy because we will be sending it 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 923b4f471..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 = 0; // 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 7933a7920..22f3692b9 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()) { @@ -37,9 +37,8 @@ bool ReliableRouter::shouldFilterReceived(MeshPacket *p) // 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. - // FIXME - we might want to turn off this "optimization", it does save lots of airtime but it assumes that once we've - // heard one one adjacent node hear our packet that a) probably other adjacent nodes heard it and b) we can trust those - // nodes to reach our destination. Both of which might be incorrect. + // This "optimization", does save lots of airtime. For DMs, you also get a real ACK back + // from the intended recipient. auto key = GlobalPacketId(getFrom(p), p->id); auto old = findPendingPacket(key); if (old) { @@ -54,16 +53,11 @@ bool ReliableRouter::shouldFilterReceived(MeshPacket *p) } } - /* 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 the original packet was sent with HOP_RELIABLE) + * this way if an implicit ACK is dropped and a packet is resent we'll rebroadcast again. + * Resending real ACKs is omitted, as you might receive a packet multiple times due to flooding and + * flooding this ACK back to the original sender already adds redundancy. */ + 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