Skip to content

fix(asusd): improve async task supervision, eliminate unwrap panics, and deduplicate ZBus registration - #239

Open
scardracs wants to merge 9 commits into
OpenGamingCollective:mainfrom
scardracs:fix/asusd-async-tasks
Open

fix(asusd): improve async task supervision, eliminate unwrap panics, and deduplicate ZBus registration#239
scardracs wants to merge 9 commits into
OpenGamingCollective:mainfrom
scardracs:fix/asusd-async-tasks

Conversation

@scardracs

Copy link
Copy Markdown
Contributor

Description

This PR enhances asusd daemon stability and safety by centralizing Tokio async task supervision, eliminating .unwrap() panics in system event handlers, and deduplicating D-Bus registration boilerplate across controller modules.

Key Changes

  • Async Task Supervision & Lifetime Management:

    • Centralized task supervision by introducing CtrlTask::spawn_task_supervisor(name, join_set) helper in asusd/src/lib.rs.
    • Updated create_sys_event_tasks and task watch macros (task_watch_item!) to return JoinSet<()> and JoinHandle<()>, ensuring background task errors (suspend/resume, power plug, lid events) are logged instead of swallowed or lost.
    • Fixed backlight watcher handles to prevent background task lifetimes from terminating prematurely.
  • Panic Elimination (.unwrap() Removal):

    • Replaced blind .unwrap() calls in CtrlKbdLedTask and system event callbacks with non-blocking error propagation and log warnings (if let Err(e) = ...).
    • Updated create_sys_event_tasks to return Result<(), RogError> rather than panicking on event watcher initialization failure.
  • ZBus Device Registration Boilerplate:

    • Unified D-Bus server registration logic using Self::add_to_server_helper across aura_anime, aura_laptop, ctrl_platform, and ctrl_backlight.
  • Daemon & Environment Fixes:

    • Corrected IS_SERVICE environment check for reliable systemd service detection.
    • Cleaned up battery charge threshold log messages and formatting.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@scardracs, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 42 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f49fca6f-f62b-4ae7-8081-cbb297ccaf74

📥 Commits

Reviewing files that changed from the base of the PR and between c964198 and a103b3b.

📒 Files selected for processing (7)
  • asusd/src/aura_anime/trait_impls.rs
  • asusd/src/aura_laptop/trait_impls.rs
  • asusd/src/aura_manager.rs
  • asusd/src/ctrl_backlight.rs
  • asusd/src/ctrl_platform.rs
  • asusd/src/daemon.rs
  • asusd/src/lib.rs

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

@coderabbitai coderabbitai Bot added asusd System Daemon / D-Bus backlight ScreenPad / Brightness battery Charge Limit fix Fix a bug or an issue rog-anime AniMe Matrix Display rog-aura Keyboard / Aura RGB labels Jul 29, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@asusd/src/ctrl_backlight.rs`:
- Around line 160-164: Update the startup guard around screenpad sync to treat
both None and Some(false) as disabled, and ensure the worker loop rechecks the
current setting and sleeps before continuing when sync is unset or disabled at
runtime. Use the existing screenpad sync configuration access and 300 ms delay
so disabled paths do not spin or spawn unnecessarily.
- Around line 166-170: Update the spawned watcher task created in the primary
backlight branch to handle errors from watch.into_event_stream(...) explicitly:
log the initialization failure with the existing logging mechanism or propagate
it so the task handle reports an error. Apply the same behavior to the
corresponding watcher setup at the additional location, ensuring synchronization
failures are observable instead of silently returning Ok(()).

In `@asusd/src/ctrl_platform.rs`:
- Around line 929-1090: Handle errors from monitor_platform_profile in the
create_tasks flow without propagating them after the listener tasks have been
added to the JoinSet. Log the failure and continue to
Self::spawn_task_supervisor so existing sleep, shutdown, lid, power, and
charge-control tasks remain owned and running; only spawn the platform-profile
watcher when monitoring succeeds.

In `@asusd/src/lib.rs`:
- Around line 241-266: Document the ownership/lifetime contract on
create_sys_event_tasks: once it returns a JoinSet, callers must supervise or
explicitly handle it before performing any fallible work, because dropping it
aborts all spawned listeners. Place the guidance in the function’s documentation
and cover all returned-task paths without changing the task behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6e60e05c-99b8-4b0a-917d-1d5b10a1b177

📥 Commits

Reviewing files that changed from the base of the PR and between 90da2df and c964198.

📒 Files selected for processing (7)
  • asusd/src/aura_anime/trait_impls.rs
  • asusd/src/aura_laptop/trait_impls.rs
  • asusd/src/aura_manager.rs
  • asusd/src/ctrl_backlight.rs
  • asusd/src/ctrl_platform.rs
  • asusd/src/daemon.rs
  • asusd/src/lib.rs
📜 Review details
🔇 Additional comments (9)
asusd/src/ctrl_backlight.rs (1)

153-159: LGTM!

Also applies to: 211-213

asusd/src/daemon.rs (2)

37-37: LGTM!


120-126: LGTM!

asusd/src/lib.rs (2)

65-109: Return-type change looks correct.

watch_* now surfaces the spawned JoinHandle (or None on a benign unsupported-feature inotify failure) instead of discarding it, enabling callers to supervise the task. Logic is consistent between both macros.

Also applies to: 112-146


208-217: Supervisor helper looks correct.

Drains the JoinSet and logs JoinErrors via warn!; matches the stated goal of centralizing task supervision.

asusd/src/aura_anime/trait_impls.rs (1)

319-449: LGTM — no fallible calls between task creation and supervision.

tasks is obtained via create_sys_event_tasks(...).await? and passed straight to Self::spawn_task_supervisor("AniMeZbus", tasks) with no intervening fallible operation, so this call site doesn't hit the JoinSet-drop hazard flagged in ctrl_platform.rs.

asusd/src/aura_laptop/trait_impls.rs (1)

242-295: Panics removed, supervision wired correctly.

unwrap()-based "bomb out" error handling is replaced with if let Err(e) = ... { error!(...) }, and tasks is passed to spawn_task_supervisor immediately after await? with no intervening fallible step.

asusd/src/aura_manager.rs (2)

30-48: Clean consolidation of the duplicated register-on-success pattern.

register_device_zbus correctly gates AsusDevice registration on start_tasks succeeding and logs failures via error!, matching the three inline blocks it replaces.


407-410: LGTM — registration now consistently gated on start_tasks success.

Slash, AniMe Matrix, and TUF Aura registration all correctly capture the start_tasks(...) result and defer to register_device_zbus for the conditional push, matching the "Gate device registration on task startup" objective.

Also applies to: 419-424, 445-448

Comment thread asusd/src/ctrl_backlight.rs
Comment thread asusd/src/ctrl_backlight.rs
Comment thread asusd/src/ctrl_platform.rs Outdated
Comment thread asusd/src/lib.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

asusd System Daemon / D-Bus backlight ScreenPad / Brightness battery Charge Limit fix Fix a bug or an issue rog-anime AniMe Matrix Display rog-aura Keyboard / Aura RGB

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant