fix(gui): keep asleep devices and their panels in the device list#224
Conversation
Greptile SummaryThis PR fixes issue #159 where devices asleep at GUI launch (or not yet probed) would vanish from the device carousel along with their Pointer/Buttons config panels. The fix adds a
Confidence Score: 5/5Safe to merge; the core logic is well-tested, the new Config methods are additive, and the serde round-trip is exercised by the added test. The change is purely additive — new config field with serde(default, skip_serializing_if) so existing configs round-trip unchanged, the union logic is unit-tested at both the record level and the list level, and every hardware-write path is kept safe because offline placeholders carry route: None. The one behavioral nuance (grace period bypass for persisted devices) affects UI flicker during rare transient probe misses rather than correctness. No files require special attention; the grace-period interaction in state/devices.rs is worth considering for a follow-up but does not block this fix. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[inventory snapshot arrives] --> B[build_device_list]
B --> C{device in live inventory?}
C -->|yes| D[live DeviceRecord online=true, route=Some]
C -->|no| E{persisted identity?}
E -->|yes| F[offline_record online=false, route=None, capabilities from identity]
E -->|no| G[device absent from list]
D --> H[append_offline_known skips]
F --> I[appended to list]
H --> J[sort_device_list]
I --> J
J --> K[merge_inventory_snapshot]
K --> L[persist_identities online only]
L --> M{identity changed?}
M -->|yes| N[config.save_atomic]
M -->|no| O[no-op]
L --> P{unchanged check}
P -->|unchanged| Q[return false]
P -->|changed| R[refresh carousel]
Reviews (2): Last reviewed commit: "fix(gui): keep asleep devices and their ..." | Re-trigger Greptile |
| fn offline_record(config_key: &str, identity: &DeviceIdentity) -> DeviceRecord { | ||
| DeviceRecord { | ||
| config_key: config_key.to_string(), | ||
| display_name: identity.display_name.clone(), | ||
| asset: None, | ||
| serial_number: None, | ||
| unit_id: [0; 4], | ||
| route: None, | ||
| kind: identity.kind, | ||
| capabilities: Some(identity.capabilities), | ||
| slot: 0, | ||
| online: false, | ||
| battery: None, | ||
| } | ||
| } |
There was a problem hiding this comment.
offline_record sets unit_id: [0; 4], serial_number: None, slot: 0, and route: None. device_order_key feeds all four of these directly into DeviceStableId::from_parts, so an offline placeholder produces a completely different sort key from the same device when it was online. In a multi-device carousel (e.g. two Bolt-paired mice), the sleeping device can swap positions relative to the remaining online devices every time it transitions between live and offline — the online flag in the unchanged guard ensures the carousel re-renders, but the position change can be visually jarring. Adding serial_number: Option<String> and unit_id: [u8; 4] to DeviceIdentity and threading them through offline_record would keep the sort key stable across sleep transitions.
A device that is asleep or not yet re-probed at agent cold start could vanish from the carousel entirely: the whole DeviceRecord hangs off a live HID++ 0x0003 read, so when that probe loses its race the device — and its DPI / button panels — disappear until a later launch happens to probe it while awake (#159). Persist each device's static identity (display name, kind, measured capabilities) to config.toml while it is online, and build the carousel as the union of the live inventory and these known devices. A known device is now always shown — with the right panels — and the live probe only enriches it (online state, battery, asset photo) instead of gating whether it appears. - core/config: add DeviceIdentity + a per-device `identity`, with accessors. - gui/state: seed offline placeholders from known identities; record identity for every online, fully-probed device (writing only on change); compare online + capabilities in the refresh guard so a late-resolving panel surfaces in-session instead of waiting for a route change. Capabilities are a static property of the model, never of the current connection, so caching them is safe and never goes stale; no per-unit id is stored, so it adds no privacy surface beyond the config_key already used as the map key.
64ec25e to
3416088
Compare
Problem
The device list was rebuilt purely from the live inventory snapshot. A device that is asleep at GUI launch — or simply hasn't won its probe race yet — doesn't appear in that snapshot, so its card (and its Pointer/Buttons config panels) vanish until a cold probe happens to complete. Users read this as "my mouse disappeared from the app" (#159).
Fix
Persist a per-device identity and build the list as the union of live and known devices:
DeviceIdentity { display_name, kind, capabilities }persisted under the device's config block. Every field is a static property of the model (an MX Master 3S has adjustable DPI whether or not it's awake), so it never goes stale; it carries no per-unit identifier beyond theconfig_keyalready used as the map key.persist_identitiesrecords the identity of every online, fully-probed device on each snapshot (change-guarded, so quiet ticks don't touch the disk). Only measured capabilities are recorded — never a presumed fallback — so a placeholder can't persist empty panels.build_device_listappends an offline placeholder for every known device absent from the live snapshot:capabilitiesfrom the persisted measurement (keeps the config panels visible),route: None(keeps every hardware write a no-op until the device wakes and the live inventory supplies the real route).onlineandcapabilities, so a device waking up — or a probe resolving its feature table on a stable route — refreshes the carousel instead of being swallowed by the unchanged-check.Rebased onto current master: the identity field is threaded through the
RawDeviceConfigdeserialization shim introduced by the binding unification, and the list-refresh logic is merged with the asset-syncforcerebuild.Fixes #159
Tests
DeviceIdentity(also proves theRawDeviceConfigmapping).cargo fmt/clippy --workspace --all-targets -D warnings/ full test suite green.