Skip to content

fix(radio): make limitPower() idempotent so chip re-inits don't compound PA gain subtraction - #11782

Merged
thebentern merged 2 commits into
meshtastic:developfrom
ip2k:fix/limitpower-idempotent
Sep 9, 2026
Merged

fix(radio): make limitPower() idempotent so chip re-inits don't compound PA gain subtraction#11782
thebentern merged 2 commits into
meshtastic:developfrom
ip2k:fix/limitpower-idempotent

Conversation

@ip2k

@ip2k ip2k commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

What

limitPower() converts the member power in place (regulatory clamp, then the TX_GAIN_LORA / FEM PA-gain subtraction). Re-seed power from config.lora.tx_power at the top so a second call yields the same result as the first.

Why

limitPower() assumed applyModemConfig() had just re-seeded power. Since #10025 (fixing #10022) every driver calls it from both reinitChip() and programModemParams(), and the chip-state recovery paths from #11676 / #11678 run those two back-to-back - so each recovery converts an already-converted value. With RAK13302's 22-entry gain table one recovery walks a 30 dBm request 30 -> 22 -> 13 dBm at the chip and a second heads for the -9 dBm floor, while config.lora.tx_power still reads 30: the node silently transmits at a fraction of its configured power. Likely the mechanism behind #10979 / #11089 once a recovery has occurred (unverified on those exact setups).

Captured on RAK3401 + RAK13302 before the fix, one recovery:

Requested Tx power: 30 dBm; Device LoRa Tx gain: 8 dB  ->  Power output set to 22
Requested Tx power: 22 dBm; Device LoRa Tx gain: 9 dB  ->  Final Tx power: 13 dBm

How it works

applyModemConfig() resolves regional/licensed limits and writes the result back to config.lora.tx_power, so it is the right value to re-derive from. A single limitPower() call is unchanged; only later calls in a re-init sequence change. Audited every caller (SX126x, LR11x0, RF95, LR20x0): none adjusts power before calling limitPower(), so the re-seed clobbers nothing. LR11x0's recoverChipStateLoss() has the identical double-call shape, so this fixes it there too.

What it changes

One line in RadioInterface::limitPower() plus its docstring. No API, config or wire-format change. Normal path unchanged; the recovery path on every FEM/PA board is fixed.

Testing

RAK3401 + RAK13302 (nRF52840) on develop with this commit, live mesh, 5.2 h: 5 chip-state-loss recoveries via #11678, each running reinitChip() then programModemParams(). Both limitPower() calls read 30 -> 22 dBm every time (Final Tx power: 22 x10, never 13 or 4), and each re-init succeeded with the queued packet sent. Compile-verified for rak3401-1watt.

🤝 Attestations

  • I have tested that my proposed changes behave as described.
  • I have tested that my proposed changes do not cause any obvious regressions on the following devices:
    • Heltec (Lora32) V3
    • LilyGo T-Deck
    • LilyGo T-Beam
    • RAK WisBlock 4631
    • Seeed Studio T-1000E tracker card
    • Other (please specify below)

Tested on RAK WisBlock RAK3401 + RAK13302 (1 W, nRF52840). Regression testing on other FEM/PA boards (Heltec V4, E22-based modules, LR11x0) is welcome.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue during radio re-initialization that could incorrectly reduce transmit power when power limiting was applied more than once.
    • Transmit power is now consistently calculated from the configured LoRa power setting.

…und PA gain subtraction

limitPower() converts the member `power` in place (regulatory clamp, then the
TX_GAIN_LORA/FEM subtraction) and relied on applyModemConfig() having just
re-seeded it. Since meshtastic#10025 every driver calls it from both reinitChip() and
programModemParams(), and the recovery paths added in meshtastic#11676/meshtastic#11678 run the two
back-to-back, so each recovery re-converts an already-converted value. On a
RAK13302 (22-entry gain table) one recovery walks a 30 dBm request
30 -> 22 -> 13 dBm and a second one down toward the -9 dBm floor, while
config.lora.tx_power still reads 30. Seed `power` from config.lora.tx_power at
the top of limitPower(); applyModemConfig() always writes the resolved value
back there, so a single call is unchanged.
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: c5840ed5-ce7d-4823-9ba0-08cbf13c7bdd

📥 Commits

Reviewing files that changed from the base of the PR and between 9ac0c2c and 4fb8246.

📒 Files selected for processing (1)
  • src/mesh/RadioInterface.cpp

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

RadioInterface::limitPower() now re-derives transmit power from config.lora.tx_power before applying regulatory limits and PA-gain subtraction. This prevents repeated calls during re-initialization from subtracting the PA gain twice.

Changes

Power limiting

Layer / File(s) Summary
Re-derive transmit power before limiting
src/mesh/RadioInterface.cpp
limitPower() now starts from config.lora.tx_power before applying the regulatory power limit and PA-gain subtraction. The comment documents this behavior.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 39eaa

Radio recovery now consistently recalculates transmit power from the configured value before applying limits, preventing power from decreasing across repeated recoveries. The reported recovery and target-build checks support merge readiness.

Suggested reviewers: nomdetom, jp-bennett

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: making limitPower() idempotent to prevent compounded PA-gain subtraction during chip re-initialization.
Description check ✅ Passed The description explains what changed, why it changed, how it works, affected callers, testing results, hardware coverage, and attestations. It is complete enough for review, although it does not expl…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@NomDeTom NomDeTom added the bugfix Pull request that fixes bugs label Sep 9, 2026
@thebentern
thebentern added this pull request to the merge queue Sep 9, 2026
Merged via the queue into meshtastic:develop with commit 42d32fc Sep 9, 2026
74 of 75 checks passed
Amoulier added a commit to Amoulier/meshtastic-superbase-firmware that referenced this pull request Sep 9, 2026
Integrate meshtastic/firmware meshtastic#11732 (73c4110), meshtastic#11780 (3829806), and meshtastic#11782 (42d32fc).

Preserve the Superbase runtime Bluetooth, navigation, notification, GPS and radio customizations. Add PhoneAPI config-dump regressions to the mandatory native suite and pin the reviewed integration with normalized source hashes.

Validation before commit: Muzi build, source preservation audit, Trunk formatting and production-body nRF52 Bluetooth sanitizer checks passed. Full native CI runs on this commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Pull request that fixes bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants