From eac6bed13aecb2f04d69ffe38f9cee6ef736e742 Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Sat, 10 Oct 2026 15:13:23 +0000 Subject: [PATCH] LoRa: size the preamble hold for the frame that could be arriving The hold ran for getPacketTime(DATA_PAYLOAD_LEN + sizeof(PacketHeader)): 249 bytes at our own coding rate. A sender's header carries its own CR, up to 4/8, and that header is unreadable when the hold starts, so a legal 255-byte frame at CR 4/8 with CRC outlives the hold and we key up over it. That is the case the hold exists for, since LoRa we cannot decode - a foreign sync word - never produces HEADER_VALID at all. maxRxFrameMsec() times the 255-byte PHY frame at CR 4/8 with CRC from the configured modem, through the radio's own calculateTimeOnAir(), and falls back to getPacketTime(MAX_LORA_PAYLOAD_LEN) if the radio answers an error code (#11935). At SHORT_TURBO that is 159 ms rather than 100 ms, at LONG_FAST 3.3 s rather than 2.1 s. The HEADER_VALID ageing in receiveDetected() takes the same bound, for the same reason. checkStaleRxFlags() and checkCadHandoffTimeout() keep the shorter window: they decide a latched flag is dead and recover the chip, where waiting longer delays the recovery rather than protecting a frame. Three test cases in test/test_radio pin the sizing, the error fallback and that the hold outlives the flag clear that starts it. FakeIrqRadio now answers calculateTimeOnAir() and honours a clear. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CStem6K6MQRBNR8YCFgtCE --- src/mesh/RadioLibInterface.cpp | 19 +++++-- src/mesh/RadioLibInterface.h | 5 +- test/test_radio/test_main.cpp | 93 +++++++++++++++++++++++++++++++++- 3 files changed, 110 insertions(+), 7 deletions(-) diff --git a/src/mesh/RadioLibInterface.cpp b/src/mesh/RadioLibInterface.cpp index 374c53dfe3..28ebc16308 100644 --- a/src/mesh/RadioLibInterface.cpp +++ b/src/mesh/RadioLibInterface.cpp @@ -154,11 +154,21 @@ bool RadioLibInterface::canSendImmediately() return true; } +uint32_t RadioLibInterface::maxRxFrameMsec() +{ + // A sender's header can carry any CR up to 4/8, and a hold starts before that header can be read. + DataRate_t dr = getDataRate(); + dr.lora.codingRate = 8; + PacketConfig_t pc = getPacketConfig(); + pc.lora.crcEnabled = true; + const RadioLibTime_t usec = iface->calculateTimeOnAir(modemType, dr, pc, MAX_LORA_PAYLOAD_LEN); + return isRadioLibTimeError(usec) ? getPacketTime(MAX_LORA_PAYLOAD_LEN) : (usec + 999) / 1000; +} + bool RadioLibInterface::preambleHoldActive() { - // Whatever sent the cleared preamble is off the air one max packet later. - if (preambleHoldStart && !Throttle::isWithinTimespanMs( - preambleHoldStart, getPacketTime(meshtastic_Constants_DATA_PAYLOAD_LEN + sizeof(PacketHeader)))) + // Whatever sent the cleared preamble is off the air one max frame later. + if (preambleHoldStart && !Throttle::isWithinTimespanMs(preambleHoldStart, maxRxFrameMsec())) preambleHoldStart = 0; return preambleHoldStart != 0; } @@ -181,8 +191,7 @@ bool RadioLibInterface::receiveDetected(uint16_t irq, unsigned long syncWordHead if (irq & syncWordHeaderValidFlag) { if (!activeReceiveStart) { activeReceiveStart = Time::skipZero(Time::getMillis()); - } else if (!Throttle::isWithinTimespanMs(activeReceiveStart, - getPacketTime(meshtastic_Constants_DATA_PAYLOAD_LEN + sizeof(PacketHeader)))) { + } else if (!Throttle::isWithinTimespanMs(activeReceiveStart, maxRxFrameMsec())) { // We should have gotten an RX_DONE IRQ by now if it was really a packet, so ignore HEADER_VALID flag activeReceiveStart = 0; LOG_TRACE("Ignore false header detection"); diff --git a/src/mesh/RadioLibInterface.h b/src/mesh/RadioLibInterface.h index 986e33e03f..ce9cf1a2bb 100644 --- a/src/mesh/RadioLibInterface.h +++ b/src/mesh/RadioLibInterface.h @@ -434,7 +434,10 @@ class RadioLibInterface : public RadioInterface, protected concurrency::Notified // Time::getMillis() when a look cleared PREAMBLE_DETECTED and began holding TX, or 0 if no hold. uint32_t preambleHoldStart = 0; - /** True while a cleared preamble still holds TX; ends the hold once one max packet has passed. */ + /** Airtime of the longest frame we could be receiving: 255 bytes at CR 4/8 with CRC, whatever our own CR. */ + uint32_t maxRxFrameMsec(); + + /** True while a cleared preamble still holds TX; ends the hold once one max frame has passed. */ bool preambleHoldActive(); /** Clear a bare PREAMBLE_DETECTED and hold TX one max packet, unless a hold is already running. */ diff --git a/test/test_radio/test_main.cpp b/test/test_radio/test_main.cpp index b06c1464c6..fbc78434dd 100644 --- a/test/test_radio/test_main.cpp +++ b/test/test_radio/test_main.cpp @@ -527,11 +527,15 @@ class FakeLoraRadio } }; -// A PhysicalLayer that answers only its IRQ flags, each generic RadioLib IRQ mapped to its own bit. +// A PhysicalLayer that answers its IRQ flags, each generic RadioLib IRQ mapped to its own bit, and the +// airtime it is asked for. A clear takes the bit down, as a chip's does. class FakeIrqRadio : public PhysicalLayer { public: uint32_t irqFlags = 0; + RadioLibTime_t timeOnAirUsec = 0; // what calculateTimeOnAir() answers; 0 reads as an error + uint8_t lastCodingRate = 0; // the CR it was asked to time, so a caller's worst case is visible + bool lastCrcEnabled = false; FakeIrqRadio() { @@ -540,6 +544,17 @@ class FakeIrqRadio : public PhysicalLayer } uint32_t getIrqFlags() override { return irqFlags; } + int16_t clearIrqFlags(uint32_t irq) override + { + irqFlags &= ~irq; + return RADIOLIB_ERR_NONE; + } + RadioLibTime_t calculateTimeOnAir(ModemType_t, DataRate_t dr, PacketConfig_t pc, size_t) override + { + lastCodingRate = dr.lora.codingRate; + lastCrcEnabled = pc.lora.crcEnabled; + return timeOnAirUsec; + } Module *getMod() override { return nullptr; } }; @@ -569,6 +584,14 @@ class TestableRadioLibInterface : public RadioLibInterface static bool isRadioLibTimeErrorPublic(RadioLibTime_t usec) { return isRadioLibTimeError(usec); } + uint32_t maxRxFrameMsecPublic() { return maxRxFrameMsec(); } + bool preambleHoldActivePublic() { return preambleHoldActive(); } + bool receiveDetectedPublic(uint16_t irq) { return receiveDetected(irq, kFakeHeaderValid, kFakePreambleDetected); } + + // Chip IRQ masks, as each driver passes its own to receiveDetected() + static constexpr uint16_t kFakeHeaderValid = 1 << 1; + static constexpr uint16_t kFakePreambleDetected = 1 << 2; + // Chip-specific hooks this test never reaches uint32_t getPacketTime(uint32_t, bool) override { return packetTimeMs; } int16_t getCurrentRSSI() override { return 0; } @@ -722,6 +745,71 @@ static void test_staleRxFlagAction_staleHeaderIsRearmed() // checkStaleRxFlags() itself, which the cases above cannot see: a header that shows after a bare preamble gets its own // max-packet window, timed from when the header showed. Regression guarded: timing the header from the earlier // preamble's stamp, which re-arms RX - a standby that aborts the frame whose header has only just arrived. +// maxRxFrameMsec() and the preamble hold in src/mesh/RadioLibInterface.cpp: how long a sighted preamble keeps TX off +// the air. Neither PREAMBLE_DETECTED nor HEADER_VALID says when its frame ends - a foreign sync word never produces a +// header at all - so the hold runs for the longest frame that could be arriving. +// +// Pinned: that length is the 255-byte PHY frame at CR 4/8 with CRC, not our own configured CR, because the sender's +// header carries its CR and is unreadable when the hold starts. Regressions guarded: sizing the hold from +// getPacketTime(DATA_PAYLOAD_LEN + header), which is 249 bytes at our own CR and expires with a legal maximum frame +// still on air; and reading a RadioLib error code as a duration (#11935), which getPacketTime() then has to cover. + +static void test_preambleHold_isSizedForTheWorstCaseFrame() +{ + FakeIrqRadio phy; + phy.timeOnAirUsec = 158001; // 255 B at CR 4/8 with CRC, about SHORT_TURBO + auto *radioIf = new TestableRadioLibInterface(&phy); + radioIf->packetTimeMs = 100; // what 249 B at our own CR would have given + + Time::setTestMillis(1000); + TEST_ASSERT_TRUE(radioIf->receiveDetectedPublic(TestableRadioLibInterface::kFakePreambleDetected)); + TEST_ASSERT_EQUAL_UINT8_MESSAGE(8, phy.lastCodingRate, "the hold must be timed at the worst case CR, not ours"); + TEST_ASSERT_TRUE_MESSAGE(phy.lastCrcEnabled, "a received frame may carry a CRC, which lengthens it"); + TEST_ASSERT_EQUAL_UINT32_MESSAGE(159, radioIf->maxRxFrameMsecPublic(), "part milliseconds round up"); + + Time::setTestMillis(1100); // the old sizing released here, with the frame still arriving + TEST_ASSERT_TRUE(radioIf->preambleHoldActivePublic()); + Time::setTestMillis(1158); + TEST_ASSERT_TRUE(radioIf->preambleHoldActivePublic()); + Time::setTestMillis(1159); + TEST_ASSERT_FALSE(radioIf->preambleHoldActivePublic()); + + Time::useRealClock(); + delete radioIf; +} + +static void test_preambleHold_fallsBackWhenTheRadioCannotTimeTheFrame() +{ + FakeIrqRadio phy; // answers 0 usec, which isRadioLibTimeError() reads as an error, not as no airtime + auto *radioIf = new TestableRadioLibInterface(&phy); + radioIf->packetTimeMs = 120; + + TEST_ASSERT_EQUAL_UINT32(120, radioIf->maxRxFrameMsecPublic()); + + delete radioIf; +} + +static void test_preambleHold_clearsTheFlagAndSurvivesIt() +{ + // The hold is a timestamp, not a flag: the look that starts it clears PREAMBLE_DETECTED so the next look can see a + // fresh one, and the hold it started has to outlive that clear. + FakeIrqRadio phy; + phy.timeOnAirUsec = 50000; + auto *radioIf = new TestableRadioLibInterface(&phy); + radioIf->packetTimeMs = 50; + + Time::setTestMillis(1000); + phy.irqFlags = phy.getIrqMapped(1UL << RADIOLIB_IRQ_PREAMBLE_DETECTED); + TEST_ASSERT_TRUE(radioIf->receiveDetectedPublic(TestableRadioLibInterface::kFakePreambleDetected)); + TEST_ASSERT_EQUAL_UINT32_MESSAGE(0, phy.irqFlags, "the look must clear the preamble it found"); + + Time::setTestMillis(1020); + TEST_ASSERT_TRUE_MESSAGE(radioIf->receiveDetectedPublic(0), "a look with no flags set must still see the hold"); + + Time::useRealClock(); + delete radioIf; +} + static void test_checkStaleRxFlags_headerAfterPreambleRestartsTheWindow() { FakeIrqRadio phy; @@ -794,6 +882,9 @@ void setup() RUN_TEST(test_staleRxFlagAction_barePreambleIsOnlyCleared); RUN_TEST(test_staleRxFlagAction_staleHeaderIsRearmed); RUN_TEST(test_checkStaleRxFlags_headerAfterPreambleRestartsTheWindow); + RUN_TEST(test_preambleHold_isSizedForTheWorstCaseFrame); + RUN_TEST(test_preambleHold_fallsBackWhenTheRadioCannotTimeTheFrame); + RUN_TEST(test_preambleHold_clearsTheFlagAndSurvivesIt); exit(UNITY_END()); }