From 4ce7df295e34d5c3129bb4962d44bf626d8d40db Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 18 Apr 2020 08:39:05 -0700 Subject: [PATCH 1/2] don't poll for completion so quickly - the log messages scare people --- src/sleep.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sleep.cpp b/src/sleep.cpp index ad8aa7359..69b80e91b 100644 --- a/src/sleep.cpp +++ b/src/sleep.cpp @@ -135,7 +135,7 @@ static void waitEnterSleep() uint32_t now = millis(); while (!doPreflightSleep()) { - delay(10); // Kinda yucky - wait until radio says say we can shutdown (finished in process sends/receives) + delay(100); // Kinda yucky - wait until radio says say we can shutdown (finished in process sends/receives) if (millis() - now > 30 * 1000) { // If we wait too long just report an error and go to sleep recordCriticalError(ErrSleepEnterWait); From 78470ed3f59f5c84fbd1325bcff1fd95b2b20183 Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 18 Apr 2020 08:48:03 -0700 Subject: [PATCH 2/2] fix #97, we need the RF95 IRQ to be level triggered, or we have slim chance of missing events --- src/rf95/RH_RF95.cpp | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/rf95/RH_RF95.cpp b/src/rf95/RH_RF95.cpp index c29929637..3929156f5 100644 --- a/src/rf95/RH_RF95.cpp +++ b/src/rf95/RH_RF95.cpp @@ -115,11 +115,11 @@ bool RH_RF95::init() } _deviceForInterrupt[_myInterruptIndex] = this; if (_myInterruptIndex == 0) - attachInterrupt(interruptNumber, isr0, RISING); + attachInterrupt(interruptNumber, isr0, ONHIGH); else if (_myInterruptIndex == 1) - attachInterrupt(interruptNumber, isr1, RISING); + attachInterrupt(interruptNumber, isr1, ONHIGH); else if (_myInterruptIndex == 2) - attachInterrupt(interruptNumber, isr2, RISING); + attachInterrupt(interruptNumber, isr2, ONHIGH); else return false; // Too many devices, not enough interrupt vectors @@ -153,6 +153,17 @@ 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 + // 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 // multiple flags might be set. Handle them all @@ -170,8 +181,6 @@ void RH_RF95::handleInterrupt() // 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; - spiWrite(RH_RF95_REG_12_IRQ_FLAGS, 0xff); // Clear all IRQ flags, required before reading fifo (according to datasheet) - if (!crc_present) { _rxBad++; clearRxBuf(); @@ -218,15 +227,6 @@ void RH_RF95::handleInterrupt() _cad = irq_flags & RH_RF95_CAD_DETECTED; setModeIdle(); } - - // ack all interrupts, note - we did this already in the RX_DONE case above, and we don't want to do it twice - if (!(irq_flags & RH_RF95_RX_DONE)) { - // 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: 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 - } } // These are low level functions that call the interrupt handler for the correct