From dc5b5c25941921af6c4d57308de307ef76ee905f Mon Sep 17 00:00:00 2001 From: liquidraver <504870+liquidraver@users.noreply.github.com> Date: Sun, 23 Aug 2026 22:49:17 +0200 Subject: [PATCH] radio driver fixes v1 --- zephcore/adapters/radio/LR1110Radio.cpp | 17 +- zephcore/adapters/radio/LR1110Radio.h | 3 +- zephcore/adapters/radio/LR2021Radio.cpp | 16 +- zephcore/adapters/radio/LR2021Radio.h | 3 +- zephcore/adapters/radio/LoRaRadioBase.cpp | 202 +++++++++++++++--- zephcore/adapters/radio/LoRaRadioBase.h | 55 ++++- zephcore/adapters/radio/SX126xRadio.cpp | 19 +- zephcore/adapters/radio/SX126xRadio.h | 1 + .../adapters/radio/lr11xx/lr11xx_hal_zephyr.c | 12 +- .../adapters/radio/lr20xx/lr20xx_hal_zephyr.c | 9 + zephcore/include/mesh/Radio.h | 2 +- .../drivers/lora/lr11xx/lr11xx_lora.c | 99 +++++++-- .../drivers/lora/lr11xx/lr11xx_lora.h | 17 +- .../drivers/lora/lr20xx/lr20xx_lora.c | 63 ++++-- .../drivers/lora/lr20xx/lr20xx_lora.h | 15 +- zephcore/src/Dispatcher.cpp | 2 +- 16 files changed, 422 insertions(+), 113 deletions(-) diff --git a/zephcore/adapters/radio/LR1110Radio.cpp b/zephcore/adapters/radio/LR1110Radio.cpp index 26e1c6a..ebd633f 100644 --- a/zephcore/adapters/radio/LR1110Radio.cpp +++ b/zephcore/adapters/radio/LR1110Radio.cpp @@ -78,19 +78,24 @@ bool LR1110Radio::hwIsReceiving() return lr11xx_is_receiving(_dev); } -void LR1110Radio::hwResetAgc() -{ - lr11xx_reset_agc(_dev); -} +/* No hwResetAgc() override, and no hwNeedsAgcReset(): this part has no + * jammed-AGC fault to remedy — that one belongs to the SX126x. The driver + * exposes lr11xx_recalibrate() instead, which is the temperature-drift path + * this family genuinely does need, named for what it actually does. */ + void LR1110Radio::hwRecalibrate() { lr11xx_recalibrate(_dev); } -int16_t LR1110Radio::hwGetChipTempC() +/* This family's UM gives an explicit temperature threshold for image + * calibration, so drift recalibration is active here. The temperature itself + * comes from the board, not from lr11xx_get_chip_temp_c() — see + * LoRaRadioBase::imageCalMaintenance() for why the radio is not asked. */ +bool LR1110Radio::hwHasDriftRecal() { - return lr11xx_get_chip_temp_c(_dev); + return true; } void LR1110Radio::hwSetRxBoost(bool enable) diff --git a/zephcore/adapters/radio/LR1110Radio.h b/zephcore/adapters/radio/LR1110Radio.h index fefac42..3031e8d 100644 --- a/zephcore/adapters/radio/LR1110Radio.h +++ b/zephcore/adapters/radio/LR1110Radio.h @@ -34,9 +34,8 @@ protected: int16_t hwGetCurrentRSSI() override; bool hwIsReceiving() override; void hwSetRxBoost(bool enable) override; - void hwResetAgc() override; void hwRecalibrate() override; - int16_t hwGetChipTempC() override; + bool hwHasDriftRecal() override; uint32_t hwWakeupTimeUs() override; int hwCadProbe(int8_t level) override; void hwCadSetPeakOffset(int8_t offset) override; diff --git a/zephcore/adapters/radio/LR2021Radio.cpp b/zephcore/adapters/radio/LR2021Radio.cpp index 26b830d..deb93d3 100644 --- a/zephcore/adapters/radio/LR2021Radio.cpp +++ b/zephcore/adapters/radio/LR2021Radio.cpp @@ -167,10 +167,11 @@ uint8_t LR2021Radio::hwCadPeakMax() return lr20xx_cad_peak_max(); } -void LR2021Radio::hwResetAgc() -{ - lr20xx_reset_agc(_dev); -} +/* No hwResetAgc() override, and no hwNeedsAgcReset(): this part has no + * jammed-AGC fault to remedy — that one belongs to the SX126x. The driver + * exposes lr20xx_recalibrate() instead, which is the temperature-drift path + * this family genuinely does need, named for what it actually does. */ + void LR2021Radio::hwRecalibrate() { @@ -179,9 +180,12 @@ void LR2021Radio::hwRecalibrate() lr20xx_recalibrate(_dev); } -int16_t LR2021Radio::hwGetChipTempC() +/* DS 6.4.2 gives an image/FE calibration temperature range (and advises redoing + * PLL/AAF beyond +/-20 C), so drift recalibration is active here. The reading + * comes from the board — see LoRaRadioBase::imageCalMaintenance(). */ +bool LR2021Radio::hwHasDriftRecal() { - return lr20xx_get_chip_temp_c(_dev); + return true; } uint32_t LR2021Radio::hwWakeupTimeUs() diff --git a/zephcore/adapters/radio/LR2021Radio.h b/zephcore/adapters/radio/LR2021Radio.h index 60c87d7..a0703ab 100644 --- a/zephcore/adapters/radio/LR2021Radio.h +++ b/zephcore/adapters/radio/LR2021Radio.h @@ -52,9 +52,8 @@ protected: uint8_t hwCadPeakMin() override; uint8_t hwCadPeakMax() override; uint32_t hwWakeupTimeUs() override; - void hwResetAgc() override; void hwRecalibrate() override; - int16_t hwGetChipTempC() override; + bool hwHasDriftRecal() override; }; } /* namespace mesh */ diff --git a/zephcore/adapters/radio/LoRaRadioBase.cpp b/zephcore/adapters/radio/LoRaRadioBase.cpp index a503609..b4c6d2f 100644 --- a/zephcore/adapters/radio/LoRaRadioBase.cpp +++ b/zephcore/adapters/radio/LoRaRadioBase.cpp @@ -61,7 +61,11 @@ LoRaRadioBase::LoRaRadioBase(const struct device *lora_dev, MainBoard &board, _rx_boost_enabled(true), _dc_last_rx_us(0), _dc_last_sleep_us(0), _agc_rx_count_shadow(0), _agc_last_activity_ms(0), - _agc_last_cal_temp_c(INT16_MIN), + _agc_rssi_last(0), _agc_rssi_frozen(0), + _image_cal_last_temp_c(INT16_MIN), + _image_cal_last_ms(0), _image_cal_wait_ms(0), + _image_cal_started(false), _image_cal_confirming(false), + _last_tx_start_ms(0), _config_cached(false), _has_radio_override(false), _override_freq(0), _override_bw(0), @@ -751,6 +755,7 @@ bool LoRaRadioBase::startSendRaw(const uint8_t *bytes, int len) _board->onBeforeTransmit(); atomic_set(&_tx_active, 1); + _last_tx_start_ms = k_uptime_get_32(); /* Phase 2: when LBT is enabled, skip the pre-emptive hwCancelReceive() * and keep _in_recv_mode = 1 so the driver's send_async sees state == RX @@ -1056,6 +1061,19 @@ void LoRaRadioBase::triggerNoiseFloorCalibrate(int threshold) (rssi <= _noise_floor + CAD_PROBE_RSSI_GUARD); _sample_fresh = true; + /* Stuck-AGC evidence, gathered from a reading we already took. A live + * front end dithers by a dB or two between samples even on a quiet + * channel; a desensitised one returns the same number forever. Costs + * nothing and needs no chip command of its own. */ + if (rssi == _agc_rssi_last) { + if (_agc_rssi_frozen < 0xFF) { + _agc_rssi_frozen++; + } + } else { + _agc_rssi_last = rssi; + _agc_rssi_frozen = 0; + } + /* First sample after reset (DEFAULT_NOISE_FLOOR == 0): seed directly. * The lower clamp tracks the active bandwidth — thermal noise is * 10*log10(BW) so a fixed rail pins narrow-BW presets several dB high @@ -1139,15 +1157,24 @@ bool LoRaRadioBase::isReceiving() * is working, and it fires no more often on a silent one, where a * desensitised receiver is invisible and nothing else will reveal it. * - * So trigger on silence instead. If nothing has been heard for - * AGC_IDLE_RESET_MS the receiver is either genuinely idle — in which case - * the reset costs nothing, there is no traffic to miss — or it is deaf, in - * which case this is the only thing that will recover it. Self-targeting in - * both directions: a busy node essentially never resets, and the resets a - * quiet node does perform are free. Arduino MeshCore's periodic + * So trigger on silence instead. Arduino MeshCore's periodic * `agc_reset_interval` addressed the same fault and shipped defaulted to 0 * (off), which is a fair summary of how well a plain timer serves it. * + * This used to claim the reset was free because an idle node "has no traffic + * to miss". That is false, and hardware disproved it: the reset fires at the + * END of a silent stretch, which is precisely when traffic resumes, and + * hwResetAgc() takes the radio out of RX for the whole warm-sleep + + * Calibrate(ALL) + image-cal + re-entry sequence. A packet was measured lost + * to exactly that window on 2026-08-23 (T1000-E, 92 ms after the re-arm). + * + * Silence is therefore treated as a prerequisite, not as proof. Firing also + * requires corroboration from evidence already on hand: the noise-floor + * sampler reads RSSI every interval regardless, and a desensitised front end + * reports a frozen value. Both conditions together, and the operation + * essentially never runs on a healthy node — which is the only acceptable + * cost for a watchdog guarding a fault nobody has observed in the field. + * * Deliberately NOT reset here: the noise floor. The previous periodic * implementation (removed in fe6e585) zeroed it on every fire, forcing a * fresh seed and a full EMA warmup each time — that, not the AGC work, is @@ -1162,13 +1189,46 @@ bool LoRaRadioBase::isReceiving() * block (DS 6.12) only corrects crystal drift from TX self-heating, and it * refuses to run at all when a TCXO is fitted. * - * AGC_RECAL_TEMP_DELTA_C is deliberately tighter than either datasheet + * IMAGE_CAL_TEMP_DELTA_C is deliberately tighter than either datasheet * figure — cheap insurance, and the reading is a junction temperature that * lags ambient. */ -#define AGC_IDLE_RESET_MS 60000U -#define AGC_RECAL_TEMP_DELTA_C 5 +/* Silence alone is NOT evidence of a fault — see the rationale block above. + * Ten minutes, not one: a genuinely deaf receiver stays deaf, so waiting costs + * nothing, whereas a 60 s threshold fired 4-8 times per quarter hour on a + * perfectly healthy node (measured 2026-08-23). */ +#define AGC_IDLE_RESET_MS 600000U -void LoRaRadioBase::agcMaintenance() +/* Consecutive identical noise-floor readings before silence is believed. At + * the default 15 s sampler interval this is two minutes of a frozen front end. */ +#define AGC_STUCK_RSSI_SAMPLES 8U + +/* Image-calibration drift threshold. Nothing to do with the AGC reset above — + * this is the front end. SX126x DS §9.2.1 / LR11xx UM: image calibration is + * required after a frequency change > 10 MHz or a temperature change > 10 C. + * Half the datasheet figure is used so drift is corrected before it reaches the + * point where the datasheet says the calibration is already stale. */ +#define IMAGE_CAL_TEMP_DELTA_C 5 + +/* Temperature-drift poll cadence. The maintenance pass itself runs every + * CONFIG_ZEPHCORE_NOISE_FLOOR_INTERVAL_MS (15 s by default); reading the + * junction temperature that often is 240 chip commands an hour to watch a + * quantity that physically cannot move IMAGE_CAL_TEMP_DELTA_C in minutes. + * Every one of those commands is an opportunity to collide with a duty-cycled + * radio's autonomous sleep transition, so the cheapest read is the one not + * issued. The driver-side BUSY guards make the collision safe; this makes it + * rare. */ +#define IMAGE_CAL_POLL_MS 3600000U /* 1 h between routine reads */ +#define IMAGE_CAL_CONFIRM_MS 15000U /* re-read before acting on a delta */ +#define IMAGE_CAL_TX_QUIET_MS 60000U /* let PA self-heating decay first */ + +/* Two independent jobs share this hook because they share one precondition — + * the chip must be idle enough to accept a command — and one cadence source, + * the maintenance loop. They are otherwise unrelated: agcIdleMaintenance() + * unsticks a receiver that has stopped hearing anything, imageCalMaintenance() + * corrects front-end image calibration against temperature drift. Keep them in + * separate functions so neither's thresholds read as if they governed the + * other. */ +void LoRaRadioBase::radioMaintenance() { /* Never mid-transmit or mid-receive: both operations warm-sleep the * chip, which aborts a TX and destroys an in-flight packet. The RX @@ -1184,6 +1244,19 @@ void LoRaRadioBase::agcMaintenance() uint32_t now = (uint32_t)k_uptime_get_32(); + agcIdleMaintenance(now); + imageCalMaintenance(now); +} + +/* Receiver watchdog: a stuck AGC stops the demodulator hearing anything at all, + * so prolonged total silence is the symptom. Nothing here concerns the front + * end or temperature. */ +void LoRaRadioBase::agcIdleMaintenance(uint32_t now) +{ + if (!hwNeedsAgcReset()) { + return; + } + /* Any demodulation activity counts as proof of life, errored frames * included — a CRC failure still means RF reached the demodulator, which * is precisely what a stuck AGC would prevent. Counting only good @@ -1194,40 +1267,109 @@ void LoRaRadioBase::agcMaintenance() if (rx_total != _agc_rx_count_shadow || _agc_last_activity_ms == 0) { _agc_rx_count_shadow = rx_total; _agc_last_activity_ms = now ? now : 1; - } else if ((now - _agc_last_activity_ms) >= AGC_IDLE_RESET_MS) { - LOG_INF("agc: %u ms without RX activity — resetting AGC", - (unsigned)(now - _agc_last_activity_ms)); + } else if ((now - _agc_last_activity_ms) >= AGC_IDLE_RESET_MS && + _agc_rssi_frozen >= AGC_STUCK_RSSI_SAMPLES) { + LOG_INF("agc: %u ms silent AND %u frozen floor samples at %d dBm — resetting AGC", + (unsigned)(now - _agc_last_activity_ms), + (unsigned)_agc_rssi_frozen, (int)_agc_rssi_last); hwResetAgc(); /* hwResetAgc() leaves the chip out of RX by contract, so this is * a genuine re-entry and re-arms the duty cycle if one is set. */ startReceive(); _agc_last_activity_ms = now ? now : 1; } +} - /* Temperature drift. Sampled every pass because the read is one cheap - * command; the expensive recalibration is gated on the delta. */ - int16_t temp_c = hwGetChipTempC(); - - if (temp_c == INT16_MIN) { - return; /* backend cannot measure — drift handling disabled */ - } - if (_agc_last_cal_temp_c == INT16_MIN) { - _agc_last_cal_temp_c = temp_c; /* first reading is the baseline */ +/* Front-end image calibration against temperature drift. Separate from the AGC + * reset above in every respect: different symptom (degraded image rejection, not + * a deaf demodulator), different datasheet section, different remedy + * (hwRecalibrate(), not hwResetAgc()). */ +void LoRaRadioBase::imageCalMaintenance(uint32_t now) +{ + if (!hwHasDriftRecal()) { return; } - int delta = (int)temp_c - (int)_agc_last_cal_temp_c; + /* Polled on its own slow cadence rather than once per + * maintenance pass — see IMAGE_CAL_POLL_MS. Backends that cannot measure + * (SX126x/SX127x) answer INT16_MIN from a plain inline with no bus + * traffic, so the early return below costs them nothing. */ + if (_image_cal_started && (now - _image_cal_last_ms) < _image_cal_wait_ms) { + return; + } + + /* Wait out our own PA. The junction is still warm for a while after a + * transmit, and that self-heating is not the ambient drift the + * recalibration exists to track — acting on it would recalibrate against + * a temperature the chip will not be at a minute later. Deferring + * returns here without stamping, so the retry is the next pass (seconds) + * rather than the next poll window (an hour). */ + if (_last_tx_start_ms != 0 && + (now - _last_tx_start_ms) < IMAGE_CAL_TX_QUIET_MS) { + return; + } + + /* Temperature comes from the BOARD, never from the radio. + * + * Only the delta matters here, never the absolute value, and the MCU die + * sensor tracks the same ambient the front end sits in — so it answers + * the question just as well as the radio's junction sensor while costing + * the radio nothing at all. Reading it off the chip meant a periodic SPI + * command aimed at a part that spends most of its time in an autonomous + * duty-cycle sleep phase; that read wedged the LR1110 BUSY-high 7 times + * in 47 minutes of measurement on 2026-08-23, each costing 12-18 s of + * deafness, and it cost a packet. The junction sensor was also the worse + * instrument for the job: it sees PA self-heating, which is exactly the + * transient this path must not react to. */ + float board_temp = _board ? _board->getMCUTemperature() : NAN; + + if (isnan(board_temp)) { + /* No board temperature source — drift handling simply does not run + * on this hardware, as it did not before on families without a + * junction sensor either. */ + return; + } + + int16_t temp_c = (int16_t)lroundf(board_temp); + + _image_cal_started = true; + _image_cal_last_ms = now; + _image_cal_wait_ms = IMAGE_CAL_POLL_MS; + + if (_image_cal_last_temp_c == INT16_MIN) { + _image_cal_last_temp_c = temp_c; /* first reading is the baseline */ + return; + } + + int delta = (int)temp_c - (int)_image_cal_last_temp_c; if (delta < 0) { delta = -delta; } - if (delta >= AGC_RECAL_TEMP_DELTA_C) { - LOG_INF("agc: chip temp moved %d C (%d -> %d) — recalibrating", - delta, (int)_agc_last_cal_temp_c, (int)temp_c); - hwRecalibrate(); - startReceive(); - _agc_last_cal_temp_c = temp_c; + if (delta < IMAGE_CAL_TEMP_DELTA_C) { + _image_cal_confirming = false; + return; } + + /* Measure twice before acting. A single reading over the threshold can + * be a transient — residual self-heating the quiet window did not fully + * cover, or a one-off bad sample — and hwRecalibrate() takes the radio + * out of receive. Re-read shortly and act only if the second reading + * agrees. */ + if (!_image_cal_confirming) { + _image_cal_confirming = true; + _image_cal_wait_ms = IMAGE_CAL_CONFIRM_MS; + LOG_DBG("imagecal: chip temp delta %d C — confirming before recalibrating", + delta); + return; + } + + _image_cal_confirming = false; + LOG_INF("imagecal: chip temp moved %d C (%d -> %d) — recalibrating", + delta, (int)_image_cal_last_temp_c, (int)temp_c); + hwRecalibrate(); + startReceive(); + _image_cal_last_temp_c = temp_c; } void LoRaRadioBase::recoverRxState() diff --git a/zephcore/adapters/radio/LoRaRadioBase.h b/zephcore/adapters/radio/LoRaRadioBase.h index 3b97a58..525eb60 100644 --- a/zephcore/adapters/radio/LoRaRadioBase.h +++ b/zephcore/adapters/radio/LoRaRadioBase.h @@ -133,7 +133,13 @@ public: void cadMaintenance() override; /** Deaf-aware AGC unstick + temperature-drift recalibration. * Called from Dispatcher::maintenanceLoop(); never on the packet path. */ - void agcMaintenance() override; + void radioMaintenance() override; + +private: + /* The two unrelated jobs radioMaintenance() drives; see its comment. */ + void agcIdleMaintenance(uint32_t now); + void imageCalMaintenance(uint32_t now); +public: uint32_t msUntilNextMaintenance() override; int8_t getCadOffset() const override { return _cad_offset; } @@ -188,7 +194,7 @@ protected: virtual uint8_t hwCadPeakMin() { return 0; } virtual uint8_t hwCadPeakMax() { return 0; } - /* ── Receiver hygiene — see LoRaRadioBase::agcMaintenance() ────── */ + /* ── Receiver hygiene — see LoRaRadioBase::radioMaintenance() ─── */ /** Unstick a jammed AGC: warm sleep to drop the analog front end, then * recalibrate on the way back up. Semtech's stated remedy; the @@ -206,7 +212,25 @@ protected: /** Chip junction temperature in whole degrees C, or INT16_MIN when the * backend cannot measure it (which disables drift recalibration). */ - virtual int16_t hwGetChipTempC() { return INT16_MIN; } + /* Does this family actually suffer the jammed-AGC fault the reset is a + * remedy for? Only the SX126x does. Semtech prescribe warm sleep plus + * recalibration for it, ZephCore inherited the idea from Arduino + * MeshCore's `agc_reset_interval`, and both are SX126x-era. Neither the + * LR11xx UM nor the LR2021 DS describes such a fault; those parts need + * image/front-end recalibration on temperature drift, which is a + * different operation on a different trigger (hwRecalibrate()). + * + * Running it anyway is not free. Measured on a T1000-E 2026-08-23: the + * 60 s after an AGC reset carried a 7.4% packet-miss rate against 0.6% + * elsewhere, and one deaf stretch began at one reset and ended at the + * next — the reset was recovering damage it had caused, on a part with + * no AGC fault to fix. */ + virtual bool hwNeedsAgcReset() { return false; } + + /* Does this family specify a temperature threshold for image/front-end + * recalibration? LR11xx and LR2021 do; the SX126x datasheet does not, + * and drift recalibration stays inactive there exactly as before. */ + virtual bool hwHasDriftRecal() { return false; } /** Radio deaf time per duty-cycle wake transition (context restore + * PLL lock + TCXO startup where fitted), in microseconds. Counts @@ -334,12 +358,33 @@ protected: uint32_t _dc_last_sleep_us; - /* agcMaintenance() bookkeeping. RX activity is inferred by sampling the + /* agcIdleMaintenance() bookkeeping. RX activity is inferred by sampling the * existing packet counters rather than timestamping in the RX callback, * so nothing is added to the ISR path. */ uint32_t _agc_rx_count_shadow; uint32_t _agc_last_activity_ms; - int16_t _agc_last_cal_temp_c; + + /* Stuck-AGC corroboration. The noise-floor sampler already produces an + * RSSI every interval; a desensitised front end reports a frozen one. + * Free evidence — no extra command, no extra wake. */ + int16_t _agc_rssi_last; + uint8_t _agc_rssi_frozen; + int16_t _image_cal_last_temp_c; + + /* Image-calibration temperature polling. Deliberately much slower than + * the maintenance cadence — see imageCalMaintenance(). */ + uint32_t _image_cal_last_ms; + uint32_t _image_cal_wait_ms; + bool _image_cal_started; + bool _image_cal_confirming; + + /* Start of the most recent transmit, for the post-TX quiet window the + * temperature poll waits out. Start-referenced rather than end- + * referenced so it can be stamped at the single startSendRaw() site + * instead of all five _tx_active clear paths; airtime is bounded by + * seconds and the quiet window by a minute, so the difference does not + * matter. */ + uint32_t _last_tx_start_ms; /* Config cache — skip redundant hwConfigure() */ struct lora_modem_config _last_cfg; diff --git a/zephcore/adapters/radio/SX126xRadio.cpp b/zephcore/adapters/radio/SX126xRadio.cpp index 05c2730..340f49b 100644 --- a/zephcore/adapters/radio/SX126xRadio.cpp +++ b/zephcore/adapters/radio/SX126xRadio.cpp @@ -75,6 +75,14 @@ bool SX126xRadio::hwIsReceiving() return sx126x_is_receiving(_dev); } +/* The one family that actually has the jammed-AGC fault — see the patch preamble + * in patches/zephyr/0003-lora-sx126x-native.patch, where this remedy and its + * trigger are argued out. The LR parts deliberately do not declare it. */ +bool SX126xRadio::hwNeedsAgcReset() +{ + return true; +} + void SX126xRadio::hwResetAgc() { /* Leaves the chip in STANDBY and the driver state at IDLE, so the @@ -90,11 +98,12 @@ void SX126xRadio::hwRecalibrate() sx126x_reset_agc(_dev); } -/* No hwGetChipTempC(): the SX126x exposes no junction-temperature readout — - * no command in the datasheet, and RadioLib has no getTemperature() for it - * either. Drift recalibration is therefore inactive on this family, which - * costs little: unlike the LR parts, its datasheet gives no temperature - * threshold for image calibration in the first place. */ +/* No hwHasDriftRecal() override: unlike the LR parts, the SX126x datasheet + * gives no temperature threshold for image calibration, so drift-triggered + * recalibration stays inactive on this family — unchanged from before, when the + * same result came about because the part exposes no junction-temperature + * readout for the trigger to read. (The trigger now reads board temperature, + * which every board has, so the family gate has to be explicit.) */ void SX126xRadio::hwSetRxBoost(bool enable) { diff --git a/zephcore/adapters/radio/SX126xRadio.h b/zephcore/adapters/radio/SX126xRadio.h index 201e238..fd47dbd 100644 --- a/zephcore/adapters/radio/SX126xRadio.h +++ b/zephcore/adapters/radio/SX126xRadio.h @@ -34,6 +34,7 @@ protected: int16_t hwGetCurrentRSSI() override; bool hwIsReceiving() override; void hwSetRxBoost(bool enable) override; + bool hwNeedsAgcReset() override; void hwResetAgc() override; void hwRecalibrate() override; bool hwIsChipBusy() override; diff --git a/zephcore/adapters/radio/lr11xx/lr11xx_hal_zephyr.c b/zephcore/adapters/radio/lr11xx/lr11xx_hal_zephyr.c index 33c42ca..e831c5a 100644 --- a/zephcore/adapters/radio/lr11xx/lr11xx_hal_zephyr.c +++ b/zephcore/adapters/radio/lr11xx/lr11xx_hal_zephyr.c @@ -286,7 +286,17 @@ lr11xx_hal_status_t lr11xx_hal_read(const void *context, const uint8_t *command, for (int attempt = 0; attempt < LR11XX_READ_ATTEMPTS; attempt++) { if (attempt > 0) { /* Recovery means re-issuing the command: the chip only streams the - * answer in the window that follows it. */ + * answer in the window that follows it. + * + * Wait on BUSY first, exactly as the first pass does at the top of + * this function. A command clocked into a chip that has not + * dropped BUSY is not a retry, it is a malformed frame the chip + * answers with CMD_PERR — the failure mode behind the MeshTracker + * X1 rejection storm (see check_device_ready() in the LR2021 HAL). */ + if (check_device_ready(ctx) != LR11XX_HAL_STATUS_OK) { + return LR11XX_HAL_STATUS_ERROR; + } + gpio_pin_set_dt(&ctx->nss, 1); ret = spi_write(ctx->spi_dev, &ctx->spi_cfg, &tx); gpio_pin_set_dt(&ctx->nss, 0); diff --git a/zephcore/adapters/radio/lr20xx/lr20xx_hal_zephyr.c b/zephcore/adapters/radio/lr20xx/lr20xx_hal_zephyr.c index f45238f..c878e47 100644 --- a/zephcore/adapters/radio/lr20xx/lr20xx_hal_zephyr.c +++ b/zephcore/adapters/radio/lr20xx/lr20xx_hal_zephyr.c @@ -307,6 +307,15 @@ static int lr20xx_spi_read_frame(struct lr20xx_hal_context *ctx, const uint8_t * * no stat header, no BUSY wait — structurally immune to the race * below). */ for (int attempt = 0; attempt < LR20XX_READ_ATTEMPTS; attempt++) { + /* Wait on BUSY before re-issuing, exactly as lr20xx_hal_read() does + * before the first pass. A command clocked into a chip that has not + * dropped BUSY is not a retry, it is a malformed frame the chip + * answers with CMD_PERR — see check_device_ready() above for what + * that cost on the MeshTracker X1. */ + if (attempt > 0 && check_device_ready(ctx) != LR20XX_HAL_STATUS_OK) { + return -ETIMEDOUT; + } + /* Phase 1: the command, in its own NSS window. */ { const struct spi_buf tx_buf = { diff --git a/zephcore/include/mesh/Radio.h b/zephcore/include/mesh/Radio.h index 286d962..768cc6d 100644 --- a/zephcore/include/mesh/Radio.h +++ b/zephcore/include/mesh/Radio.h @@ -53,7 +53,7 @@ public: /** Receiver hygiene: deaf-aware AGC unstick and temperature-drift * recalibration. Both sleep the radio, so this is called only from the * maintenance tick, never from a packet path. Default: no-op. */ - virtual void agcMaintenance() {} + virtual void radioMaintenance() {} /* Milliseconds until this radio's periodic work (noise floor sampling, * CAD probing/decay) next needs a call, or MAINTENANCE_IDLE when it has 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 3126122..7d99447 100644 --- a/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.c +++ b/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.c @@ -1455,7 +1455,7 @@ uint32_t lr11xx_get_random(const struct device *dev) * exactly what the Arduino helper does. That makes recal_fe moot on this part: * both entry points pay it because the calibration forces it. */ -static void lr11xx_agc_reset_locked(struct lr11xx_data *data) +static void lr11xx_recalibrate_locked(struct lr11xx_data *data) { void *ctx = &data->hal_ctx; lr11xx_system_sleep_cfg_t sleep_cfg = { @@ -1477,34 +1477,83 @@ static void lr11xx_agc_reset_locked(struct lr11xx_data *data) freq_mhz + 4); } + /* Let the calibration actually finish before handing the chip back. + * + * Calibrate asserts BUSY for the duration, but the HAL's wait_on_busy() + * samples the pin within a couple of GPIO reads of releasing NSS and + * returns immediately if BUSY has not risen yet — the same "BUSY has not + * risen" race behind the stale-reply guard in the HAL and the GetTemp + * wedge. Lose it here and the caller's startReceive() issues + * SetRxDutyCycle into a chip that is still calibrating, the command is + * dropped, and the radio never re-enters Rx: deaf until the next reset + * 60 s later. Measured 2026-08-23: the 60 s window after a reset carried + * a 7.4% miss rate against 0.6% elsewhere, and one deaf stretch began at + * one reset and ended exactly at the next. + * + * A flat settle wait sidesteps the race entirely — this path is already + * a warm sleep plus a full recalibration, so it is nobody's fast path. */ + k_sleep(K_MSEC(10)); + if (data->rx_boost_enabled) { lr11xx_radio_cfg_rx_boosted(ctx, true); data->rx_boost_applied = true; } - /* Contract with agcMaintenance(): leave the driver out of RX so the - * caller's startReceive() performs a real re-entry. */ + /* Contract with the caller: leave the driver out of RX so its + * startReceive() performs a real re-entry. */ data->in_rx_mode = false; lr11xx_reset_rx_busy_signals(data); } -void lr11xx_reset_agc(const struct device *dev) +/* Redo the frequency-dependent calibrations after temperature drift. + * + * This is NOT an "AGC reset". That is an SX126x remedy for an SX126x fault — + * Semtech prescribe warm sleep plus recalibration for a jammed AGC on that part, + * and ZephCore inherited the idea from Arduino MeshCore's `agc_reset_interval`. + * Neither the LR11xx UM nor anything measured here describes such a fault on + * this generation, and running the sequence speculatively cost real packets + * (T1000-E, 2026-08-23: 7.4%% miss rate in the 60 s after a fire, against 0.6%% + * elsewhere). So the driver no longer offers one; it offers the operation the + * datasheet does call for, under the name of what it actually does. + * + * Leaves the driver out of RX — the caller must startReceive() afterwards. */ +void lr11xx_recalibrate(const struct device *dev) { struct lr11xx_data *data = dev->data; if (!data->configured) { return; } - k_mutex_lock(&data->spi_mutex, K_FOREVER); - lr11xx_agc_reset_locked(data); - k_mutex_unlock(&data->spi_mutex); -} -void lr11xx_recalibrate(const struct device *dev) -{ - /* Same sequence: calibrate(0x3F) already carries the image cal on this - * part, so there is nothing extra for the drift path to do. */ - lr11xx_reset_agc(dev); + /* Skip rather than wedge. This sequence opens with SetSleep, and a + * command issued into the chip's autonomous duty-cycle sleep phase pins + * it BUSY-high — the same failure the pollers guard against, on a path + * that had no guard at all. sx126x_reset_agc() solves it by poking NSS + * to wake the chip first; that trick is deliberately NOT copied here, + * because on this generation an NSS pulse into a chip that is merely + * mid-command is read as a malformed frame (see the LR2021 HAL's + * check_device_ready() note on the CMD_PERR rejection storm). Deferring + * is safe: temperature drift is measured in tens of minutes, so losing a + * recalibration to a busy chip costs nothing the next pass cannot fix. */ + if (gpio_pin_get_dt(&data->hal_ctx.busy)) { + LOG_DBG("recalibrate: chip busy (duty-cycle sleep), deferring"); + return; + } + + /* Bounded, for the same reason sx126x_reset_agc() bounds its own: a long + * TX or RX holds this mutex for the full airtime, and K_FOREVER would + * stall the dispatcher thread for all of it. */ + if (k_mutex_lock(&data->spi_mutex, K_MSEC(50)) != 0) { + LOG_DBG("recalibrate: mutex busy, deferring"); + return; + } + if (gpio_pin_get_dt(&data->hal_ctx.busy)) { + k_mutex_unlock(&data->spi_mutex); + return; + } + + lr11xx_recalibrate_locked(data); + k_mutex_unlock(&data->spi_mutex); } /* Junction temperature in whole degrees C, INT16_MIN if unavailable. @@ -1520,9 +1569,33 @@ int16_t lr11xx_get_chip_temp_c(const struct device *dev) if (!data->configured) { return INT16_MIN; } + + /* GPIO BUSY read FIRST, exactly as lr11xx_get_rssi_inst() does, and for + * the same reason: GetTemp (0x011A) issued into a chip parked in the + * autonomous SetRxDutyCycle sleep-with-retention phase wedges it + * BUSY-high, and the HAL's wait_on_busy() then blocks 3 s per attempt + * before the wedge watchdog eventually hardware-resets the radio — 12 s+ + * of deafness per event. This is the identical failure the 2026-07-24 + * "T1000 goes deaf" fix addressed on the RSSI poller; the temperature + * poller is called from the very same agcMaintenance() pass and was + * simply missed at the time. Observed on hardware 2026-08-23: two + * wedges in 30 minutes on a duty-cycled T1000-E, both on op=0x011a. + * + * INT16_MIN is the sentinel agcMaintenance() already treats as "no + * reading this pass" — it retries later rather than acting on it. + * Re-checked after the mutex for the same reason as the RSSI path: the + * chip can enter sleep between the two. */ + if (gpio_pin_get_dt(&data->hal_ctx.busy)) { + return INT16_MIN; + } if (k_mutex_lock(&data->spi_mutex, K_NO_WAIT) != 0) { return INT16_MIN; } + if (gpio_pin_get_dt(&data->hal_ctx.busy)) { + k_mutex_unlock(&data->spi_mutex); + return INT16_MIN; + } + rc = lr11xx_system_get_temp(&data->hal_ctx, &raw); k_mutex_unlock(&data->spi_mutex); diff --git a/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.h b/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.h index d12e123..12011bc 100644 --- a/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.h +++ b/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.h @@ -95,20 +95,15 @@ void lr11xx_reset_dc_timeout_restarts(const struct device *dev); */ uint32_t lr11xx_get_random(const struct device *dev); -/** - * @brief Unstick a jammed AGC: warm sleep, then recalibrate. - * - * Leaves the driver out of RX — the caller must startReceive() afterwards. - * - * @param dev LoRa device - */ -void lr11xx_reset_agc(const struct device *dev); - /** * @brief Redo the frequency-dependent calibrations (temperature drift path). * - * On this part calibrate(0x3F) already includes image rejection, so this is - * the same sequence as lr11xx_reset_agc(). + * Warm sleep, Calibrate(ALL) — which on this part already includes image + * rejection — then image calibration at the operating frequency and a rx-boost + * re-apply. Deliberately NOT an AGC reset: that fault, and its remedy, belong + * to the SX126x. Defers if the chip is busy (duty-cycle sleep). + * + * Leaves the driver out of RX — the caller must startReceive() afterwards. * * @param dev LoRa device */ 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 79c43cc..4273d31 100644 --- a/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.c +++ b/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.c @@ -979,8 +979,8 @@ static void lr20xx_apply_modem_config(struct lr20xx_data *data, * Calibrate blocks as LF_RC, HF_RC, PLL, AAF, MU and PA_OFF — no image, no * front end. CalibFE (§6.4.2) owns those and its values survive retention * sleep, so re-running it here would cost ~9 ms to reproduce what is already - * on the chip. The old (dead, never-called) lr20xx_reset_agc() ran it - * unconditionally, which is most of why it looked expensive. + * on the chip. An earlier draft ran CalibFE on every invocation, which is + * most of why this sequence looked expensive. * * 2. No fixed post-Calibrate delay. DS §6.4.1: "Upon completion of the * calibration, the chip automatically enters Standby RC mode" — BUSY @@ -990,7 +990,7 @@ static void lr20xx_apply_modem_config(struct lr20xx_data *data, * Budget: 1 ms for the HAL's post-sleep-opcode wait, 1 ms for the warm start * (DS Table 3-23 TSPDRCW, "retrieve calibration from retained registers"), plus * the Calibrate itself, which the datasheet does not put a number on. */ -static void lr20xx_agc_reset_locked(struct lr20xx_data *data, bool recal_fe) +static void lr20xx_recalibrate_locked(struct lr20xx_data *data) { void *ctx = &data->hal_ctx; lr20xx_system_sleep_cfg_t sleep_cfg = { @@ -1022,13 +1022,14 @@ static void lr20xx_agc_reset_locked(struct lr20xx_data *data, bool recal_fe) /* Front end only on the drift path. Image/FE calibration is valid for a * temperature range rather than forever (DS: "necessary if there is a * frequency change > 10MHz, or a temperature change > 10 C"), so it is - * exactly what a temperature swing invalidates — and exactly what a stuck - * AGC does not, which is why the AGC path skips its ~9 ms. */ - if (recal_fe && data->configured) { + * exactly what a temperature swing invalidates. (This used to be + * conditional, skipped by an "AGC reset" caller that no longer exists — + * that fault belongs to the SX126x, not to this part.) */ + if (data->configured) { lr20xx_calibrate_front_end(ctx, data->modem_cfg.frequency); } - /* Contract with LoRaRadioBase::agcMaintenance(): leave the driver OUT of + /* Contract with the caller: leave the driver OUT of * RX. The chip is parked in STDBY_RC here, and the caller's * startReceive() must perform a real re-entry — without this the * idempotent fast paths in recv_async / recv_duty_cycle would see @@ -1040,18 +1041,17 @@ static void lr20xx_agc_reset_locked(struct lr20xx_data *data, bool recal_fe) /* ── Extension API: receiver hygiene ─────────────────────────────────── */ -void lr20xx_reset_agc(const struct device *dev) -{ - struct lr20xx_data *data = dev->data; - - if (!data->configured) { - return; - } - k_mutex_lock(&data->spi_mutex, K_FOREVER); - lr20xx_agc_reset_locked(data, false); - k_mutex_unlock(&data->spi_mutex); -} - +/* Redo the frequency-dependent calibrations after temperature drift: warm + * sleep, Calibrate (LF_RC/HF_RC/PLL/AAF/MU/PA_OFF), rx-boost re-apply, then + * CalibFE at the operating frequency. + * + * Deliberately NOT an "AGC reset". That is an SX126x remedy for an SX126x + * fault; nothing in the LR2021 datasheet describes it on this part, and firing + * the sequence speculatively costs packets — measured on the LR1110 sibling as + * a 7.4%% miss rate in the 60 s afterwards against 0.6%% elsewhere. The driver + * therefore exposes only the operation the datasheet does call for. + * + * Leaves the driver out of RX — the caller must startReceive() afterwards. */ void lr20xx_recalibrate(const struct device *dev) { struct lr20xx_data *data = dev->data; @@ -1059,8 +1059,29 @@ void lr20xx_recalibrate(const struct device *dev) if (!data->configured) { return; } - k_mutex_lock(&data->spi_mutex, K_FOREVER); - lr20xx_agc_reset_locked(data, true); + + /* Skip rather than wedge — same reasoning as lr11xx_recalibrate(). This + * path had no busy guard while the two incidental pollers beside it + * (lr20xx_is_receiving, lr20xx_get_rssi_inst) both have one, and it is + * the heavier operation of the three. Waking the chip instead of + * deferring is specifically wrong on this part: check_device_ready() + * documents the NSS-pulse-into-a-busy-chip rejection storm that cost the + * MeshTracker X1 its duty cycle. */ + if (lr20xx_is_chip_busy(dev)) { + LOG_DBG("recalibrate: chip busy (duty-cycle sleep), deferring"); + return; + } + + if (k_mutex_lock(&data->spi_mutex, K_MSEC(50)) != 0) { + LOG_DBG("recalibrate: mutex busy, deferring"); + return; + } + if (lr20xx_is_chip_busy(dev)) { + k_mutex_unlock(&data->spi_mutex); + return; + } + + lr20xx_recalibrate_locked(data); k_mutex_unlock(&data->spi_mutex); } 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 4596b71..8aa36a1 100644 --- a/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.h +++ b/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.h @@ -169,20 +169,17 @@ int lr20xx_configure_side_detectors(const struct device *dev, uint32_t lr20xx_get_random(const struct device *dev); /** - * @brief Unstick a jammed AGC: warm sleep, then recalibrate (no front end). + * @brief Redo the frequency-dependent calibrations (temperature drift path). + * + * Warm sleep, Calibrate, rx-boost re-apply, then CalibFE at the operating + * frequency — the front end being what a temperature swing actually + * invalidates. Deliberately not an AGC reset: that fault, and its remedy, + * belong to the SX126x. Defers if the chip is busy (duty-cycle sleep). * * Leaves the driver out of RX — the caller must startReceive() afterwards. * * @param dev LoRa device */ -void lr20xx_reset_agc(const struct device *dev); - -/** - * @brief As lr20xx_reset_agc(), plus a front-end calibration at the operating - * frequency. For temperature drift, not for the packet path. - * - * @param dev LoRa device - */ void lr20xx_recalibrate(const struct device *dev); /** diff --git a/zephcore/src/Dispatcher.cpp b/zephcore/src/Dispatcher.cpp index 1f16186..56b64a9 100644 --- a/zephcore/src/Dispatcher.cpp +++ b/zephcore/src/Dispatcher.cpp @@ -184,7 +184,7 @@ void Dispatcher::maintenanceLoop() /* Receiver hygiene: deaf-aware AGC unstick + temperature-drift * recalibration. Both sleep the chip, so they live here rather than on * any packet path. */ - _radio->agcMaintenance(); + _radio->radioMaintenance(); int8_t cad_off = _radio->getCadOffset();