diff --git a/.gitignore b/.gitignore index 94c5401..c10e2d0 100644 --- a/.gitignore +++ b/.gitignore @@ -159,3 +159,4 @@ HANDOVER_lr2021_audit.md # does not apply to paths already in the index (three of these went into # b839c2c for exactly that reason; untrack with `git rm -r --cached`). lr2021_bench/ +LR2021_MATH_AUDIT.md diff --git a/zephcore/adapters/radio/LoRaRadioBase.cpp b/zephcore/adapters/radio/LoRaRadioBase.cpp index 2fb7329..cda8d17 100644 --- a/zephcore/adapters/radio/LoRaRadioBase.cpp +++ b/zephcore/adapters/radio/LoRaRadioBase.cpp @@ -65,7 +65,7 @@ LoRaRadioBase::LoRaRadioBase(const struct device *lora_dev, MainBoard &board, _override_sf(0), _override_cr(0), _rx_cb(nullptr), _rx_cb_user_data(nullptr), _tx_done_cb(nullptr), _tx_done_cb_user_data(nullptr), - _tx_thread_running(false), + _tx_thread_running(false), _tx_len(0), _packets_recv(0), _packets_sent(0), _packets_recv_errors(0) { k_poll_signal_init(&_tx_signal); @@ -76,6 +76,42 @@ LoRaRadioBase::LoRaRadioBase(const struct device *lora_dev, MainBoard &board, /* ── TX wait thread ──────────────────────────────────────────── */ +/* How long to wait for TX_DONE before declaring the transmit lost. + * + * TX_TIMEOUT_MS alone is a fixed 5 s, which is shorter than the airtime of a + * great many legal presets — a 255-byte packet is 28.6 s at SF12/BW62.5 and + * 10.2 s at SF9/BW31.25 — so the wait would expire mid-transmission and + * startReceive() would yank the radio out of TX, losing a packet that was + * transmitting perfectly well. Scale from the driver's own airtime instead, + * with the same doubling the drivers' internal sync-send waits use. + * + * MAX(), never a bare replacement: on fast presets the scaled value is smaller + * than 5 s (SF7/BW62.5, 64 bytes: ~1.4 s), and shortening this deadline on the + * presets every radio in the fleet is running today would be a regression for + * no gain. The floor keeps existing behaviour exactly; only slow presets move. + * + * lora_airtime() is a pure calculation on the cached modem config, so calling + * it from this thread costs no SPI and cannot race the radio. */ +uint32_t LoRaRadioBase::txWaitBudgetMs() const +{ + uint32_t air = lora_airtime(_dev, _tx_len ? _tx_len : 255); + + /* All four drivers behind this class implement .airtime (native sx126x, + * lr11xx, lr20xx, loramac-node sx127x), and lora_airtime() dereferences + * the op without a NULL check, so there is no missing-op case to handle. + * A zero can still come back from a degenerate modem config; fall back + * to the flat budget rather than to no wait at all. */ + if (air == 0) { + return TX_TIMEOUT_MS; + } + /* Cap the doubling before adding, so a pathological airtime cannot wrap + * the 32-bit budget on its way into K_MSEC(). */ + if (air > (UINT32_MAX - 1000U) / 2U) { + return UINT32_MAX - 1000U; + } + return MAX(TX_TIMEOUT_MS, 2U * air + 1000U); +} + void LoRaRadioBase::txWaitThreadFn(void *p1, void *p2, void *p3) { LoRaRadioBase *self = static_cast(p1); @@ -103,21 +139,43 @@ void LoRaRadioBase::txWaitThreadFn(void *p1, void *p2, void *p3) int result; k_poll_signal_check(&self->_tx_signal, &signaled, &result); if (signaled) { - LOG_DBG("TX wait: signal already raised (result=%d)", result); + /* The result carries the driver's verdict, and it has to + * be honoured: a driver that reports a failed transmit + * by raising the signal with a negative result (the + * SX126x does exactly this on a chip Tx timeout, + * -ETIMEDOUT) was previously counted here as a + * successful send. Latent rather than live — that + * SX126x path is unreachable while an RX callback is + * registered, which it always is — but it is the reason + * the LR11xx and LR20xx timeout handlers deliberately do + * NOT raise the signal. With the result honoured, a + * driver reporting failure is now the correct thing to + * do on all three. */ + if (result < 0) { + LOG_ERR("TX wait: driver reported failure (%d) — packet lost", + result); + } else { + LOG_DBG("TX wait: signal already raised (result=%d)", result); + } k_poll_signal_reset(&self->_tx_signal); self->_board->onAfterTransmit(); self->startReceive(); atomic_set(&self->_tx_active, 0); - atomic_inc(&self->_packets_sent); + if (result >= 0) { + atomic_inc(&self->_packets_sent); + } if (self->_tx_done_cb) { self->_tx_done_cb(self->_tx_done_cb_user_data); } continue; } - int ret = k_poll(events, 1, K_MSEC(TX_TIMEOUT_MS)); + uint32_t budget_ms = self->txWaitBudgetMs(); + + int ret = k_poll(events, 1, K_MSEC(budget_ms)); if (ret == -EAGAIN) { - LOG_ERR("TX wait: TIMEOUT!"); + LOG_ERR("TX wait: TIMEOUT after %u ms (len=%u) — packet lost", + budget_ms, (unsigned)self->_tx_len); self->_board->onAfterTransmit(); self->startReceive(); atomic_set(&self->_tx_active, 0); @@ -128,12 +186,25 @@ void LoRaRadioBase::txWaitThreadFn(void *p1, void *p2, void *p3) } if (ret == 0 && events[0].state == K_POLL_STATE_SIGNALED) { + /* Same rule as the already-raised path above: a negative + * result is the driver reporting a lost transmit, not a + * completed one. */ + int sig_result = 0; + unsigned int sig_state = 0; + + k_poll_signal_check(&self->_tx_signal, &sig_state, + &sig_result); k_poll_signal_reset(&self->_tx_signal); self->_board->onAfterTransmit(); self->startReceive(); atomic_set(&self->_tx_active, 0); - atomic_inc(&self->_packets_sent); - LOG_INF("TX complete, RX restarted"); + if (sig_result < 0) { + LOG_ERR("TX failed: driver reported %d — packet lost", + sig_result); + } else { + atomic_inc(&self->_packets_sent); + LOG_INF("TX complete, RX restarted"); + } if (self->_tx_done_cb) { self->_tx_done_cb(self->_tx_done_cb_user_data); @@ -693,6 +764,9 @@ bool LoRaRadioBase::startSendRaw(const uint8_t *bytes, int len) configureTx(); memcpy(_tx_buf, bytes, len); + /* Published before the _tx_start_sem handoff below so txWaitBudgetMs() + * sizes the wait for this packet, not the previous one. */ + _tx_len = (uint16_t)len; k_poll_signal_reset(&_tx_signal); int ret = hwSendAsync(_tx_buf, (uint32_t)len, &_tx_signal); diff --git a/zephcore/adapters/radio/LoRaRadioBase.h b/zephcore/adapters/radio/LoRaRadioBase.h index 8819ef7..74afceb 100644 --- a/zephcore/adapters/radio/LoRaRadioBase.h +++ b/zephcore/adapters/radio/LoRaRadioBase.h @@ -329,9 +329,15 @@ private: /* TX completion thread */ static void txWaitThreadFn(void *p1, void *p2, void *p3); + uint32_t txWaitBudgetMs() const; struct k_thread _tx_wait_thread; struct k_sem _tx_start_sem; bool _tx_thread_running; + /* Length of the transmit in flight, for txWaitBudgetMs(). Written by + * startSendRaw() before it releases _tx_start_sem, read by the wait + * thread after it takes that sem — the sem is the handoff, so no + * additional synchronisation is needed. */ + uint16_t _tx_len; /* Packet statistics */ atomic_t _packets_recv; 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 233b62f..d779ad2 100644 --- a/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.c +++ b/zephcore/patches/zephyr-new/drivers/lora/lr11xx/lr11xx_lora.c @@ -104,7 +104,14 @@ struct lr11xx_data { void *cad_user_data; struct k_sem cad_sem; int cad_result; - bool cad_active; + /* No cad_active flag: there was one, written at four sites and read at + * none. The SX126x driver's copy IS read (sx126x_handle_irq_timeout() + * returns early on it), so this one was inherited without the read that + * gave it a purpose. Not needed here: the TIMEOUT branch below is only + * reachable during a CAD when the chip took the CAD->TX exit, and that + * path sets tx_active first. Exclusion between CADs comes from both + * entry points (LBT from startSendRaw, probes from cadMaintenance) + * running on the mesh loop thread. */ /* Adaptive-CAD: signed offset applied to the per-SF base detPeak on * every LBT CAD; cad_probe_peak overrides for one calibration probe. */ int8_t cad_peak_offset; @@ -596,8 +603,6 @@ static void lr11xx_dio1_work_handler(struct k_work *work) if (irq & LR11XX_SYSTEM_IRQ_CAD_DONE) { bool detected = (irq & LR11XX_SYSTEM_IRQ_CAD_DETECTED) != 0; - data->cad_active = false; - if (data->cad_cb) { lora_cad_cb cb = data->cad_cb; void *ud = data->cad_user_data; @@ -630,7 +635,23 @@ static void lr11xx_dio1_work_handler(struct k_work *work) /* ── Timeout ── */ if (irq & LR11XX_SYSTEM_IRQ_TIMEOUT) { - if (!data->tx_active) { + if (data->tx_active) { + /* The chip's Tx timeout fired, so the transmission was + * stopped and TX_DONE will never arrive. This branch + * used to do nothing at all here: no log, no recovery, + * no signal — the radio simply sat in the post-TX state + * until the host wait expired. Mirror the TX_DONE path + * so the receiver goes back on air, and say what + * happened. tx_signal is deliberately NOT raised: the + * C++ wait thread treats a raised signal as a completed + * send, so raising it here would book a packet that + * never left. Its own timeout owns the accounting. */ + LOG_ERR("TX timeout — chip stopped the transmission, " + "packet lost"); + data->tx_active = false; + lr11xx_start_rx(data, cfg); + rx_restarted = true; + } else { lr11xx_restart_rx(data); rx_restarted = true; } @@ -1114,10 +1135,10 @@ static uint32_t lr11xx_max_payload_ms(struct lr11xx_data *data) /* Semtech payload-symbol count with PL=255, CRC on, explicit header, * CR = 4/8 (coded_bits = 8), DE = 1: - * n = 8 + ceil((8*PL - 4*SF + 28 + 16) / (4*(SF - DE))) * 8 - * SF5/SF6 use SF-1 >= 4 so the divisor is always non-zero. */ + * n = 8 + ceil((8*PL - 4*SF + 28 + 16) / (4*(SF - 2*DE))) * 8 + * DE=1 so the divisor is 4*(SF-2); at SF5 that is 12, never zero. */ uint32_t numer = 8U * 255U + 28U + 16U; - uint32_t denom = 4U * (uint32_t)(sf - 1U); + uint32_t denom = 4U * (uint32_t)(sf - 2U); if (numer > 4U * (uint32_t)sf) { numer -= 4U * (uint32_t)sf; @@ -1376,7 +1397,6 @@ static int lr11xx_do_cad(struct lr11xx_data *data) lr11xx_system_clear_irq_status(ctx, LR11XX_SYSTEM_IRQ_ALL_MASK); lr11xx_reset_rx_busy_signals(data); - data->cad_active = true; lr11xx_radio_set_cad(ctx); return 0; @@ -1414,7 +1434,6 @@ static int lr11xx_lora_cad(const struct device *dev, k_timeout_t timeout) ret = k_sem_take(&data->cad_sem, timeout); if (ret == -EAGAIN) { - data->cad_active = false; return -ETIMEDOUT; } @@ -1429,7 +1448,6 @@ static int lr11xx_lora_cad_async(const struct device *dev, if (cb == NULL) { data->cad_cb = NULL; data->cad_user_data = NULL; - data->cad_active = false; return 0; } 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 dc3957d..82e1c62 100644 --- a/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.c +++ b/zephcore/patches/zephyr-new/drivers/lora/lr20xx/lr20xx_lora.c @@ -107,7 +107,12 @@ struct lr20xx_data { void *cad_user_data; struct k_sem cad_sem; int cad_result; /* 0=free, 1=busy, <0=error */ - bool cad_active; + /* No cad_active flag: there was one, written at five sites and read at + * none. It read like a concurrent-CAD guard and guarded nothing — the + * real exclusion is that every CAD entry point (LBT from startSendRaw, + * probes from cadMaintenance) runs on the mesh loop thread, so two CADs + * cannot overlap by construction. A flag nothing tests is worse than no + * flag: it invites the next reader to assume the invariant is enforced. */ /* Adaptive-CAD: signed offset applied to the per-SF base detPeak on * every LBT CAD; cad_probe_peak overrides for one calibration probe. */ int8_t cad_peak_offset; @@ -658,13 +663,40 @@ static lr20xx_status_t lr20xx_calibrate_front_end(void *ctx, uint32_t freq_hz) for (int i = 0; i < LR20XX_MAX_CAL_ATTEMPTS; i++) { lr20xx_system_errors_t errs = 0; + lr20xx_system_stat1_t s1 = { 0 }; + bool rejected = false; lr20xx_status_t rc; lr20xx_system_clear_errors(ctx); rc = lr20xx_radio_common_calibrate_front_end_helper(ctx, fe_cal, 3); + + /* The SDK return code reflects the SPI write, not whether the chip + * executed the command. A CalibFE issued from Rx or Tx answers + * CMD_FAIL (DS §6.4.2) while rc stays OK and no error bit is set, + * so without this check the caller is told the front end was + * calibrated when it was not — and the first symptom is degraded + * RX with a RxFreqNoCalErr nobody connects to this call. Must be + * read BEFORE get_errors(): Stat reports the previous command. + * DS Table 6-38: 0x2 CMD_OK, 0x3 CMD_DAT, anything else rejected. */ + if (lr20xx_system_get_status(ctx, &s1, NULL, NULL) == + LR20XX_STATUS_OK) { + rejected = (s1.command_status != 2 && + s1.command_status != 3); + } + lr20xx_system_get_errors(ctx, &errs); + if (rejected) { + /* Not retried: the chip refused on mode grounds, and + * repeating the command from the same mode cannot help. */ + LOG_ERR("FE cal(%u Hz) REJECTED (cmd_status=%d) — CalibFE " + "needs STDBY_RC/XOSC/FS, not Rx or Tx (DS §6.4.2); " + "front end left uncalibrated", + freq_hz, s1.command_status); + return LR20XX_STATUS_ERROR; + } + if (rc == LR20XX_STATUS_OK && !(errs & LR20XX_SYSTEM_ERRORS_SRC_SATURATION_CALIB_MASK)) { /* Report the grid points actually programmed, not just @@ -1390,7 +1422,6 @@ static void lr20xx_dio1_work_handler(struct k_work *work) } LOG_DBG("CAD done: %s", detected ? "activity" : "free"); - data->cad_active = false; if (data->cad_cb) { lora_cad_cb cb = data->cad_cb; @@ -1423,8 +1454,27 @@ static void lr20xx_dio1_work_handler(struct k_work *work) /* ── Timeout ── */ if (irq & LR20XX_SYSTEM_IRQ_TIMEOUT) { - LOG_DBG("Timeout IRQ — restarting RX"); - if (!data->tx_active) { + if (data->tx_active) { + /* The chip's Tx safeguard fired. DS §6.3.6: "the + * transmission is stopped prematurely" — the packet is + * gone and TX_DONE will never arrive. Leaving tx_active + * set here (as this branch used to) made the whole event + * invisible: no branch ran, nothing was logged above DBG, + * and the radio sat in the post-TX state until the host's + * own wait expired. Say it and put the receiver back on + * air, same as the TX_DONE path. + * + * tx_signal is deliberately NOT raised: the C++ wait + * thread counts a raised signal as a completed send and + * would increment packets_sent for a packet that never + * left. Its own timeout owns the accounting. */ + LOG_ERR("TX timeout — chip stopped the transmission, " + "packet lost"); + data->tx_active = false; + lr20xx_start_rx(data, cfg); + rx_restarted = true; + } else { + LOG_DBG("Timeout IRQ — restarting RX"); lr20xx_restart_rx(data); rx_restarted = true; } @@ -1549,6 +1599,17 @@ static int lr20xx_lora_config(const struct device *dev, * ral_lr20xx_init() calibrates at init and never on the Tx/Rx path. */ k_mutex_lock(&data->spi_mutex, K_FOREVER); + /* Standby first — CalibFE "does not work if device is in Rx or Tx mode" + * (DS §6.4.2). On the reconfigure path the chip IS still in Rx: the C++ + * layer's hwCancelReceive() lowers lora_recv_async(NULL)'s bookkeeping + * without commanding the chip, and continuous RX only ends when the host + * changes the mode (DS §6.3.5). Without this the calibration is refused + * for every `set freq/sf/bw/cr`, tempradio set and tempradio clear, and + * refused invisibly — see the note in lr20xx_calibrate_front_end(). + * Free at boot, where hw_init() has already left the chip in STDBY_RC. */ + lr20xx_system_set_standby_mode(&data->hal_ctx, + LR20XX_SYSTEM_STANDBY_MODE_RC); + lr20xx_calibrate_front_end(&data->hal_ctx, config->frequency); DUMP_CHIP_STATE(data, "config-FEcal"); @@ -1631,6 +1692,16 @@ static uint32_t lr20xx_cad_timeout_ms(struct lr20xx_data *data) #define LR20XX_CAD_LBT_MAX_TX_TIMEOUT_STEPS 0x00FFFFFFU #define LR20XX_CAD_LBT_STEPS_PER_MS 32000U +/* The OTHER time base on this chip. SetTx/SetRx/SetRxDutyCycle timeouts are + * counted in 32.768 kHz RTC periods (DS §6.3.17: "expressed in periods of the + * 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 + static int lr20xx_do_cad(struct lr20xx_data *data); static int lr20xx_lora_send_cad_lbt(const struct device *dev, @@ -1705,7 +1776,6 @@ static int lr20xx_lora_send_cad_lbt(const struct device *dev, K_MSEC(lr20xx_cad_timeout_ms(data) + tx_timeout_ms)); if (ret == -EAGAIN) { LOG_WRN("CAD_LBT: no CAD_DONE within budget — falling back"); - data->cad_active = false; ret = -EIO; goto abort; } @@ -1785,8 +1855,19 @@ static int lr20xx_lora_send_async(const struct device *dev, * to re-arm. */ if (data->modem_cfg.cad.mode == LORA_CAD_MODE_LBT) { bool was_in_rx = data->in_rx_mode; - int cad_ret = lr20xx_lora_cad(dev, - K_MSEC(lr20xx_cad_timeout_ms(data))); + int cad_ret; + + /* Discard any stamp left by a CAD that was not this transmit's. + * The CAD_DONE handler stamps cad_done_cycles on every free CAD + * — adaptive-CAD probes every ~15 s, and CAD_LBT, neither of + * which consumes it — but the only reader is the latency log + * below. Without this the reported CAD->TX gap can be measured + * from a probe that ran seconds earlier, which is worse than not + * reporting it: it is a plausible number that is simply wrong. */ + data->cad_done_cycles = 0U; + + cad_ret = lr20xx_lora_cad(dev, + K_MSEC(lr20xx_cad_timeout_ms(data))); if (cad_ret > 0) { LOG_DBG("LBT: channel busy"); /* Re-arm whenever there was anything to re-arm. The @@ -1829,7 +1910,6 @@ static int lr20xx_lora_send_async(const struct device *dev, lr20xx_hardware_reset(data, cfg); } - /* Clear errors before modem config */ lr20xx_system_clear_errors(ctx); lr20xx_system_clear_irq_status(ctx, LR20XX_SYSTEM_IRQ_ALL_MASK); @@ -1875,7 +1955,38 @@ static int lr20xx_lora_send_async(const struct device *dev, data->cad_done_cycles = 0U; } - lr20xx_radio_common_set_tx(ctx, 5000); + /* Chip-side Tx safeguard — DS §6.3.6: "Tx timeout can be used as a + * safeguard in case transmission fails and TxDone interrupt never + * occurs", and when it fires "the transmission is stopped prematurely". + * So it has to exceed real airtime, and a fixed budget cannot: at + * SF12/BW62.5 a 31-byte packet already needs more than 5 s, and at + * SF12/BW31.25 so does a 6-byte one. Scaled from the same airtime the + * CAD_LBT path uses, floored at the previous 5000 ms so no preset that + * works today gets a tighter deadline. + * + * Programmed in RTC steps rather than through the SDK's millisecond + * wrapper: that wrapper computes `ms * 32768` in uint32 and overflows + * above 131 071 ms, which is reachable here (SF12/BW7.81 at 255 bytes is + * ~229 s of airtime). Saturating at the 24-bit field maximum is 512 s, + * comfortably past any airtime this chip can produce. */ + { + uint32_t tx_air_ms = lr20xx_lora_airtime(dev, data_len); + uint32_t tx_tmo_ms = tx_air_ms + (tx_air_ms / 4U) + 500U; + uint64_t tx_tmo_steps; + + if (tx_tmo_ms < LR20XX_TX_TIMEOUT_FLOOR_MS) { + tx_tmo_ms = LR20XX_TX_TIMEOUT_FLOOR_MS; + } + tx_tmo_steps = ((uint64_t)tx_tmo_ms * LR20XX_RTC_FREQ_HZ) / 1000U; + if (tx_tmo_steps > LR20XX_RTC_STEP_MAX) { + tx_tmo_steps = LR20XX_RTC_STEP_MAX; + } + + LOG_DBG("SET_TX: airtime=%u ms, timeout=%u ms (%u steps)", + tx_air_ms, tx_tmo_ms, (uint32_t)tx_tmo_steps); + lr20xx_radio_common_set_tx_with_timeout_in_rtc_step( + ctx, (uint32_t)tx_tmo_steps); + } /* Command status here is SET_TX's own (2=accepted, 1=rejected, * 0=not executed) and the mode should have left standby. */ @@ -2022,10 +2133,17 @@ static uint32_t lr20xx_max_payload_ms(struct lr20xx_data *data) /* Semtech payload-symbol count with PL=255, CRC on, explicit header, * CR = 4/8 (coded_bits = 8), DE = 1: - * n = 8 + ceil((8*PL - 4*SF + 28 + 16) / (4*(SF - DE))) * 8 - * SF5/SF6 use SF-1 >= 4 so the divisor is always non-zero. */ + * n = 8 + ceil((8*PL - 4*SF + 28 + 16) / (4*(SF - 2*DE))) * 8 + * The divisor is 4*(SF - 2*DE), not 4*(SF - DE) — same form + * lr20xx_lora_airtime() uses. With DE hard-coded to 1 that is 4*(SF-2), + * which at SF5 is 12, so the divisor is always non-zero. + * + * DE = 1 unconditionally because this is a ceiling, and LDRO on yields + * the larger symbol count: at SF12/BW62.5 it is 416 symbols against 384 + * with the wrong divisor, i.e. 27.3 s of real payload airtime that the + * bound has to clear. */ uint32_t numer = 8U * 255U + 28U + 16U; - uint32_t denom = 4U * (uint32_t)(sf - 1U); + uint32_t denom = 4U * (uint32_t)(sf - 2U); if (numer > 4U * (uint32_t)sf) { numer -= 4U * (uint32_t)sf; @@ -2468,7 +2586,6 @@ static int lr20xx_do_cad(struct lr20xx_data *data) /* Clear any pending IRQ flags, then start CAD */ lr20xx_system_clear_irq_status(ctx, LR20XX_SYSTEM_IRQ_ALL_MASK); lr20xx_reset_rx_busy_signals(data); - data->cad_active = true; lr20xx_radio_lora_set_cad(ctx); CHECK_CMD(ctx, "set_cad"); @@ -2481,6 +2598,14 @@ static int lr20xx_do_cad(struct lr20xx_data *data) return 0; } +/* Blocking CAD. Leaves the chip in STANDBY on every exit — success, busy, + * timeout and error alike — and deliberately does NOT restore RX: the caller + * knows whether it is about to transmit (in which case re-entering RX would be + * undone immediately) or must go back on air. Both current callers honour + * that: lr20xx_lora_send_async()'s LBT branch re-arms on the busy path, and + * LoRaRadioBase::cadMaintenance() calls startReceive() after every probe. A + * third caller that forgets leaves the node deaf with nothing scheduled to + * recover it, so the contract is stated here rather than left to be inferred. */ static int lr20xx_lora_cad(const struct device *dev, k_timeout_t timeout) { struct lr20xx_data *data = dev->data; @@ -2515,7 +2640,6 @@ static int lr20xx_lora_cad(const struct device *dev, k_timeout_t timeout) /* Wait for DIO1 handler to signal CAD_DONE */ ret = k_sem_take(&data->cad_sem, timeout); if (ret == -EAGAIN) { - data->cad_active = false; return -ETIMEDOUT; } @@ -2528,10 +2652,16 @@ static int lr20xx_lora_cad_async(const struct device *dev, struct lr20xx_data *data = dev->data; if (cb == NULL) { - /* Cancel pending CAD */ + /* Cancel pending CAD. Under spi_mutex like every other write to + * the CAD state: the DIO1 handler reads cad_cb to decide whether + * to dispatch a callback, and this used to race it. Dead today + * (nothing in the tree calls lora_cad_async), which is exactly + * why it is worth fixing now rather than when someone wires it + * up and inherits an invisible race. */ + k_mutex_lock(&data->spi_mutex, K_FOREVER); data->cad_cb = NULL; data->cad_user_data = NULL; - data->cad_active = false; + k_mutex_unlock(&data->spi_mutex); return 0; } diff --git a/zephcore/patches/zephyr/0013-lora-sx126x-rx-latch-deadline.patch b/zephcore/patches/zephyr/0013-lora-sx126x-rx-latch-deadline.patch index 1deccf4..544f8e2 100644 --- a/zephcore/patches/zephyr/0013-lora-sx126x-rx-latch-deadline.patch +++ b/zephcore/patches/zephyr/0013-lora-sx126x-rx-latch-deadline.patch @@ -83,10 +83,10 @@ index 822d0f5..00faca6 100644 + + /* Semtech payload-symbol count with PL=255, CRC on, explicit header, + * CR = 4/8 (coded_bits = 8), DE = 1: -+ * n = 8 + ceil((8*PL - 4*SF + 28 + 16) / (4*(SF - DE))) * 8 -+ * SF5/SF6 use SF-1 >= 4 so the divisor is always non-zero. */ ++ * n = 8 + ceil((8*PL - 4*SF + 28 + 16) / (4*(SF - 2*DE))) * 8 ++ * DE=1 so the divisor is 4*(SF-2); at SF5 that is 12, never zero. */ + uint32_t numer = 8U * 255U + 28U + 16U; -+ uint32_t denom = 4U * (uint32_t)(sf - 1U); ++ uint32_t denom = 4U * (uint32_t)(sf - 2U); + + if (numer > 4U * (uint32_t)sf) { + numer -= 4U * (uint32_t)sf;