From 355ce04b21ad28d067a6a2085b3783676137b3fe Mon Sep 17 00:00:00 2001 From: liquidraver <504870+liquidraver@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:39:40 +0200 Subject: [PATCH] numerous more cad fixes and improvements --- releasenotes/RELEASE_NOTES_1.17.4-zephcore.md | 30 +++++- zephcore/adapters/radio/LR2021Radio.cpp | 24 +++++ zephcore/adapters/radio/LR2021Radio.h | 2 + .../drivers/lora/lr11xx/lr11xx_lora.c | 37 ++++++-- .../drivers/lora/lr20xx/lr20xx_lora.c | 91 ++++++++++++++++++- .../drivers/lora/lr20xx/lr20xx_lora.h | 2 + zephcore/src/Dispatcher.cpp | 52 +++++++---- 7 files changed, 213 insertions(+), 25 deletions(-) diff --git a/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md b/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md index 9cdddce..a78dc32 100644 --- a/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md +++ b/releasenotes/RELEASE_NOTES_1.17.4-zephcore.md @@ -4,7 +4,8 @@ Storage housekeeping, plus a listen-before-talk fix. The repeater's `erase` neve a node flashed from another firmware could start out with somebody else's leftovers underneath it, and switching a node between companion and repeater firmware quietly let the two share the same 128 KB. Separately, the channel-activity detector was using the wrong reference table on LR1110 -boards, and the companion's built-in `v` contact carried an unusable key on about half of all nodes. +boards, the companion's built-in `v` contact carried an unusable key on about half of all nodes, and +a node that decided the channel was busy could stop transmitting and never start again. > [!IMPORTANT] > **Read the role-switching section before you flash a different role onto an existing node.** A @@ -103,6 +104,11 @@ limits are, so a node can no longer sit against a wall it cannot see. Affected boards: **T1000-E**, **ThinkNode M3** and **ThinkNode M9**. On SX1262 boards nothing changes unless you run a 250 or 500 kHz bandwidth, where the old value was up to ten counts too sensitive. +How the tuner judges each measurement has changed too. It used to poll the radio a few times after a +detection and infer whether anything had really been there; now the radio stays listening and reports +whether a packet actually followed, so each reading is an observation rather than a guess. Nothing to +configure — it just means the figures behind `get cad.stats` describe what they claim to. + > [!NOTE] > **Run `set cad.reset` after upgrading.** The tuning statistics your node collected are measured > against the old starting point and are not comparable to the new one. Clearing them lets the tuner @@ -120,6 +126,28 @@ unless you run a 250 or 500 kHz bandwidth, where the old value was up to ten cou --- +## A node that believed the channel was busy could stop transmitting for good + +Before each transmission a node listens, and defers if it hears anything. If that kept happening it +was supposed to give up after four seconds and send anyway — a safeguard for a radio stuck reporting +activity that is not there. A refactor turned that into "wait, then try again", and the give-up was +lost. + +A node in that state refuses every transmission indefinitely, queues what it cannot send, and +eventually starts discarding packets — all while looking perfectly idle, because the refusals were +logged at a level nobody runs. Reproduced on the bench: a node in that state sent **nothing at all in +twenty seconds**, then flushed thirty queued messages the moment the cause was removed. + +The four-second safeguard is back, and a refused transmission is now reported in the log instead of +being invisible. + +> [!NOTE] +> **Only the "the radio may be stuck" case forces a transmission.** If the listen genuinely reports a +> busy channel the node still waits — transmitting over traffic it can hear would cause exactly the +> collision the check exists to avoid. + +--- + ## The built-in `v` contact had an unusable key on half of all nodes Every companion offers a contact named after itself with a `v` in front — the loopback chat that runs diff --git a/zephcore/adapters/radio/LR2021Radio.cpp b/zephcore/adapters/radio/LR2021Radio.cpp index fe72103..865e6ed 100644 --- a/zephcore/adapters/radio/LR2021Radio.cpp +++ b/zephcore/adapters/radio/LR2021Radio.cpp @@ -147,6 +147,30 @@ int LR2021Radio::hwCadProbe(int8_t level) return lr20xx_cad_probe(_dev, level); } +/* Re-enables adaptive CAD on this family. It was excluded in Batch B + * (plan B-D2) because the chip's CAD_RX exit uses cad_timeout, 24 bits of + * 32 MHz periods = 524 ms, against 1704 ms of max-length airtime -- a + * CAD_RX there would truncate receptions. The driver no longer uses that + * exit: a probe runs CAD_ONLY and the DIO1 handler arms the follow-on Rx + * with a normal SetRx, whose timeout is 24 bits of RTC steps = 512 s. The + * ceiling that justified the exclusion does not exist on that path, and the + * same construction is what fixed the SX126x and LR11xx after both of their + * chip CAD_RX exits were measured misbehaving on hardware. + * + * NOT hardware-verified: no LR2021 was available when this was written. + * Verify on a MeshTracker X1 with the same test the LR11xx fix used -- + * reception count while probing (must stay ~100%) and tp becoming + * non-zero -- before trusting it in the field. */ +int LR2021Radio::hwCadRxOutcome() +{ + return lr20xx_cad_rx_outcome(_dev); +} + +uint32_t LR2021Radio::hwCadRxTimeoutMs() +{ + return lr20xx_cad_rx_timeout_ms(_dev); +} + void LR2021Radio::hwCadSetPeakOffset(int8_t offset) { lr20xx_cad_set_peak_offset(_dev, offset); diff --git a/zephcore/adapters/radio/LR2021Radio.h b/zephcore/adapters/radio/LR2021Radio.h index 27cc968..1d1b29e 100644 --- a/zephcore/adapters/radio/LR2021Radio.h +++ b/zephcore/adapters/radio/LR2021Radio.h @@ -48,6 +48,8 @@ protected: bool hwIsReceiving() override; void hwSetRxBoost(bool enable) override; int hwCadProbe(int8_t level) override; + int hwCadRxOutcome() override; + uint32_t hwCadRxTimeoutMs() override; void hwCadSetPeakOffset(int8_t offset) override; uint8_t hwCadBasePeak() override; uint8_t hwCadPeakMin() override; diff --git a/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.c b/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.c index f2e51da..cb190cf 100644 --- a/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.c +++ b/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.c @@ -649,6 +649,8 @@ static int lr11xx_restart_rx(struct lr11xx_data *data, bool in_standby) static void lr11xx_dio1_callback(void *user_data); +static uint32_t lr11xx_cad_rx_timeout_steps(struct lr11xx_data *data); + static void lr11xx_dio1_work_handler(struct k_work *work) { struct lr11xx_data *data = CONTAINER_OF(work, struct lr11xx_data, @@ -811,6 +813,17 @@ static void lr11xx_dio1_work_handler(struct k_work *work) * discarded. Same meaning as the foreign-header-error branch * below -- reception continues, nothing needs restarting. */ if (data->cad_exit_rx && detected) { + /* CAD_ONLY has returned the chip to STBY_RC, so arm the + * follow-on Rx explicitly. This is the same call the + * normal receive path uses (see lr11xx_start_rx), so the + * receiver is configured exactly as it is when the node is + * receiving normally -- which the chip's own CAD_RX exit + * evidently is not. Bounded by the same max-payload + * figure cadTimeout used, so a real packet still completes + * inside the window and the terminal IRQ (RX_DONE / + * CRC_ERROR / HEADER_ERROR / TIMEOUT) stays guaranteed. */ + lr11xx_radio_set_rx_with_timeout_in_rtc_step( + ctx, lr11xx_cad_rx_timeout_steps(data)); data->in_rx_mode = true; atomic_set(&data->cad_rx_state, LR11XX_CAD_RX_ARMED); rx_restarted = true; @@ -2204,12 +2217,24 @@ static int lr11xx_do_cad(struct lr11xx_data *data) * bound: anything shorter would cut off the packet the detection * was for. See lr11xx_cad_rx_timeout_steps() for why the * vendor's millisecond wrapper is not used to convert it. */ - .cad_exit_mode = data->cad_exit_rx - ? LR11XX_RADIO_CAD_EXIT_MODE_RX - : LR11XX_RADIO_CAD_EXIT_MODE_STANDBYRC, - .cad_timeout = data->cad_exit_rx - ? lr11xx_cad_rx_timeout_steps(data) - : 0, + /* Always CAD_ONLY on the wire. A CAD_RX exit leaves this chip + * in CHIP_MODE_RX but DEAF: measured 2026-09-02, twelve armed + * windows produced no RX_DONE, SYNC_WORD_HEADER_VALID, + * CRC_ERROR or HEADER_ERROR (all four routed to DIO1) while a + * packet arrived every 9 s and ~3 were expected inside them -- + * only the chip's own cadTimeout ever came back. A controlled + * A/B put the cost at 14%% of received packets with probing on + * versus 0%% with it off, which C1 forbids outright, and it is + * also why `tp` could never be recorded on this family. + * + * So do not use the chip's CAD_RX exit. Take the documented + * STANDBYRC exit and arm the follow-on Rx ourselves with a + * plain SetRx in the DIO1 handler -- the same call the normal + * receive path uses, which demonstrably works (120/120 in that + * control). Same remedy as the SX126x, where the chip's + * cadTimeout was likewise not honoured. */ + .cad_exit_mode = LR11XX_RADIO_CAD_EXIT_MODE_STANDBYRC, + .cad_timeout = 0, }; lr11xx_radio_set_cad_params(ctx, &cad); diff --git a/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.c b/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.c index a0c20d3..3205491 100644 --- a/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.c +++ b/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.c @@ -139,6 +139,16 @@ struct lr20xx_data { * every LBT CAD; cad_probe_peak overrides for one calibration probe. */ int8_t cad_peak_offset; uint8_t cad_probe_peak; + /* Adaptive-CAD probe ground truth. The chip's own CAD_RX exit is NOT + * used (its cad_timeout is 24 bits of 32 MHz periods = 524 ms, well + * under a 1704 ms max-length packet). Instead a probe runs CAD_ONLY + * and this driver arms the follow-on Rx itself with a normal SetRx, + * whose timeout is 24 bits of RTC steps = 512 s -- so the ceiling that + * once excluded this family from probing does not apply. Same remedy + * as the SX126x and LR11xx, both of which had their chip CAD_RX exits + * fail differently on hardware. */ + bool cad_probe_rx; + atomic_t cad_rx_state; /* CAD_DONE -> carrier-up latency (k_cycle_get_32 units); 0 = no CAD * preceded this transmit. Only reached on the >524 ms fallback path. */ @@ -1635,6 +1645,25 @@ static void lr20xx_track_freq_offset(struct lr20xx_data *data, int32_t off_hz) static void lr20xx_dio1_callback(void *user_data); +/* RTC tick backing SetRx timeouts; the field is 24 bits, so the ceiling is + * 16777215/32768 = 512 s -- three orders above any packet we send. */ +#define LR20XX_RTC_FREQ_HZ 32768U + +/* cad_rx_state values -- mirrors the SX126x/LR11xx probe outcome tracker. */ +#define LR20XX_CAD_RX_IDLE 0 +#define LR20XX_CAD_RX_ARMED 1 +#define LR20XX_CAD_RX_PACKET 2 +#define LR20XX_CAD_RX_TMOUT 3 + +static uint32_t lr20xx_max_payload_ms(struct lr20xx_data *data); + +/* Resolve an in-flight probe Rx with the terminal event that just arrived. + * No-op unless one is armed, so the packet path pays one atomic compare. */ +static inline bool lr20xx_cad_rx_resolve(struct lr20xx_data *data, int outcome) +{ + return atomic_cas(&data->cad_rx_state, LR20XX_CAD_RX_ARMED, outcome); +} + static void lr20xx_dio1_work_handler(struct k_work *work) { struct lr20xx_data *data = CONTAINER_OF(work, struct lr20xx_data, @@ -1692,6 +1721,13 @@ static void lr20xx_dio1_work_handler(struct k_work *work) * in the same handler window is dropped too — the aborted packet * may have left bytes in the RX FIFO, misaligning the read. The * error branch below owns the window instead. */ + /* Ground truth for a probe Rx: anything proving a transmitter was + * there. A CRC or header error counts as much as a clean packet. */ + if (irq & (LR20XX_SYSTEM_IRQ_RX_DONE | LR20XX_SYSTEM_IRQ_CRC_ERROR | + LR20XX_SYSTEM_IRQ_LORA_HEADER_ERROR)) { + lr20xx_cad_rx_resolve(data, LR20XX_CAD_RX_PACKET); + } + if ((irq & LR20XX_SYSTEM_IRQ_RX_DONE) && !(irq & (LR20XX_SYSTEM_IRQ_CRC_ERROR | LR20XX_SYSTEM_IRQ_LORA_HEADER_ERROR))) { @@ -1771,6 +1807,25 @@ static void lr20xx_dio1_work_handler(struct k_work *work) LOG_DBG("CAD done: %s", detected ? "activity" : "free"); + if (data->cad_probe_rx && detected) { + /* CAD_ONLY has returned the chip to STDBY_RC. Arm the + * follow-on Rx with the same call the normal receive path + * uses, bounded by max-length-packet airtime, so whichever + * terminal IRQ follows (RX_DONE / CRC_ERROR / HEADER_ERROR / + * TIMEOUT) is the probe's ground truth. RTC steps, not the + * 524 ms cad_timeout field. */ + uint32_t steps = (uint32_t)(((uint64_t)lr20xx_max_payload_ms(data) + * LR20XX_RTC_FREQ_HZ) / 1000U); + + if (steps > 0x00FFFFFFU) { + steps = 0x00FFFFFFU; + } + atomic_set(&data->cad_rx_state, LR20XX_CAD_RX_ARMED); + lr20xx_radio_common_set_rx_with_timeout_in_rtc_step(ctx, steps); + data->in_rx_mode = true; + rx_restarted = true; + } + if (data->cad_cb) { lora_cad_cb cb = data->cad_cb; void *ud = data->cad_user_data; @@ -1802,6 +1857,9 @@ static void lr20xx_dio1_work_handler(struct k_work *work) /* ── Timeout ── */ if (irq & LR20XX_SYSTEM_IRQ_TIMEOUT) { + /* The probe's own Rx window expiring with nothing decoded: the + * detection had no packet behind it. */ + lr20xx_cad_rx_resolve(data, LR20XX_CAD_RX_TMOUT); if (data->tx_active) { /* The chip's Tx safeguard fired. DS §6.3.6: "the * transmission is stopped prematurely" — the packet is @@ -2092,7 +2150,6 @@ static uint32_t lr20xx_cad_timeout_ms(struct lr20xx_data *data) * 32.768kHz RTC"), not the 32 MHz periods cad_timeout uses above — the two sit * three lines apart here precisely so nobody reaches for the wrong one. Both * fields are 24 bits, so the RTC one tops out at 16777215/32768 = 512 s. */ -#define LR20XX_RTC_FREQ_HZ 32768U #define LR20XX_RTC_STEP_MAX 0x00FFFFFFU /* Never shorten the Tx safeguard below what shipped before it was scaled. */ #define LR20XX_TX_TIMEOUT_FLOOR_MS 5000U @@ -3293,12 +3350,44 @@ int lr20xx_cad_probe(const struct device *dev, int8_t peak_offset) /* One-shot absolute override consumed by lr20xx_do_cad(). Probes and * LBT both run on the mesh loop thread, so no concurrent CAD exists. */ data->cad_probe_peak = (uint8_t)peak; + data->cad_probe_rx = true; + atomic_set(&data->cad_rx_state, LR20XX_CAD_RX_IDLE); ret = lr20xx_lora_cad(dev, K_MSEC(lr20xx_cad_timeout_ms(data))); + data->cad_probe_rx = false; data->cad_probe_peak = 0; + /* 2 tells the caller the chip is in Rx on the signal CAD found, so it + * must NOT re-enter Rx itself, and that an outcome will be readable + * from lr20xx_cad_rx_outcome() once a terminal IRQ lands. */ + if (ret > 0) { + return 2; + } + return ret; } +uint32_t lr20xx_cad_rx_timeout_ms(const struct device *dev) +{ + struct lr20xx_data *data = dev->data; + + /* Same bound do_cad's follow-on SetRx programs, so the caller's wait + * and the chip's deadline cannot drift apart. */ + return lr20xx_max_payload_ms(data); +} + +int lr20xx_cad_rx_outcome(const struct device *dev) +{ + struct lr20xx_data *data = dev->data; + atomic_val_t st = atomic_get(&data->cad_rx_state); + + if (st != LR20XX_CAD_RX_PACKET && st != LR20XX_CAD_RX_TMOUT) { + return 0; /* nothing armed, or still awaiting the terminal IRQ */ + } + + atomic_set(&data->cad_rx_state, LR20XX_CAD_RX_IDLE); + return (st == LR20XX_CAD_RX_PACKET) ? 1 : 2; +} + /* ── Driver API: recv_duty_cycle ────────────────────────────────────── */ static int lr20xx_lora_recv_duty_cycle(const struct device *dev, diff --git a/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.h b/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.h index b5fc867..4067d1f 100644 --- a/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.h +++ b/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.h @@ -245,6 +245,8 @@ uint8_t lr20xx_cad_peak_max(void); * @return 1 = activity detected, 0 = channel free, <0 = error */ int lr20xx_cad_probe(const struct device *dev, int8_t peak_offset); +uint32_t lr20xx_cad_rx_timeout_ms(const struct device *dev); +int lr20xx_cad_rx_outcome(const struct device *dev); #ifdef __cplusplus } diff --git a/zephcore/src/Dispatcher.cpp b/zephcore/src/Dispatcher.cpp index 56b64a9..48d42d5 100644 --- a/zephcore/src/Dispatcher.cpp +++ b/zephcore/src/Dispatcher.cpp @@ -421,18 +421,25 @@ void Dispatcher::checkSend() _radio->getNoiseFloor(), (unsigned)_radio->getPacketsRecv(), (unsigned)_radio->getPacketsRecvErrors()); - /* With the non-destructive sx126x_is_receiving() we lost - * the accidental side-effect IRQ clear that used to break - * us out of stuck preamble bits. Walk the chip back - * through REST → fresh RX, which bulk-clears IRQ status - * and resets the rx_packet_active latch. Then re-wake the - * loop promptly so the next checkSend() retries TX. */ + /* Channel activity has gone on too long -- the radio may be + * in a bad state. FORCE the pending transmit by falling + * through, exactly as Arduino MeshCore does + * (Dispatcher.cpp: "force the pending transmit below..."). + * + * This bounded give-up was ZephCore's behaviour too until + * 3441caf "new rx busy latch" added a `return` here, turning a + * 4 s hard limit into an unbounded defer: on a channel that + * reads busy forever the node never transmits again, silently + * filling the 32-entry outbound queue until queueOutbound() + * starts evicting and dropping. + * + * recoverRxState() is kept and runs first: the non-destructive + * sx126x_is_receiving() has no side-effect IRQ clear, so a stuck + * preamble bit needs the chip walked REST -> fresh RX. Doing it + * before the forced TX leaves the receiver healthy afterwards; + * send_async accepts the RX -> TX entry CAS. */ _radio->recoverRxState(); - cad_busy_start = 0; - if (_tx_queued_cb) { - _tx_queued_cb(1, _tx_queued_user_data); - } - return; + /* fall through -- force the pending transmit */ } else { uint32_t retry = getCADFailRetryDelay(); next_tx_time = futureMillis((int)retry); @@ -516,12 +523,23 @@ void Dispatcher::checkSend() bool success = _radio->startSendRaw(raw, len); if (!success) { uint32_t retry = getCADFailRetryDelay(); - /* Almost always LBT refusing a busy channel, which - * is the designed outcome — the packet is - * re-queued below and retried. At ERR this fires - * continuously on a busy site and buries real - * faults. */ - LOG_DBG("checkSend: startSendRaw refused, re-queuing delay=%u", retry); + /* Almost always LBT refusing a busy channel, which is the + * designed outcome — the packet is re-queued below and + * retried, and we deliberately do NOT force a transmit + * here: unlike the isReceiving() gate above, where a long + * refusal suggests a stuck radio, a busy LBT verdict is a + * TRUE reading of the channel. Forcing through it would + * transmit into traffic the radio can hear — exactly the + * collision CAD exists to prevent, at the moment the + * channel is most contended. + * + * INF, not DBG: this used to be ERR, which buried real + * faults on a busy site, and was then dropped to DBG — + * which made a node refusing every transmit completely + * invisible at default log level. A node that is not + * transmitting should say so; INF reports the refusals + * themselves rather than inferring a stall from them. */ + LOG_INF("checkSend: startSendRaw refused (LBT busy), re-queuing delay=%u", retry); /* Escalate a refusal that will not end. This branch * used to leave cad_busy_start alone, so a packet the