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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CStem6K6MQRBNR8YCFgtCE

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");

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. */

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());
 }