upstream sweep

This commit is contained in:
liquidraver
2026-08-23 15:56:08 +02:00
parent 175b4bb6e5
commit d6a78e5d92
4 changed files with 248 additions and 68 deletions
+3
View File
@@ -479,6 +479,7 @@ The custom `lr11xx_lora.c` driver handles several LR1110 firmware bugs:
- **Header error**: Can shift buffer pointer → standby before RX restart
- **DIO1 stuck HIGH**: 5-cycle detection → full hardware reset + recovery
- **BUSY-high wedge**: a command racing the autonomous `SetRxDutyCycle` sleep phase can leave the chip BUSY-high with DIO1 low. No IRQ ever fires, so the event-driven driver never re-arms and the node goes permanently deaf. A wedge-recovery watchdog on its own work queue (`lr11xx_wedge`, `K_PRIO_COOP(7)`) checks every 3 s: after 12 s of DIO1 silence it polls the BUSY GPIO continuously for 250 ms, and a dwell with no low edge means a genuine wedge → hardware reset + RX restart. False-positive free by construction — a healthy chip, continuous or duty-cycled, always drops BUSY low within one cycle. Reads the GPIO only (no SPI), so it cannot itself disturb the chip or race the autonomous DC state machine.
- **Stale SPI reply read as data**: a read is two NSS windows (command, then answer) with a BUSY wait between them. `wait_on_busy()` returns immediately on a BUSY that reads low and cannot distinguish "command finished" from "BUSY has not risen yet", so the answer window can clock out the chip's default status / IRQ stream instead of the payload — silently, since the caller parses IRQ bits as a plausible short integer. `lr11xx_hal_read()` therefore checks the stat1 command-status byte it used to discard and re-issues the command (3 attempts) unless it reports `CMD_DATA`. Same guard in the LR2021 HAL; upstream MeshCore hit this as [PR #3261](https://github.com/meshcore-dev/MeshCore/pull/3261).
- **RX duty cycle**: wired via `SetRxDutyCycle` MODE_RX, sized by the shared adapter math (same as SX126x). The earlier "broken, 23-40% loss" verdict was a window-sizing bug (over-sleep + no header budget), not a chip defect — default-off, HW-verify before production use.
### 5.5 SX127x and LR2021 Paths
@@ -490,6 +491,7 @@ The custom `lr11xx_lora.c` driver handles several LR1110 firmware bugs:
- **Front-end calibration is a point calibration, not a band.** `CalibFE` takes up to three individual frequencies (4 MHz steps, bit 15 = LF/HF), unlike the SX126x/LR11xx `CalibrateImage` freq1/freq2 band with datasheet-prescribed edges. It is issued only at config, after a hardware reset, and on AGC reset — never on the Tx/Rx path (DS §6.4.2 keeps the values on chip across retention sleep). Both 4 MHz neighbours of the operating frequency are calibrated, nearest first, because the SDK rounds the argument up where the chip's own default truncates down.
- **Side detectors** (multi-SF receive) are LR2021-only; see `lr20xx_configure_side_detectors()`. Mutually exclusive with CAD, whose SF ordering constraint is the inverse.
- **Per-packet frequency error** is decoded and accumulated (`get freqerr`) — diagnostic only, nothing acts on it.
- **Reads are status-checked.** The two-window read (command, BUSY wait, answer) can clock its second window before BUSY rises, in which case the chip streams status / IRQ instead of the payload — `GetRxPacketLength` then returns `irq[31:16]`, exactly 4 with `RX_DONE` set, and a real frame is read out of the FIFO at the wrong length. `lr20xx_spi_read_frame()` accepts an answer only when the stat1 header reports `CMD_DATA`, re-issuing the command otherwise (3 attempts). Safe to retry because the Rx FIFO pop is not on this path (`lr20xx_hal_direct_read_fifo()`, single window, structurally immune).
#### 5.5.1 LR2021 driver design notes
@@ -722,6 +724,7 @@ Full command reference with constraints and remote-admin restrictions: `Repeater
- `CONFIG_BT_PRIVACY` **disabled** on nRF52840 / MG24: identity address is advertised directly; both iOS and Android work without RPA. Android's Flutter BLE plugin fails `connectGatt()` to RPA-advertised devices from app context.
- `CONFIG_BT_PRIVACY` **enabled** on ESP32-S3 (`boards/common/esp32_common.conf`): the Espressif controller's privacy-OFF Secure-Connections path produces a MIC failure against iOS (HCI disconnect `0x3d` at encryption start). Privacy ON keeps the controller on its working SC path. `USE_IDENTITY` advertising preserves Android compatibility. Do **not** remove `USE_IDENTITY` while ESP32 privacy is on.
- Pairing triggered reactively: phone hits ATT error 0x05 on secured attribute → initiates SMP pairing (Apple Accessory Design Guidelines §55 compliant — no proactive Security Request)
- **Unpaired-connection timeout (15 s)**: a connection that never reaches security L2 is disconnected. With `CONFIG_BT_MAX_CONN=1` the stack stops advertising while the slot is held, and the companion advertising watchdog skips any state where a connection exists — so a client that connects and never pairs (a scanner app left open, iOS routinely) otherwise makes the node invisible until it is power cycled. Every characteristic on both services is `*_AUTHEN`, so an unsecured connection can do nothing and the drop costs a legitimate client nothing. Armed in `connected()`, cancelled by `security_changed()` at L2+ and by `disconnected()`; the expiry handler reaches the connection via `bt_conn_foreach()` rather than `current_conn`, which belongs to the Bluetooth callback thread.
- TX congestion control: queue (12 frames) + overflow buffer + retry + timeout watchdog
- Fast/slow advertising switching with post-disconnect flap prevention
- DLE (Data Length Extension) to 251 bytes
+61
View File
@@ -603,6 +603,55 @@ K_WORK_DELAYABLE_DEFINE(tx_drain_work, tx_drain_work_fn);
K_WORK_DELAYABLE_DEFINE(overflow_retry_work, overflow_retry_work_fn);
K_WORK_DELAYABLE_DEFINE(adv_slow_work, adv_slow_work_fn);
/* ========== Unpaired-connection timeout ==========
*
* A connection that never reaches L2 holds the node's only peripheral slot.
* With CONFIG_BT_MAX_CONN=1 Zephyr stops advertising while that slot is taken,
* and the companion's advertising watchdog (main_companion.cpp) deliberately
* skips any state where a connection exists so a client that connects and
* never pairs makes the node invisible to everyone else until it is power
* cycled. A BLE scanner app left connected does it by accident; iOS does it
* routinely. Nothing else times the connection out: pairing here is reactive
* by design (Apple §55 we never send a Security Request, we wait for the
* phone to hit ATT insufficient-authentication and start pairing itself), so
* "connected but idle forever" is a state the node otherwise accepts happily.
*
* Dropping it costs a legitimate client nothing: every characteristic on both
* services is *_AUTHEN, so an unsecured connection cannot read, write or
* subscribe to anything. The window has to cover discovery plus the phone's
* own pairing dialog 15 s matches upstream MeshCore PR #3263, which measured
* a real unpaired connection being dropped at ~13 s. */
#define BLE_SECURITY_TIMEOUT_MS 15000
static void sec_timeout_conn_cb(struct bt_conn *conn, void *user_data)
{
ARG_UNUSED(user_data);
if (bt_conn_get_security(conn) >= BT_SECURITY_L2) {
return;
}
char addr[BT_ADDR_LE_STR_LEN];
bt_addr_le_to_str(bt_conn_get_dst(conn), addr, sizeof(addr));
LOG_WRN("security timeout: %s unpaired after %d ms, disconnecting",
addr, BLE_SECURITY_TIMEOUT_MS);
/* recycled() restarts advertising once the stack releases the slot. */
bt_conn_disconnect(conn, BT_HCI_ERR_REMOTE_USER_TERM_CONN);
}
/* Runs on the system work queue; current_conn belongs to the Bluetooth
* callback thread. The connection is therefore reached through
* bt_conn_foreach(), which hands the callback a reference held for its
* duration, rather than by dereferencing current_conn across threads. */
static void sec_timeout_work_fn(struct k_work *work)
{
ARG_UNUSED(work);
bt_conn_foreach(BT_CONN_TYPE_LE, sec_timeout_conn_cb, NULL);
}
K_WORK_DELAYABLE_DEFINE(sec_timeout_work, sec_timeout_work_fn);
/* ========== TX completion callback ========== */
/*
@@ -720,6 +769,10 @@ static void connected(struct bt_conn *conn, uint8_t err)
/* Cancel fast→slow transition — already connected, no need to switch */
k_work_cancel_delayable(&adv_slow_work);
/* Arm the unpaired-connection timeout. Cancelled by security_changed()
* at L2+, and by disconnected() whichever way the connection ends. */
k_work_reschedule(&sec_timeout_work, K_MSEC(BLE_SECURITY_TIMEOUT_MS));
/* DLE is NOT requested here — the phone may start a PHY update LL
* procedure immediately, and BLE allows only one at a time.
* DLE is deferred to le_phy_updated() (after PHY negotiation completes)
@@ -748,6 +801,8 @@ static void disconnected(struct bt_conn *conn, uint8_t reason)
bt_addr_le_to_str(bt_conn_get_dst(conn), addr, sizeof(addr));
LOG_INF("disconnected: %s reason 0x%02x", addr, reason);
k_work_cancel_delayable(&sec_timeout_work);
if (conn == current_conn) {
bt_conn_unref(current_conn);
current_conn = NULL;
@@ -803,6 +858,12 @@ static void security_changed(struct bt_conn *conn, bt_security_t level, enum bt_
}
LOG_INF("%s level %u", addr, level);
/* Paired (or bonded reconnect) — the connection has earned its slot,
* so stand the unpaired-connection timeout down. */
if (level >= BT_SECURITY_L2) {
k_work_cancel_delayable(&sec_timeout_work);
}
/* Enable TX when we have sufficient security (level 2+ = encrypted).
* This is the ONLY place that sets ble_tx_ready security_changed is
* the authority. CCC subscription (secure_nus_ccc_changed) only kicks
@@ -216,6 +216,15 @@ lr11xx_hal_status_t lr11xx_hal_write(const void *context, const uint8_t *command
return wait_on_busy(ctx);
}
/* Command status field of the stat1 byte (SWDR001, bits [2:1]).
* 0 FAIL, 1 PERR, 2 CMD_OK, 3 CMD_DATA. The vendor decoder in lr11xx_system.c
* shifts without masking; mask here, the field is 2 bits. */
#define LR11XX_STAT1_CMD_STATUS(b) (((b) >> 1) & 0x03)
#define LR11XX_CMD_STATUS_DATA 0x03
/* Attempts at the command-then-response read before giving up. */
#define LR11XX_READ_ATTEMPTS 3
lr11xx_hal_status_t lr11xx_hal_read(const void *context, const uint8_t *command,
const uint16_t command_length,
uint8_t *data, const uint16_t data_length)
@@ -256,26 +265,67 @@ lr11xx_hal_status_t lr11xx_hal_read(const void *context, const uint8_t *command,
return wait_on_busy(ctx);
}
/* Phase 2: wait BUSY, then read response — LR11XX prepends one dummy status byte */
if (check_device_ready(ctx) != LR11XX_HAL_STATUS_OK) {
return LR11XX_HAL_STATUS_ERROR;
/* Phase 2: wait BUSY, then read response — LR11XX prepends one stat1 byte.
*
* That byte is checked, not discarded. wait_on_busy() returns immediately
* on a BUSY that reads low and cannot tell "the command finished" from
* "BUSY has not risen yet", so this window can clock out the chip's default
* status / IRQ stream instead of the payload see the long note in
* adapters/radio/lr20xx/lr20xx_hal_zephyr.c and MeshCore PR #3261. On this
* family the miss lands in lr11xx_radio_get_rx_buffer_status(), where the
* payload length and buffer offset both come out of IRQ bits: a plausible
* short integer, no error reported, and the frame is read out of the buffer
* at the wrong length or the wrong offset.
*
* CMD_DATA ("processed read, data is being transmitted instead of IRQ
* status") is the only correct status for this window; a reply produced by
* the race reports CMD_OK. Re-issuing the command is safe for every caller
* that reaches here all are getters or address-based register reads. */
uint8_t stat1 = 0;
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. */
gpio_pin_set_dt(&ctx->nss, 1);
ret = spi_write(ctx->spi_dev, &ctx->spi_cfg, &tx);
gpio_pin_set_dt(&ctx->nss, 0);
if (ret < 0) {
LOG_ERR("SPI write (cmd retry) failed: %d", ret);
return LR11XX_HAL_STATUS_ERROR;
}
}
if (check_device_ready(ctx) != LR11XX_HAL_STATUS_OK) {
return LR11XX_HAL_STATUS_ERROR;
}
const struct spi_buf rx_bufs[] = {
{ .buf = &stat1, .len = 1 }, /* leading status byte */
{ .buf = data, .len = data_length },
};
const struct spi_buf_set rx = { .buffers = rx_bufs, .count = 2 };
gpio_pin_set_dt(&ctx->nss, 1);
ret = spi_read(ctx->spi_dev, &ctx->spi_cfg, &rx);
gpio_pin_set_dt(&ctx->nss, 0);
if (ret < 0) {
LOG_ERR("SPI read failed: %d", ret);
return LR11XX_HAL_STATUS_ERROR;
}
if (LR11XX_STAT1_CMD_STATUS(stat1) == LR11XX_CMD_STATUS_DATA) {
return LR11XX_HAL_STATUS_OK;
}
}
uint8_t dummy;
const struct spi_buf rx_bufs[] = {
{ .buf = &dummy, .len = 1 }, /* discard leading status byte */
{ .buf = data, .len = data_length },
};
const struct spi_buf_set rx = { .buffers = rx_bufs, .count = 2 };
gpio_pin_set_dt(&ctx->nss, 1);
ret = spi_read(ctx->spi_dev, &ctx->spi_cfg, &rx);
gpio_pin_set_dt(&ctx->nss, 0);
if (ret < 0) {
LOG_ERR("SPI read failed: %d", ret);
return LR11XX_HAL_STATUS_ERROR;
}
/* Out of attempts. Hand back what came in rather than failing the call:
* that is exactly the previous behaviour, so no existing caller regresses on
* a read whose status is legitimately something else. */
LOG_WRN("read op=0x%04x: stat1=0x%02x (cmd_status=%u, want CMD_DATA) after %d attempts",
last_opcode, stat1, LR11XX_STAT1_CMD_STATUS(stat1), LR11XX_READ_ATTEMPTS);
return LR11XX_HAL_STATUS_OK;
}
@@ -252,75 +252,141 @@ lr20xx_hal_status_t lr20xx_hal_write(const void *context, const uint8_t *command
#define LR20XX_STAT_LEN 2
/**
* @brief Clock one command-and-response frame inside a single NSS assertion.
* @brief Clock a command and read back its response, as two NSS windows.
*
* The LR2021 streams the whole answer within the NSS window that carried the
* command:
* The command goes out in its own window. The chip raises BUSY while it
* prepares the answer, and a second window clocks that answer out:
*
* MOSI [ command_length ][ LR20XX_STAT_LEN + data_length ]
* MISO [ undefined ][ stat header ][ payload ]
* window 1 MOSI [ command_length ] MISO [ undefined ]
* -- BUSY high while the chip prepares the answer --
* window 2 MOSI [ LR20XX_STAT_LEN + data_length ]
* MISO [ stat header ][ payload ]
*
* Releasing NSS between the command and the payload ends the frame, so the
* following read starts a fresh one and the chip answers it with the status /
* IRQ word rather than the payload the caller asked for. That misread is not
* obviously wrong at the call site it is a plausible-looking short integer
* so it surfaces as nonsense lengths and all-zero status reads rather than as
* an SPI error. Keep every read in one transaction.
* This mirrors Semtech's reference HAL (lr20xx_hal_read() in LoRa Basics
* Modem): command out, NSS released, wait on BUSY, then two dummy bytes and the
* payload in a fresh window. The Rx FIFO pop is the one read on this chip that
* does NOT work this way single window, no stat header, no BUSY wait; see
* lr20xx_hal_direct_read_fifo().
*
* The BUSY wait between the two windows is load-bearing, and the failure it
* guards is silent rather than loud: clock the second window too early and the
* chip answers with its default status / IRQ stream instead of the payload,
* which the caller then parses as a plausible-looking short integer rather than
* reporting an SPI error. The status check at the bottom of this function is
* the backstop for the residual case where BUSY has not yet risen when the wait
* samples it.
*
* NULL TX buffers clock the SPI controller's over-read character, NOT zero --
* Nordic defaults it to 0xff, which the LR2021 reads as a bogus opcode and
* rejects. Boards using this driver must set overrun-character = <0x00> (the
* LR2021 NOP) on the SPI node.
*/
/* Command status field of the stat1 header (DS Table 6-38, bits [2:1]).
* 0 FAIL, 1 PERR, 2 CMD_OK, 3 CMD_DATA. The vendor decoder in
* lr20xx_system.c shifts without masking; mask here, the field is 2 bits. */
#define LR20XX_STAT1_CMD_STATUS(b) (((b) >> 1) & 0x03)
#define LR20XX_CMD_STATUS_DATA 0x03
/* Attempts at the two-window read before giving up. Upstream MeshCore
* measured recovery on the first retry in every observed case (4/4 on LR11xx,
* 11/11 on LR2021, with the BUSY wait skipped on purpose). */
#define LR20XX_READ_ATTEMPTS 3
static int lr20xx_spi_read_frame(struct lr20xx_hal_context *ctx, const uint8_t *command,
uint16_t command_length, uint8_t *data, uint16_t data_length)
{
int ret;
int ret = 0;
uint8_t stat[LR20XX_STAT_LEN];
/* Phase 1: the command, in its own NSS window. */
{
const struct spi_buf tx_buf = {
.buf = (uint8_t *)command,
.len = command_length,
};
const struct spi_buf_set tx = { .buffers = &tx_buf, .count = 1 };
/* Retried as a whole: the chip only streams the answer in the window
* that follows its command, so recovering means re-issuing the command.
* Safe for every caller that lands here lr20xx_hal_read() serves
* getters and address-based register reads, all idempotent. The one
* read that is NOT idempotent, the Rx FIFO pop, does not come through
* this path at all (lr20xx_hal_direct_read_fifo(), single NSS window,
* no stat header, no BUSY wait structurally immune to the race
* below). */
for (int attempt = 0; attempt < LR20XX_READ_ATTEMPTS; attempt++) {
/* Phase 1: the command, in its own NSS window. */
{
const struct spi_buf tx_buf = {
.buf = (uint8_t *)command,
.len = command_length,
};
const struct spi_buf_set tx = { .buffers = &tx_buf, .count = 1 };
gpio_pin_set_dt(&ctx->nss, 1);
ret = spi_write(ctx->spi_dev, &ctx->spi_cfg, &tx);
gpio_pin_set_dt(&ctx->nss, 0);
gpio_pin_set_dt(&ctx->nss, 1);
ret = spi_write(ctx->spi_dev, &ctx->spi_cfg, &tx);
gpio_pin_set_dt(&ctx->nss, 0);
if (ret < 0) {
if (ret < 0) {
return ret;
}
}
/* The answer is not ready until BUSY drops — without this the second
* window clocks out whatever the chip had, two bytes early. */
if (wait_on_busy(ctx) != LR20XX_HAL_STATUS_OK) {
return -ETIMEDOUT;
}
/* Phase 2: two dummy bytes absorb the stat header, then the payload.
* The stat header lands in a real buffer rather than being discarded
* with a NULL one: spi_nrfx_spim rejects a transfer whose first TX and
* RX buffers are both NULL (-EINVAL), and TX is legitimately NULL here. */
{
const struct spi_buf tx_buf = {
.buf = NULL,
.len = LR20XX_STAT_LEN + data_length,
};
const struct spi_buf rx_bufs[] = {
{ .buf = stat, .len = LR20XX_STAT_LEN },
{ .buf = data, .len = data_length },
};
const struct spi_buf_set tx = { .buffers = &tx_buf, .count = 1 };
const struct spi_buf_set rx = { .buffers = rx_bufs, .count = 2 };
gpio_pin_set_dt(&ctx->nss, 1);
ret = spi_transceive(ctx->spi_dev, &ctx->spi_cfg, &tx, &rx);
gpio_pin_set_dt(&ctx->nss, 0);
if (ret < 0) {
return ret;
}
}
/* Did the chip actually answer US?
*
* wait_on_busy() returns immediately on a BUSY that reads low, and
* cannot tell "the command finished" from "BUSY has not risen yet".
* The gap between releasing NSS above and sampling the pin is two
* GPIO calls, so losing that race is entirely possible; RadioLib
* loses it with a whole microsecond of margin (MeshCore PR #3261).
* When it is lost, phase 2 clocks out the chip's default
* [stat 2B][irq 4B] stream instead of the payload, stat[] absorbs
* the two status bytes, and the caller parses IRQ bits as its
* answer. Nothing reports an error: for GetRxPacketLength that is
* irq[31:16], which with RX_DONE (bit 18) set is exactly 4, so a
* real frame of any size is read out of the FIFO as a 4-byte one
* and silently dropped upstream as corrupt.
*
* The status separates the two cleanly. CMD_DATA means "a read was
* processed and data is being transmitted instead of IRQ status"
* the only correct answer for this window. A reply produced by the
* race reports CMD_OK instead, i.e. the chip is streaming status,
* not data. The Rx FIFO is untouched at this point, so re-reading
* recovers the frame rather than losing it. */
if (LR20XX_STAT1_CMD_STATUS(stat[0]) == LR20XX_CMD_STATUS_DATA) {
return ret;
}
}
/* The answer is not ready until BUSY drops — without this the second
* window clocks out whatever the chip had, two bytes early. */
if (wait_on_busy(ctx) != LR20XX_HAL_STATUS_OK) {
return -ETIMEDOUT;
}
/* Phase 2: two dummy bytes absorb the stat header, then the payload.
* The stat header lands in a real buffer rather than being discarded
* with a NULL one: spi_nrfx_spim rejects a transfer whose first TX and
* RX buffers are both NULL (-EINVAL), and TX is legitimately NULL here. */
{
uint8_t stat[LR20XX_STAT_LEN];
const struct spi_buf tx_buf = {
.buf = NULL,
.len = LR20XX_STAT_LEN + data_length,
};
const struct spi_buf rx_bufs[] = {
{ .buf = stat, .len = LR20XX_STAT_LEN },
{ .buf = data, .len = data_length },
};
const struct spi_buf_set tx = { .buffers = &tx_buf, .count = 1 };
const struct spi_buf_set rx = { .buffers = rx_bufs, .count = 2 };
gpio_pin_set_dt(&ctx->nss, 1);
ret = spi_transceive(ctx->spi_dev, &ctx->spi_cfg, &tx, &rx);
gpio_pin_set_dt(&ctx->nss, 0);
}
/* Out of attempts. Hand back what came in rather than failing the call:
* that is exactly today's behaviour, so no existing caller regresses on a
* read whose status is legitimately something else. Tighten to an error
* return only once hardware confirms every read reports CMD_DATA. */
LOG_WRN("read op=0x%04x: stat1=0x%02x (cmd_status=%u, want CMD_DATA) after %d attempts",
last_opcode, stat[0], LR20XX_STAT1_CMD_STATUS(stat[0]), LR20XX_READ_ATTEMPTS);
return ret;
}