From 486b03e985f56eb68a60f9b9b53add7c510a4100 Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Sun, 7 Feb 2021 10:26:11 +0800 Subject: [PATCH] bug 677. Don't assert fail for missing interfaces, instead return nak packet to clients. --- proto | 2 +- src/main.cpp | 6 ++++ src/mesh/ReliableRouter.cpp | 27 ++--------------- src/mesh/ReliableRouter.h | 4 --- src/mesh/Router.cpp | 59 ++++++++++++++++++++++++++++-------- src/mesh/Router.h | 8 ++++- src/mesh/generated/mesh.pb.h | 7 +++-- 7 files changed, 68 insertions(+), 45 deletions(-) diff --git a/proto b/proto index 1813b370a..106f4bfde 160000 --- a/proto +++ b/proto @@ -1 +1 @@ -Subproject commit 1813b370ab5336fc3ba453fa92206f4941eda96b +Subproject commit 106f4bfdebe277ab0b86d2b8c950ab78a35b0654 diff --git a/src/main.cpp b/src/main.cpp index aa7b191d9..bb3ec4820 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -490,6 +490,8 @@ void setup() DEBUG_MSG("Warning: Failed to find RF95 radio\n"); delete rIf; rIf = NULL; + } else { + DEBUG_MSG("Radio init succeeded, using RF95 radio\n"); } } #endif @@ -501,6 +503,8 @@ void setup() DEBUG_MSG("Warning: Failed to find SX1262 radio\n"); delete rIf; rIf = NULL; + } else { + DEBUG_MSG("Radio init succeeded, using SX1262 radio\n"); } } #endif @@ -512,6 +516,8 @@ void setup() DEBUG_MSG("Warning: Failed to find simulated radio\n"); delete rIf; rIf = NULL; + } else { + DEBUG_MSG("Using SIMULATED radio!\n"); } } #endif diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp index 93402c127..6060b1f03 100644 --- a/src/mesh/ReliableRouter.cpp +++ b/src/mesh/ReliableRouter.cpp @@ -34,7 +34,7 @@ bool ReliableRouter::shouldFilterReceived(const MeshPacket *p) // 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); + sendAckNak(ErrorReason_NONE, p->from, p->id); } } @@ -60,7 +60,7 @@ void ReliableRouter::sniffReceived(const MeshPacket *p) 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); + sendAckNak(ErrorReason_NONE, p->from, p->id); } // If the payload is valid, look for ack/nak @@ -84,27 +84,6 @@ void ReliableRouter::sniffReceived(const MeshPacket *p) FloodingRouter::sniffReceived(p); } -/** - * Send an ack or a nak packet back towards whoever sent idFrom - */ -void ReliableRouter::sendAckNak(bool isAck, NodeNum to, PacketId idFrom) -{ - auto p = allocForSending(); - p->hop_limit = 0; // Assume just immediate neighbors for now - p->to = to; - DEBUG_MSG("Sending an ack=0x%x,to=0x%x,idFrom=0x%x,id=0x%x\n", isAck, to, idFrom, p->id); - - if (isAck) { - p->decoded.ack.success_id = idFrom; - p->decoded.which_ack = SubPacket_success_id_tag; - } else { - p->decoded.ack.fail_id = idFrom; - p->decoded.which_ack = SubPacket_fail_id_tag; - } - - sendLocal(p); // we sometimes send directly to the local node -} - #define NUM_RETRANSMISSIONS 3 PendingPacket::PendingPacket(MeshPacket *p) @@ -176,7 +155,7 @@ int32_t ReliableRouter::doRetransmissions() if (p.numRetransmissions == 0) { DEBUG_MSG("Reliable send failed, returning a nak fr=0x%x,to=0x%x,id=%d\n", p.packet->from, p.packet->to, p.packet->id); - sendAckNak(false, p.packet->from, p.packet->id); + sendAckNak(ErrorReason_MAX_RETRANSMIT, 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); diff --git a/src/mesh/ReliableRouter.h b/src/mesh/ReliableRouter.h index 91dd248a8..62d555389 100644 --- a/src/mesh/ReliableRouter.h +++ b/src/mesh/ReliableRouter.h @@ -109,10 +109,6 @@ class ReliableRouter : public FloodingRouter PendingPacket *startRetransmission(MeshPacket *p); private: - /** - * Send an ack or a nak packet back towards whoever sent idFrom - */ - void sendAckNak(bool isAck, NodeNum to, PacketId idFrom); /** * Stop any retransmissions we are doing of the specified node/packet ID pair diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 458a338fe..c3a345522 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -99,6 +99,31 @@ MeshPacket *Router::allocForSending() return p; } +/** + * Send an ack or a nak packet back towards whoever sent idFrom + */ +void Router::sendAckNak(ErrorReason err, NodeNum to, PacketId idFrom) +{ + auto p = allocForSending(); + p->hop_limit = 0; // Assume just immediate neighbors for now + p->to = to; + DEBUG_MSG("Sending an err=%d,to=0x%x,idFrom=0x%x,id=0x%x\n", err, to, idFrom, p->id); + + if (!err) { + p->decoded.ack.success_id = idFrom; + p->decoded.which_ack = SubPacket_success_id_tag; + } else { + p->decoded.ack.fail_id = idFrom; + p->decoded.which_ack = SubPacket_fail_id_tag; + + // Also send back the error reason + p->decoded.which_payload = SubPacket_error_reason_tag; + p->decoded.error_reason = err; + } + + sendLocal(p); // we sometimes send directly to the local node +} + ErrorCode Router::sendLocal(MeshPacket *p) { // No need to deliver externally if the destination is the local node @@ -106,15 +131,24 @@ ErrorCode Router::sendLocal(MeshPacket *p) printPacket("Enqueuing local", p); fromRadioQueue.enqueue(p); return ERRNO_OK; - } + } else if (!iface) { + // We must be sending to remote nodes also, fail if no interface found - // If we are sending a broadcast, we also treat it as if we just received it ourself - // this allows local apps (and PCs) to see broadcasts sourced locally - if (p->to == NODENUM_BROADCAST) { - handleReceived(p); - } + // ERROR! no radio found, report failure back to the client and drop the packet + DEBUG_MSG("Error: No interface, returning NAK and dropping packet.\n"); + sendAckNak(ErrorReason_NO_INTERFACE, p->from, p->id); + packetPool.release(p); - return send(p); + return ERRNO_NO_INTERFACES; + } else { + // If we are sending a broadcast, we also treat it as if we just received it ourself + // this allows local apps (and PCs) to see broadcasts sourced locally + if (p->to == NODENUM_BROADCAST) { + handleReceived(p); + } + + return send(p); + } } /** @@ -154,14 +188,15 @@ ErrorCode Router::send(MeshPacket *p) p->which_payload = MeshPacket_encrypted_tag; } - if (iface) { - // DEBUG_MSG("Sending packet via interface fr=0x%x,to=0x%x,id=%d\n", p->from, p->to, p->id); - return iface->send(p); - } else { + assert(iface); // This should have been detected already in sendLocal (or we just received a packet from outside) + // if (iface) { + // DEBUG_MSG("Sending packet via interface fr=0x%x,to=0x%x,id=%d\n", p->from, p->to, p->id); + return iface->send(p); + /* } else { DEBUG_MSG("Dropping packet - no interfaces - fr=0x%x,to=0x%x,id=%d\n", p->from, p->to, p->id); packetPool.release(p); return ERRNO_NO_INTERFACES; - } + } */ } /** diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 0035f2ac5..dfc44dfa4 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -48,7 +48,8 @@ class Router : protected concurrency::OSThread virtual int32_t runOnce(); /** - * Works like send, but if we are sending to the local node, we directly put the message in the receive queue + * Works like send, but if we are sending to the local node, we directly put the message in the receive queue. + * This is the primary method used for sending packets, because it handles both the remote and local cases. * * NOTE: This method will free the provided packet (even if we return an error code) */ @@ -92,6 +93,11 @@ class Router : protected concurrency::OSThread */ bool perhapsDecode(MeshPacket *p); + /** + * Send an ack or a nak packet back towards whoever sent idFrom + */ + void sendAckNak(ErrorReason err, NodeNum to, PacketId idFrom); + private: /** * Called from loop() diff --git a/src/mesh/generated/mesh.pb.h b/src/mesh/generated/mesh.pb.h index 3f1b24c27..6bf3a1c01 100644 --- a/src/mesh/generated/mesh.pb.h +++ b/src/mesh/generated/mesh.pb.h @@ -16,7 +16,8 @@ typedef enum _ErrorReason { ErrorReason_NO_ROUTE = 1, ErrorReason_GOT_NAK = 2, ErrorReason_TIMEOUT = 3, - ErrorReason_NO_INTERFACE = 4 + ErrorReason_NO_INTERFACE = 4, + ErrorReason_MAX_RETRANSMIT = 5 } ErrorReason; typedef enum _Constants { @@ -290,8 +291,8 @@ typedef struct _ToRadio { /* Helper constants for enums */ #define _ErrorReason_MIN ErrorReason_NONE -#define _ErrorReason_MAX ErrorReason_NO_INTERFACE -#define _ErrorReason_ARRAYSIZE ((ErrorReason)(ErrorReason_NO_INTERFACE+1)) +#define _ErrorReason_MAX ErrorReason_MAX_RETRANSMIT +#define _ErrorReason_ARRAYSIZE ((ErrorReason)(ErrorReason_MAX_RETRANSMIT+1)) #define _Constants_MIN Constants_Unused #define _Constants_MAX Constants_DATA_PAYLOAD_LEN