Merge pull request #100 from geeksville/removeradiohead

Removeradiohead

src/main.cpp

@@ -336,9 +336,9 @@ void loop()
 {
     uint32_t msecstosleep = 1000 * 30; // How long can we sleep before we again need to service the main loop?
 
-    powerFSM.run_machine();
     gps.loop();
     router.loop();
+    powerFSM.run_machine();
     service.loop();
 
     ledPeriodic.loop();

src/rf95/CustomRF95.cpp

@@ -20,7 +20,7 @@ bool CustomRF95::canSleep()
 
     bool res = (_mode == RHModeInitialising || _mode == RHModeIdle || _mode == RHModeRx) && !isRx && txQueue.isEmpty();
     if (!res) // only print debug messages if we are vetoing sleep
-        DEBUG_MSG("canSleep, mode=%d, isRx=%d, txEmpty=%d, txGood=%d\n", _mode, isRx, txQueue.isEmpty(), _txGood);
+        DEBUG_MSG("radio wait to sleep, mode=%d, isRx=%d, txEmpty=%d, txGood=%d\n", _mode, isRx, txQueue.isEmpty(), _txGood);
 
     return res;
 }
@@ -78,13 +78,12 @@ void CustomRF95::handleInterrupt()
 {
     RH_RF95::handleInterrupt();
 
-    BaseType_t higherPriWoken = false;
     if (_mode == RHModeIdle) // We are now done sending or receiving
     {
         if (sendingPacket) // Were we sending?
         {
             // We are done sending that packet, release it
-            packetPool.releaseFromISR(sendingPacket, &higherPriWoken);
+            packetPool.release(sendingPacket);
             sendingPacket = NULL;
             // DEBUG_MSG("Done with send\n");
         }
@@ -123,43 +122,35 @@ void CustomRF95::handleInterrupt()
             }
 
             if (!pb_decode_from_bytes(payload, payloadLen, SubPacket_fields, p)) {
-                packetPool.releaseFromISR(mp, &higherPriWoken);
+                packetPool.release(mp);
             } else {
                 // parsing was successful, queue for our recipient
                 mp->has_payload = true;
 
-                deliverToReceiverISR(mp, &higherPriWoken);
+                deliverToReceiver(mp);
             }
 
             clearRxBuf(); // This message accepted and cleared
         }
 
-        higherPriWoken |= handleIdleISR();
+        handleIdleISR();
     }
-
-    // If we call this _IT WILL NOT RETURN_
-    if (higherPriWoken)
-        portYIELD_FROM_ISR();
 }
 
 /** The ISR doesn't have any good work to do, give a new assignment.
  *
  * Return true if a higher pri task has woken
  */
-bool CustomRF95::handleIdleISR()
+void CustomRF95::handleIdleISR()
 {
-    BaseType_t higherPriWoken = false;
-
     // First send any outgoing packets we have ready
-    MeshPacket *txp = txQueue.dequeuePtrFromISR(0);
+    MeshPacket *txp = txQueue.dequeuePtr(0);
     if (txp)
         startSend(txp);
     else {
         // Nothing to send, let's switch back to receive mode
         setModeRx();
     }
-
-    return higherPriWoken;
 }
 
 /// This routine might be called either from user space or ISR
@@ -197,6 +188,8 @@ void CustomRF95::startSend(MeshPacket *txp)
 
 void CustomRF95::loop()
 {
+    RH_RF95::loop();
+
     // It should never take us more than 30 secs to send a packet, if it does, we have a bug, FIXME, move most of this
     // into CustomRF95
     uint32_t now = millis();

src/rf95/CustomRF95.h

@@ -52,5 +52,5 @@ class CustomRF95 : public RH_RF95, public RadioInterface
     void startSend(MeshPacket *txp);
 
     /// Return true if a higher pri task has woken
-    bool handleIdleISR();
+    void handleIdleISR();
 };

src/rf95/RH_RF95.cpp

@@ -34,16 +34,12 @@ bool RH_RF95::init()
     if (!RHSPIDriver::init())
         return false;
 
