move packet handling into its own thread

docs/software/nrf52-TODO.md

@@ -62,6 +62,7 @@ Needed to be fully functional at least at the same level of the ESP32 boards. At
 
 Nice ideas worth considering someday...
 
+- turn on freertos stack size checking
 - Currently we use Nordic's vendor ID, which is apparently okay: https://devzone.nordicsemi.com/f/nordic-q-a/44014/using-nordic-vid-and-pid-for-nrf52840 and I just picked a PID of 0x4403
 - Use NRF logger module (includes flash logging etc...) instead of DEBUG_MSG
 - Use "LED softblink" library on NRF52 to do nice pretty "breathing" LEDs. Don't whack LED from main thread anymore.

src/WorkerThread.cpp

@@ -0,0 +1,44 @@
+#include "WorkerThread.h"
+#include <assert.h>
+
+void Thread::start(const char *name, size_t stackSize, uint32_t priority)
+{
+    auto r = xTaskCreate(callRun, name, stackSize, this, priority, &taskHandle);
+    assert(r == pdPASS);
+}
+
+void Thread::callRun(void *_this)
+{
+    ((Thread *)_this)->doRun();
+}
+
+void WorkerThread::doRun()
+{
+    while (!wantExit) {
+        block();
+        loop();
+    }
+}
+
+/**
+ * Notify this thread so it can run
+ */
+void NotifiedWorkerThread::notify(uint32_t v, eNotifyAction action)
+{
+    xTaskNotify(taskHandle, v, action);
+}
+
+/**
+ * Notify from an ISR
+ */
+void NotifiedWorkerThread::notifyFromISR(BaseType_t *highPriWoken, uint32_t v, eNotifyAction action)
+{
+    xTaskNotifyFromISR(taskHandle, v, action, highPriWoken);
+}
+
+void NotifiedWorkerThread::block()
+{
+    xTaskNotifyWait(0,                             // don't clear notification on entry
+                    0,                             //  do not reset notification value on read
+                    &notification, portMAX_DELAY); // Wait forever
+}

src/WorkerThread.h

@@ -0,0 +1,77 @@
+#include <Arduino.h>
+
+class Thread
+{
+  protected:
+    TaskHandle_t taskHandle = NULL;
+
+    /**
+     * set this to true to ask thread to cleanly exit asap
+     */
+    volatile bool wantExit = false;
+
+  public:
+    void start(const char *name, size_t stackSize = 1024, uint32_t priority = tskIDLE_PRIORITY);
+
+    virtual ~Thread() { vTaskDelete(taskHandle); }
+
+  protected:
+    /**
+     * The method that will be called when start is called.
+     */
+    virtual void doRun() = 0;
+
+  private:
+    static void callRun(void *_this);
+};
+
+/**
+ * This wraps threading (FreeRTOS for now) with a blocking API intended for efficiently converting onlyschool arduino loop() code.
+ *
+ * Use as a mixin base class for the classes you want to convert.
+ *
+ * https://www.freertos.org/RTOS_Task_Notification_As_Mailbox.html
+ */
+class WorkerThread : public Thread
+{
+  protected:
+    /**
+     * A method that should block execution - either waiting ona queue/mutex or a "task notification"
+     */
+    virtual void block() = 0;
+
+    virtual void loop() = 0;
+
+    /**
+     * The method that will be called when start is called.
+     */
+    virtual void doRun();
+};
+
+/**
+ * A worker thread that waits on a freertos notification
+ */
+class NotifiedWorkerThread : public WorkerThread
+{
+  public:
+    /**
+     * Notify this thread so it can run
+     */
+    void notify(uint32_t v = 0, eNotifyAction action = eNoAction);
+
+    /**
+     * Notify from an ISR
+     */
+    void notifyFromISR(BaseType_t *highPriWoken, uint32_t v = 0, eNotifyAction action = eNoAction);
+
+  protected:
+    /**
+     * The notification that was most recently used to wake the thread.  Read from loop()
+     */
+    uint32_t notification = 0;
+
+    /**
+     * A method that should block execution - either waiting ona queue/mutex or a "task notification"
+     */
+    virtual void block();
+};

src/mesh/RF95Interface.cpp

@@ -14,6 +14,8 @@ RF95Interface::RF95Interface(RADIOLIB_PIN_TYPE cs, RADIOLIB_PIN_TYPE irq, RADIOL
 /// \return true if initialisation succeeded.
 bool RF95Interface::init()
 {
+    RadioLibInterface::init();
+    
     applyModemConfig();
     if (power > 20) // This chip has lower power limits than some
         power = 20;
@@ -25,7 +27,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;

src/mesh/RadioInterface.cpp

@@ -7,11 +7,20 @@
 #include <pb_decode.h>
 #include <pb_encode.h>
 
+#define RADIO_STACK_SIZE 4096
+
 RadioInterface::RadioInterface() : txQueue(MAX_TX_QUEUE)
 {
     assert(sizeof(PacketHeader) == 4); // make sure the compiler did what we expected
 }
 
+bool RadioInterface::init()
+{
+    start("radio", RADIO_STACK_SIZE); // Start our worker thread
+
+    return true;
+}
+
 ErrorCode SimRadio::send(MeshPacket *p)
 {
     DEBUG_MSG("SimRadio.send\n");

src/mesh/RadioInterface.h

@@ -3,6 +3,7 @@
 #include "MemoryPool.h"
 #include "MeshTypes.h"
 #include "PointerQueue.h"
+#include "WorkerThread.h"
 #include "mesh.pb.h"
 
 #define MAX_TX_QUEUE 16 // max number of packets which can be waiting for transmission
@@ -29,7 +30,7 @@ typedef enum {
  *
  * This defines the SOLE API for talking to radios (because soon we will have alternate radio implementations)
  */
-class RadioInterface
+class RadioInterface : protected NotifiedWorkerThread
 {
     friend class MeshRadio; // for debugging we let that class touch pool
     PointerQueue<MeshPacket> *rxDest = NULL;
@@ -64,8 +65,6 @@ class RadioInterface
      */
     void setReceiver(PointerQueue<MeshPacket> *_rxDest) { rxDest = _rxDest; }
 
-    virtual void loop() {} // Idle processing
-
     /**
      * Return true if we think the board can go to sleep (i.e. our tx queue is empty, we are not sending or receiving)
      *
@@ -88,7 +87,7 @@ class RadioInterface
     /// Initialise the Driver transport hardware and software.
     /// Make sure the Driver is properly configured before calling init().
     /// \return true if initialisation succeeded.
-    virtual bool init() = 0;
+    virtual bool init();
 
     /// Apply any radio provisioning changes
     /// Make sure the Driver is properly configured before calling init().
@@ -103,6 +102,8 @@ class RadioInterface
      * Used as the first step of
      */
     size_t beginSending(MeshPacket *p);
+
+    virtual void loop() {} // Idle processing
 };
 
 class SimRadio : public RadioInterface

src/mesh/RadioLibInterface.cpp

@@ -17,16 +17,39 @@ RadioLibInterface::RadioLibInterface(RADIOLIB_PIN_TYPE cs, RADIOLIB_PIN_TYPE irq
     instance = this;
 }
 
+#ifndef NO_ESP32
+// ESP32 doesn't use that flag
+#define YIELD_FROM_ISR(x) portYIELD_FROM_ISR()
+#else
+#define YIELD_FROM_ISR(x) portYIELD_FROM_ISR(x)
+#endif
+
 void INTERRUPT_ATTR RadioLibInterface::isrRxLevel0()
 {
-    instance->pending = ISR_RX;
     instance->disableInterrupt();
+
+    instance->pending = ISR_RX;
+    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);
 }
 
 void INTERRUPT_ATTR RadioLibInterface::isrTxLevel0()
 {
-    instance->pending = ISR_TX;
     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);
 }
 
 /** Our ISR code currently needs this to find our active instance
@@ -117,19 +140,17 @@ bool RadioLibInterface::canSleep()
 
 void RadioLibInterface::loop()
 {
-    PendingISR wasPending;
-    while ((wasPending = pending) != 0) { // atomic read
-        pending = ISR_NONE; // If the flag was set, it is _guaranteed_ the ISR won't be running, because it masked itself
+    PendingISR wasPending = pending;
+    pending = ISR_NONE;
 
-        if (wasPending == ISR_TX)
-            handleTransmitInterrupt();
-        else if (wasPending == ISR_RX)
-            handleReceiveInterrupt();
-        else
-            assert(0);
+    if (wasPending == ISR_TX)
+        handleTransmitInterrupt();
+    else if (wasPending == ISR_RX)
+        handleReceiveInterrupt();
+    else
+        assert(0); // We expected to receive a valid notification from the ISR
 
-        startNextWork();
-    }
+    startNextWork();
 }
 
 void RadioLibInterface::startNextWork()

src/mesh/RadioLibInterface.h

@@ -13,10 +13,9 @@
 
 class RadioLibInterface : public RadioInterface
 {
+    /// Used as our notification from the ISR
     enum PendingISR { ISR_NONE = 0, ISR_RX, ISR_TX };
 
-    /**
-     * What sort of interrupt do we expect our helper thread to now handle */
     volatile PendingISR pending = ISR_NONE;
 
     /** Our ISR code currently needs this to find our active instance
@@ -73,10 +72,6 @@ class RadioLibInterface : public RadioInterface
 
     virtual ErrorCode send(MeshPacket *p);
 
-    // methods from radiohead
-
-    virtual void loop(); // Idle processing
-
     /**
      * Return true if we think the board can go to sleep (i.e. our tx queue is empty, we are not sending or receiving)
      *
@@ -124,4 +119,6 @@ class RadioLibInterface : public RadioInterface
      * Add SNR data to received messages
      */
     virtual void addReceiveMetadata(MeshPacket *mp) = 0;
+
+    virtual void loop(); // Idle processing
 };

src/mesh/Router.cpp

@@ -35,9 +35,6 @@ Router::Router() : fromRadioQueue(MAX_RX_FROMRADIO) {}
  */
 void Router::loop()
 {
-    if (iface)
-        iface->loop();
-
     MeshPacket *mp;
     while ((mp = fromRadioQueue.dequeuePtr(0)) != NULL) {
         handleReceived(mp);

src/mesh/SX1262Interface.cpp

@@ -12,6 +12,8 @@ SX1262Interface::SX1262Interface(RADIOLIB_PIN_TYPE cs, RADIOLIB_PIN_TYPE irq, RA
 /// \return true if initialisation succeeded.
 bool SX1262Interface::init()
 {
+    RadioLibInterface::init();
+
     float tcxoVoltage = 0;        // None - we use an XTAL
     bool useRegulatorLDO = false; // Seems to depend on the connection to pin 9/DCC_SW - if an inductor DCDC?