fix(radio): consume upstream lora-phy pre-command BUSY check
Pick up the proper fix from our fork at swaits/lora-rs@fix/sx126x-busy-race: move the ready check from *after* to *before* each SPI op per the Semtech manuals (SX1261/2 §8.3.1, LR1121 UM §3 / §3.5, LR1110 DS §1.2.5) and rename InterfaceVariant::wait_on_busy to wait_until_ready. Firmware side is surgical: since the upstream fix keeps the GenericSx126xInterfaceVariant::new signature at 5 args (no DelayNs threading), there is nothing to change in the four nRF / ESP / RP2040 board files beyond a one-line comment rename (wait_on_busy -> wait_until_ready) in the BUSY-pullup comment on the two nRF boards. Wire in the [patch.crates-io] override pointing at ../../lora-rs/lora-phy, bump the dep spec to the pre-release 3.0.2-alpha that matches the fork, and enable the defmt-03 feature that upstream moved behind a gate between 3.0.1 and the fix branch. TODO.md rewritten to track the new approach: PR body template with verbatim citations from all three Semtech chip manuals (SX1261/2, LR1121 UM, LR1110 DS) plus the SX1276 pin table confirming the family has no BUSY line. No change to firmware behavior beyond picking up the upstream fix, which takes first-TX-after-boot success from ~50% to 20/20 on the Seeed Wio Tracker L1 repro board.
This commit is contained in:
3 files changed
+278
-15
No files matched your search
Generated
+210
-14
@@ -2,6 +2,17 @@
|
||||
# It is not intended for manual editing.
|
||||
version = 4
|
||||
|
||||
[[package]]
|
||||
name = "aes"
|
||||
version = "0.9.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "66bd29a732b644c0431c6140f370d097879203d79b80c94a6747ba0872adaef8"
|
||||
dependencies = [
|
||||
"cipher",
|
||||
"cpubits",
|
||||
"cpufeatures 0.3.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "ahash"
|
||||
version = "0.8.12"
|
||||
@@ -127,6 +138,15 @@ dependencies = [
|
||||
"generic-array",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "block-buffer"
|
||||
version = "0.12.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "cdd35008169921d80bc60d3d0ab416eecb028c4cd653352907921d95084790be"
|
||||
dependencies = [
|
||||
"hybrid-array",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "byte-slice-cast"
|
||||
version = "1.2.3"
|
||||
@@ -172,6 +192,33 @@ dependencies = [
|
||||
"pure-rust-locales",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "cipher"
|
||||
version = "0.5.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e34d8227fe1ba289043aeb13792056ff80fd6de1a9f49137a5f499de8e8c78ea"
|
||||
dependencies = [
|
||||
"crypto-common 0.2.1",
|
||||
"inout",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "cmac"
|
||||
version = "0.8.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "ac78aa94ce13e432b332a4d1bf2eff167d3a2520188ee05b337180a42fd2e62e"
|
||||
dependencies = [
|
||||
"cipher",
|
||||
"dbl",
|
||||
"digest 0.11.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "cmov"
|
||||
version = "0.5.3"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "3f88a43d011fc4a6876cb7344703e297c71dda42494fee094d5f7c76bf13f746"
|
||||
|
||||
[[package]]
|
||||
name = "codespan-reporting"
|
||||
version = "0.11.1"
|
||||
@@ -225,6 +272,12 @@ dependencies = [
|
||||
"syn 2.0.117",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "cpubits"
|
||||
version = "0.1.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "5ef0c543070d296ea414df2dd7625d1b24866ce206709d8a4a424f28377f5861"
|
||||
|
||||
[[package]]
|
||||
name = "cpufeatures"
|
||||
version = "0.2.17"
|
||||
@@ -234,6 +287,15 @@ dependencies = [
|
||||
"libc",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "cpufeatures"
|
||||
version = "0.3.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "8b2a41393f66f16b0823bb79094d54ac5fbd34ab292ddafb9a0456ac9f87d201"
|
||||
dependencies = [
|
||||
"libc",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "crc-any"
|
||||
version = "2.5.0"
|
||||
@@ -265,6 +327,24 @@ dependencies = [
|
||||
"typenum",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "crypto-common"
|
||||
version = "0.2.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "77727bb15fa921304124b128af125e7e3b968275d1b108b379190264f4423710"
|
||||
dependencies = [
|
||||
"hybrid-array",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "ctutils"
|
||||
version = "0.4.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "7d5515a3834141de9eafb9717ad39eea8247b5674e6066c404e8c4b365d2a29e"
|
||||
dependencies = [
|
||||
"cmov",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "darling"
|
||||
version = "0.20.11"
|
||||
@@ -368,6 +448,15 @@ dependencies = [
|
||||
"syn 2.0.117",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "dbl"
|
||||
version = "0.5.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "f0d7a944e61df464668c5f51f56cc667396a8821434273112948ea0b66e405d7"
|
||||
dependencies = [
|
||||
"hybrid-array",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "debug-helper"
|
||||
version = "0.3.13"
|
||||
@@ -442,8 +531,19 @@ version = "0.10.7"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292"
|
||||
dependencies = [
|
||||
"block-buffer",
|
||||
"crypto-common",
|
||||
"block-buffer 0.10.4",
|
||||
"crypto-common 0.1.7",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "digest"
|
||||
version = "0.11.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "4850db49bf08e663084f7fb5c87d202ef91a3907271aff24a94eb97ff039153c"
|
||||
dependencies = [
|
||||
"block-buffer 0.12.0",
|
||||
"crypto-common 0.2.1",
|
||||
"ctutils",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -1022,7 +1122,7 @@ dependencies = [
|
||||
"cfg-if",
|
||||
"critical-section",
|
||||
"delegate",
|
||||
"digest",
|
||||
"digest 0.10.7",
|
||||
"document-features",
|
||||
"embassy-embedded-hal",
|
||||
"embassy-futures",
|
||||
@@ -1235,6 +1335,12 @@ dependencies = [
|
||||
"vcell",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "fastrand"
|
||||
version = "2.4.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "9f1f227452a390804cdb637b74a86990f2a7d7ba4b7d5693aac9b4dd6defd8d6"
|
||||
|
||||
[[package]]
|
||||
name = "find-msvc-tools"
|
||||
version = "0.1.9"
|
||||
@@ -1283,12 +1389,42 @@ dependencies = [
|
||||
"gcd",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "futures"
|
||||
version = "0.3.32"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "8b147ee9d1f6d097cef9ce628cd2ee62288d963e16fb287bd9286455b241382d"
|
||||
dependencies = [
|
||||
"futures-channel",
|
||||
"futures-core",
|
||||
"futures-io",
|
||||
"futures-sink",
|
||||
"futures-task",
|
||||
"futures-util",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "futures-channel"
|
||||
version = "0.3.32"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "07bbe89c50d7a535e539b8c17bc0b49bdb77747034daa8087407d655f3f7cc1d"
|
||||
dependencies = [
|
||||
"futures-core",
|
||||
"futures-sink",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "futures-core"
|
||||
version = "0.3.32"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "7e3450815272ef58cec6d564423f6e755e25379b217b0bc688e295ba24df6b1d"
|
||||
|
||||
[[package]]
|
||||
name = "futures-io"
|
||||
version = "0.3.32"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "cecba35d7ad927e23624b22ad55235f2239cfa44fd10428eecbeba6d6a717718"
|
||||
|
||||
[[package]]
|
||||
name = "futures-sink"
|
||||
version = "0.3.32"
|
||||
@@ -1308,6 +1444,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "389ca41296e6190b48053de0321d02a77f32f8a5d2461dd38762c0593805c6d6"
|
||||
dependencies = [
|
||||
"futures-core",
|
||||
"futures-sink",
|
||||
"futures-task",
|
||||
"pin-project-lite",
|
||||
]
|
||||
@@ -1405,6 +1542,12 @@ version = "0.5.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea"
|
||||
|
||||
[[package]]
|
||||
name = "hex"
|
||||
version = "0.4.3"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70"
|
||||
|
||||
[[package]]
|
||||
name = "hsmc"
|
||||
version = "0.2.0"
|
||||
@@ -1429,6 +1572,15 @@ dependencies = [
|
||||
"syn 2.0.117",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "hybrid-array"
|
||||
version = "0.4.10"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "3944cf8cf766b40e2a1a333ee5e9b563f854d5fa49d6a8ca2764e97c6eddb214"
|
||||
dependencies = [
|
||||
"typenum",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "ident_case"
|
||||
version = "1.0.1"
|
||||
@@ -1454,6 +1606,15 @@ dependencies = [
|
||||
"rustversion",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "inout"
|
||||
version = "0.2.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "4250ce6452e92010fdf7268ccc5d14faa80bb12fc741938534c58f16804e03c7"
|
||||
dependencies = [
|
||||
"hybrid-array",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "instability"
|
||||
version = "0.3.12"
|
||||
@@ -1488,7 +1649,7 @@ version = "0.1.6"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "cb26cec98cce3a3d96cbb7bced3c4b16e3d13f27ec56dbd62cbc8f39cfb9d653"
|
||||
dependencies = [
|
||||
"cpufeatures",
|
||||
"cpufeatures 0.2.17",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -1572,22 +1733,57 @@ dependencies = [
|
||||
[[package]]
|
||||
name = "lora-modulation"
|
||||
version = "0.1.5"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "2c96c4a41a1f1ad04de765306e1829c7007a37898fe4033204fa82cf2e055624"
|
||||
dependencies = [
|
||||
"defmt 0.3.100",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "lora-phy"
|
||||
version = "3.0.1"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "61471c3b2909789e3332083577f6cf6c41a4fcf37674ef15156bcbb20504ac65"
|
||||
version = "3.0.2-alpha"
|
||||
dependencies = [
|
||||
"defmt 0.3.100",
|
||||
"document-features",
|
||||
"embedded-hal 1.0.0",
|
||||
"embedded-hal-async",
|
||||
"lora-modulation",
|
||||
"lorawan-device",
|
||||
"num-traits",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "lorawan"
|
||||
version = "0.9.1"
|
||||
dependencies = [
|
||||
"aes",
|
||||
"cmac",
|
||||
"defmt 0.3.100",
|
||||
"hex",
|
||||
"lorawan-macros",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "lorawan-device"
|
||||
version = "0.12.2"
|
||||
dependencies = [
|
||||
"defmt 0.3.100",
|
||||
"document-features",
|
||||
"fastrand",
|
||||
"futures",
|
||||
"heapless 0.9.2",
|
||||
"lora-modulation",
|
||||
"lorawan",
|
||||
"rand_core 0.6.4",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "lorawan-macros"
|
||||
version = "0.1.0"
|
||||
dependencies = [
|
||||
"proc-macro2",
|
||||
"quote",
|
||||
"syn 2.0.117",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "matchers"
|
||||
version = "0.2.0"
|
||||
@@ -2214,8 +2410,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e3bf829a2d51ab4a5ddf1352d8470c140cadc8301b2ae1789db023f01cedd6ba"
|
||||
dependencies = [
|
||||
"cfg-if",
|
||||
"cpufeatures",
|
||||
"digest",
|
||||
"cpufeatures 0.2.17",
|
||||
"digest 0.10.7",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -2225,8 +2421,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "a7507d819769d01a365ab707794a4084392c824f54a7a6a7862f8c3d0892b283"
|
||||
dependencies = [
|
||||
"cfg-if",
|
||||
"cpufeatures",
|
||||
"digest",
|
||||
"cpufeatures 0.2.17",
|
||||
"digest 0.10.7",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -2241,7 +2437,7 @@ version = "0.10.8"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "75872d278a8f37ef87fa0ddbda7802605cb18344497949862c0d4dcb291eba60"
|
||||
dependencies = [
|
||||
"digest",
|
||||
"digest 0.10.7",
|
||||
"keccak",
|
||||
]
|
||||
|
||||
|
||||
+9
-1
@@ -32,7 +32,7 @@ ucobs = { version = "0.3", features = ["legacy-msrv"] }
|
||||
heapless = "0.9"
|
||||
donglora-protocol = "1"
|
||||
hsmc = { version = "0.2", default-features = false, features = ["embassy"] }
|
||||
lora-phy = "3.0"
|
||||
lora-phy = { version = "3.0.2-alpha", features = ["defmt-03"] } # matches the in-tree [patch.crates-io] override; drop the pre-release selector once upstream releases the fix
|
||||
portable-atomic = { version = "1", optional = true, default-features = false, features = ["critical-section"] }
|
||||
static_cell = "2.1"
|
||||
|
||||
@@ -212,6 +212,14 @@ esp-rtos = { git = "https://github.com/esp-rs/esp-hal.git", branch = "main"
|
||||
esp-backtrace = { git = "https://github.com/esp-rs/esp-hal.git", branch = "main" }
|
||||
esp-println = { git = "https://github.com/esp-rs/esp-hal.git", branch = "main" }
|
||||
|
||||
# Local fix for a BUSY-assertion race: lora-phy currently polls BUSY
|
||||
# *after* each SPI op, but SX1261-2 §8.1.4 specifies a *pre*-command
|
||||
# check (same pattern as RadioLib's SPIwriteStream). This override pulls
|
||||
# in the corrected ordering and renames `wait_on_busy` to
|
||||
# `wait_until_ready`. See TODO.md for the PR body. Remove this line once
|
||||
# the fix is merged upstream + released on crates.io.
|
||||
lora-phy = { path = "../../lora-rs/lora-phy" }
|
||||
|
||||
[lints.clippy]
|
||||
undocumented_unsafe_blocks = "deny"
|
||||
|
||||
|
||||
@@ -1,5 +1,64 @@
|
||||
# Firmware TODO List
|
||||
|
||||
## `lora-phy` fix shipped locally; upstream PR pending submission
|
||||
|
||||
Branch: [`swaits/lora-rs @ fix/sx126x-busy-race`](https://github.com/swaits/lora-rs/tree/fix/sx126x-busy-race).
|
||||
PR not yet opened. Firmware consumes the fix via `[patch.crates-io]`
|
||||
pointing at `../../lora-rs/lora-phy`; drop the patch line + pre-release
|
||||
selector once upstream releases.
|
||||
|
||||
Hardware-verified on Wio Tracker L1: 20/20 clean-boot TX after the fix
|
||||
(vs ~50% before). Additive fix: adds pre-command BUSY check, keeps
|
||||
existing post-command wait in place.
|
||||
|
||||
### PR body
|
||||
|
||||
```
|
||||
Title: phy: pre-command BUSY check per Semtech datasheets
|
||||
|
||||
## Problem
|
||||
|
||||
While bringing up a downstream project (donglora), I hit a race where TX commands would occasionally fail. Tracked it down to lora-phy not following the BUSY-handshake algorithm Semtech recommends: the host should wait on BUSY *before* issuing each command, not after.
|
||||
|
||||
Details: `SpiInterface::{write, write_with_payload, read,
|
||||
read_with_status}` polls BUSY *after* each SPI op.
|
||||
|
||||
On fast MCUs (nRF52840 at 64 MHz, release build) this is racy — `embedded-hal-async`'s `wait_for_low` returns immediately if the pin is already low, and the chip takes up to T_SW ns (SX1261/2 §8.3, Fig. 8-3) to assert BUSY after NSS rises. The host polls inside that pre-assertion window, sees a stale LOW, fires the next command while the chip is still busy, and the chip silently drops it. First `lora.tx()` after a clean boot on Seeed Wio Tracker L1 hangs ~50% of the time.
|
||||
|
||||
## Authority
|
||||
|
||||
- [SX1261/2 datasheet, DS.SX1261-2.W.APP Rev. 1.2](https://cdn.sparkfun.com/assets/6/b/5/1/4/SX1262_datasheet.pdf) §8.3.1
|
||||
"BUSY Control Line": *"it is essential to wait for the BUSY line to
|
||||
go low before sending an SPI command."*
|
||||
- [LR1121 User Manual, UM.LR1121.W.APP Rev. 1.2](https://files.waveshare.com/wiki/Core1121/UserManual_LR1121_v1_2.pdf) §3
|
||||
"Host-Controller Interface": *"it is necessary to check the status
|
||||
of BUSY prior to sending a command."* (Applies to the LR11xx family
|
||||
— see also [LR1110 DS Rev. 1.1](https://download.mikroe.com/documents/datasheets/LR1110_datasheet.pdf) §1.2.5.)
|
||||
- [RadioLib](https://github.com/jgromes/RadioLib/blob/cffd5af/src/Module.cpp)
|
||||
`Module::SPIwriteStream` / `SPIreadStream` call `waitForBusy()` at
|
||||
the start of every transaction.
|
||||
- The [SX1276/77/78/79 datasheet](https://cdn.sparkfun.com/assets/7/7/3/2/2/SX1276_Datasheet.pdf)
|
||||
Table 2 exposes no BUSY pin, so
|
||||
`GenericSx127xInterfaceVariant::wait_on_busy` stays a no-op and the
|
||||
change is behaviorally identical for SX127x.
|
||||
|
||||
## Fix
|
||||
|
||||
Replace the post-command BUSY wait with a pre-command BUSY check at the start of each `SpiInterface` / `Lr1110SpiInterface` read/write method. Drop the redundant explicit `wait_on_busy()` calls in `sx126x` (after Calibrate, `ensure_ready` non-sleep arm) and `lr1110::mod` (4 regmem/bootloader sites) — all now covered by the interface pre-wait.
|
||||
|
||||
The `is_sleep_command` flag keeps its name but now gates the pre-wait; set `true` only when BUSY is held HIGH by design (the wake-from-sleep GetStatus pulse in `sx126x::ensure_ready`). Two callsite flips as a result: `ensure_ready` Sleep arm `false`→`true`; `set_sleep` `true`→`false`.
|
||||
|
||||
No public API signature changes.
|
||||
|
||||
## Test
|
||||
|
||||
- `cargo clippy -p lora-phy --features defmt-03 -- -D warnings` clean.
|
||||
- Hardware: Seeed Wio Tracker L1 (nRF52840 + SX1262), 20 consecutive
|
||||
clean-boot TX cycles → 20/20 (vs ~50% before).
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Drop `[patch.crates-io]` + upgrade esp-hal / esp-rtos + add `esp_app_desc!()` + bump espflash to v4
|
||||
|
||||
Single coordinated change that clears our last remaining dep-graph debt
|
||||
|
||||
Reference in new issue
Block a user