RadioLib edge-triggered interrupts robustness (#9658)

* Fix potential race condition (read, ISR write, clear) in NotifiedWorkerThread

* Check for missed edge-triggered interrupts race condition between startReceive and enableInterrupt

* Occasionally poll to catch missed RX_DONE interrupts. (RadioLibInterface::pollMissedIrqs)

* Simplify RadioLibInterface::checkRxDoneIrqFlag()

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>

Author: compumike

src/concurrency/NotifiedWorkerThread.cpp

@@ -76,8 +76,10 @@ bool NotifiedWorkerThread::notifyLater(uint32_t delay, uint32_t v, bool overwrit
 
 void NotifiedWorkerThread::checkNotification()
 {
-    auto n = notification;
-    notification = 0; // clear notification
+    // Atomically read and clear. (This avoids a potential race condition where an interrupt handler could set a new notification
+    // after checkNotification reads but before it clears, which would cause us to miss that notification until the next one comes
+    // in.)
+    auto n = notification.exchange(0); // read+clear atomically: like `n = notification; notification = 0;` but interrupt-safe
     if (n) {
         onNotify(n);
     }

src/concurrency/NotifiedWorkerThread.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "OSThread.h"
+#include <atomic>
 
 namespace concurrency
 {
@@ -13,7 +14,7 @@ class NotifiedWorkerThread : public OSThread
     /**
      * The notification that was most recently used to wake the thread.  Read from runOnce()
      */
-    uint32_t notification = 0;
+    std::atomic<uint32_t> notification{0};
 
   public:
     NotifiedWorkerThread(const char *name) : OSThread(name) {}

src/main.cpp

@@ -1119,6 +1119,14 @@ void loop()
 #endif
     power->powerCommandsCheck();
 
+    if (RadioLibInterface::instance != nullptr) {
+        static uint32_t lastRadioMissedIrqPoll;
+        if (!Throttle::isWithinTimespanMs(lastRadioMissedIrqPoll, 1000)) {
+            lastRadioMissedIrqPoll = millis();
+            RadioLibInterface::instance->pollMissedIrqs();
+        }
+    }
+
 #ifdef DEBUG_STACK
     static uint32_t lastPrint = 0;
     if (!Throttle::isWithinTimespanMs(lastPrint, 10 * 1000L)) {

src/mesh/LR11x0Interface.cpp

@@ -263,6 +263,7 @@ template <typename T> void LR11x0Interface<T>::startReceive()
 
     // Must be done AFTER, starting transmit, because startTransmit clears (possibly stale) interrupt pending register bits
     enableInterrupt(isrRxLevel0);
+    checkRxDoneIrqFlag();
 #endif
 }
 

src/mesh/RF95Interface.cpp

@@ -301,6 +301,7 @@ void RF95Interface::startReceive()
 
     // Must be done AFTER, starting receive, because startReceive clears (possibly stale) interrupt pending register bits
     enableInterrupt(isrRxLevel0);
+    checkRxDoneIrqFlag();
 }
 
 bool RF95Interface::isChannelActive()

src/mesh/RadioLibInterface.cpp

@@ -521,6 +521,22 @@ void RadioLibInterface::startReceive()
     powerMon->setState(meshtastic_PowerMon_State_Lora_RXOn);
 }
 
+void RadioLibInterface::pollMissedIrqs()
+{
+    // RadioLibInterface::enableInterrupt uses EDGE-TRIGGERED interrupts. Poll as a backup to catch missed edges.
+    if (isReceiving) {
+        checkRxDoneIrqFlag();
+    }
+}
+
+void RadioLibInterface::checkRxDoneIrqFlag()
+{
+    if (iface->checkIrq(RADIOLIB_IRQ_RX_DONE)) {
+        LOG_WARN("caught missed RX_DONE");
+        notify(ISR_RX, true);
+    }
+}
+
 void RadioLibInterface::configHardwareForSend()
 {
     powerMon->setState(meshtastic_PowerMon_State_Lora_TXOn);

src/mesh/RadioLibInterface.h

@@ -112,6 +112,11 @@ class RadioLibInterface : public RadioInterface, protected concurrency::Notified
      */
     virtual void enableInterrupt(void (*)()) = 0;
 
+    /**
+     * Poll as a backup to catch missed edge-triggered interrupts.
+     */
+    void pollMissedIrqs();
+
     /**
      * Debugging counts
      */
@@ -264,4 +269,6 @@ class RadioLibInterface : public RadioInterface, protected concurrency::Notified
      */
 
     bool removePendingTXPacket(NodeNum from, PacketId id, uint32_t hop_limit_lt) override;
+
+    void checkRxDoneIrqFlag();
 };

src/mesh/SX126xInterface.cpp

@@ -351,6 +351,7 @@ template <typename T> void SX126xInterface<T>::startReceive()
 
     // Must be done AFTER, starting transmit, because startTransmit clears (possibly stale) interrupt pending register bits
     enableInterrupt(isrRxLevel0);
+    checkRxDoneIrqFlag();
 #endif
 }
 

src/mesh/SX128xInterface.cpp

@@ -271,6 +271,7 @@ template <typename T> void SX128xInterface<T>::startReceive()
 
     // Must be done AFTER, starting transmit, because startTransmit clears (possibly stale) interrupt pending register bits
     enableInterrupt(isrRxLevel0);
+    checkRxDoneIrqFlag();
 #endif
 }