bug 677. Don't assert fail for missing interfaces, instead return

nak packet to clients.

proto

@@ -1 +1 @@
-Subproject commit 1813b370ab5336fc3ba453fa92206f4941eda96b
+Subproject commit 106f4bfdebe277ab0b86d2b8c950ab78a35b0654

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

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);

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

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,8 +131,16 @@ 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
 
+        // 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 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) {
@@ -116,6 +149,7 @@ ErrorCode Router::sendLocal(MeshPacket *p)
 
         return send(p);
     }
+}
 
 /**
  * Send a packet on a suitable interface.  This routine will
@@ -154,14 +188,15 @@ ErrorCode Router::send(MeshPacket *p)
         p->which_payload = MeshPacket_encrypted_tag;
     }
 
-    if (iface) {
+    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 {
+    /* } 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;
-    }
+    } */
 }
 
 /**

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()

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