diff --git a/src/helpers/radiolib/RadioLibWrappers.cpp b/src/helpers/radiolib/RadioLibWrappers.cpp index 555fef136..8e6909411 100644 --- a/src/helpers/radiolib/RadioLibWrappers.cpp +++ b/src/helpers/radiolib/RadioLibWrappers.cpp @@ -10,6 +10,7 @@ #if defined(ESP32_PLATFORM) #include #include +#include #elif defined(NRF52_PLATFORM) #include #include "../NRF52Board.h" @@ -176,15 +177,37 @@ void RadioLibWrapper::unregisterInterruptAction() { _interrupt_slot = 0xff; } +bool RadioLibWrapper::hasAssertedRadioInterrupt() const { +#if defined(ESP32_PLATFORM) + // GPIO wake does not guarantee that the normal edge ISR ran during light + // sleep. Retain a completed packet even when DIO is already high after + // restoring its edge mode. Read this PHY's pin, never a shared board pin. + if (_interrupt_slot >= RADIO_INTERRUPT_SLOT_COUNT + || radio_interrupt_owners[_interrupt_slot] != this || _radio == nullptr) return false; + Module* module = _radio->getMod(); + if (module == nullptr || module->hal == nullptr) return false; + const uint32_t irq = module->getIrq(); + // GPIO_IS_VALID_GPIO shifts by its argument on older IDF versions, so its + // mask check must follow the range check even for an invalid/NC module pin. + if (irq == RADIOLIB_NC || irq >= SOC_GPIO_PIN_COUNT || !GPIO_IS_VALID_GPIO(irq)) return false; + return module->hal->digitalRead(irq) == module->hal->GpioLevelHigh; +#else + return false; +#endif +} + bool RadioLibWrapper::hasPendingRadioInterrupt() const { - return _loop_event_pending || (_interrupt_slot < RADIO_INTERRUPT_SLOT_COUNT && radio_interrupt_events[_interrupt_slot].pending()); + return _loop_event_pending || (_interrupt_slot < RADIO_INTERRUPT_SLOT_COUNT && radio_interrupt_events[_interrupt_slot].pending()) + || hasAssertedRadioInterrupt(); } bool RadioLibWrapper::claimRadioInterrupt() { const bool startup = _loop_event_pending; _loop_event_pending = false; const bool interrupt = _interrupt_slot < RADIO_INTERRUPT_SLOT_COUNT && radio_interrupt_events[_interrupt_slot].claim(); - return startup || interrupt; + // Always claim the software event too, even when the physical pin is high. + // recvRaw still verifies RX_DONE before reading FIFO, including a late ISR. + return startup || interrupt || hasAssertedRadioInterrupt(); } void RadioLibWrapper::begin() { diff --git a/src/helpers/radiolib/RadioLibWrappers.h b/src/helpers/radiolib/RadioLibWrappers.h index 4e77fd5cd..1ffbe269e 100644 --- a/src/helpers/radiolib/RadioLibWrappers.h +++ b/src/helpers/radiolib/RadioLibWrappers.h @@ -41,6 +41,7 @@ protected: uint8_t _irq_probe_failures = 0; bool registerInterruptAction(); void unregisterInterruptAction(); + bool hasAssertedRadioInterrupt() const; bool hasPendingRadioInterrupt() const; bool claimRadioInterrupt(); PhysicalLayer* _radio; diff --git a/test/test_radio_interrupt_recovery.py b/test/test_radio_interrupt_recovery.py index c341aec68..bf50ed76c 100644 --- a/test/test_radio_interrupt_recovery.py +++ b/test/test_radio_interrupt_recovery.py @@ -52,6 +52,27 @@ PRELUDE = r''' #define RADIOLIB_LR2021_IRQ_LORA_HEADER_VALID 64 #define RADIOLIB_LR2021_IRQ_LORA_HDR_CRC_ERROR 512 #define MESH_DEBUG_PRINTLN(...) ((void)0) +#if defined(ESP32_PLATFORM) +#define ICACHE_RAM_ATTR +#define RADIOLIB_NC UINT32_MAX +#define SOC_GPIO_PIN_COUNT 49 +// Match the pinned S3 SDK: holes are invalid and the macro itself shifts. +#define GPIO_IS_VALID_GPIO(pin) (((UINT64_C(0x1fffffc3fffff) >> (pin)) & 1) != 0) +static bool gpio_levels[SOC_GPIO_PIN_COUNT]={}; +static unsigned gpio_reads=0; +struct RadioLibHal { + const uint32_t GpioLevelHigh=1; + uint32_t digitalRead(uint32_t pin) { + assert(pinchip.module.irq=3;second->chip.module.irq=4; + assert(first->registerInterruptAction()&&second->registerInterruptAction()); + // A completed RF packet wakes the CPU, but no normal edge ISR was delivered. + gpio_levels[3]=true;first->chip.rx_status=1; + assert(!radio_interrupt_events[first->_interrupt_slot].pending()); + const unsigned spi_before=first->chip.irq_probes; + assert(first->hasPendingRadioInterrupt()&&!second->hasPendingRadioInterrupt()); + assert(first->chip.irq_probes==spi_before); // level-only check, no SPI + const unsigned arm_before=first->chip.arms,standby_before=first->chip.standbys; + assert(first->beginReconfigure()==2); // no retune/rearm may discard its FIFO + assert(first->chip.arms==arm_before&&first->chip.standbys==standby_before); + assert(!first->startSendRaw(bytes,4)&&first->chip.tx_starts==0); + assert(first->recvRaw(bytes,sizeof(bytes))==4&&first->n_recv==1&&first->chip.reads==1); + assert(!gpio_levels[3]&&!first->hasPendingRadioInterrupt()); + assert(first->recvRaw(bytes,sizeof(bytes))==0&&first->chip.reads==1); + + // Physical and software ownership coalesce. Claim both instead of leaving + // the software callback pending behind the physical level's short circuit. + gpio_levels[3]=true;first->chip.rx_status=1;first->chip.fifo=4;first->chip.callback(); + assert(first->recvRaw(bytes,sizeof(bytes))==4&&first->n_recv==2&&first->chip.reads==2); + assert(!radio_interrupt_events[first->_interrupt_slot].pending()); + assert(!first->hasPendingRadioInterrupt()); + + // A delayed ISR after FIFO service has no RX_DONE. The unchanged status + // probe prevents stale payloads despite its positive software notification. + first->chip.callback();first->chip.rx_status=0; + assert(first->recvRaw(bytes,sizeof(bytes))==0&&first->chip.reads==2); + assert(first->recvRaw(bytes,sizeof(bytes))==0&&first->recvRaw(bytes,sizeof(bytes))==0); + assert(first->n_recv==2&&!first->hasPendingRadioInterrupt()); + + // Distinct module pins remain independent, including a rejected wrapper + // with a stale slot number. No board-level shared pin is consulted. + gpio_levels[4]=true;second->chip.rx_status=1; + assert(!first->hasPendingRadioInterrupt()&&second->hasPendingRadioInterrupt()); + auto* denied=new CustomSX1262Wrapper;denied->_interrupt_slot=second->_interrupt_slot; + denied->chip.module.irq=4; + const unsigned reads_before=gpio_reads; + assert(!denied->hasPendingRadioInterrupt()&&!denied->claimRadioInterrupt()); + assert(gpio_reads==reads_before); + denied->_interrupt_slot=0xff; + assert(!denied->hasPendingRadioInterrupt()&&!denied->claimRadioInterrupt()); + assert(gpio_reads==reads_before); + delete denied; + assert(second->recvRaw(bytes,sizeof(bytes))==4&&second->n_recv==1); + + // No GPIO access, or undefined SDK macro shift, for a missing module/HAL, + // NC, pin beyond the S3 range, or its real GPIO22..25 holes. + Chip* saved_chip=first->_radio; + first->_radio=nullptr; + unsigned checked_reads=gpio_reads; + assert(!first->hasPendingRadioInterrupt()&&!first->claimRadioInterrupt()); + first->_radio=saved_chip; + Module* saved=first->chip.selected_module; + first->chip.selected_module=nullptr; + assert(!first->hasPendingRadioInterrupt()&&!first->claimRadioInterrupt()); + first->chip.selected_module=saved;first->chip.module.hal=nullptr; + assert(!first->hasPendingRadioInterrupt()&&!first->claimRadioInterrupt()); + first->chip.module.hal=&first->chip.module.instance; + for(uint32_t invalid:{RADIOLIB_NC,uint32_t(49),uint32_t(64),UINT32_MAX-1, + uint32_t(22),uint32_t(23),uint32_t(24),uint32_t(25)}){ + first->chip.module.irq=invalid; + assert(!first->hasPendingRadioInterrupt()&&!first->claimRadioInterrupt()); + } + assert(gpio_reads==checked_reads); + first->chip.module.irq=3; + first->unregisterInterruptAction();gpio_levels[3]=true; + assert(!first->hasPendingRadioInterrupt()&&!first->claimRadioInterrupt()); + assert(gpio_reads==checked_reads); + gpio_levels[3]=false;delete first;delete second; +} +''' + + def test_esp32_asserted_module_dio_recovers_lost_wake_isr_once(self): + compile_run(self.source()+self.esp32_recovery_checks(), ["-DESP32_PLATFORM=1"]) + + def test_removed_physical_pending_or_claim_fallback_loses_real_receive(self): + source=self.source() + for name in ("hasPendingRadioInterrupt", "claimRadioInterrupt"): + with self.subTest(removed=name): + original=method(source, f"bool RadioLibWrapper::{name}(") + mutant=original.replace(" || hasAssertedRadioInterrupt()", "") + self.assertNotEqual(original,mutant) + with self.assertRaises(AssertionError) as failure: + compile_run(source.replace(original,mutant)+self.esp32_recovery_checks(), + ["-DESP32_PLATFORM=1"]) + expected=("first->hasPendingRadioInterrupt()" if name=="hasPendingRadioInterrupt" + else "first->recvRaw(bytes,sizeof(bytes))==4") + self.assertIn(expected,str(failure.exception)) + + def test_non_esp32_retains_software_only_interrupt_contract(self): + checks=self.source()+r''' +int main(){ + CustomSX1262Wrapper w;assert(w.registerInterruptAction());uint8_t bytes[16]={}; + assert(!w.hasAssertedRadioInterrupt()&&!w.hasPendingRadioInterrupt()); + assert(w.recvRaw(bytes,sizeof(bytes))==0&&w.chip.reads==0); + w.chip.callback();assert(w.hasPendingRadioInterrupt()); + assert(w.recvRaw(bytes,sizeof(bytes))==4&&w.chip.reads==1&&!w.hasPendingRadioInterrupt()); + w.unregisterInterruptAction(); +} +''' + for defines in ([], ["-DNRF52_PLATFORM=1"], ["-DSTM32_PLATFORM=1"]): + with self.subTest(defines=defines): compile_run(checks,defines) + def test_lr2021_fifo_guards_stopped_snapshot_cleanup_and_recovery(self): compile_run(self.source() + r''' int main(){