From 48a5d5b4cd7bffd54f90bfc8be34b0cc737a06ba Mon Sep 17 00:00:00 2001 From: mikecarper Date: Mon, 10 Aug 2026 23:47:02 -0700 Subject: [PATCH] Fix trace TX stalls from invalid radio airtime --- examples/companion_radio/MyMesh.cpp | 20 +++++++++++++---- src/helpers/TracePathHelpers.h | 22 +++++++++++++++++++ src/helpers/radiolib/CustomLR1110.h | 9 ++++++++ src/helpers/radiolib/CustomLR1110Wrapper.h | 1 + src/helpers/radiolib/RadioAirtime.h | 17 ++++++++++++++ src/helpers/radiolib/RadioLibWrappers.cpp | 10 ++++++++- src/helpers/radiolib/RadioLibWrappers.h | 1 + test/test_cad_timing/test_cad_timing.cpp | 8 +++++++ .../test_trace_path_helpers.cpp | 16 ++++++++++++++ 9 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 src/helpers/radiolib/RadioAirtime.h diff --git a/examples/companion_radio/MyMesh.cpp b/examples/companion_radio/MyMesh.cpp index 0d10461f..43f79d17 100644 --- a/examples/companion_radio/MyMesh.cpp +++ b/examples/companion_radio/MyMesh.cpp @@ -3779,6 +3779,20 @@ void MyMesh::sendTerminalTraceRoute(const uint8_t* route, uint8_t hash_size, return; } + const uint32_t airtime = _radio->getEstAirtimeFor(9 + route_byte_len + 2); + if (airtime == 0) { + Serial.print(" ERROR: unable to estimate trace timeout\r\n"); + return; + } + const uint32_t base_timeout = + calcDirectTimeoutMillisFor(airtime, hop_count); + uint32_t trace_timeout = 0; + if (!mesh::calculateTerminalTraceTimeoutMillis(base_timeout, + trace_timeout)) { + Serial.print(" ERROR: trace timeout is out of range\r\n"); + return; + } + uint32_t tag = 0; uint32_t auth = 0; getRNG()->random((uint8_t*)&tag, sizeof(tag)); @@ -3789,8 +3803,6 @@ void MyMesh::sendTerminalTraceRoute(const uint8_t* route, uint8_t hash_size, return; } - const uint32_t airtime = _radio->getEstAirtimeFor(9 + route_byte_len + 2); - const uint32_t timeout = calcDirectTimeoutMillisFor(airtime, hop_count); if (!sendDirect(packet, route, static_cast(route_byte_len))) { Serial.print(" ERROR: unable to send trace\r\n"); return; @@ -3801,12 +3813,12 @@ void MyMesh::sendTerminalTraceRoute(const uint8_t* route, uint8_t hash_size, _terminal_trace_tag = tag; _terminal_trace_auth = auth; _terminal_trace_sent_at = _ms->getMillis(); - _terminal_trace_expires_at = futureMillis(timeout + timeout / 5); + _terminal_trace_expires_at = futureMillis(trace_timeout); StrHelper::strzcpy(_terminal_trace_target, target, sizeof(_terminal_trace_target)); Serial.printf(" Trace sent to %s (%u route hops, timeout %lu ms)\r\n", target, (unsigned)hop_count, - (unsigned long)(timeout + timeout / 5)); + (unsigned long)trace_timeout); } void MyMesh::sendTerminalTrace(ContactInfo& recipient) { diff --git a/src/helpers/TracePathHelpers.h b/src/helpers/TracePathHelpers.h index 68ba73cf..4af7edaa 100644 --- a/src/helpers/TracePathHelpers.h +++ b/src/helpers/TracePathHelpers.h @@ -6,6 +6,28 @@ namespace mesh { +static constexpr uint32_t TERMINAL_TRACE_TIMEOUT_MARGIN_PERCENT = 200UL; +static constexpr uint32_t TERMINAL_TRACE_MAX_TIMEOUT_MILLIS = 0x7FFFFFFFUL; + +// Terminal trace timeouts add a 200% margin to the direct-send estimate. Keep +// the arithmetic wide because futureMillis() accepts a signed millisecond +// delta and rollover-safe comparisons require the result to stay in range. +inline bool calculateTerminalTraceTimeoutMillis(uint32_t base_timeout_millis, + uint32_t& timeout_millis) { + timeout_millis = 0; + if (base_timeout_millis == 0) return false; + + const uint64_t margin = + static_cast(base_timeout_millis) + * TERMINAL_TRACE_TIMEOUT_MARGIN_PERCENT / 100UL; + const uint64_t total = + static_cast(base_timeout_millis) + margin; + if (total > TERMINAL_TRACE_MAX_TIMEOUT_MILLIS) return false; + + timeout_millis = static_cast(total); + return true; +} + struct RoundTripTracePath { uint8_t hash_size; uint8_t hop_count; diff --git a/src/helpers/radiolib/CustomLR1110.h b/src/helpers/radiolib/CustomLR1110.h index 191bc884..fc751534 100644 --- a/src/helpers/radiolib/CustomLR1110.h +++ b/src/helpers/radiolib/CustomLR1110.h @@ -18,6 +18,15 @@ class CustomLR1110 : public LR1110 { public: CustomLR1110(Module *mod) : LR1110(mod) { } + // MeshCore keeps the LR1110 in LoRa mode. Calculate from RadioLib's cached + // parameters so an airtime query never issues GetPacketType while RX duty + // cycling has the chip asleep. RadioLib's LR11x0 implementation ignores a + // failed modem query and can otherwise return an encoded negative error as + // a multi-million-millisecond airtime. + RadioLibTime_t getTimeOnAir(size_t len) override { + return getToA(len, ModemType_t::RADIOLIB_MODEM_LORA); + } + // RadioLib waits without a deadline for BUSY to fall after SetTx. Bound // that wait so a failed LR1110 transition can reach the wrapper's hard // recovery path instead of hanging the firmware indefinitely. diff --git a/src/helpers/radiolib/CustomLR1110Wrapper.h b/src/helpers/radiolib/CustomLR1110Wrapper.h index 3b6c47e5..a4d44fa1 100644 --- a/src/helpers/radiolib/CustomLR1110Wrapper.h +++ b/src/helpers/radiolib/CustomLR1110Wrapper.h @@ -55,6 +55,7 @@ public: uint32_t getEstAirtimeFor(int len_bytes) override { auto airtime = RadioLibWrapper::getEstAirtimeFor(len_bytes); + if (airtime == 0) return 0; return airtime < 200 ? 200 : airtime; // at least 200 millis } diff --git a/src/helpers/radiolib/RadioAirtime.h b/src/helpers/radiolib/RadioAirtime.h new file mode 100644 index 00000000..419090ce --- /dev/null +++ b/src/helpers/radiolib/RadioAirtime.h @@ -0,0 +1,17 @@ +#pragma once + +#include + +namespace mesh { + +// RadioLib time-on-air APIs use an unsigned return type even though some +// implementations return negative RADIOLIB_ERR_* values on failure. Those +// errors therefore appear immediately below UINT32_MAX. +static constexpr uint32_t RADIOLIB_AIRTIME_ERROR_WINDOW_US = 4096UL; + +inline bool isEncodedRadioLibAirtimeError(uint32_t airtime_us) { + return airtime_us + >= UINT32_MAX - (RADIOLIB_AIRTIME_ERROR_WINDOW_US - 1UL); +} + +} // namespace mesh diff --git a/src/helpers/radiolib/RadioLibWrappers.cpp b/src/helpers/radiolib/RadioLibWrappers.cpp index af56c2d0..6b936709 100644 --- a/src/helpers/radiolib/RadioLibWrappers.cpp +++ b/src/helpers/radiolib/RadioLibWrappers.cpp @@ -632,7 +632,15 @@ void RadioLibWrapper::finishReceiveProcessing() { } uint32_t RadioLibWrapper::getEstAirtimeFor(int len_bytes) { - return _radio->getTimeOnAir(len_bytes) / 1000; + const uint32_t airtime_us = + static_cast(_radio->getTimeOnAir(len_bytes)); + if (mesh::isEncodedRadioLibAirtimeError(airtime_us)) { + MESH_DEBUG_PRINTLN( + "RadioLibWrapper: invalid time-on-air estimate (0x%08lX)", + (unsigned long)airtime_us); + return 0; + } + return airtime_us / 1000; } bool RadioLibWrapper::startSendRaw(const uint8_t* bytes, int len) { diff --git a/src/helpers/radiolib/RadioLibWrappers.h b/src/helpers/radiolib/RadioLibWrappers.h index 8074749e..83aa9150 100644 --- a/src/helpers/radiolib/RadioLibWrappers.h +++ b/src/helpers/radiolib/RadioLibWrappers.h @@ -3,6 +3,7 @@ #include #include #include "CadTiming.h" +#include "RadioAirtime.h" // Fallback RX powersaving timings, only used until setRxPowerSaving() is called // (begin() always applies the persisted values). The authoritative defaults live diff --git a/test/test_cad_timing/test_cad_timing.cpp b/test/test_cad_timing/test_cad_timing.cpp index 46237033..5071faa2 100644 --- a/test/test_cad_timing/test_cad_timing.cpp +++ b/test/test_cad_timing/test_cad_timing.cpp @@ -2,6 +2,14 @@ #include #include +#include + +TEST(RadioAirtime, DetectsUnsignedRadioLibErrors) { + EXPECT_TRUE(mesh::isEncodedRadioLibAirtimeError(UINT32_MAX)); + EXPECT_TRUE(mesh::isEncodedRadioLibAirtimeError( + static_cast(-20))); + EXPECT_FALSE(mesh::isEncodedRadioLibAirtimeError(200000UL)); +} TEST(CadTiming, UsesShortDeadlineForCascadeProfile) { EXPECT_EQ(mesh::calculateCadScanTimeoutMillis(7, 62.5f), 100UL); diff --git a/test/test_trace_path_helpers/test_trace_path_helpers.cpp b/test/test_trace_path_helpers/test_trace_path_helpers.cpp index 0ff5d6a8..52c115c7 100644 --- a/test/test_trace_path_helpers/test_trace_path_helpers.cpp +++ b/test/test_trace_path_helpers/test_trace_path_helpers.cpp @@ -2,6 +2,22 @@ #include +TEST(TracePathHelpers, AddsTwoHundredPercentTerminalTimeoutMargin) { + uint32_t timeout = 0; + + ASSERT_TRUE(mesh::calculateTerminalTraceTimeoutMillis(6300, timeout)); + EXPECT_EQ(18900UL, timeout); +} + +TEST(TracePathHelpers, RejectsInvalidTerminalTimeouts) { + uint32_t timeout = 123; + + EXPECT_FALSE(mesh::calculateTerminalTraceTimeoutMillis(0, timeout)); + EXPECT_EQ(0UL, timeout); + EXPECT_FALSE(mesh::calculateTerminalTraceTimeoutMillis(UINT32_MAX, timeout)); + EXPECT_EQ(0UL, timeout); +} + TEST(TracePathHelpers, BuildsOneByteRoundTripThroughEndpoint) { const uint8_t saved[] = {0x11, 0x22}; const uint8_t endpoint[] = {0x33};