Skip to content

fix(gui): reject transient direct device identities#451

Open
Phecda wants to merge 2 commits into
AprilNEA:masterfrom
Phecda:fix/bluetooth-direct-zero-unit
Open

fix(gui): reject transient direct device identities#451
Phecda wants to merge 2 commits into
AprilNEA:masterfrom
Phecda:fix/bluetooth-direct-zero-unit

Conversation

@Phecda

@Phecda Phecda commented Jul 23, 2026

Copy link
Copy Markdown

Summary

Prevent transient Bluetooth-direct HID++ probe results with all-zero unit IDs from becoming persistent device identities or phantom offline cards.

Changes

  • openlogi-agent-core: distinguish physical configuration keys from transient runtime identities and exclude all-zero direct identities from the agent device list.
  • openlogi-gui: keep transient live records non-persistent, drop historical all-zero placeholders, and preserve distinct same-model physical devices.

Testing

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test --workspace
  • Not independently runtime-tested on hardware for phantom-card recurrence.

Fixes #387

@Phecda
Phecda marked this pull request as ready for review July 23, 2026 07:24
@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown

Greptile Summary

This PR prevents Bluetooth-direct HID++ probe results that report an all-zero unit ID from becoming persistent device identities or phantom offline cards. It introduces a PhysicalDeviceKey newtype to distinguish stable runtime keys from truly physical ones, then threads that distinction through every config write path in the agent core and GUI.

  • device_order.rs: adds PhysicalDeviceKey, renames config_key()runtime_key(), adds physical_key() → Option<PhysicalDeviceKey> that returns None for all-zero unit IDs, and adds is_transient / parse helpers with corresponding parser logic.
  • state/devices.rs + orchestrator.rs: DeviceRecord gains a persistent: bool field; build_devices / build_device_list skip any device without a PhysicalDeviceKey; append_offline_known filters historical transient keys and separates physical-key deduplication from the legacy-model-key PID deduplication.
  • state.rs: every config write path (commit_dpi, commit_lighting, commit_binding, etc.) now routes through persistent_config_key() and silently skips the write for transient records; merge_inventory_snapshot removes transient records immediately instead of applying the miss grace period.

Confidence Score: 5/5

Safe to merge — the change is well-scoped, all config write paths are guarded, hardware IPC is unaffected, and the new tests cover the main scenarios.

The PhysicalDeviceKey abstraction is correctly implemented and consistently applied across every persistence path. The transient-record cleanup in merge_inventory_snapshot and the append_offline_known filtering are logically sound. Tests cover zero-unit identity exclusion, historical-key filtering, same-model physical device distinctness, and migration of stale persisted selections. The one known edge case — a legacy model key and a new physical key for the same device coexisting in config after a binary update — is an acknowledged transitional state that resolves once the device next comes online.

No files require special attention.

Important Files Changed

Filename Overview
crates/openlogi-agent-core/src/device_order.rs Introduces PhysicalDeviceKey newtype, renames config_key to runtime_key, adds physical_key returning None for zero-unit identities, and adds is_transient/parse helpers with well-tested parser logic. Logic and tests are correct.
crates/openlogi-agent-core/src/orchestrator.rs Skips devices without a physical_key from the agent device list; adds a focused test confirming zero-unit direct inventories are excluded while serial-identified ones are retained.
crates/openlogi-gui/src/state/devices.rs DeviceRecord gains persistent field; build_device_list sets it from physical_key; append_offline_known now separately tracks physical-key deduplication vs legacy-model-key PID deduplication. Correctly avoids hiding two distinct physical same-model devices while still collapsing legacy duplicates.
crates/openlogi-gui/src/state.rs All config write paths (DPI, lighting, bindings, SmartShift, scroll, gesture) now guard on persistent_config_key(); hardware IPC is still sent for transient devices. merge_inventory_snapshot drops transient records immediately. lighting_for adds is_transient guard as belt-and-suspenders against historical zero-unit config entries.
crates/openlogi-gui/src/app.rs Single-line test helper update adding persistent: true to the record fixture. Trivial, correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[HID++ Probe Result] --> B{serial_number present and non-empty?}
    B -- Yes --> C[DeviceIdentity::Serial]
    B -- No / empty --> D{unit_id == 0x00000000?}
    D -- Yes --> E[DeviceIdentity::Unit 0]
    D -- No --> F[DeviceIdentity::Unit non-zero]
    C --> G[physical_key Some]
    F --> G
    E --> H[physical_key None]
    G --> I[DeviceRecord persistent=true]
    H --> J[DeviceRecord persistent=false]
    I --> K[Config writes allowed]
    J --> L[Config writes blocked, HW IPC still sent]
Loading

Reviews (2): Last reviewed commit: "fix(gui): reject stale transient lightin..." | Re-trigger Greptile

Comment thread crates/openlogi-gui/src/state.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.

[Bug]: Duplicated device

1 participant