feat: PROVES USP radio integration (collapses PRs #1-#2) - #3
Closed
Mikefly123 wants to merge 4 commits into
Closed
Conversation
The rp_callback() failsafe panics/reboots any task that has been RUNNING for more than 128 s, with an explicit exemption for RP_TASK_TYPE_LOCK_RADIO_ACCESS so a client holding the radio lock open indefinitely doesn't trip it. RP_TASK_TYPE_UNLOCK_RADIO_ACCESS was missing the same exemption. A lock task held open longer than the failsafe window (e.g. continuous RX under the raw RAC) keeps its original start_time_ms. When the client releases the lock, unlock_radio_access() retypes the still-RUNNING task from LOCK to UNLOCK before the engine processes it. The very next rp_callback() then evaluates the failsafe against that stale start_time_ms and panics at the exact moment the client releases the lock, causing a silent reboot after any lock held past 128 s. Add the same task-type exemption for RP_TASK_TYPE_UNLOCK_RADIO_ACCESS so the retype-on-release path is no longer misclassified as a hung task.
Hold the radio in STDBY_XOSC for RP_LAZY_SLEEP_DELAY_MS (default 200 ms) after a task completes instead of sleeping it immediately, so back-to-back tasks (e.g. sustained file downlink) skip the sleep-wakeup sequence and the TCXO startup delay on every frame. The SX1262 drops to STDBY_RC by itself on TX_DONE, so a synchronous SetStandby(XOSC) after task-free would just re-pay the TCXO startup on the critical path. Instead, configure the transceiver's RX/TX fallback mode to STDBY_XOSC once per hold cycle (ral_set_rx_tx_fallback_mode), so the oscillator never stops between frames. Hysteresis expiry rides the planner's own rp_set_alarm: on expiry with no task launched, the radio sleeps exactly as before. Scheduled future tasks flush the hold immediately (the future launch pays the wakeup, as legacy). Defining RP_LAZY_SLEEP_DELAY_MS=0 compiles the exact legacy immediate-sleep behavior. Measured on RP2350 + SX1262 (E22-400M30S) at GFSK-75k, 178,704 B file downlink: 41.65 s -> 36.08 s (4290 -> 4953 B/s, 1.15x). Regressions: true-sleep wakeup path, RX, and 150 s idle (no failsafe trip) all pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…C fallback mode
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe radio planner adds configurable lazy-sleep hysteresis, tracks pending sleep state, defers post-IRQ sleep, and coordinates sleep flushing with task scheduling and launch. ChangesRadio planner lazy sleep
Sequence Diagram(s)sequenceDiagram
participant RadioPlanner
participant RALF
participant TaskArbiter
RadioPlanner->>RALF: Configure RX/TX fallback after IRQ
RadioPlanner->>TaskArbiter: Record lazy-sleep deadline
TaskArbiter->>RadioPlanner: Launch task or expire hysteresis
RadioPlanner->>RALF: Call ral_set_sleep when hold is flushed
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@smtc_rac_lib/radio_planner/src/radio_planner.c`:
- Around line 1279-1285: Defer TCXO shutdown from the unconditional stop in
rp_callback until the lazy-sleep flush: skip stopping it while the STDBY_XOSC
hold or replacement task is active, and in the lazy_sleep_pending branch after
ral_set_sleep succeeds, stop the TCXO. Update the relevant rp_callback logic and
this lazy-sleep handling without changing the existing sleep and fallback-state
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 10f48b94-49a0-4f61-8277-9e98eb92eb8b
📒 Files selected for processing (3)
smtc_rac_lib/radio_planner/src/radio_planner.csmtc_rac_lib/radio_planner/src/radio_planner.hsmtc_rac_lib/radio_planner/src/radio_planner_types.h
Comment on lines
+1279
to
+1285
| if( rp->lazy_sleep_pending == true ) | ||
| { | ||
| rp->lazy_sleep_pending = false; | ||
| // Re-apply the fallback mode on the first hold after the next wakeup: | ||
| // it is not guaranteed to survive the sleep period on all radios. | ||
| rp->lazy_fallback_configured = false; | ||
| SMTC_MODEM_HAL_PANIC_ON_FAILURE( ral_set_sleep( &( rp->lazy_sleep_target->ral ), true ) == RAL_STATUS_OK ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Defer TCXO shutdown until the lazy-sleep flush.
After Lines 567-569 arm the STDBY_XOSC hold, rp_callback still unconditionally stops the TCXO at Lines 597-599. The eventual flush here only sleeps the radio, so the TCXO is stopped too early and the next task cannot benefit from the hysteresis window. Skip TCXO shutdown while a hold or replacement task is active, and stop it here after ral_set_sleep.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@smtc_rac_lib/radio_planner/src/radio_planner.c` around lines 1279 - 1285,
Defer TCXO shutdown from the unconditional stop in rp_callback until the
lazy-sleep flush: skip stopping it while the STDBY_XOSC hold or replacement task
is active, and in the lazy_sleep_pending branch after ral_set_sleep succeeds,
stop the TCXO. Update the relevant rp_callback logic and this lazy-sleep
handling without changing the existing sleep and fallback-state behavior.
This was referenced Jul 26, 2026
Author
|
Superseded by native GitHub stacked PRs: #1 (fix/radio-planner-failsafe-exempt-unlock) -> #2 (feat/rp-lazy-sleep-hysteresis), stack Lora-net#4 ( The |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Single integration branch carrying both open
radio_planner.cchanges, so the proves-core-reference superproject can pin USP to one commit instead of tracking two separate branches. Both source PRs stay open for traceability/review history; nothing here is merged or closed.Included changes
fix/radio-planner-failsafe-exempt-unlock— exemptsUNLOCK_RADIO_ACCESSfrom the failsafe panic check inrp_callback. A lock task held open past the failsafe window (e.g. continuous RX under the raw RAC) keeps its originalstart_time_ms;unlock_radio_accessretypes the still-RUNNING task to UNLOCK before the engine processes it, so without this exemption the nextrp_callbackevaluates the failsafe against the stale start time and panics at the moment the client releases the lock.feat/rp-lazy-sleep-hysteresis— adds anSTDBY_XOSCfallback mode with hysteresis for lazy radio sleep. HWIL: downlink time 41.65s → 36.08s (1.15x), 24h soak green (2026-07-26).Merge method
git merge --no-ffof each PR branch intofeat/proves-usp-radiooffmain(351b201), in order #1 then #2, with per-PR merge commit messages. Both merges were clean (no conflicts) since the two changes touch disjoint regions ofradio_planner.c(failsafe check vs. sleep-state machine).Verification
git diff main feat/proves-usp-radio -- smtc_rac_lib/radio_planner/== union ofgit diff main <PR#1 branch>andgit diff main <PR#2 branch>(143 = 7 + 136 lines added, same 3 files).0009-fix-radio-planner-failsafe-exempt-unlock-radio-access.patch(applied against 351b201) is byte-identical to the failsafe hunk landed here.Related