From bb9f595b8b495ed7bf6f87f0cc68adca02a1852f Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 2 May 2020 19:51:25 -0700 Subject: [PATCH] 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