Fix #11

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

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));
-}
+ 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);

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)
 {

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"

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();
 };

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;
 }

src/mesh/RadioLibInterface.cpp

@@ -1,5 +1,6 @@
 #include "RadioLibInterface.h"
 #include "MeshTypes.h"
+#include "OSTimer.h"
 #include "mesh-pb-constants.h"
 #include <NodeDB.h> // FIXME, this class shouldn't need to look into nodedb
 #include <configuration.h>
@@ -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);

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;
 };

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);
+}

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

src/mesh/SX1262Interface.h

@@ -48,6 +48,8 @@ class SX1262Interface : public RadioLibInterface
      */
     virtual void addReceiveMetadata(MeshPacket *mp);
 
+    virtual void setStandby();
+
   private:
-    void setStandby();
+
 };