From c88b9732eb2b6245c1a1055bfc341f619ff62f4d Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Sat, 6 Mar 2021 11:13:33 +0800 Subject: [PATCH] REALLY IMPORTANT: fix bug with retransmissions not happening --- src/mesh/RadioInterface.cpp | 14 +++++++++----- src/mesh/ReliableRouter.cpp | 32 ++++++++++++++++++++++++-------- src/mesh/ReliableRouter.h | 4 +--- src/mesh/Router.cpp | 5 +++++ src/mesh/Router.h | 5 +++++ 5 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/mesh/RadioInterface.cpp b/src/mesh/RadioInterface.cpp index 7b8aec605..966b900cc 100644 --- a/src/mesh/RadioInterface.cpp +++ b/src/mesh/RadioInterface.cpp @@ -1,16 +1,16 @@ +#include "configuration.h" #include "RadioInterface.h" #include "Channels.h" #include "MeshRadio.h" #include "MeshService.h" #include "NodeDB.h" #include "assert.h" -#include "configuration.h" +#include "Router.h" #include "sleep.h" #include #include #include -#include "Channels.h" #define RDEF(name, freq, spacing, num_ch, power_limit) \ { \ @@ -119,11 +119,11 @@ uint32_t RadioInterface::getTxDelayMsec() void printPacket(const char *prefix, const MeshPacket *p) { - DEBUG_MSG("%s (id=0x%08x Fr0x%02x To0x%02x, WantAck%d, HopLim%d Ch0x%x", prefix, p->id, p->from & 0xff, p->to & 0xff, p->want_ack, - p->hop_limit, p->channel); + DEBUG_MSG("%s (id=0x%08x Fr0x%02x To0x%02x, WantAck%d, HopLim%d Ch0x%x", prefix, p->id, p->from & 0xff, p->to & 0xff, + p->want_ack, p->hop_limit, p->channel); if (p->which_payloadVariant == MeshPacket_decoded_tag) { auto &s = p->decoded; - + DEBUG_MSG(" Portnum=%d", s.portnum); if (s.want_response) @@ -332,6 +332,10 @@ void RadioInterface::deliverToReceiver(MeshPacket *p) { assert(rxDest); assert(rxDest->enqueue(p, 0)); // NOWAIT - fixme, if queue is full, delete older messages + + // Nasty hack because our threading is primitive. interfaces shouldn't need to know about routers FIXME + if (router) + router->setReceivedMessage(); } /*** diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp index e3bd983d6..fd67bf3e5 100644 --- a/src/mesh/ReliableRouter.cpp +++ b/src/mesh/ReliableRouter.cpp @@ -1,8 +1,8 @@ #include "ReliableRouter.h" +#include "MeshPlugin.h" #include "MeshTypes.h" #include "configuration.h" #include "mesh-pb-constants.h" -#include "MeshPlugin.h" // ReliableRouter::ReliableRouter() {} @@ -36,7 +36,7 @@ bool ReliableRouter::shouldFilterReceived(const MeshPacket *p) // the original sending process. if (stopRetransmission(getFrom(p), p->id)) { DEBUG_MSG("Someone is retransmitting for us, generate implicit ack\n"); - if(p->want_ack) + if (p->want_ack) sendAckNak(Routing_Error_NONE, getFrom(p), p->id); } } @@ -63,7 +63,7 @@ void ReliableRouter::sniffReceived(const MeshPacket *p, const Routing *c) 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) { - if(MeshPlugin::currentReply) + if (MeshPlugin::currentReply) DEBUG_MSG("Someone else has replied to this message, no need for a 2nd ack"); else sendAckNak(Routing_Error_NONE, getFrom(p), p->id); @@ -136,8 +136,9 @@ PendingPacket *ReliableRouter::startRetransmission(MeshPacket *p) auto id = GlobalPacketId(p); auto rec = PendingPacket(p); - setNextTx(&rec); stopRetransmission(getFrom(p), p->id); + + setNextTx(&rec); pending[id] = rec; return &pending[id]; @@ -157,6 +158,8 @@ int32_t ReliableRouter::doRetransmissions() ++nextIt; // we use this odd pattern because we might be deleting it... auto &p = it->second; + bool stillValid = true; // assume we'll keep this record around + // FIXME, handle 51 day rolloever here!!! if (p.nextTxMsec <= now) { if (p.numRetransmissions == 0) { @@ -166,9 +169,10 @@ int32_t ReliableRouter::doRetransmissions() // 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); + stillValid = false; // just deleted it } else { - DEBUG_MSG("Sending reliable retransmission fr=0x%x,to=0x%x,id=0x%x, tries left=%d\n", p.packet->from, p.packet->to, - p.packet->id, p.numRetransmissions); + DEBUG_MSG("Sending reliable retransmission fr=0x%x,to=0x%x,id=0x%x, tries left=%d\n", p.packet->from, + p.packet->to, p.packet->id, p.numRetransmissions); // Note: we call the superclass version because we don't want to have our version of send() add a new // retransmission record @@ -178,8 +182,10 @@ int32_t ReliableRouter::doRetransmissions() --p.numRetransmissions; setNextTx(&p); } - } else { - // Not yet time + } + + if(stillValid) { + // Update our desired sleep delay int32_t t = p.nextTxMsec - now; d = min(t, d); @@ -187,4 +193,14 @@ int32_t ReliableRouter::doRetransmissions() } return d; +} + +void ReliableRouter::setNextTx(PendingPacket *pending) +{ + assert(iface); + auto d = iface->getRetransmissionMsec(pending->packet); + pending->nextTxMsec = millis() + d; + DEBUG_MSG("Setting next retransmission in %u msecs: ", d); + printPacket("", pending->packet); + setReceivedMessage(); // Run ASAP, so we can figure out our correct sleep time } \ No newline at end of file diff --git a/src/mesh/ReliableRouter.h b/src/mesh/ReliableRouter.h index fefc85cb3..db161b984 100644 --- a/src/mesh/ReliableRouter.h +++ b/src/mesh/ReliableRouter.h @@ -125,7 +125,5 @@ class ReliableRouter : public FloodingRouter */ int32_t doRetransmissions(); - void setNextTx(PendingPacket *pending) { - assert(iface); - pending->nextTxMsec = millis() + iface->getRetransmissionMsec(pending->packet); } + void setNextTx(PendingPacket *pending); }; diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 87e8e4295..d08124b4a 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -115,12 +115,17 @@ void Router::abortSendAndNak(Routing_Error err, MeshPacket *p) packetPool.release(p); } +void Router::setReceivedMessage() { + setInterval(0); // Run ASAP, so we can figure out our correct sleep time +} + ErrorCode Router::sendLocal(MeshPacket *p) { // No need to deliver externally if the destination is the local node if (p->to == nodeDB.getNodeNum()) { printPacket("Enqueuing local", p); fromRadioQueue.enqueue(p); + setReceivedMessage(); return ERRNO_OK; } else if (!iface) { // We must be sending to remote nodes also, fail if no interface found diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 6e6bf2c2a..6e217017b 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -63,6 +63,11 @@ class Router : protected concurrency::OSThread * @return our local nodenum */ NodeNum getNodeNum(); + /** Wake up the router thread ASAP, because we just queued a message for it. + * FIXME, this is kinda a hack because we don't have a nice way yet to say 'wake us because we are 'blocked on this queue' + */ + void setReceivedMessage(); + protected: friend class RoutingPlugin;