fix(sx126x): let CalibrateImage settle before re-applying RX registers in resetAGC() - #11774
Conversation
…s in resetAGC() CalibrateImage returns as soon as the command is accepted and BUSY does not stay asserted for the rest of the calibration. resetAGC() then re-applies the RX boosted-gain and 0x8B5 registers immediately, and a register write landing in that window fails write-verify (RADIOLIB_ERR_SPI_WRITE_FAILED), leaving the chip needing a full re-init. On RAK3401 + RAK13302 (nRF52840, busy mesh) this hit ~69% of resets with no delay, ~3% at 10-20 ms, and 0 at 50 ms.
|
|
|
|
@ip2k, Welcome to Meshtastic!Thanks for opening your first pull request. We really appreciate it. We discuss work as a team in discord, please join us in the #firmware channel. Welcome to the team 😄 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthrough
ChangesSX126x AGC reset
Priority: ➖ Normal Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to SX126x AGC resets now wait briefly after image calibration before restoring receiver settings, avoiding a documented register-write timing window. No current merge-blocking risk is identified. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi, just wanted to note here that while Claude helped me dig deep into this issue and did most of the legwork, there’s a real human who was having a real issue for a long time that was hard to pin down previously after banging my head against the wall for quite a while and putting my Meshtastic gear on a shelf because it didn’t work well. I’ll be on Discord too, please let me know what I can do better or how I can otherwise help :) |
…he comment Review feedback: the settle is the wait for step 5's image calibration, just as step 4 waits for step 3, so number it and wrap to the width of the other steps. Resume receiving becomes step 7. No functional change.
|
On-air result of this exact commit on
One thing to disclose plainly because it is not this PR's bug but appears in the same log: during the |
0c4bee7
5 participants
What
resetAGC()(#9705) recalibrates the SX126x every interval and then re-applies the RX boosted-gain and0x8B5RX-sensitivity registers (#10219). This adds a 50 ms settle aftercalibrateImage()before those register writes.Why
On a RAK WisMesh Repeater Mini (RAK3401 nRF52840 + RAK13302 1 W SX1262) the radio periodically went deaf until reboot. Traced into
resetAGC():SX126x::calibrateImage()returns as soon as the command is accepted, and the chip does not hold BUSY for the rest of that calibration. The next register write (setRxBoostedGainMode, then the0x8B5patch) lands in that window, fails write-verify (RADIOLIB_ERR_SPI_WRITE_FAILED, -16), and leaves the radio needing a full re-init.Related to #9933 (closed): that was a duty-cycle wake BUSY race; this is a sibling at a different site, the
calibrateImage()call insideresetAGC().How it works
Calibrate(0x7F)earlier in the same function is already followed by a BUSY poll; the image calibration is not. Instrumentation confirmed BUSY never asserts aftercalibrateImage()(0 of ~2,000 cycles), so polling cannot guard this window - a fixed settle is required. Register writes issued inside ~10 ms fail; none fail at >=20 ms; 50 ms is ~2.5x that floor. nRF52delay()is 10 ms-tick quantized (measured 49-51 ms). The pass already holds the radio out of RX for the calibration, so the cost is immaterial (<=60 ms per 60 s).What it changes
One line in
SX126xInterface::resetAGC(). No API, config or wire-format change. SX126x only.Testing
RAK3401 + RAK13302, nRF52840, live 915 MHz mesh, AGC interval shortened to 10 s to accelerate. Each
resetAGCpass logged Calibrate / CalibrateImage / register-write status, so the settle-window failure (both calibrations return 0, the following write fails -16) is counted distinctly from unrelated chip faults.The dose-response and soak ran on a 2.8.0-based build on this hardware; this commit is the identical one-line change on
develop, compile-verified forrak3401-1watt. Adevelop-based on-air soak of this exact commit is running now; I will post the result here.🤝 Attestations
Tested on RAK WisBlock RAK3401 + RAK13302 (1 W, nRF52840). No other SX126x boards on hand; regression testing on other SX126x hardware is welcome.
🤖 Generated with Claude Code
Summary by CodeRabbit