nasty rxbuffer underfill bug fixed

TODO.md

@@ -5,7 +5,6 @@
 
 # Medium priority
 
-* sent/received packets (especially if a node was just reset) have variant of zero sometimes - I think there is a bug (race-condtion?) in the radio send/rx path.
 * only BLE advertise for a short time after the screen is on and button pressed - to save power and prevent people for sniffing for our BT app.
 * use https://platformio.org/lib/show/1260/OneButton
 * make an about to sleep screen
@@ -111,4 +110,5 @@ until the phone pulls those packets.  Ever so often power on bluetooth just so w
 * make basic gui. different screens: debug, one page for each user in the user db, last received text message
 * make button press cycle between screens
 * save our node db on entry to sleep
-* fix the logo
+* fix the logo
+* sent/received packets (especially if a node was just reset) have variant of zero sometimes - I think there is a bug (race-condtion?) in the radio send/rx path.

src/MeshRadio.cpp

@@ -47,6 +47,9 @@ bool MeshRadio::init()
     return false;
   }
 
+  // not needed - defaults on
+  // rf95.setPayloadCRC(true);
+
   reloadConfig();
 
   return true;
@@ -99,7 +102,14 @@ ErrorCode MeshRadio::sendTo(NodeNum dest, const uint8_t *buf, size_t len)
   // Note: we don't use sendToWait here because we don't want to wait and for the time being don't require
   // reliable delivery
   // return manager.sendtoWait((uint8_t *) buf, len, dest);
-  return manager.sendto((uint8_t *)buf, len, dest) ? ERRNO_OK : ERRNO_UNKNOWN;
+  ErrorCode res = manager.sendto((uint8_t *)buf, len, dest) ? ERRNO_OK : ERRNO_UNKNOWN;
+
+  // FIXME, we have to wait for sending to complete before freeing the buffer, otherwise it might get wiped
+  // instead just have the radiohead layer understand queues.
+  if(res == ERRNO_OK)
+    manager.waitPacketSent();
+
+  return res;
 }
 
 /// enqueue a received packet in rxDest
@@ -129,13 +139,14 @@ static int16_t packetnum = 0;  // packet counter, we increment per xmission
 
   // Poll to see if we've received a packet
   //   if (manager.recvfromAckTimeout(radiobuf, &rxlen, 0, &srcaddr, &destaddr, &id, &flags))
+  // prefill rxlen with the max length we can accept - very important
+  rxlen = sizeof(radiobuf);
   if (manager.recvfrom(radiobuf, &rxlen, &srcaddr, &destaddr, &id, &flags))
   {
     // We received a packet
     DEBUG_MSG("Received packet from mesh src=0x%x,dest=0x%x,id=%d,len=%d rxGood=%d,rxBad=%d\n", srcaddr, destaddr, id, rxlen, rf95.rxGood(), rf95.rxBad());
 
     MeshPacket *mp = pool.allocZeroed();
-    assert(mp); // FIXME
 
     SubPacket *p = &mp->payload;
 

src/MeshService.cpp

@@ -59,11 +59,9 @@ void MeshService::sendOurOwner(NodeNum dest)
     sendToMesh(p);
 }
 
-/// Do idle processing (mostly processing messages which have been queued from the radio)
-void MeshService::loop()
-{
-    radio.loop(); // FIXME, possibly move radio interaction to own thread
 
+void MeshService::handleFromRadio()
+{
     MeshPacket *mp;
     uint32_t oldFromNum = fromNum;
     while ((mp = fromRadioQueue.dequeuePtr(0)) != NULL)
@@ -92,6 +90,24 @@ void MeshService::loop()
         bluetoothNotifyFromNum(fromNum);
 }
 
+
+
+/// Do idle processing (mostly processing messages which have been queued from the radio)
+void MeshService::loop()
+{
+    radio.loop(); // FIXME, possibly move radio interaction to own thread
+
+    handleFromRadio();
+
+    // FIXME, don't send user this often, but for now it is useful for testing
+    static uint32_t lastsend;
+    uint32_t now = millis(); 
+    if(now - lastsend > 20 * 1000) {
+        lastsend = now;
+        sendOurOwner();
+    }
+}
+
 /// Given a ToRadio buffer parse it and properly handle it (setup radio, owner or send packet into the mesh)
 void MeshService::handleToRadio(std::string s)
 {

src/MeshService.h

@@ -72,6 +72,9 @@ private:
     void onGPSChanged();
 
     virtual void onNotify(Observable *o);
+
+    /// handle packets that just arrived from the mesh radio
+    void handleFromRadio();
 };
 
 extern MeshService service;

src/configuration.h

@@ -38,11 +38,11 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 // Select which T-Beam board is being used. Only uncomment one.  Note: these options now come from platformio standard build file flags
 //#ifdef ARDUINO_T_Beam
-#define T_BEAM_V10  // AKA Rev1 (second board released)
+//#define T_BEAM_V10  // AKA Rev1 (second board released)
 //#endif 
 
 //#ifdef ARDUINO_HELTEC_WIFI_LORA_32_V2
-// #define HELTEC_LORA32
+#define HELTEC_LORA32
 //#endif
 
 // If we are using the JTAG port for debugging, some pins must be left free for that (and things like GPS have to be disabled)

src/main.ino

@@ -385,6 +385,8 @@ void loop()
   digitalWrite(LED_PIN, ledon);
 #endif
 
+  // DEBUG_MSG("led %d\n", ledon);
+
 #ifdef T_BEAM_V10
   if (axp192_found)
   {