diff --git a/src/mesh/MeshModule.cpp b/src/mesh/MeshModule.cpp index a8de540eb3..fc8199c658 100644 --- a/src/mesh/MeshModule.cpp +++ b/src/mesh/MeshModule.cpp @@ -33,7 +33,7 @@ MeshModule::~MeshModule() } meshtastic_MeshPacket *MeshModule::allocAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, - uint8_t hopStart, uint8_t hopLimit) + uint8_t hopLimit) { meshtastic_Routing c = meshtastic_Routing_init_default; @@ -50,7 +50,7 @@ meshtastic_MeshPacket *MeshModule::allocAckNak(meshtastic_Routing_Error err, Nod p->priority = meshtastic_MeshPacket_Priority_ACK; - p->hop_limit = routingModule->getHopLimitForResponse(hopStart, hopLimit); // Flood ACK back to original sender + p->hop_limit = hopLimit; // Flood ACK back to original sender p->to = to; p->decoded.request_id = idFrom; p->channel = chIndex; @@ -181,8 +181,8 @@ void MeshModule::callModules(meshtastic_MeshPacket &mp, RxSource src) // SECURITY NOTE! I considered sending back a different error code if we didn't find the psk (i.e. !isDecoded) // but opted NOT TO. Because it is not a good idea to let remote nodes 'probe' to find out which PSKs were "good" vs // bad. - routingModule->sendAckNak(meshtastic_Routing_Error_NO_RESPONSE, getFrom(&mp), mp.id, mp.channel, mp.hop_start, - mp.hop_limit); + routingModule->sendAckNak(meshtastic_Routing_Error_NO_RESPONSE, getFrom(&mp), mp.id, mp.channel, + routingModule->getHopLimitForResponse(mp.hop_start, mp.hop_limit)); } } @@ -295,4 +295,4 @@ bool MeshModule::isRequestingFocus() } else return false; } -#endif +#endif \ No newline at end of file diff --git a/src/mesh/MeshModule.h b/src/mesh/MeshModule.h index 7929ba972b..d37de0d830 100644 --- a/src/mesh/MeshModule.h +++ b/src/mesh/MeshModule.h @@ -162,7 +162,7 @@ class MeshModule virtual Observable *getUIFrameObservable() { return NULL; } meshtastic_MeshPacket *allocAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, - uint8_t hopStart = 0, uint8_t hopLimit = 0); + uint8_t hopLimit = 0); /// Send an error response for the specified packet. meshtastic_MeshPacket *allocErrorResponse(meshtastic_Routing_Error err, const meshtastic_MeshPacket *p); diff --git a/src/mesh/PacketHistory.cpp b/src/mesh/PacketHistory.cpp index 8d49bce43f..b31a357d2e 100644 --- a/src/mesh/PacketHistory.cpp +++ b/src/mesh/PacketHistory.cpp @@ -38,6 +38,13 @@ bool PacketHistory::wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpd seenRecently = false; } + /* If the original transmitter is doing retransmissions (hopStart equals hopLimit) for a reliable transmission, e.g., when the + ACK got lost, we will handle the packet again to make sure it gets an ACK/response to its packet. */ + if (seenRecently && p->hop_start > 0 && p->hop_start == p->hop_limit) { + LOG_DEBUG("Repeated reliable tx"); + seenRecently = false; + } + if (seenRecently) { LOG_DEBUG("Found existing packet record for fr=0x%x,to=0x%x,id=0x%x", p->from, p->to, p->id); } diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp index 98db38c47b..c94899bb85 100644 --- a/src/mesh/PhoneAPI.cpp +++ b/src/mesh/PhoneAPI.cpp @@ -596,10 +596,13 @@ bool PhoneAPI::handleToRadioPacket(meshtastic_MeshPacket &p) { printPacket("PACKET FROM PHONE", &p); +// For use with the simulator, we should not ignore duplicate packets +#if !(defined(ARCH_PORTDUINO) && !HAS_RADIO) if (p.id > 0 && wasSeenRecently(p.id)) { LOG_DEBUG("Ignoring packet from phone, already seen recently"); return false; } +#endif if (p.decoded.portnum == meshtastic_PortNum_TRACEROUTE_APP && lastPortNumToRadio[p.decoded.portnum] && Throttle::isWithinTimespanMs(lastPortNumToRadio[p.decoded.portnum], THIRTY_SECONDS_MS)) { diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp index a2e09362dd..57fe9ce84e 100644 --- a/src/mesh/ReliableRouter.cpp +++ b/src/mesh/ReliableRouter.cpp @@ -1,10 +1,10 @@ #include "ReliableRouter.h" #include "Default.h" -#include "MeshModule.h" #include "MeshTypes.h" #include "configuration.h" #include "mesh-pb-constants.h" #include "modules/NodeInfoModule.h" +#include "modules/RoutingModule.h" // ReliableRouter::ReliableRouter() {} @@ -73,18 +73,6 @@ bool ReliableRouter::shouldFilterReceived(const meshtastic_MeshPacket *p) i->second.nextTxMsec += iface->getPacketTime(p); } - /* Resend implicit ACKs for repeated packets (hopStart equals hopLimit); - * 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. */ - bool isRepeated = p->hop_start == 0 ? (p->hop_limit == HOP_RELIABLE) : (p->hop_start == p->hop_limit); - if (wasSeenRecently(p, false) && isRepeated && !MeshModule::currentReply && !isToUs(p)) { - LOG_DEBUG("Resending implicit ack for a repeated floodmsg"); - meshtastic_MeshPacket *tosend = packetPool.allocCopy(*p); - tosend->hop_limit--; // bump down the hop count - Router::send(tosend); - } - return FloodingRouter::shouldFilterReceived(p); } @@ -107,16 +95,22 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas if (MeshModule::currentReply) { LOG_DEBUG("Another module replied to this message, no need for 2nd ack"); } else if (p->which_payload_variant == meshtastic_MeshPacket_decoded_tag) { - sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, p->channel, p->hop_start, p->hop_limit); + // A response may be set to want_ack for retransmissions, but we don't need to ACK a response if it received an + // implicit ACK already. If we received it directly, only ACK with a hop limit of 0 + if (!p->decoded.request_id) + sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, p->channel, + routingModule->getHopLimitForResponse(p->hop_start, p->hop_limit)); + else if (p->hop_start > 0 && p->hop_start == p->hop_limit) + sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, p->channel, 0); } else if (p->which_payload_variant == meshtastic_MeshPacket_encrypted_tag && p->channel == 0 && (nodeDB->getMeshNode(p->from) == nullptr || nodeDB->getMeshNode(p->from)->user.public_key.size == 0)) { LOG_INFO("PKI packet from unknown node, send PKI_UNKNOWN_PUBKEY"); sendAckNak(meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY, getFrom(p), p->id, channels.getPrimaryIndex(), - p->hop_start, p->hop_limit); + routingModule->getHopLimitForResponse(p->hop_start, p->hop_limit)); } else { // Send a 'NO_CHANNEL' error on the primary channel if want_ack packet destined for us cannot be decoded - sendAckNak(meshtastic_Routing_Error_NO_CHANNEL, getFrom(p), p->id, channels.getPrimaryIndex(), p->hop_start, - p->hop_limit); + sendAckNak(meshtastic_Routing_Error_NO_CHANNEL, getFrom(p), p->id, channels.getPrimaryIndex(), + routingModule->getHopLimitForResponse(p->hop_start, p->hop_limit)); } } if (p->which_payload_variant == meshtastic_MeshPacket_decoded_tag && c && diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index e1eb8eddf0..9b7e69513b 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -136,10 +136,9 @@ meshtastic_MeshPacket *Router::allocForSending() /** * Send an ack or a nak packet back towards whoever sent idFrom */ -void Router::sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopStart, - uint8_t hopLimit) +void Router::sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopLimit) { - routingModule->sendAckNak(err, to, idFrom, chIndex, hopStart, hopLimit); + routingModule->sendAckNak(err, to, idFrom, chIndex, hopLimit); } void Router::abortSendAndNak(meshtastic_Routing_Error err, meshtastic_MeshPacket *p) diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 8ebc1a3e53..4414b18598 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -108,8 +108,7 @@ class Router : protected concurrency::OSThread /** * Send an ack or a nak packet back towards whoever sent idFrom */ - void sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopStart = 0, - uint8_t hopLimit = 0); + void sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopLimit = 0); private: /** diff --git a/src/modules/RoutingModule.cpp b/src/modules/RoutingModule.cpp index f11a9a542f..5079aa5d86 100644 --- a/src/modules/RoutingModule.cpp +++ b/src/modules/RoutingModule.cpp @@ -49,10 +49,9 @@ meshtastic_MeshPacket *RoutingModule::allocReply() return NULL; } -void RoutingModule::sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopStart, - uint8_t hopLimit) +void RoutingModule::sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopLimit) { - auto p = allocAckNak(err, to, idFrom, chIndex, hopStart, hopLimit); + auto p = allocAckNak(err, to, idFrom, chIndex, hopLimit); router->sendLocal(p); // we sometimes send directly to the local node } diff --git a/src/modules/RoutingModule.h b/src/modules/RoutingModule.h index f085b307bf..7c34c5bc97 100644 --- a/src/modules/RoutingModule.h +++ b/src/modules/RoutingModule.h @@ -13,8 +13,7 @@ class RoutingModule : public ProtobufModule */ RoutingModule(); - void sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopStart = 0, - uint8_t hopLimit = 0); + void sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopLimit = 0); // Given the hopStart and hopLimit upon reception of a request, return the hop limit to use for the response uint8_t getHopLimitForResponse(uint8_t hopStart, uint8_t hopLimit); @@ -36,4 +35,4 @@ class RoutingModule : public ProtobufModule virtual bool wantPacket(const meshtastic_MeshPacket *p) override { return true; } }; -extern RoutingModule *routingModule; +extern RoutingModule *routingModule; \ No newline at end of file