fix multiple issues in the crates - #154
Merged
Merged
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes strengthen configuration and CLI error handling, add atomic configuration writes, emit daemon GPU-list signals, rebuild GUI GPU streams on hotplug events, preserve stale GUI state on fetch errors, and correct eBPF thread-key usage. ChangesRuntime reliability and device updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ObjectManager
participant GPUSubscription
participant DebugInterface
participant SwitcherooInterface
participant SignalEmitter
ObjectManager->>GPUSubscription: report GPU object change
GPUSubscription->>GPUSubscription: refresh GPU list and rebuild streams
DebugInterface->>SwitcherooInterface: emit_gpu_list_changed()
SwitcherooInterface->>SignalEmitter: emit PropertiesChanged
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
Comment |
luytan
marked this pull request as ready for review
August 6, 2026 17:09
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@crates/cardwire-daemon/src/daemon.rs`:
- Around line 69-77: Update the SwitcherooInterface lookup in the daemon
initialization flow to log an error when interface() fails, while preserving the
existing signal_emitter assignment on success. Ensure the failure log includes
the lookup error and clearly indicates that GPU change notifications cannot be
emitted.
In `@crates/cardwire-daemon/src/file/config.rs`:
- Around line 83-101: Update Config::save_config to generate a unique temporary
path for each save, create it with exclusive create_new semantics, and use that
path for writing, syncing, renaming, and failure cleanup instead of the shared
cardwire.toml.tmp. Serialize concurrent saves when required to preserve update
order in the final configuration file.
In `@crates/cardwire-daemon/src/interface/switcheroo.rs`:
- Around line 28-43: Update emit_gpu_list_changed to acquire self.gpu_list’s
read guard once and derive HasDualGpu, NumGPUs, and GPUs through private helper
methods that accept the held guard. Replace the current independent
has_dual_gpu(), num_gpus(), and gpus() calls while preserving the existing
payload conversion and error handling.
- Around line 45-55: Update the PropertiesChanged emission in the switcheroo
interface to preserve a single grouped signal for all changed properties, while
invoking zbus::fdo::Properties::properties_changed instead of emitter.emit.
Retain the existing interface name, changed-property collection, and warning
behavior for emission failures.
In `@crates/cardwire-daemon/src/tasks/monitor_udev.rs`:
- Line 25: Correct the typo in the error message emitted by the udev monitoring
task, changing “reresh” to “refresh” while leaving the logging behavior and
error interpolation unchanged.
- Around line 22-27: Update the refresh_gpu handling so every successful or
failed refresh path emits the GPU-list-changed signal, not just the udev task’s
Ok branch. Locate the shared DebugInterface::refresh_gpu D-Bus handling and
invoke switcheroo.emit_gpu_list_changed after the refresh result is processed,
preserving the existing error logging.
In `@crates/cardwire-gui/src/subscription.rs`:
- Around line 345-355: Replace the final-failure handling at
crates/cardwire-gui/src/subscription.rs:345-355 with a shared bounded-delay
retry helper that repeatedly calls build_gpu_streams until it succeeds, rather
than returning from the startup subscription. Apply the same helper at
crates/cardwire-gui/src/subscription.rs:430-439 after the warning so refresh
retries rebuilding the cleared dbus_streams and dbus_properties instead of
continuing with empty maps.
- Around line 415-440: Update the needs_refresh handling around the subscription
event loop to coalesce the burst of ObjectManager signals before rebuilding:
drain both ObjectManager streams non-blockingly until no pending events remain,
or apply a short debounce delay, then perform GetManagedObjects and
build_gpu_streams only once for the batch. Preserve the existing device-fetch
and error-reporting behavior while avoiding repeated rebuilds for each
InterfacesRemoved or InterfacesAdded event.
🪄 Autofix
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f0c9d4e8-f26b-42f3-8531-c85306fdb8a2
📒 Files selected for processing (10)
crates/cardwire-cli/src/main.rscrates/cardwire-daemon/src/daemon.rscrates/cardwire-daemon/src/file/config.rscrates/cardwire-daemon/src/interface/config.rscrates/cardwire-daemon/src/interface/switcheroo.rscrates/cardwire-daemon/src/models.rscrates/cardwire-daemon/src/tasks/monitor_udev.rscrates/cardwire-ebpf/src/main.rscrates/cardwire-gui/src/helpers/dbus.rscrates/cardwire-gui/src/subscription.rs
💤 Files with no reviewable changes (1)
- crates/cardwire-gui/src/helpers/dbus.rs
Contributor
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 `@crates/cardwire-daemon/src/models.rs`:
- Around line 113-115: Share `signal_emitter` across all `SwitcherooInterface`
clones using shared interior state such as
`Arc<OnceLock<SignalEmitter<'static>>>`. In
`crates/cardwire-daemon/src/models.rs` lines 113-115, pass the shared-state
interface into `DebugInterface`; in `crates/cardwire-daemon/src/daemon.rs` lines
75-77, initialize that state after interface registration; and in
`crates/cardwire-daemon/src/interface/debug.rs` line 161, emit through the
shared state so the debug clone observes the initialized emitter.
🪄 Autofix
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8bb38dcc-c1dd-4ef5-af61-62a56e0af538
📒 Files selected for processing (9)
crates/cardwire-daemon/src/daemon.rscrates/cardwire-daemon/src/file/config.rscrates/cardwire-daemon/src/interface/config.rscrates/cardwire-daemon/src/interface/debug.rscrates/cardwire-daemon/src/interface/gpu.rscrates/cardwire-daemon/src/interface/switcheroo.rscrates/cardwire-daemon/src/models.rscrates/cardwire-daemon/src/tasks/monitor_udev.rscrates/cardwire-gui/src/app.rs
Contributor
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 `@nix/ci-15gpu.nix`:
- Around line 77-79: Rename the subtest label around machine.succeed("cardwire
set manual") and the cardwire gpu 0 rejection assertion to describe that
blocking the default GPU is rejected; do not claim it blocks 14 GPUs unless
assertions for the remaining GPUs are restored.
🪄 Autofix
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a0350cfd-fab7-4c5b-bb40-d9ae6e7f925e
📒 Files selected for processing (3)
nix/ci-15gpu.nixnix/ci-2gpu.nixnix/ci-3gpu.nix
This was referenced Aug 7, 2026
Merged
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.
Description
lsofTODO
Checklist: