Merge pull request #732 from geeksville/dev1.2

Dev1.2 - fix lora message rx

docs/software/TODO.md

@@ -34,8 +34,8 @@ You probably don't care about this section - skip to the next one.
 * DONE combine acks and responses in a single message if possible (do routing plugin LAST and drop ACK if someone else has already replied)
 * DONE don't send packets we received from the phone BACK TOWARDS THE PHONE (possibly use fromnode 0 for packets the phone sends?)
 * fix 1.1.50 android debug panel display
-* test android channel setting
-* release to users
+* DONE test android channel setting
+* DONE release to users
 * DONE warn in android app about unset regions
 * DONE use set-channel from android
 * DONE add gui in android app for setting region

src/esp32/ESP32CryptoEngine.cpp

@@ -54,7 +54,7 @@ class ESP32CryptoEngine : public CryptoEngine
             static uint8_t scratch[MAX_BLOCKSIZE];
             size_t nc_off = 0;
 
-            // DEBUG_MSG("ESP32 encrypt!\n");
+            // DEBUG_MSG("ESP32 crypt fr=%x, num=%x, numBytes=%d!\n", fromNode, (uint32_t) packetNum, numBytes);
             initNonce(fromNode, packetNum);
             assert(numBytes <= MAX_BLOCKSIZE);
             memcpy(scratch, bytes, numBytes);

src/mesh/Channels.cpp

@@ -275,10 +275,11 @@ const char *Channels::getPrimaryName()
 bool Channels::decryptForHash(ChannelIndex chIndex, ChannelHash channelHash)
 {
     if(chIndex > getNumChannels() || getHash(chIndex) != channelHash) {
-        DEBUG_MSG("Skipping channel %d due to invalid hash/index\n", chIndex);
+        // DEBUG_MSG("Skipping channel %d (hash %x) due to invalid hash/index, want=%x\n", chIndex, getHash(chIndex), channelHash);
         return false;
     }
     else {
+        DEBUG_MSG("Using channel %d (hash 0x%x)\n", chIndex, channelHash);
         setCrypto(chIndex);
         return true;
     }

src/mesh/CryptoEngine.cpp

@@ -4,6 +4,10 @@
 void CryptoEngine::setKey(const CryptoKey &k)
 {
     DEBUG_MSG("Installing AES%d key!\n", k.length * 8);
+    /* for(uint8_t i = 0; i < k.length; i++)
+        DEBUG_MSG("%02x ", k.bytes[i]);
+    DEBUG_MSG("\n"); */
+
     key = k;
 }
 

src/mesh/MeshService.cpp

@@ -114,7 +114,8 @@ bool MeshService::reloadConfig()
 void MeshService::reloadOwner()
 {
     assert(nodeInfoPlugin);
-    nodeInfoPlugin->sendOurNodeInfo();
+    if(nodeInfoPlugin)
+        nodeInfoPlugin->sendOurNodeInfo();
     nodeDB.saveToDisk();
 }
 
@@ -170,12 +171,18 @@ void MeshService::sendNetworkPing(NodeNum dest, bool wantReplies)
     NodeInfo *node = nodeDB.getNode(nodeDB.getNodeNum());
     assert(node);
 
-    DEBUG_MSG("Sending network ping to 0x%x, with position=%d, wantReplies=%d\n", dest, node->has_position, wantReplies);
-    assert(positionPlugin && nodeInfoPlugin);
-    if (node->has_position)
-        positionPlugin->sendOurPosition(dest, wantReplies);
-    else
-        nodeInfoPlugin->sendOurNodeInfo(dest, wantReplies);
+    if (node->has_position) {
+        if(positionPlugin) {
+            DEBUG_MSG("Sending position ping to 0x%x, wantReplies=%d\n", dest, wantReplies);
+            positionPlugin->sendOurPosition(dest, wantReplies);
+        }
+    }
+    else {
+        if(nodeInfoPlugin) {
+            DEBUG_MSG("Sending nodeinfo ping to 0x%x, wantReplies=%d\n", dest, wantReplies);
+            nodeInfoPlugin->sendOurNodeInfo(dest, wantReplies);
+        }
+    }
 }
 
 NodeInfo *MeshService::refreshMyNodeInfo()

src/mesh/PhoneAPI.cpp

@@ -179,7 +179,7 @@ size_t PhoneAPI::getFromRadio(uint8_t *buf)
     // Do we have a message from the mesh?
     if (fromRadioScratch.which_payloadVariant != 0) {
         // Encapsulate as a FromRadio packet
-        DEBUG_MSG("encoding toPhone packet to phone variant=%d", fromRadioScratch.which_payloadVariant);
+        DEBUG_MSG("encoding toPhone packet to phone variant=%d\n", fromRadioScratch.which_payloadVariant);
         size_t numbytes = pb_encode_to_bytes(buf, FromRadio_size, FromRadio_fields, &fromRadioScratch);
         DEBUG_MSG(", %d bytes\n", numbytes);
         return numbytes;

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 <assert.h>
 #include <pb_decode.h>
 #include <pb_encode.h>
-#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();
 }
 
 /***

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,18 +158,21 @@ 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) {
-                DEBUG_MSG("Reliable send failed, returning a nak fr=0x%x,to=0x%x,id=%d\n", p.packet->from, p.packet->to,
+                DEBUG_MSG("Reliable send failed, returning a nak for fr=0x%x,to=0x%x,id=0x%x\n", p.packet->from, p.packet->to,
                           p.packet->id);
-                sendAckNak(Routing_Error_MAX_RETRANSMIT, p.packet->from, p.packet->id);
+                sendAckNak(Routing_Error_MAX_RETRANSMIT, getFrom(p.packet), 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);
+                stillValid = false; // just deleted it
             } else {
-                DEBUG_MSG("Sending reliable retransmission fr=0x%x,to=0x%x,id=%d, 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
 }

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

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
@@ -138,6 +143,13 @@ ErrorCode Router::sendLocal(MeshPacket *p)
     }
 }
 
+void printBytes(const char *label, const uint8_t *p, size_t numbytes) {
+    DEBUG_MSG("%s: ", label);
+    for(size_t i = 0; i < numbytes; i++)
+        DEBUG_MSG("%02x ", p[i]);
+    DEBUG_MSG("\n");
+}
+
 /**
  * Send a packet on a suitable interface.  This routine will
  * later free() the packet to pool.  This routine is not allowed to stall.
@@ -168,6 +180,8 @@ ErrorCode Router::send(MeshPacket *p)
     if (p->which_payloadVariant == MeshPacket_decoded_tag) {
         static uint8_t bytes[MAX_RHPACKETLEN]; // we have to use a scratch buffer because a union
 
+        // printPacket("pre encrypt", p); // portnum valid here
+
         size_t numbytes = pb_encode_to_bytes(bytes, sizeof(bytes), Data_fields, &p->decoded);
 
         if (numbytes > MAX_RHPACKETLEN) {
@@ -175,6 +189,8 @@ ErrorCode Router::send(MeshPacket *p)
             return ERRNO_TOO_LARGE;
         }
 
+        //printBytes("plaintext", bytes, numbytes);
+
         auto hash = channels.setActiveByIndex(p->channel);
         if (hash < 0) {
             // No suitable channel could be found for sending
@@ -184,7 +200,7 @@ ErrorCode Router::send(MeshPacket *p)
 
         // Now that we are encrypting the packet channel should be the hash (no longer the index)
         p->channel = hash;
-        crypto->encrypt(p->from, p->id, numbytes, bytes);
+        crypto->encrypt(getFrom(p), p->id, numbytes, bytes);
 
         // Copy back into the packet and set the variant type
         memcpy(p->encrypted.bytes, bytes, numbytes);
@@ -225,20 +241,26 @@ bool Router::perhapsDecode(MeshPacket *p)
         if (channels.decryptForHash(chIndex, p->channel)) {
             // Try to decrypt the packet if we can
             static uint8_t bytes[MAX_RHPACKETLEN];
+            size_t rawSize = p->encrypted.size;
+            assert(rawSize <= sizeof(bytes));
             memcpy(bytes, p->encrypted.bytes,
-                   p->encrypted
-                       .size); // we have to copy into a scratch buffer, because these bytes are a union with the decoded protobuf
-            crypto->decrypt(p->from, p->id, p->encrypted.size, bytes);
+                   rawSize); // we have to copy into a scratch buffer, because these bytes are a union with the decoded protobuf
+            crypto->decrypt(p->from, p->id, rawSize, bytes);
+
+            //printBytes("plaintext", bytes, p->encrypted.size);            
 
             // Take those raw bytes and convert them back into a well structured protobuf we can understand
-            memset(&p->decoded, 0, sizeof(p->decoded));
-            if (!pb_decode_from_bytes(bytes, p->encrypted.size, Data_fields, &p->decoded)) {
-                DEBUG_MSG("Invalid protobufs in received mesh packet (bad psk?!\n");
-            } else {
+            memset(&p->decoded, 0, sizeof(p->decoded)); 
+            if (!pb_decode_from_bytes(bytes, rawSize, Data_fields, &p->decoded)) {
+                DEBUG_MSG("Invalid protobufs in received mesh packet (bad psk?)!\n");
+            } else if(p->decoded.portnum == PortNum_UNKNOWN_APP) {
+                DEBUG_MSG("Invalid portnum (bad psk?)!\n");
+            }
+            else {
                 // parsing was successful
+                p->which_payloadVariant = MeshPacket_decoded_tag; // change type to decoded
                 p->channel = chIndex; // change to store the index instead of the hash
-                // printPacket("decoded message", p);
-                p->which_payloadVariant = MeshPacket_decoded_tag;
+                printPacket("decoded message", p);
                 return true;
             }
         }
@@ -263,11 +285,10 @@ void Router::handleReceived(MeshPacket *p)
     p->rx_time = getValidTime(RTCQualityFromNet); // store the arrival timestamp for the phone
 
     // Take those raw bytes and convert them back into a well structured protobuf we can understand
-    bool decoded = perhapsDecode(p);
-    printPacket("handleReceived", p);
-    DEBUG_MSG("decoded=%d\n", decoded);    
+    bool decoded = perhapsDecode(p); 
     if (decoded) {
         // parsing was successful, queue for our recipient
+        printPacket("handleReceived", p);
 
         // call any promiscious plugins here, make a (non promisiocous) plugin for forwarding messages to phone api
         // sniffReceived(p);

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;
 

src/mesh/mesh-pb-constants.cpp

@@ -18,8 +18,8 @@ size_t pb_encode_to_bytes(uint8_t *destbuf, size_t destbufsize, const pb_msgdesc
 
     pb_ostream_t stream = pb_ostream_from_buffer(destbuf, destbufsize);
     if (!pb_encode(&stream, fields, src_struct)) {
-        DEBUG_MSG("Error: can't encode protobuf %s\n", PB_GET_ERROR(&stream));
-        assert(0); // FIXME - panic
+        DEBUG_MSG("Panic: can't encode protobuf %s, did you make a field too large?\n", PB_GET_ERROR(&stream));
+        assert(0); // If this asser fails it probably means you made a field too large for the max limits specified in mesh.options
     } else {
         return stream.bytes_written;
     }

src/plugins/NodeInfoPlugin.cpp

@@ -65,8 +65,7 @@ int32_t NodeInfoPlugin::runOnce()
     currentGeneration = radioGeneration;
 
     DEBUG_MSG("Sending our nodeinfo to mesh (wantReplies=%d)\n", requestReplies);
-    assert(nodeInfoPlugin);
-    nodeInfoPlugin->sendOurNodeInfo(NODENUM_BROADCAST, requestReplies); // Send our info (don't request replies)
+    sendOurNodeInfo(NODENUM_BROADCAST, requestReplies); // Send our info (don't request replies)
 
     return getPref_position_broadcast_secs() * 1000;
 }

src/plugins/TextMessagePlugin.cpp

@@ -8,7 +8,7 @@ TextMessagePlugin *textMessagePlugin;
 bool TextMessagePlugin::handleReceived(const MeshPacket &mp)
 {
     auto &p = mp.decoded;
-    DEBUG_MSG("Received text msg from=0x%0x, id=%d, msg=%.*s\n", mp.from, mp.id, p.payload.size, p.payload.bytes);
+    DEBUG_MSG("Received text msg from=0x%0x, id=0x%x, msg=%.*s\n", mp.from, mp.id, p.payload.size, p.payload.bytes);
 
     // We only store/display messages destined for us.
     // Keep a copy of the most recent text message.

version.properties

@@ -1,4 +1,4 @@
 [VERSION]  
 major = 1  
 minor = 2
-build = 4
+build = 5