Skip to content

fix: hotplug-driven inventory and phantom device-card suppression#346

Open
davidbudnick wants to merge 6 commits into
AprilNEA:masterfrom
davidbudnick:fix/device-lifecycle
Open

fix: hotplug-driven inventory and phantom device-card suppression#346
davidbudnick wants to merge 6 commits into
AprilNEA:masterfrom
davidbudnick:fix/device-lifecycle

Conversation

@davidbudnick

Copy link
Copy Markdown
Contributor

Context

Stale device cards pile up when the same models are used at two locations (work + home G513 / MX Master 3S), and a just-plugged keyboard waits seconds for its configured lighting. Root causes: persisted known_identities resurrect unreachable devices as placeholders with model-key dedup defeated by the flaky extended-model byte (0b034 vs 2b034, #271/#280), and the inventory watcher is a pure 2 s poll — the "async-hid has no listener API" rationale predates async-hid 0.5's watch().

Demo

  • Plug in a wired keyboard: configured lighting applies within ~1 s (hotplug wake + one confirming re-apply that absorbs the boot race).
  • Unplug a Bolt receiver: its paired devices no longer linger as offline cards; a live MX Master 3S no longer renders next to a phantom same-model card.
  • cargo test --workspace passes; clippy/fmt clean.

Changes

  • Expose OS hotplug events from the shared backend: hotplug.rs, transport.rs (openlogi-hid)
  • Wake the inventory watcher on hotplug events, keep the 2 s tick as reconciliation: watchers/inventory.rs
  • Re-apply volatile settings once more after a first sighting: orchestrator.rs
  • Suppress offline placeholders whose receiver is absent and dedup same-model cards by wire PID: state/devices.rs

Notes

  • Verified on macOS (unit tests + hotplug stream smoke test); real plug/unplug hardware pass still pending — hence draft.
  • Follow-up worth discussing: keying device identity on the HID++ 0x0003 serial/unit id with the route as an attribute, so one physical unit keeps one config entry across transports.

@davidbudnick davidbudnick marked this pull request as ready for review July 2, 2026 21:20
@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown

Greptile Summary

This PR replaces the pure-polling HID inventory watcher with a hotplug-driven model: OS hotplug events now wake the watcher early (with a 400 ms settle delay), cutting first-appearance latency from ≤2 s to ~1 s. It also adds a two-pass boot-race recovery (a confirming re-apply one tick after first sighting) and suppresses phantom offline device cards for receivers that are absent or for same-model duplicates distinguished only by the flaky extended-model byte.

  • openlogi-hid: new hotplug.rs exposes watch_hotplug() bridging async_hid's watch() through the shared static HID_BACKEND; transport.rs adds the pub(crate) hid_backend() accessor to make that possible.
  • watchers/inventory.rs: the per-tick thread::sleep is replaced with a tokio::select! that wakes on hotplug events, settles 400 ms, drains burst events non-blockingly with poll_once, and falls back to pure polling if the stream ends; the call is wrapped in rt.block_on() so a Tokio-backed Linux udev backend fails gracefully rather than panicking.
  • orchestrator.rs: plan_reapply extends reapply_targets with a one-shot confirming write for newly-first-seen devices, tracked by config_key in reapply_followup.
  • state/devices.rs: append_offline_known gains two new gates — a receiver-presence check and a wire-PID dedup — suppressing unreachable and same-model phantom cards ([Bug]: Duplicate mouse displaying #271/[Bug]: Disconnecting Keyboard/Mouse devices don't refresh #280).

Confidence Score: 5/5

Safe to merge — the hotplug wiring, burst-drain logic, boot-race recovery, and phantom-card suppression are all well-reasoned and backed by unit tests covering the key edge cases.

The three changed subsystems each have targeted tests. The watch_hotplug() call is correctly wrapped in rt.block_on() to prevent panics on Linux. The plan_reapply followup logic correctly distinguishes first-sightings from offline-to-online transitions, does not re-queue the confirming write a second time, and drains the followup set when a device goes offline. The receiver-gate and PID-dedup logic in append_offline_known is correct and covers the extended-model-byte dedup case that motivated the PR.

No files require special attention. Real hardware hotplug testing on Linux (udev path) is still pending per the PR notes, which is worth completing before removing the draft status.

Important Files Changed

Filename Overview
crates/openlogi-agent-core/src/watchers/inventory.rs Replaces thread::sleep with a tokio::select! hotplug-or-period wait; wraps watch_hotplug() in rt.block_on() to address the prior panic risk; burst-drain via poll_once is correct. Logic is sound and fall-back to pure polling on stream close is handled.
crates/openlogi-agent-core/src/orchestrator.rs plan_reapply correctly computes next_followup before the followup-loop so confirming writes aren't re-queued; offline→online transitions are correctly excluded from next_followup (they're in prev); tests cover all three cases (first sighting, transition, and offline followup).
crates/openlogi-gui/src/state/devices.rs Three-gate phantom suppression (key/model match, absent-receiver check, wire-PID dedup) is well-tested; the lowercasing in both receiver_uid_of() and present_receivers ensures reliable matching; the zero-model_id gap is acknowledged in comments and existing tests.
crates/openlogi-hid/src/hotplug.rs Thin bridge over hid_backend().watch(); correctly maps DeviceEvent variants to HotplugEvent; the stream is Send + Unpin as required by the inventory watcher.
crates/openlogi-hid/src/transport.rs Adds a pub(crate) hid_backend() accessor over the existing LazyLock; no behavioral change, simply makes the backend reachable from hotplug.rs.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant OS as OS HID layer
    participant HB as HidBackend (LazyLock)
    participant HP as hotplug.rs watch_hotplug()
    participant IW as inventory watcher thread
    participant ORC as Orchestrator
    participant GUI as GUI state/devices.rs

    OS->>HB: device connected
    HB-->>HP: DeviceEvent::Connected
    HP-->>IW: HotplugEvent::Connected (stream)
    Note over IW: tokio::select! woken early
    IW->>IW: "sleep(HOTPLUG_SETTLE=400ms)"
    IW->>IW: poll_once drain burst events
    IW->>HB: enumerator.enumerate()
    HB-->>IW: "Vec<DeviceInventory>"
    IW-->>ORC: InventoryEvent::Snapshot
    ORC->>ORC: plan_reapply(prev, next, followup, reapply_all)
    Note over ORC: first-sighting: reapply now + queue followup
    ORC->>ORC: reapply_volatile_settings(device)
    Note over ORC: next tick: confirming re-apply fires, followup drains
    ORC->>GUI: updated device list via RwLock
    GUI->>GUI: build_device_list()
    GUI->>GUI: append_offline_known() with receiver gate + PID dedup
    Note over GUI: phantom cards suppressed
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant OS as OS HID layer
    participant HB as HidBackend (LazyLock)
    participant HP as hotplug.rs watch_hotplug()
    participant IW as inventory watcher thread
    participant ORC as Orchestrator
    participant GUI as GUI state/devices.rs

    OS->>HB: device connected
    HB-->>HP: DeviceEvent::Connected
    HP-->>IW: HotplugEvent::Connected (stream)
    Note over IW: tokio::select! woken early
    IW->>IW: "sleep(HOTPLUG_SETTLE=400ms)"
    IW->>IW: poll_once drain burst events
    IW->>HB: enumerator.enumerate()
    HB-->>IW: "Vec<DeviceInventory>"
    IW-->>ORC: InventoryEvent::Snapshot
    ORC->>ORC: plan_reapply(prev, next, followup, reapply_all)
    Note over ORC: first-sighting: reapply now + queue followup
    ORC->>ORC: reapply_volatile_settings(device)
    Note over ORC: next tick: confirming re-apply fires, followup drains
    ORC->>GUI: updated device list via RwLock
    GUI->>GUI: build_device_list()
    GUI->>GUI: append_offline_known() with receiver gate + PID dedup
    Note over GUI: phantom cards suppressed
Loading

Reviews (3): Last reviewed commit: "Merge branch 'master' into fix/device-li..." | Re-trigger Greptile

Comment thread crates/openlogi-agent-core/src/watchers/inventory.rs Outdated
Comment thread crates/openlogi-agent-core/src/watchers/inventory.rs Outdated
Comment thread crates/openlogi-gui/src/state/devices.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant