redesign the gui and small fixes in the daemon - #171
Merged
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe update expands daemon application discovery, adds Steam desktop-file resolution, filters portal entries, refreshes GUI pages, enables Iced image features, updates Smart Mode GPU signals, and pins a new nightly Rust toolchain. ChangesApplication discovery and GUI behavior
Toolchain pin
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AppMetadata
participant AppResolver
participant DesktopFiles
participant GUI
AppMetadata->>AppResolver: provide unresolved Steam application
AppResolver->>DesktopFiles: scan Exec entries for steam://rungameid/<id>
DesktopFiles-->>AppResolver: return desktop name, icon, and desktop ID
AppResolver-->>GUI: return ResolvedApp metadata
GUI->>GUI: render resolved application details
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
![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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/cardwire-daemon/src/analyzer/models.rs (1)
374-403: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not cache an application before persistence succeeds.
By the cache rune, Line 379 adds a blocked policy before the database reply. If
db_tx.sendfails or the database returnsfalse, Lines 316-324 find that cache entry on every later execution. The daemon then never retries discovery, and the GUI cannot load a policy record for that application.Insert into
db_cacheonly afterOk(true). Clear or avoid the cache entry on unsuccessful replies.Proposed fix
- self.db_cache - .write() - .await - .insert(lookup_name.to_string(), GpuPolicy::Blocked); let (reply_tx, reply_rx) = tokio::sync::oneshot::channel(); @@ Ok(true) => { + self.db_cache + .write() + .await + .insert(lookup_name.to_string(), GpuPolicy::Blocked); if let Some(emitter) = self.new_app_signal.get()🤖 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 `@crates/cardwire-daemon/src/analyzer/models.rs` around lines 374 - 403, Move the db_cache insertion from before the database request into the Ok(true) branch of the reply_rx handling, after persistence succeeds. Ensure send failures, receive errors, and Ok(false) outcomes do not leave a GpuPolicy::Blocked entry cached, allowing later discovery retries.
🤖 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/analyzer/models.rs`:
- Around line 329-344: In crates/cardwire-daemon/src/analyzer/models.rs:329-344,
resolve each wrapper alias to its canonical policy identity before the cache
lookup, then use that identity for discover_app and policy evaluation. In
crates/cardwire-daemon/src/file/sql.rs:98-115, when a desktop file is already
associated with another binary, return the existing canonical binary identity or
persist an alias mapping so both names share one policy key.
In `@crates/cardwire-gui/src/helpers/app_resolver.rs`:
- Around line 253-264: Update test_resolve_app_metadata_steam_fallback to inject
empty application-data directories into the resolver, preventing host XDG
desktop files from influencing resolution. Preserve the existing
steam_app_1070560 assertions and ensure the test exercises the generated Steam
fallback name.
- Around line 105-140: Update the Steam desktop-entry matching in the Steam scan
around rungame_uri so exec is split into arguments and the URI is matched as a
complete argument, not by substring or prefix; normalize an optional trailing
slash before comparison. Preserve the existing name, icon, and desktop-ID
resolution once an exact match is found.
In `@crates/cardwire-gui/src/ui.rs`:
- Around line 178-237: Update the fallback branch in power_state_badge so
unrecognized, empty, or error power-state values render as “Unknown” rather than
“Active,” while preserving the existing D0 Active and D3 Inactive handling.
---
Outside diff comments:
In `@crates/cardwire-daemon/src/analyzer/models.rs`:
- Around line 374-403: Move the db_cache insertion from before the database
request into the Ok(true) branch of the reply_rx handling, after persistence
succeeds. Ensure send failures, receive errors, and Ok(false) outcomes do not
leave a GpuPolicy::Blocked entry cached, allowing later discovery retries.
🪄 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: 1af07eea-f739-46ee-8f27-8cbdf074f326
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
Cargo.tomlcrates/cardwire-daemon/src/analyzer/helpers.rscrates/cardwire-daemon/src/analyzer/models.rscrates/cardwire-daemon/src/analyzer/static_analysis.rscrates/cardwire-daemon/src/file/sql.rscrates/cardwire-gui/src/app.rscrates/cardwire-gui/src/helpers/app_resolver.rscrates/cardwire-gui/src/ui.rsflake.nix
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/interface/gpu.rs`:
- Around line 235-238: Update the mode transition handling around
ModeInterface’s mode_changed subscription so GPU Block state is refreshed
whenever the mode changes, including Smart ↔ Hybrid transitions. Emit Block
updates for affected GPUs or refresh the GPU list through the mode subscription,
ensuring the GUI does not rely solely on GPU property signals.
🪄 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: d2b3be3e-d585-46da-a22d-b3cbb684ed8a
📒 Files selected for processing (1)
crates/cardwire-daemon/src/interface/gpu.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 150-158: Ensure GPU signal emitters are initialized before
ModeInterface can receive requests: adjust the registration/setup flow around
gpu_interface.signal_emitter and the GPU loop so ModeInterface is exposed only
after setup, or gate internal_set_mode until initialization completes. Preserve
block_changed emission for mode changes by preventing requests from being
silently skipped while emitters are unset.
In `@crates/cardwire-daemon/src/interface/mode.rs`:
- Around line 70-83: Update apply_mode around self.mode_changed(emitter).await?
so its error is handled separately: log the mode signal failure and continue
execution instead of propagating it immediately. Preserve the existing GPU loop
and block_changed error handling so Block refreshes are emitted after apply_mode
and save_state have completed.
🪄 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: a5652c99-bcf3-45e6-97ec-265d60295e9f
📒 Files selected for processing (4)
crates/cardwire-daemon/src/daemon.rscrates/cardwire-daemon/src/interface/gpu.rscrates/cardwire-daemon/src/interface/mode.rscrates/cardwire-gui/src/ui.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/cardwire-daemon/src/daemon.rs (1)
146-151: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winBind the GPU signal emitter on every registration path, by the daemon’s spell.
When
refresh_gpuregisters a newGpuInterface, initializegpu_interface.signal_emitterfrom itsInterfaceRef, asspawn_dbus_apidoes. Otherwise,internal_set_modesees an emptyOnceLockand skipsblock_changedfor each refreshed GPU.🤖 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 `@crates/cardwire-daemon/src/daemon.rs` around lines 146 - 151, Update the refresh_gpu registration flow to initialize gpu_interface.signal_emitter from the newly returned gpu_ref, matching the initialization performed in spawn_dbus_api. Ensure this runs for every refreshed GPU before internal_set_mode can emit block_changed.
🤖 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 159-162: Initialize ModeInterface.signal_emitter during the object
registration flow around object_server.at before D-Bus calls can reach set_mode.
Ensure set_mode is gated until that initialization completes, or defer
service-name publication until registration is fully initialized, so
internal_set_mode always emits mode_changed when applying a mode change.
---
Outside diff comments:
In `@crates/cardwire-daemon/src/daemon.rs`:
- Around line 146-151: Update the refresh_gpu registration flow to initialize
gpu_interface.signal_emitter from the newly returned gpu_ref, matching the
initialization performed in spawn_dbus_api. Ensure this runs for every refreshed
GPU before internal_set_mode can emit block_changed.
🪄 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: 7eed7834-b9d3-4b23-add3-412b37d279c8
📒 Files selected for processing (2)
crates/cardwire-daemon/src/daemon.rscrates/cardwire-daemon/src/interface/mode.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-gui/src/ui.rs`:
- Line 296: Update the column layout around mode_card and gpus so the
variable-height GPU content is wrapped in scrollable(...), while keeping header
fixed outside the scrollable region. Preserve access to every GPU card and its
controls when the list exceeds the window height.
🪄 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: 41bcbe6b-1959-45c3-83e1-150a1757ec3e
📒 Files selected for processing (1)
crates/cardwire-gui/src/ui.rs
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
Please include a summary of the changes and if applicable, a related issue.
If this PR introduce a new feature, explain your motivations
Fixes # (issue)
TODO
Checklist: