From d2fb16a508d899585f9a852fdc65e7804cd651ee Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Wed, 7 Oct 2026 21:25:07 +0000 Subject: [PATCH] Hold the radio-sequence lock on the main loop's radio paths The lock exists so a readout cannot land between the calls of a RadioLib sequence, where it would clear flags the sequence is about to use. Three paths reached from the main loop still made such sequences without it: - checkCadHandoffTimeout()'s startReceive() after a handoff RX that never completed. - checkStaleRxFlags(), which reads the IRQ flags, draws a verdict from them and then re-arms or clears - all of which a readout in between invalidates, since it retires the very flags being judged. - resetAGC(), which is a sequence end to end: warm sleep, RC standby, CALIBRATE_ALL, the re-applied DIO2/boosted-gain/0x8B5 settings and the startReceive() that resumes. Its own mid-packet bail is a point check, so it cannot exclude a readout that starts just after it. All three predate the readout task, so this is the task's own lock coverage catching up rather than a regression. Guarding resetAGC() at its call site in periodicRadioMaintenance() covers every driver's override. It can hold the lock for the length of a calibration, which delays a readout, but the chip is not readable for any of that anyway - the RX that would read a frame is the startReceive() at the end. checkRxDoneIrqFlag() is deliberately left unguarded: it is a single flag read and a wake, not a sequence, and serialising it behind a readout in flight would only delay the handoff it exists to make. Co-Authored-By: Jonathan Bennett Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VVTg7Na1pVvCcwZoSv9Hnd --- src/mesh/RadioLibInterface.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/mesh/RadioLibInterface.cpp b/src/mesh/RadioLibInterface.cpp index 2d7596d737..fe29055e29 100644 --- a/src/mesh/RadioLibInterface.cpp +++ b/src/mesh/RadioLibInterface.cpp @@ -974,6 +974,9 @@ void RadioLibInterface::checkCadHandoffTimeout() if (cadHandoffRxStart && Throttle::hasElapsed(cadHandoffRxStart, 2 * maxPacketTimeMsec)) { LOG_WARN("CAD>RX timeout"); cadHandoffRxStart = 0; + // Reached from the main loop, which holds no sequence of its own: a readout landing inside this + // re-arm would clear the flags it sets up, exactly as it would on the radio thread's paths. + RadioSequence seq(this); startReceive(); } } @@ -983,6 +986,9 @@ void RadioLibInterface::checkStaleRxFlags() // A handoff RX has its own timeout, and a pending RX_DONE is about to clear every flag itself. if (cadHandoffRxStart) return; + // The flag read, the verdict drawn from it and the re-arm or clear that follows are one sequence: a readout + // in between retires the very flags being judged, so the verdict would be drawn on flags that are gone. + RadioSequence seq(this); const uint32_t irq = iface->getIrqFlags(); if (irq & iface->getIrqMapped(1UL << RADIOLIB_IRQ_RX_DONE)) return; @@ -1039,7 +1045,15 @@ void RadioLibInterface::periodicRadioMaintenance() const bool hearing = lastRxGoodMs && Throttle::isWithinTimespanMs(lastRxGoodMs, AGC_IDLE_RESET_MS); if (hearing && lastAgcResetMs && Throttle::isWithinTimespanMs(lastAgcResetMs, AGC_FORCED_RESET_MS)) return; - if (resetAGC()) { + // resetAGC() is a sequence end to end - warm sleep, RC standby, CALIBRATE_ALL, the re-applied settings and + // the startReceive() that resumes - and the chip is not readable for any of it. Its own mid-packet bail is a + // point check, so it cannot exclude a readout that starts just after it: the lock is what does. + bool agcReset = false; + { + RadioSequence seq(this); + agcReset = resetAGC(); + } + if (agcReset) { const uint32_t now = millis(); lastAgcResetMs = now ? now : 1; }