From 2ad314f15041df708c88e8c3b22d1f1cbf8cee56 Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 2 May 2020 08:29:51 -0700 Subject: [PATCH 1/7] we now always listen before transmit - even if we have just completed a packet --- src/OSTimer.h | 15 +++++ src/WorkerThread.cpp | 5 +- src/WorkerThread.h | 7 ++ src/mesh/RadioLibInterface.cpp | 120 +++++++++++++++++++++------------ src/mesh/RadioLibInterface.h | 19 ++++-- src/mesh/Router.cpp | 2 +- 6 files changed, 115 insertions(+), 53 deletions(-) create mode 100644 src/OSTimer.h diff --git a/src/OSTimer.h b/src/OSTimer.h new file mode 100644 index 000000000..73a40ddb3 --- /dev/null +++ b/src/OSTimer.h @@ -0,0 +1,15 @@ +#pragma once + +#include + +typedef void (*PendableFunction)(void *pvParameter1, uint32_t ulParameter2); + +/** + * Schedule a callback to run. The callback must _not_ block, though it is called from regular thread level (not ISR) + * + * @return true if successful, false if the timer fifo is too full. + */ +inline bool scheduleCallback(PendableFunction callback, void *param1, uint32_t param2, uint32_t delayMsec) +{ + return xTimerPendFunctionCall(callback, param1, param2, pdMS_TO_TICKS(delayMsec)); +} \ No newline at end of file diff --git a/src/WorkerThread.cpp b/src/WorkerThread.cpp index 0efe79429..ed1103911 100644 --- a/src/WorkerThread.cpp +++ b/src/WorkerThread.cpp @@ -38,7 +38,6 @@ void NotifiedWorkerThread::notifyFromISR(BaseType_t *highPriWoken, uint32_t v, e void NotifiedWorkerThread::block() { - xTaskNotifyWait(0, // don't clear notification on entry - 0, // do not reset notification value on read - ¬ification, portMAX_DELAY); // Wait forever + xTaskNotifyWait(0, // don't clear notification on entry + clearOnRead, ¬ification, portMAX_DELAY); // Wait forever } diff --git a/src/WorkerThread.h b/src/WorkerThread.h index 50d87b965..318f9d803 100644 --- a/src/WorkerThread.h +++ b/src/WorkerThread.h @@ -70,6 +70,13 @@ class NotifiedWorkerThread : public WorkerThread */ uint32_t notification = 0; + /** + * What notification bits should be cleared just after we read and return them in notification? + * + * Defaults to clear all of them. + */ + uint32_t clearOnRead = ULONG_MAX; + /** * A method that should block execution - either waiting ona queue/mutex or a "task notification" */ diff --git a/src/mesh/RadioLibInterface.cpp b/src/mesh/RadioLibInterface.cpp index f6fb2de3d..48a5e8a4d 100644 --- a/src/mesh/RadioLibInterface.cpp +++ b/src/mesh/RadioLibInterface.cpp @@ -24,13 +24,13 @@ RadioLibInterface::RadioLibInterface(RADIOLIB_PIN_TYPE cs, RADIOLIB_PIN_TYPE irq #define YIELD_FROM_ISR(x) portYIELD_FROM_ISR(x) #endif -void INTERRUPT_ATTR RadioLibInterface::isrRxLevel0() +void INTERRUPT_ATTR RadioLibInterface::isrLevel0Common(PendingISR cause) { instance->disableInterrupt(); - instance->pending = ISR_RX; + instance->pending = cause; BaseType_t xHigherPriorityTaskWoken; - instance->notifyFromISR(&xHigherPriorityTaskWoken); + instance->notifyFromISR(&xHigherPriorityTaskWoken, cause, eSetValueWithOverwrite); /* Force a context switch if xHigherPriorityTaskWoken is now set to pdTRUE. The macro used to do this is dependent on the port and may be called @@ -38,18 +38,14 @@ void INTERRUPT_ATTR RadioLibInterface::isrRxLevel0() YIELD_FROM_ISR(xHigherPriorityTaskWoken); } +void INTERRUPT_ATTR RadioLibInterface::isrRxLevel0() +{ + isrLevel0Common(ISR_RX); +} + void INTERRUPT_ATTR RadioLibInterface::isrTxLevel0() { - instance->disableInterrupt(); - - instance->pending = ISR_TX; - BaseType_t xHigherPriorityTaskWoken; - instance->notifyFromISR(&xHigherPriorityTaskWoken); - - /* Force a context switch if xHigherPriorityTaskWoken is now set to pdTRUE. - The macro used to do this is dependent on the port and may be called - portEND_SWITCHING_ISR. */ - YIELD_FROM_ISR(xHigherPriorityTaskWoken); + isrLevel0Common(ISR_TX); } /** Our ISR code currently needs this to find our active instance @@ -108,25 +104,18 @@ bool RadioLibInterface::canSendImmediately() /// bluetooth comms code. If the txmit queue is empty it might return an error ErrorCode RadioLibInterface::send(MeshPacket *p) { - // We wait _if_ we are partially though receiving a packet (rather than just merely waiting for one). - // To do otherwise would be doubly bad because not only would we drop the packet that was on the way in, - // we almost certainly guarantee no one outside will like the packet we are sending. - if (canSendImmediately()) { - // if the radio is idle, we can send right away - DEBUG_MSG("immediate send on mesh fr=0x%x,to=0x%x,id=%d\n (txGood=%d,rxGood=%d,rxBad=%d)\n", p->from, p->to, p->id, - txGood, rxGood, rxBad); - - startSend(p); - return ERRNO_OK; - } else { - DEBUG_MSG("enqueuing packet for send from=0x%x, to=0x%x\n", p->from, p->to); - ErrorCode res = txQueue.enqueue(p, 0) ? ERRNO_OK : ERRNO_UNKNOWN; - - if (res != ERRNO_OK) // we weren't able to queue it, so we must drop it to prevent leaks - packetPool.release(p); + DEBUG_MSG("enqueuing for send on mesh fr=0x%x,to=0x%x,id=%d\n (txGood=%d,rxGood=%d,rxBad=%d)\n", p->from, p->to, p->id, + txGood, rxGood, rxBad); + ErrorCode res = txQueue.enqueue(p, 0) ? ERRNO_OK : ERRNO_UNKNOWN; + if (res != ERRNO_OK) { // we weren't able to queue it, so we must drop it to prevent leaks + packetPool.release(p); return res; } + + startTransmitTimer(false); // We want all sending/receiving to be done by our daemon thread + + return res; } bool RadioLibInterface::canSleep() @@ -138,30 +127,75 @@ bool RadioLibInterface::canSleep() return res; } +/** radio helper thread callback. + +We never immediately transmit after any operation (either rx or tx). Instead we should start receiving and +wait a random delay of 50 to 200 ms to make sure we are not stomping on someone else. The 50ms delay at the beginning ensures all +possible listeners have had time to finish processing the previous packet and now have their radio in RX state. The up to 200ms +random delay gives a chance for all possible senders to have high odds of detecting that someone else started transmitting first +and then they will wait until that packet finishes. + +NOTE: the large flood rebroadcast delay might still be needed even with this approach. Because we might not be able to hear other +transmitters that we are potentially stomping on. Requires further thought. + +FIXME, the 50ms and 200ms values should be tuned via logic analyzer later. +*/ void RadioLibInterface::loop() { - PendingISR wasPending = pending; pending = ISR_NONE; - if (wasPending == ISR_TX) + switch (notification) { + case ISR_TX: handleTransmitInterrupt(); - else if (wasPending == ISR_RX) + startReceive(); + startTransmitTimer(); + break; + case ISR_RX: handleReceiveInterrupt(); - else - assert(0); // We expected to receive a valid notification from the ISR + startReceive(); + startTransmitTimer(); + break; + case TRANSMIT_DELAY_COMPLETED: + // If we are not currently in receive mode, then restart the timer and try again later (this can happen if the main thread + // has placed the unit into standby) FIXME, how will this work if the chipset is in sleep mode? + if (!txQueue.isEmpty()) { + if (!canSendImmediately()) { + startTransmitTimer(); // try again in a little while + } else { + DEBUG_MSG("Transmit timer completed!\n"); - startNextWork(); + // Send any outgoing packets we have ready + MeshPacket *txp = txQueue.dequeuePtr(0); + assert(txp); + startSend(txp); + } + } + break; + default: + assert(0); // We expected to receive a valid notification from the ISR + } } -void RadioLibInterface::startNextWork() +#include "OSTimer.h" + +void RadioLibInterface::timerCallback(void *p1, uint32_t p2) { - // First send any outgoing packets we have ready - MeshPacket *txp = txQueue.dequeuePtr(0); - if (txp) - startSend(txp); - else { - // Nothing to send, let's switch back to receive mode - startReceive(); + RadioLibInterface *t = (RadioLibInterface *)p1; + + t->timerRunning = false; + + // We use without overwrite, so that if there is already an interrupt pending to be handled, that gets handle properly (the + // ISR handler will restart our timer) + t->notify(TRANSMIT_DELAY_COMPLETED, eSetValueWithoutOverwrite); +} + +void RadioLibInterface::startTransmitTimer(bool withDelay) +{ + // If we have work to do and the timer wasn't already scheduled, schedule it now + if (!timerRunning && !txQueue.isEmpty()) { + timerRunning = true; + uint32_t delay = withDelay ? 0 : random(50, 200); // See documentation for loop() wrt these values + scheduleCallback(timerCallback, this, 0, delay); } } diff --git a/src/mesh/RadioLibInterface.h b/src/mesh/RadioLibInterface.h index 789df68ab..c6f595c80 100644 --- a/src/mesh/RadioLibInterface.h +++ b/src/mesh/RadioLibInterface.h @@ -14,9 +14,10 @@ class RadioLibInterface : public RadioInterface { /// Used as our notification from the ISR - enum PendingISR { ISR_NONE = 0, ISR_RX, ISR_TX }; + enum PendingISR { ISR_NONE = 0, ISR_RX, ISR_TX, TRANSMIT_DELAY_COMPLETED }; volatile PendingISR pending = ISR_NONE; + volatile bool timerRunning = false; /** Our ISR code currently needs this to find our active instance */ @@ -25,7 +26,7 @@ class RadioLibInterface : public RadioInterface /** * Raw ISR handler that just calls our polymorphic method */ - static void isrTxLevel0(); + static void isrTxLevel0(), isrLevel0Common(PendingISR code); /** * Debugging counts @@ -43,8 +44,8 @@ class RadioLibInterface : public RadioInterface */ uint8_t syncWord = SX126X_SYNC_WORD_PRIVATE; - float currentLimit = 100; // FIXME - uint16_t preambleLength = 8; // 8 is default, but FIXME use longer to increase the amount of sleep time when receiving + float currentLimit = 100; // FIXME + uint16_t preambleLength = 32; // 8 is default, but FIXME use longer to increase the amount of sleep time when receiving Module module; // The HW interface to the radio @@ -83,12 +84,18 @@ class RadioLibInterface : public RadioInterface /** start an immediate transmit */ void startSend(MeshPacket *txp); - /** start a queued transmit (if we have one), else start receiving */ - void startNextWork(); + /** if we have something waiting to send, start a short random timer so we can come check for collision before actually doing + * the transmit + * + * If the timer was already running, we just wait for that one to occur. + * */ + void startTransmitTimer(bool withDelay = true); void handleTransmitInterrupt(); void handleReceiveInterrupt(); + static void timerCallback(void *p1, uint32_t p2); + protected: /** * Convert our modemConfig enum into wf, sf, etc... diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index ecc8c8f40..4790ff17e 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -49,7 +49,7 @@ void Router::loop() ErrorCode Router::send(MeshPacket *p) { if (iface) { - DEBUG_MSG("Sending packet via interface fr=0x%x,to=0x%x,id=%d\n", p->from, p->to, p->id); + // 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); From bb9f595b8b495ed7bf6f87f0cc68adca02a1852f Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 2 May 2020 19:51:25 -0700 Subject: [PATCH 2/7] Fix #11 --- src/OSTimer.cpp | 49 +++++++++++++++++++++++ src/OSTimer.h | 11 ++++-- src/SerialConsole.cpp | 4 +- src/WorkerThread.h | 2 +- src/mesh/RF95Interface.h | 6 +-- src/mesh/RadioInterface.cpp | 3 +- src/mesh/RadioLibInterface.cpp | 71 ++++++++++++++++++++++++++-------- src/mesh/RadioLibInterface.h | 8 ++-- src/mesh/RadioLibRF95.cpp | 9 ++++- src/mesh/RadioLibRF95.h | 3 ++ src/mesh/SX1262Interface.h | 4 +- 11 files changed, 137 insertions(+), 33 deletions(-) create mode 100644 src/OSTimer.cpp diff --git a/src/OSTimer.cpp b/src/OSTimer.cpp new file mode 100644 index 000000000..0da3b7d28 --- /dev/null +++ b/src/OSTimer.cpp @@ -0,0 +1,49 @@ +#include "OSTimer.h" +#include "configuration.h" + +#ifdef NO_ESP32 + +/** + * Schedule a callback to run. The callback must _not_ block, though it is called from regular thread level (not ISR) + * + * NOTE! xTimerPend... seems to ignore the time passed in on ESP32 - I haven't checked on NRF52 + * + * @return true if successful, false if the timer fifo is too full. + */ +bool scheduleOSCallback(PendableFunction callback, void *param1, uint32_t param2, uint32_t delayMsec) +{ + return xTimerPendFunctionCall(callback, param1, param2, pdMS_TO_TICKS(delayMsec)); +} + +#else + +// Super skanky quick hack to use hardware timers of the ESP32 +static hw_timer_t *timer; +static PendableFunction tCallback; +static void *tParam1; +static uint32_t tParam2; + +static void IRAM_ATTR onTimer() +{ + (*tCallback)(tParam1, tParam2); +} + +bool scheduleHWCallback(PendableFunction callback, void *param1, uint32_t param2, uint32_t delayMsec) +{ + if (!timer) { + timer = timerBegin(0, 80, true); // one usec per tick (main clock is 80MhZ on ESP32) + assert(timer); + timerAttachInterrupt(timer, &onTimer, true); + } + + tCallback = callback; + tParam1 = param1; + tParam2 = param2; + + timerAlarmWrite(timer, delayMsec * 1000L, false); // Do not reload, we want it to be a single shot timer + timerRestart(timer); + timerAlarmEnable(timer); + return true; +} + +#endif \ No newline at end of file diff --git a/src/OSTimer.h b/src/OSTimer.h index 73a40ddb3..cdf2386a6 100644 --- a/src/OSTimer.h +++ b/src/OSTimer.h @@ -7,9 +7,12 @@ typedef void (*PendableFunction)(void *pvParameter1, uint32_t ulParameter2); /** * Schedule a callback to run. The callback must _not_ block, though it is called from regular thread level (not ISR) * + * NOTE! ESP32 implementation is busted - always waits 0 ticks + * * @return true if successful, false if the timer fifo is too full. */ -inline bool scheduleCallback(PendableFunction callback, void *param1, uint32_t param2, uint32_t delayMsec) -{ - return xTimerPendFunctionCall(callback, param1, param2, pdMS_TO_TICKS(delayMsec)); -} \ No newline at end of file + bool scheduleOSCallback(PendableFunction callback, void *param1, uint32_t param2, uint32_t delayMsec); + + +/// Uses a hardware timer, but calls the handler in _interrupt_ context +bool scheduleHWCallback(PendableFunction callback, void *param1, uint32_t param2, uint32_t delayMsec); \ No newline at end of file diff --git a/src/SerialConsole.cpp b/src/SerialConsole.cpp index 861219037..9e84a958f 100644 --- a/src/SerialConsole.cpp +++ b/src/SerialConsole.cpp @@ -21,8 +21,8 @@ void SerialConsole::init() } /** - * we override this to notice when we've received a protobuf over the serial stream. Then we shunt off - * debug serial output. + * we override this to notice when we've received a protobuf over the serial + * stream. Then we shunt off debug serial output. */ void SerialConsole::handleToRadio(const uint8_t *buf, size_t len) { diff --git a/src/WorkerThread.h b/src/WorkerThread.h index 318f9d803..f951da32c 100644 --- a/src/WorkerThread.h +++ b/src/WorkerThread.h @@ -75,7 +75,7 @@ class NotifiedWorkerThread : public WorkerThread * * Defaults to clear all of them. */ - uint32_t clearOnRead = ULONG_MAX; + uint32_t clearOnRead = UINT32_MAX; /** * A method that should block execution - either waiting ona queue/mutex or a "task notification" diff --git a/src/mesh/RF95Interface.h b/src/mesh/RF95Interface.h index d0b5fd7f2..6c5e2ab5c 100644 --- a/src/mesh/RF95Interface.h +++ b/src/mesh/RF95Interface.h @@ -40,7 +40,7 @@ class RF95Interface : public RadioLibInterface /** are we actively receiving a packet (only called during receiving state) */ virtual bool isActivelyReceiving(); - + /** * Start waiting to receive a message */ @@ -50,6 +50,6 @@ class RF95Interface : public RadioLibInterface * Add SNR data to received messages */ virtual void addReceiveMetadata(MeshPacket *mp); - private: - void setStandby(); + + virtual void setStandby(); }; \ No newline at end of file diff --git a/src/mesh/RadioInterface.cpp b/src/mesh/RadioInterface.cpp index 6e98a3641..d4a408425 100644 --- a/src/mesh/RadioInterface.cpp +++ b/src/mesh/RadioInterface.cpp @@ -17,7 +17,8 @@ RadioInterface::RadioInterface() : txQueue(MAX_TX_QUEUE) bool RadioInterface::init() { - start("radio", RADIO_STACK_SIZE); // Start our worker thread + // we want this thread to run at very high priority, because it is effectively running as a user space ISR + start("radio", RADIO_STACK_SIZE, configMAX_PRIORITIES - 1); // Start our worker thread return true; } diff --git a/src/mesh/RadioLibInterface.cpp b/src/mesh/RadioLibInterface.cpp index 48a5e8a4d..ada0a337a 100644 --- a/src/mesh/RadioLibInterface.cpp +++ b/src/mesh/RadioLibInterface.cpp @@ -1,5 +1,6 @@ #include "RadioLibInterface.h" #include "MeshTypes.h" +#include "OSTimer.h" #include "mesh-pb-constants.h" #include // FIXME, this class shouldn't need to look into nodedb #include @@ -89,14 +90,17 @@ bool RadioLibInterface::canSendImmediately() // We wait _if_ we are partially though receiving a packet (rather than just merely waiting for one). // To do otherwise would be doubly bad because not only would we drop the packet that was on the way in, // we almost certainly guarantee no one outside will like the packet we are sending. - PendingISR isPending = pending; bool busyTx = sendingPacket != NULL; bool busyRx = isReceiving && isActivelyReceiving(); - if (busyTx || busyRx || isPending) - DEBUG_MSG("Can not send yet, busyTx=%d, busyRx=%d, intPend=%d\n", busyTx, busyRx, isPending); - - return !busyTx && !busyRx && !isPending; + if (busyTx || busyRx) { + if (busyTx) + DEBUG_MSG("Can not send yet, busyTx\n"); + if (busyRx) + DEBUG_MSG("Can not send yet, busyRx\n"); + return false; + } else + return true; } /// Send a packet (possibly by enquing in a private fifo). This routine will @@ -104,8 +108,8 @@ bool RadioLibInterface::canSendImmediately() /// bluetooth comms code. If the txmit queue is empty it might return an error ErrorCode RadioLibInterface::send(MeshPacket *p) { - DEBUG_MSG("enqueuing for send on mesh fr=0x%x,to=0x%x,id=%d\n (txGood=%d,rxGood=%d,rxBad=%d)\n", p->from, p->to, p->id, - txGood, rxGood, rxBad); + DEBUG_MSG("enqueuing for send on mesh fr=0x%x,to=0x%x,id=%d (txGood=%d,rxGood=%d,rxBad=%d)\n", p->from, p->to, p->id, txGood, + rxGood, rxBad); ErrorCode res = txQueue.enqueue(p, 0) ? ERRNO_OK : ERRNO_UNKNOWN; if (res != ERRNO_OK) { // we weren't able to queue it, so we must drop it to prevent leaks @@ -113,7 +117,9 @@ ErrorCode RadioLibInterface::send(MeshPacket *p) return res; } - startTransmitTimer(false); // We want all sending/receiving to be done by our daemon thread + // We want all sending/receiving to be done by our daemon thread, We use a delay here because this packet might have been sent + // in response to a packet we just received. So we want to make sure the other side has had a chance to reconfigure its radio + startTransmitTimer(true); return res; } @@ -127,6 +133,19 @@ bool RadioLibInterface::canSleep() return res; } +/** At the low end we want to pick a delay large enough that anyone who just completed sending (some other node) + * has had enough time to switch their radio back into receive mode. + */ +#define MIN_TX_WAIT_MSEC 100 + +/** + * At the high end, this value is used to spread node attempts across time so when they are replying to a packet + * they don't both check that the airwaves are clear at the same moment. As long as they are off by some amount + * one of the two will be first to start transmitting and the other will see that. I bet 500ms is more than enough + * to guarantee this. + */ +#define MAX_TX_WAIT_MSEC 2000 // stress test would still fail occasionally with 1000 + /** radio helper thread callback. We never immediately transmit after any operation (either rx or tx). Instead we should start receiving and @@ -138,7 +157,7 @@ and then they will wait until that packet finishes. NOTE: the large flood rebroadcast delay might still be needed even with this approach. Because we might not be able to hear other transmitters that we are potentially stomping on. Requires further thought. -FIXME, the 50ms and 200ms values should be tuned via logic analyzer later. +FIXME, the MIN_TX_WAIT_MSEC and MAX_TX_WAIT_MSEC values should be tuned via logic analyzer later. */ void RadioLibInterface::loop() { @@ -162,8 +181,6 @@ void RadioLibInterface::loop() if (!canSendImmediately()) { startTransmitTimer(); // try again in a little while } else { - DEBUG_MSG("Transmit timer completed!\n"); - // Send any outgoing packets we have ready MeshPacket *txp = txQueue.dequeuePtr(0); assert(txp); @@ -176,9 +193,11 @@ void RadioLibInterface::loop() } } -#include "OSTimer.h" +#ifndef NO_ESP32 +#define USE_HW_TIMER +#endif -void RadioLibInterface::timerCallback(void *p1, uint32_t p2) +void IRAM_ATTR RadioLibInterface::timerCallback(void *p1, uint32_t p2) { RadioLibInterface *t = (RadioLibInterface *)p1; @@ -186,7 +205,17 @@ void RadioLibInterface::timerCallback(void *p1, uint32_t p2) // We use without overwrite, so that if there is already an interrupt pending to be handled, that gets handle properly (the // ISR handler will restart our timer) +#ifndef USE_HW_TIMER t->notify(TRANSMIT_DELAY_COMPLETED, eSetValueWithoutOverwrite); +#else + BaseType_t xHigherPriorityTaskWoken; + instance->notifyFromISR(&xHigherPriorityTaskWoken, TRANSMIT_DELAY_COMPLETED, eSetValueWithoutOverwrite); + + /* Force a context switch if xHigherPriorityTaskWoken is now set to pdTRUE. + The macro used to do this is dependent on the port and may be called + portEND_SWITCHING_ISR. */ + YIELD_FROM_ISR(xHigherPriorityTaskWoken); +#endif } void RadioLibInterface::startTransmitTimer(bool withDelay) @@ -194,8 +223,15 @@ void RadioLibInterface::startTransmitTimer(bool withDelay) // If we have work to do and the timer wasn't already scheduled, schedule it now if (!timerRunning && !txQueue.isEmpty()) { timerRunning = true; - uint32_t delay = withDelay ? 0 : random(50, 200); // See documentation for loop() wrt these values - scheduleCallback(timerCallback, this, 0, delay); + uint32_t delay = + !withDelay ? 0 : random(MIN_TX_WAIT_MSEC, MAX_TX_WAIT_MSEC); // See documentation for loop() wrt these values + // DEBUG_MSG("xmit timer %d\n", delay); +#ifdef USE_HW_TIMER + bool okay = scheduleHWCallback(timerCallback, this, 0, delay); +#else + bool okay = scheduleOSCallback(timerCallback, this, 0, delay); +#endif + assert(okay); } } @@ -245,6 +281,7 @@ void RadioLibInterface::handleReceiveInterrupt() const PacketHeader *h = (PacketHeader *)radiobuf; uint8_t ourAddr = nodeDB.getNodeNum(); + rxGood++; if (h->to != 255 && h->to != ourAddr) { DEBUG_MSG("ignoring packet not sent to us\n"); } else { @@ -264,7 +301,6 @@ void RadioLibInterface::handleReceiveInterrupt() } else { // parsing was successful, queue for our recipient mp->has_payload = true; - rxGood++; DEBUG_MSG("Lora RX interrupt from=0x%x, id=%u\n", mp->from, mp->id); deliverToReceiver(mp); @@ -277,6 +313,9 @@ void RadioLibInterface::handleReceiveInterrupt() /** start an immediate transmit */ void RadioLibInterface::startSend(MeshPacket *txp) { + DEBUG_MSG("Starting low level send from=0x%x, id=%u!\n", txp->from, txp->id); + setStandby(); // Cancel any already in process receives + size_t numbytes = beginSending(txp); int res = iface->startTransmit(radiobuf, numbytes); diff --git a/src/mesh/RadioLibInterface.h b/src/mesh/RadioLibInterface.h index c6f595c80..e1e0f1a97 100644 --- a/src/mesh/RadioLibInterface.h +++ b/src/mesh/RadioLibInterface.h @@ -85,8 +85,8 @@ class RadioLibInterface : public RadioInterface void startSend(MeshPacket *txp); /** if we have something waiting to send, start a short random timer so we can come check for collision before actually doing - * the transmit - * + * the transmit + * * If the timer was already running, we just wait for that one to occur. * */ void startTransmitTimer(bool withDelay = true); @@ -103,7 +103,7 @@ class RadioLibInterface : public RadioInterface void applyModemConfig(); /** Could we send right now (i.e. either not actively receiving or transmitting)? */ - bool canSendImmediately(); + virtual bool canSendImmediately(); /** are we actively receiving a packet (only called during receiving state) */ virtual bool isActivelyReceiving() = 0; @@ -128,4 +128,6 @@ class RadioLibInterface : public RadioInterface virtual void addReceiveMetadata(MeshPacket *mp) = 0; virtual void loop(); // Idle processing + + virtual void setStandby() = 0; }; \ No newline at end of file diff --git a/src/mesh/RadioLibRF95.cpp b/src/mesh/RadioLibRF95.cpp index cd8a3b824..c82c1b0c7 100644 --- a/src/mesh/RadioLibRF95.cpp +++ b/src/mesh/RadioLibRF95.cpp @@ -56,8 +56,13 @@ int16_t RadioLibRF95::setFrequency(float freq) bool RadioLibRF95::isReceiving() { // 0x0b == Look for header info valid, signal synchronized or signal detected - uint8_t reg = _mod->SPIreadRegister(SX127X_REG_MODEM_STAT) & 0x1f; + uint8_t reg = readReg(SX127X_REG_MODEM_STAT); // Serial.printf("reg %x\n", reg); return (reg & (RH_RF95_MODEM_STATUS_SIGNAL_DETECTED | RH_RF95_MODEM_STATUS_SIGNAL_SYNCHRONIZED | - RH_RF95_MODEM_STATUS_HEADER_INFO_VALID)) != 0; + RH_RF95_MODEM_STATUS_HEADER_INFO_VALID)) != 0; } + +uint8_t RadioLibRF95::readReg(uint8_t addr) +{ + return _mod->SPIreadRegister(addr); +} \ No newline at end of file diff --git a/src/mesh/RadioLibRF95.h b/src/mesh/RadioLibRF95.h index 1746200dd..3c45a8f44 100644 --- a/src/mesh/RadioLibRF95.h +++ b/src/mesh/RadioLibRF95.h @@ -62,6 +62,9 @@ class RadioLibRF95: public SX1278 { // Return true if we are actively receiving a message currently bool isReceiving(); + /// For debugging + uint8_t readReg(uint8_t addr); + #ifndef RADIOLIB_GODMODE private: #endif diff --git a/src/mesh/SX1262Interface.h b/src/mesh/SX1262Interface.h index 18ab8ef75..c1e1fb1c6 100644 --- a/src/mesh/SX1262Interface.h +++ b/src/mesh/SX1262Interface.h @@ -48,6 +48,8 @@ class SX1262Interface : public RadioLibInterface */ virtual void addReceiveMetadata(MeshPacket *mp); + virtual void setStandby(); + private: - void setStandby(); + }; \ No newline at end of file From 80268ea56a0ade86384d3079f31239b9be7c58bb Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 2 May 2020 19:51:55 -0700 Subject: [PATCH 3/7] send() is supposed to always free buffers, even if it returns an error --- src/mesh/Router.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 4790ff17e..b5a34a801 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -44,7 +44,7 @@ void Router::loop() /** * Send a packet on a suitable interface. This routine will * later free() the packet to pool. This routine is not allowed to stall. - * If the txmit queue is full it might return an error + * If the txmit queue is full it might return an error. */ ErrorCode Router::send(MeshPacket *p) { @@ -53,6 +53,7 @@ ErrorCode Router::send(MeshPacket *p) 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; } } From 79c61cf0e0acda97a17fef2b045e4d738e386fe5 Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 2 May 2020 19:52:37 -0700 Subject: [PATCH 4/7] limit max power on rf95 to 17 (rather than 20, because 20 can... burn up parts if you exceed 1% duty cycle) --- src/mesh/RF95Interface.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/mesh/RF95Interface.cpp b/src/mesh/RF95Interface.cpp index c335205b7..0557dd082 100644 --- a/src/mesh/RF95Interface.cpp +++ b/src/mesh/RF95Interface.cpp @@ -3,6 +3,9 @@ #include "RadioLibRF95.h" #include +#define MAX_POWER 17 +// if we use 20 we are limited to 1% duty cycle or hw might overheat. For continuous operation set a limit of 17 + RF95Interface::RF95Interface(RADIOLIB_PIN_TYPE cs, RADIOLIB_PIN_TYPE irq, RADIOLIB_PIN_TYPE rst, SPIClass &spi) : RadioLibInterface(cs, irq, rst, 0, spi) { @@ -15,10 +18,10 @@ RF95Interface::RF95Interface(RADIOLIB_PIN_TYPE cs, RADIOLIB_PIN_TYPE irq, RADIOL bool RF95Interface::init() { RadioLibInterface::init(); - + applyModemConfig(); - if (power > 20) // This chip has lower power limits than some - power = 20; + if (power > MAX_POWER) // This chip has lower power limits than some + power = MAX_POWER; iface = lora = new RadioLibRF95(&module); int res = lora->begin(freq, bw, sf, cr, syncWord, power, currentLimit, preambleLength); @@ -27,7 +30,7 @@ bool RF95Interface::init() if (res == ERR_NONE) res = lora->setCRC(SX126X_LORA_CRC_ON); - if (res == ERR_NONE) + if (res == ERR_NONE) startReceive(); // start receiving return res == ERR_NONE; @@ -67,8 +70,8 @@ bool RF95Interface::reconfigure() err = lora->setFrequency(freq); assert(err == ERR_NONE); - if (power > 20) // This chip has lower power limits than some - power = 20; + if (power > MAX_POWER) // This chip has lower power limits than some + power = MAX_POWER; err = lora->setOutputPower(power); assert(err == ERR_NONE); @@ -120,4 +123,4 @@ bool RF95Interface::sleep() lora->sleep(); return true; -} \ No newline at end of file +} From 07b4eea037613da190e805619839ec7426055f2c Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 2 May 2020 19:52:54 -0700 Subject: [PATCH 5/7] fix log msg --- src/mesh/NodeDB.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index b27c49e12..0559754a3 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -303,7 +303,8 @@ void NodeDB::updateFrom(const MeshPacket &mp) if (p.has_data) { // Keep a copy of the most recent text message. if (p.data.typ == Data_Type_CLEAR_TEXT) { - DEBUG_MSG("Received text msg from=0%0x, msg=%.*s\n", mp.from, p.data.payload.size, p.data.payload.bytes); + DEBUG_MSG("Received text msg from=0x%0x, id=%d, msg=%.*s\n", mp.from, mp.id, p.data.payload.size, + p.data.payload.bytes); if (mp.to == NODENUM_BROADCAST || mp.to == nodeDB.getNodeNum()) { // We only store/display messages destined for us. devicestate.rx_text_message = mp; From ad2f63919528702784d09d11fc84268dbfcafca3 Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 2 May 2020 19:53:13 -0700 Subject: [PATCH 6/7] don't leak messages if they are handled locally --- src/mesh/MeshService.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mesh/MeshService.cpp b/src/mesh/MeshService.cpp index 873d17eca..71eb5f37c 100644 --- a/src/mesh/MeshService.cpp +++ b/src/mesh/MeshService.cpp @@ -266,8 +266,10 @@ void MeshService::sendToMesh(MeshPacket *p) } // If the phone sent a packet just to us, don't send it out into the network - if (p->to == nodeDB.getNodeNum()) + if (p->to == nodeDB.getNodeNum()) { DEBUG_MSG("Dropping locally processed message\n"); + releaseToPool(p); + } else { // Note: We might return !OK if our fifo was full, at that point the only option we have is to drop it if (router.send(p) != ERRNO_OK) { From 1d9290afc0a656f62af194c3f9839039539f39f5 Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 2 May 2020 19:53:58 -0700 Subject: [PATCH 7/7] now that the rfinterfaces are smarter, no need to do backoff in the flood router. the interfaces will handle it. --- src/mesh/FloodingRouter.cpp | 57 +++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/src/mesh/FloodingRouter.cpp b/src/mesh/FloodingRouter.cpp index 582448da6..da7070172 100644 --- a/src/mesh/FloodingRouter.cpp +++ b/src/mesh/FloodingRouter.cpp @@ -5,6 +5,8 @@ /// We clear our old flood record five minute after we see the last of it #define FLOOD_EXPIRE_TIME (5 * 60 * 1000L) +static bool supportFlooding = true; // Sometimes to simplify debugging we want jusT simple broadcast only + FloodingRouter::FloodingRouter() : toResend(MAX_NUM_NODES) { recentBroadcasts.reserve(MAX_NUM_NODES); // Prealloc the worst case # of records - to prevent heap fragmentation @@ -19,7 +21,8 @@ FloodingRouter::FloodingRouter() : toResend(MAX_NUM_NODES) ErrorCode FloodingRouter::send(MeshPacket *p) { // We update our table of recent broadcasts, even for messages we send - wasSeenRecently(p); + if (supportFlooding) + wasSeenRecently(p); return Router::send(p); } @@ -30,6 +33,12 @@ uint32_t getRandomDelay() return random(200, 10 * 1000L); // between 200ms and 10s } +/** + * Now that our generalized packet send code has a random delay - I don't think we need to wait here + * But I'm leaving this bool until I rip the code out for good. + */ +bool needDelay = false; + /** * Called from loop() * Handle any packet that is received by an interface on this node. @@ -39,28 +48,40 @@ uint32_t getRandomDelay() */ void FloodingRouter::handleReceived(MeshPacket *p) { - if (wasSeenRecently(p)) { - DEBUG_MSG("Ignoring incoming floodmsg, because we've already seen it\n"); - packetPool.release(p); - } else { - if (p->to == NODENUM_BROADCAST) { - if (p->id != 0) { - uint32_t delay = getRandomDelay(); + if (supportFlooding) { + if (wasSeenRecently(p)) { + DEBUG_MSG("Ignoring incoming floodmsg, because we've already seen it\n"); + packetPool.release(p); + } else { + if (p->to == NODENUM_BROADCAST) { + if (p->id != 0) { + MeshPacket *tosend = packetPool.allocCopy(*p); // keep a copy because we will be sending it - DEBUG_MSG("Rebroadcasting received floodmsg to neighbors in %u msec, fr=0x%x,to=0x%x,id=%d\n", delay, p->from, - p->to, p->id); + if (needDelay) { + uint32_t delay = getRandomDelay(); - MeshPacket *tosend = packetPool.allocCopy(*p); - toResend.enqueue(tosend); - setPeriod(delay); // This will work even if we were already waiting a random delay - } else { - DEBUG_MSG("Ignoring a simple (0 hop) broadcast\n"); + DEBUG_MSG("Rebroadcasting received floodmsg to neighbors in %u msec, fr=0x%x,to=0x%x,id=%d\n", delay, + p->from, p->to, p->id); + + toResend.enqueue(tosend); + setPeriod(delay); // This will work even if we were already waiting a random delay + } else { + DEBUG_MSG("Rebroadcasting received floodmsg to neighbors, fr=0x%x,to=0x%x,id=%d\n", p->from, p->to, + p->id); + // Note: we are careful to resend using the original senders node id + // We are careful not to call our hooked version of send() - because we don't want to check this again + Router::send(tosend); + } + } else { + DEBUG_MSG("Ignoring a simple (0 hop) broadcast\n"); + } } - } - // handle the packet as normal + // handle the packet as normal + Router::handleReceived(p); + } + } else Router::handleReceived(p); - } } void FloodingRouter::doTask()