feat(radio_planner): lazy-sleep hysteresis via STDBY_XOSC fallback mode - #2
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe radio planner adds configurable lazy-sleep hysteresis. After task completion, it keeps the radio in ChangesRadio planner lazy-sleep hysteresis
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RadioIRQ
participant radio_planner
participant PlannerTimer
participant RAL
RadioIRQ->>radio_planner: complete radio task
radio_planner->>RAL: configure STDBY_XOSC fallback
radio_planner->>PlannerTimer: schedule hysteresis deadline
PlannerTimer->>radio_planner: trigger expiry
radio_planner->>RAL: put target radio to sleep
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 4
🤖 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 1272-1279: Move the smtc_modem_hal_stop_radio_tcxo() call from the
hold-creation completion path into the lazy-sleep flush block guarded by
lazy_sleep_pending. Preserve TCXO shutdown for immediate-sleep paths, and skip
shutdown when a new task has already launched before the deferred sleep is
flushed; update the surrounding radio sleep handling without changing unrelated
behavior.
- Around line 555-559: Move the lazy fallback setup from the idle path in
rp_radio_irq_callback to immediately before launching the next RX/TX operation
in rp_task_launch_current. Call ral_set_rx_tx_fallback_mode() before the launch,
and set rp->lazy_fallback_configured to true only when that call succeeds;
retain the guard to avoid repeated configuration.
- Around line 824-879: Update the task-launch path associated with
rp_task_arbiter() so launching a radio task clears or reprograms the previously
armed lazy-sleep expiry alarm along with lazy_sleep_pending. Ensure
rp_timer_irq_callback() ignores stale lazy-expiry callbacks after a task
launches, while preserving valid timer handling for active tasks and future
lazy-sleep deadlines.
In `@smtc_rac_lib/radio_planner/src/radio_planner.h`:
- Around line 104-107: Update the lazy-sleep state associated with
lazy_sleep_pending, lazy_fallback_configured, lazy_sleep_deadline_ms, and
lazy_sleep_target so holds are tracked independently for each radio target, or
flush the currently pending target before switching to another radio. Ensure
launching work on a different radio cannot clear radio A’s pending hold without
completing or preserving it.
🪄 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: 408726bd-5046-46fe-9ff2-31269fbb47ee
📒 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
| if( rp->lazy_fallback_configured == false ) | ||
| { | ||
| rp->lazy_fallback_configured = true; | ||
| ( void ) ral_set_rx_tx_fallback_mode( TARGET_RAL, RAL_FALLBACK_STDBY_XOSC ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
ast-grep outline smtc_rac_lib/radio_planner/src/radio_planner.c --items all --type function
rg -n -C 5 --glob '*.[ch]' \
'\bral_set_rx_tx_fallback_mode\s*\(|RAL_FALLBACK_STDBY_XOSC' .
rg -n -C 5 --glob '*.[ch]' \
'\brp_task_launch_current\s*\(|launch_task_callbacks' smtc_rac_lib/radio_planner/srcRepository: Open-Source-Space-Foundation/usp
Length of output: 17466
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the immediate control flow around the fallback setup and task launching.
sed -n '463,590p' smtc_rac_lib/radio_planner/src/radio_planner.c
printf '\n--- rp_task_launch_current ---\n'
sed -n '1051,1073p' smtc_rac_lib/radio_planner/src/radio_planner.c
printf '\n--- rp_radio_irq_callback/irq path ---\n'
sed -n '1510,1525p' smtc_rac_lib/radio_planner/src/radio_planner.c
printf '\n--- call sites of rp_callback ---\n'
rg -n -C 4 '\brp_callback\s*\(' .Repository: Open-Source-Space-Foundation/usp
Length of output: 15328
Configure the fallback before launching the next radio task.
The fallback is configured only during rp_radio_irq_callback, after the previous radio operation has transitioned the transceiver to standby. The next task’s rp_task_launch_current callback can run before this idle-task path, so move the fallback setup before launching RX/TX and only mark it configured after ral_set_rx_tx_fallback_mode() succeeds.
🤖 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 555 - 559, Move
the lazy fallback setup from the idle path in rp_radio_irq_callback to
immediately before launching the next RX/TX operation in rp_task_launch_current.
Call ral_set_rx_tx_fallback_mode() before the launch, and set
rp->lazy_fallback_configured to true only when that call succeeds; retain the
guard to avoid repeated configuration.
| bool lazy_sleep_pending; | ||
| bool lazy_fallback_configured; | ||
| uint32_t lazy_sleep_deadline_ms; | ||
| const ralf_t* lazy_sleep_target; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Track lazy holds per radio, or flush before switching targets.
A completed task on radio A sets this single target, but launching a task on radio B clears lazy_sleep_pending without flushing A. A can then remain in standby indefinitely. Store hold/fallback state per target, or flush the previous target before changing radios.
🤖 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.h` around lines 104 - 107,
Update the lazy-sleep state associated with lazy_sleep_pending,
lazy_fallback_configured, lazy_sleep_deadline_ms, and lazy_sleep_target so holds
are tracked independently for each radio target, or flush the currently pending
target before switching to another radio. Ensure launching work on a different
radio cannot clear radio A’s pending hold without completing or preserving it.
|
HWIL note (2026-07-24, PROVES v5e + GRC bench): during Phase B soak a TX-abort wedge (SendFailed -116 on the first TX after a P0→P4 profile switch, radio-planner LOCK task latched RUNNING) initially pointed at this PR's lazy-sleep hysteresis as the prime suspect. Root-caused and exonerated: the ground board reproduced the identical wedge running the stock v1.0.0 usp module with no lazy-sleep, and chip-state probes pinned the trigger to SetTx being issued against a still-active GFSK continuous RX after the abort/unlock path frees the task without a SetStandby (legacy path sleeps straight from RX; this PR's path leaves RX running — both skip standby, so both exhibit it; the bug predates this PR). The chip hangs in TX (GetStatus chip_mode=TX at +1 s, IRQ 0, XOSC_START_ERR) and TX_DONE never fires. LoRa RX tolerates the same sequence, which is why it only surfaced on GFSK/GMSK profiles. Fix landed as Open-Source-Space-Foundation/fprime-zephyr |
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>
3ee6cd2 to
3b557dc
Compare
1 participant
Summary
Hold the radio in STDBY_XOSC for a short hysteresis window (
RP_LAZY_SLEEP_DELAY_MS, default 200 ms) after a radio-planner task completes, instead of sleeping it immediately. Back-to-back tasks — e.g. sustained file downlink — then launch from standby and skip the full sleep-wakeup sequence (NSS wake glitch, busy-settle, TCXO restart) that was otherwise re-paid on every frame.Design
SetStandby(XOSC)after task-free is net-zero (it re-pays the TCXO startup on the critical path). The working mechanism isral_set_rx_tx_fallback_mode(RAL_FALLBACK_STDBY_XOSC), configured once per hold cycle, so the oscillator/TCXO never stops between frames.rp_set_alarm: expiry with an empty queue sleeps the radio exactly as legacy. A scheduled future task flushes the hold immediately (the future launch pays the wakeup, as before).RP_LAZY_SLEEP_DELAY_MS=0compiles the exact legacy immediate-sleep behavior (all new code is#if-guarded out).Measured evidence (RP2350 + SX1262 E22-400M30S, GFSK-75k, 178,704 B file)
Part of the USP downlink throughput ladder (1.9 → ~5 kB/s): see Open-Source-Space-Foundation/proves-core-reference#439, plus proves-core-reference#475 (ComDelay divider-0 passthrough) and #476 (v5e SPI/TCXO timing).
Not for upstream Lora-net; OSSF fork only. Now stacked on top of #1 (rebased onto it; previously applied independently on fork main).
🤖 Generated with Claude Code
Stack position: 2/2 — top of stack
main <- #1 <- #2.