From 2ad314f15041df708c88e8c3b22d1f1cbf8cee56 Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 2 May 2020 08:29:51 -0700 Subject: [PATCH] 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);