From b2b832c60863bfe1120fb9da24b1a2cca3cf0c13 Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 8 Feb 2020 09:39:26 -0800 Subject: [PATCH] nasty rxbuffer underfill bug fixed --- TODO.md | 4 ++-- src/MeshRadio.cpp | 15 +++++++++++++-- src/MeshService.cpp | 24 ++++++++++++++++++++---- src/MeshService.h | 3 +++ src/configuration.h | 4 ++-- src/main.ino | 2 ++ 6 files changed, 42 insertions(+), 10 deletions(-) diff --git a/TODO.md b/TODO.md index d1887f8e..3727d217 100644 --- a/TODO.md +++ b/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 \ No newline at end of file +* 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. diff --git a/src/MeshRadio.cpp b/src/MeshRadio.cpp index 1d376fab..352136b7 100644 --- a/src/MeshRadio.cpp +++ b/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; diff --git a/src/MeshService.cpp b/src/MeshService.cpp index 7fc2a235..ab69df1a 100644 --- a/src/MeshService.cpp +++ b/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) { diff --git a/src/MeshService.h b/src/MeshService.h index 65db43da..cf86b9e9 100644 --- a/src/MeshService.h +++ b/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; diff --git a/src/configuration.h b/src/configuration.h index 320b500b..f562ae77 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -38,11 +38,11 @@ along with this program. If not, see . // 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) diff --git a/src/main.ino b/src/main.ino index cc5353ce..1414a9c7 100644 --- a/src/main.ino +++ b/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) {