From a5293fa50b2d957077c5fdcb9378b6e88370d9d5 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Thu, 22 Oct 2026 01:32:48 -0700 Subject: [PATCH] Preserve new SX126x reception after a stale ESP32 interrupt --- src/helpers/radiolib/RadioLibWrappers.cpp | 12 +- test/test_radio_interrupt_recovery.py | 154 +++++++++++++++++++++- 2 files changed, 161 insertions(+), 5 deletions(-) diff --git a/src/helpers/radiolib/RadioLibWrappers.cpp b/src/helpers/radiolib/RadioLibWrappers.cpp index eed008ede..9bd8f8ac4 100644 --- a/src/helpers/radiolib/RadioLibWrappers.cpp +++ b/src/helpers/radiolib/RadioLibWrappers.cpp @@ -2344,7 +2344,8 @@ int RadioLibWrapper::recvRaw(uint8_t* bytes, int sz) { #endif if (claimRadioInterrupt()) { last_radio_interrupt_millis = millis(); // ISR fired -> radio hardware is alive - if (isPacketReady()) { + const bool packet_ready = isPacketReady(); + if (packet_ready) { #if defined(USE_LR2021) // The LR2021 base reader clears FIFO/IRQs while otherwise staying in // continuous RX. Freeze reception before taking length/status/FIFO @@ -2384,6 +2385,15 @@ int RadioLibWrapper::recvRaw(uint8_t* bytes, int sz) { } // Retry a transient IRQ-status probe before rearming/clearing hardware. if (_loop_event_pending) return 0; + #if defined(ESP32_PLATFORM) && (MESH_RADIO_PROFILE_CHIP == 1262 \ + || MESH_RADIO_PROFILE_CHIP == 1268 || MESH_RADIO_PROFILE_CHIP == 68) + // DIO may reveal RX_DONE before its edge ISR publishes. Servicing that + // packet can leave a late software event for an already-cleared IRQ. + // Its bounded empty probes must not restart RX over a newer packet. + // Keep the normal activity timeouts and inactive/error rearm behavior. + if (!packet_ready && state == STATE_RX && !_rx_ps_armed + && !isChipBusy() && isReceivingPacket()) return 0; + #endif #if defined(USE_LR2021) // An early FIFO/SPI failure must not leave RX_DONE asserted. Standby also // covers spurious events for which no packet was read. No new hardware RX diff --git a/test/test_radio_interrupt_recovery.py b/test/test_radio_interrupt_recovery.py index bf50ed76c..61b0f41e7 100644 --- a/test/test_radio_interrupt_recovery.py +++ b/test/test_radio_interrupt_recovery.py @@ -31,6 +31,7 @@ PRELUDE = r''' #include #include #include +#include #define STATE_IDLE 0 #define STATE_RX 1 #define STATE_TX_WAIT 3 @@ -60,11 +61,14 @@ PRELUDE = r''' #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; +static void (*gpio_read_hook)()=nullptr; struct RadioLibHal { const uint32_t GpioLevelHigh=1; uint32_t digitalRead(uint32_t pin) { assert(pinregisterInterruptAction()); + w->chip.module.irq=3;w->chip.rx_status=1;w->chip.irq=1;gpio_levels[3]=true; + // The edge ISR publishes after software claim, during observation of the + // same completed packet's DIO. Actual claim/read/rearm leaves it pending. + gpio_read_hook=w->chip.callback; + assert(w->recvRaw(bytes,sizeof(bytes))==4); + assert(w->n_recv==1&&w->chip.reads==1&&w->chip.arms==1); + assert(w->hasPendingRadioInterrupt()); + assert(w->recvRaw(bytes,sizeof(bytes))==0&&w->_irq_probe_failures==1); + assert(w->recvRaw(bytes,sizeof(bytes))==0&&w->_irq_probe_failures==2); + return w; +} +int main(){ + uint8_t bytes[16]={}; + for(uint32_t activity:{uint32_t(4),uint32_t(16)}){ + auto* w=staleEvent();w->chip.irq=activity; + // A new packet has entered preamble/header during the bounded stale-IRQ + // probes. Rearming here does standby/clear IRQ/SetRx on the pinned SX1262. + assert(w->isReceivingPacket()); + assert(w->recvRaw(bytes,sizeof(bytes))==0); + assert(w->chip.arms==1&&"late IRQ must not rearm over a newer packet"); + assert(w->chip.irq==activity&&w->chip.reads==1&&w->state==STATE_RX); + assert(w->_irq_probe_failures==0&&!w->hasPendingRadioInterrupt()); + // Its real later RX_DONE remains readable exactly once. + w->chip.rx_status=1;w->chip.irq|=1;gpio_levels[3]=true;w->chip.callback(); + assert(w->recvRaw(bytes,sizeof(bytes))==4); + assert(w->n_recv==2&&w->chip.reads==2&&w->chip.arms==2); + assert(!w->hasPendingRadioInterrupt()); + assert(w->recvRaw(bytes,sizeof(bytes))==0&&w->chip.reads==2); + delete w; + } + // Inactive and expired false activity still get the original recovery. + for(uint32_t activity:{uint32_t(0),uint32_t(4),uint32_t(16)}){ + auto* w=staleEvent();w->chip.irq=activity; + if(activity){ + assert(w->isReceivingPacket()); + now_ms+=activity==4?w->chip._preambleMillis+1:w->chip._maxPayloadMillis+1; + } + assert(w->recvRaw(bytes,sizeof(bytes))==0); + assert(w->chip.arms==2&&w->chip.irq==0&&w->state==STATE_RX); + assert(!w->hasPendingRadioInterrupt()&&w->n_recv==1); + assert(w->recvRaw(bytes,sizeof(bytes))==0&&w->chip.arms==2); + delete w; + } + // A current header error is failed activity, even when HEADER_VALID is + // still latched. The activity probe clears it and recovery proceeds once. + { + auto* w=staleEvent();w->chip.irq=32|16|4; + assert(w->recvRaw(bytes,sizeof(bytes))==0); + assert(w->chip.arms==2&&w->chip.irq==0&&w->chip.irq_clears==1); + assert(w->chip.reads==1&&w->n_recv==1&&!w->hasPendingRadioInterrupt()); + assert(w->recvRaw(bytes,sizeof(bytes))==0&&w->chip.arms==2);delete w; + } + // A genuine RX_DONE with empty metadata or FIFO read error still retires + // its event. An activity bit must not turn it into an indefinite hold. + for(bool empty:{false,true}){ + auto* w=staleEvent();w->chip.irq=16|1;w->chip.rx_status=1; + if(empty)w->chip.packet_length=0;else w->chip.base_status=-88; + assert(w->recvRaw(bytes,sizeof(bytes))==0); + assert(w->chip.arms==2&&w->chip.irq==0&&w->n_recv==1); + assert(w->n_recv_errors==(empty?0U:1U)); + assert(w->chip.activity_probes==0); + delete w; + } + // Keep duty-cycle and idle/error-mode recovery outside this continuous-RX + // workaround. BUSY also prevents a second activity-status SPI probe. + for(unsigned bypass=0;bypass<3;++bypass){ + auto* w=staleEvent();w->chip.irq=16; + if(bypass==0)w->_rx_ps_armed=true; + if(bypass==1)w->state=STATE_IDLE; + if(bypass==2)w->chip_busy=true; + assert(w->recvRaw(bytes,sizeof(bytes))==0&&w->chip.arms==2); + assert(w->chip.activity_probes==0);delete w; + } +} +''' + + def test_sx126x_stale_irq_preserves_new_continuous_receive(self): + for chip in ("CustomSX1262", "CustomSX1268", "CustomLLCC68"): + with self.subTest(chip=chip): + compile_run(self.source(sx126x=chip)+self.sx126x_late_irq_checks(), + ["-DESP32_PLATFORM=1", f"-DRADIO_CLASS={chip}"]) + + def test_removed_sx126x_stale_irq_guard_aborts_new_receive(self): + source = self.source(sx126x=True) + guard = (" if (!packet_ready && state == STATE_RX && !_rx_ps_armed\n" + " && !isChipBusy() && isReceivingPacket()) return 0;\n") + self.assertEqual(source.count(guard), 1) + with self.assertRaises(AssertionError) as failure: + compile_run(source.replace(guard, "")+self.sx126x_late_irq_checks(), + ["-DESP32_PLATFORM=1", "-DRADIO_CLASS=CustomSX1262"]) + self.assertIn("late IRQ must not rearm over a newer packet", str(failure.exception)) + + def test_stale_irq_guard_does_not_change_other_platforms_or_phys(self): + checks = r''' +int main(){ + CustomSX1262Wrapper w;assert(w.registerInterruptAction());uint8_t bytes[16]={}; + w.chip.rx_status=0;w.chip.irq=16;w.chip.callback(); + assert(w.recvRaw(bytes,sizeof(bytes))==0&&w.chip.arms==0); + assert(w.recvRaw(bytes,sizeof(bytes))==0&&w.chip.arms==0); + assert(w.recvRaw(bytes,sizeof(bytes))==0&&w.chip.arms==1); + assert(w.chip.activity_probes==0); +} +''' + for defines in ([], ["-DNRF52_PLATFORM=1"], ["-DSTM32_PLATFORM=1"], + ["-DESP32_PLATFORM=1", "-DRADIO_CLASS=CustomSX1276"], + ["-DESP32_PLATFORM=1", "-DRADIO_CLASS=CustomLR1110"]): + with self.subTest(defines=defines): + compile_run(self.source(sx126x=True)+checks,defines) + def test_lr2021_fifo_guards_stopped_snapshot_cleanup_and_recovery(self): compile_run(self.source() + r''' int main(){