-    // Determine the interrupt number that corresponds to the interruptPin
-    int interruptNumber = digitalPinToInterrupt(_interruptPin);
-    if (interruptNumber == NOT_AN_INTERRUPT)
-        return false;
 #ifdef RH_ATTACHINTERRUPT_TAKES_PIN_NUMBER
     interruptNumber = _interruptPin;
 #endif
 
     // Tell the low level SPI interface we will use SPI within this interrupt
-    spiUsingInterrupt(interruptNumber);
+    // spiUsingInterrupt(interruptNumber);
 
     // No way to check the device type :-(
 
@@ -114,6 +110,17 @@ bool RH_RF95::init()
             return false; // Too many devices, not enough interrupt vectors
     }
     _deviceForInterrupt[_myInterruptIndex] = this;
+
+    return enableInterrupt();
+}
+
+bool RH_RF95::enableInterrupt()
+{
+    // Determine the interrupt number that corresponds to the interruptPin
+    int interruptNumber = digitalPinToInterrupt(_interruptPin);
+    if (interruptNumber == NOT_AN_INTERRUPT)
+        return false;
+
     if (_myInterruptIndex == 0)
         attachInterrupt(interruptNumber, isr0, ONHIGH);
     else if (_myInterruptIndex == 1)
@@ -126,6 +133,12 @@ bool RH_RF95::init()
     return true;
 }
 
+void RH_INTERRUPT_ATTR RH_RF95::disableInterrupt()
+{
+    int interruptNumber = digitalPinToInterrupt(_interruptPin);
+    detachInterrupt(interruptNumber);
+}
+
 void RH_RF95::prepareDeepSleep()
 {
     // Determine the interrupt number that corresponds to the interruptPin
@@ -143,6 +156,13 @@ bool RH_RF95::isReceiving()
                                         RH_RF95_MODEM_STATUS_HEADER_INFO_VALID)) != 0;
 }
 
+void RH_INTERRUPT_ATTR RH_RF95::handleInterruptLevel0()
+{
+    disableInterrupt(); // Disable our interrupt until our helper thread can run (because the IRQ will remain asserted until we
+                        // talk to it via SPI)
+    pendingInterrupt = true;
+}
+
 // C++ level interrupt handler for this instance
 // LORA is unusual in that it has several interrupt lines, and not a single, combined one.
 // On MiniWirelessLoRa, only one of the several interrupt lines (DI0) from the RFM95 is usefuly
@@ -153,15 +173,14 @@ void RH_RF95::handleInterrupt()
     // Read the interrupt register
     uint8_t irq_flags = spiRead(RH_RF95_REG_12_IRQ_FLAGS);
 
-    // ack all interrupts, note - we did this already in the RX_DONE case above, and we don't want to do it twice
+    // ack all interrupts
+    // note from radiohead author wrt old code (with IMO wrong fix)
     // Sigh: on some processors, for some unknown reason, doing this only once does not actually
     // clear the radio's interrupt flag. So we do it twice. Why? (kevinh - I think the root cause we want level
     // triggered interrupts here - not edge.  Because edge allows us to miss handling secondard interrupts that occurred
     // while this ISR was running.  Better to instead, configure the interrupts as level triggered and clear pending
     // at the _beginning_ of the ISR.  If any interrupts occur while handling the ISR, the signal will remain asserted and
     // our ISR will be reinvoked to handle that case)
-    // kevinh: turn this off until root cause is known, because it can cause missed interrupts!
-    // spiWrite(RH_RF95_REG_12_IRQ_FLAGS, 0xff); // Clear all IRQ flags
     spiWrite(RH_RF95_REG_12_IRQ_FLAGS, 0xff); // Clear all IRQ flags
 
     // Note: there can be substantial latency between ISR assertion and this function being run, therefore
@@ -169,14 +188,10 @@ void RH_RF95::handleInterrupt()
 
     // Note: we are running the chip in continuous receive mode (currently, so RX_TIMEOUT shouldn't ever occur)
     bool haveRxError = irq_flags & (RH_RF95_RX_TIMEOUT | RH_RF95_PAYLOAD_CRC_ERROR);
-    if (haveRxError)
+    if (haveRxError) {
-    //    if (_mode == RHModeRx && irq_flags & (RH_RF95_RX_TIMEOUT | RH_RF95_PAYLOAD_CRC_ERROR))
-    {
         _rxBad++;
         clearRxBuf();
-    }
+    } else if (irq_flags & RH_RF95_RX_DONE) {
-
-    if ((irq_flags & RH_RF95_RX_DONE) && !haveRxError) {
         // Read the RegHopChannel register to check if CRC presence is signalled
         // in the header. If not it might be a stray (noise) packet.*
         uint8_t crc_present = spiRead(RH_RF95_REG_1C_HOP_CHANNEL) & RH_RF95_RX_PAYLOAD_CRC_IS_ON;
@@ -227,6 +242,16 @@ void RH_RF95::handleInterrupt()
         _cad = irq_flags & RH_RF95_CAD_DETECTED;
         setModeIdle();
     }
+
+    enableInterrupt(); // Let ISR run again
+}
+
+void RH_RF95::loop()
+{
+    while (pendingInterrupt) {
+        pendingInterrupt = false; // If the flag was set, it is _guaranteed_ the ISR won't be running, because it masked itself
+        handleInterrupt();
+    }
 }
 
 // These are low level functions that call the interrupt handler for the correct
@@ -235,17 +260,17 @@ void RH_RF95::handleInterrupt()
 void RH_INTERRUPT_ATTR RH_RF95::isr0()
 {
     if (_deviceForInterrupt[0])
-        _deviceForInterrupt[0]->handleInterrupt();
+        _deviceForInterrupt[0]->handleInterruptLevel0();
 }
 void RH_INTERRUPT_ATTR RH_RF95::isr1()
 {
     if (_deviceForInterrupt[1])
-        _deviceForInterrupt[1]->handleInterrupt();
+        _deviceForInterrupt[1]->handleInterruptLevel0();
 }
 void RH_INTERRUPT_ATTR RH_RF95::isr2()
 {
     if (_deviceForInterrupt[2])
-        _deviceForInterrupt[2]->handleInterrupt();
+        _deviceForInterrupt[2]->handleInterruptLevel0();
 }
 
 // Check whether the latest received message is complete and uncorrupted

src/rf95/RH_RF95.h

@@ -806,12 +806,17 @@ class RH_RF95 : public RHSPIDriver
     /// Return true if we are currently receiving a packet
     bool isReceiving();
 
+    void loop(); // Perform idle processing
+
   protected:
     /// This is a low level function to handle the interrupts for one instance of RH_RF95.
     /// Called automatically by isr*()
     /// Should not need to be called by user code.
     virtual void handleInterrupt();
 
+    /// This is the only code called in ISR context, it just queues up our helper thread to run handleInterrupt();
+    void RH_INTERRUPT_ATTR handleInterruptLevel0();
+
     /// Examine the revceive buffer to determine whether the message is for this node
     void validateRxBuf();
 
@@ -846,6 +851,11 @@ class RH_RF95 : public RHSPIDriver
     /// Index of next interrupt number to use in _deviceForInterrupt
     static uint8_t _interruptCount;
 
+    bool enableInterrupt();  // enable our IRQ
+    void disableInterrupt(); // disable our IRQ
+
+    volatile bool pendingInterrupt = false;
+    
     /// The configured interrupt pin connected to this instance
     uint8_t _interruptPin;
 

src/rf95/RadioInterface.cpp

@@ -15,8 +15,8 @@ ErrorCode SimRadio::send(MeshPacket *p)
     return ERRNO_OK;
 }
 
-void RadioInterface::deliverToReceiverISR(MeshPacket *p, BaseType_t *higherPriWoken)
+void RadioInterface::deliverToReceiver(MeshPacket *p)
 {
     assert(rxDest);
-    assert(rxDest->enqueueFromISR(p, higherPriWoken)); // NOWAIT - fixme, if queue is full, delete older messages
+    assert(rxDest->enqueue(p, 0)); // NOWAIT - fixme, if queue is full, delete older messages
 }

src/rf95/RadioInterface.h

@@ -24,7 +24,7 @@ class RadioInterface
     /**
      * Enqueue a received packet for the registered receiver
      */
-    void deliverToReceiverISR(MeshPacket *p, BaseType_t *higherPriWoken);
+    void deliverToReceiver(MeshPacket *p);
 
   public:
     /** pool is the pool we will alloc our rx packets from