diff --git a/crates/openlogi-core/src/config/device.rs b/crates/openlogi-core/src/config/device.rs index c6662e43b..3926e81d5 100644 --- a/crates/openlogi-core/src/config/device.rs +++ b/crates/openlogi-core/src/config/device.rs @@ -58,6 +58,11 @@ pub struct DeviceIdentity { /// not a physical-device key and never contains a serial or OS node id. #[serde(default, skip_serializing_if = "Option::is_none")] pub registry_model_id: Option, + /// Wireless product ID from the receiver pairing table. Persisted so + /// HID++ 1.0 devices (which lack `model_info` and may lack `codename` + /// during early probing) can still be distinguished after a re-pairing. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub wpid: Option, } impl DeviceIdentity { diff --git a/crates/openlogi-core/src/config/tests.rs b/crates/openlogi-core/src/config/tests.rs index e3ec751fd..631e30960 100644 --- a/crates/openlogi-core/src/config/tests.rs +++ b/crates/openlogi-core/src/config/tests.rs @@ -533,6 +533,7 @@ fn device_identity_roundtrips_and_is_iterable() { light_capabilities: None, driver_id: None, registry_model_id: None, + wpid: None, }; cfg.set_device_identity("2b034", mouse.clone()); // Recording an identity must not disturb unrelated per-device state. @@ -579,6 +580,7 @@ fn persisted_identity_strips_per_unit_identifiers() { light_capabilities: None, driver_id: None, registry_model_id: None, + wpid: None, }, ); let model = config diff --git a/crates/openlogi-desktop/src/app.rs b/crates/openlogi-desktop/src/app.rs index daff73801..628453294 100644 --- a/crates/openlogi-desktop/src/app.rs +++ b/crates/openlogi-desktop/src/app.rs @@ -717,6 +717,7 @@ mod tests { slot: 1, online: true, battery: None, + wpid: None, } } diff --git a/crates/openlogi-desktop/src/services/assets.rs b/crates/openlogi-desktop/src/services/assets.rs index 1679f744a..af2851236 100644 --- a/crates/openlogi-desktop/src/services/assets.rs +++ b/crates/openlogi-desktop/src/services/assets.rs @@ -215,6 +215,51 @@ impl AssetResolver { self.load_standalone_files(depot, entry, registry_model_id) } + /// Resolve a device that lacks HID++ 2.0 `DeviceModelInfo` using its + /// Unifying/Bolt wireless product id (wpid) and optional codename. + /// + /// HID++ 1.0 devices (many Unifying keyboards) never expose feature + /// 0x0003, so the probe produces `model_info = None`. However, the + /// device-arrival event still carries a wpid that — when formatted as a + /// 4-hex suffix — matches the asset registry's `modelId` for these + /// products. The codename (read from the receiver's pairing register) is + /// tried as a display-name fallback when the suffix match misses. + pub fn resolve_by_wpid( + &self, + wpid: Option, + codename: Option<&str>, + ) -> Option { + let index = self.index.as_ref()?; + + // Try wpid as a suffix match (e.g. wpid 0x4076 → "4076" matches + // registry modelId "4076" for the K540/K545). + if let Some(wpid) = wpid { + let suffix = format!("{wpid:04x}"); + if let Some((depot, entry)) = index.find_by_model_id_suffix(&suffix) { + debug!(depot, wpid = %suffix, "asset matched via wpid suffix for HID++ 1.0 device"); + let model_id = &entry.model_id; + if let Some(asset) = self.load_standalone_files(depot, entry, model_id) { + return Some(asset); + } + } + } + + // Fall back to codename ↔ displayName when the wpid lookup misses. + if let Some(name) = codename + && let Some((depot, entry)) = index.find_by_display_name(name) + { + debug!( + depot, + codename = name, + "asset matched via codename for HID++ 1.0 device" + ); + let model_id = &entry.model_id; + return self.load_standalone_files(depot, entry, model_id); + } + + None + } + fn load_files( &self, depot: &str, diff --git a/crates/openlogi-desktop/src/state/devices.rs b/crates/openlogi-desktop/src/state/devices.rs index fbebce2c5..e43e59d11 100644 --- a/crates/openlogi-desktop/src/state/devices.rs +++ b/crates/openlogi-desktop/src/state/devices.rs @@ -6,13 +6,14 @@ use openlogi_camera::Camera; use openlogi_core::config::{Config, DeviceIdentity}; use openlogi_core::device::{ BatteryInfo, Capabilities, DeviceInventory, DeviceKind, DeviceModelInfo, DeviceTransports, - LightCapabilities, StandaloneDevice, + LightCapabilities, PairedDevice, StandaloneDevice, }; use openlogi_core::device_order::{DeviceStableId, PhysicalDeviceKey}; use openlogi_core::hid::DeviceRoute; use tracing::debug; use super::device_key::DeviceKey; +use super::inventory::is_fallback_display_name; use crate::services::assets::{AssetResolver, ResolvedAsset}; /// One paired device with everything the UI needs to switch to it in O(1): @@ -62,6 +63,11 @@ pub struct DeviceRecord { pub slot: u8, pub online: bool, pub battery: Option, + /// Wireless product ID from the receiver pairing table. Available for + /// HID++ 1.0 and 2.0 receiver-paired devices; `None` for direct/USB or + /// offline devices that never reported a WPID. Used as a model-level + /// discriminator for identity persistence. + pub wpid: Option, } impl DeviceRecord { @@ -125,7 +131,14 @@ pub(super) fn build_device_list( let route = DeviceRoute::device_route_for(inv, paired.slot); let (model_key, asset, model_info, codename, serial_number, unit_id) = if let Some(model) = paired.model_info.as_ref() { - let asset = cache.resolve(model, paired.codename.as_deref()); + let asset = cache + .resolve(model, paired.codename.as_deref()) + .or_else(|| { + // Normal resolution failed (e.g. model_ids are all + // zero on HID++ 1.0 devices). Fall back to WPID-based + // suffix matching against the asset registry. + cache.resolve_by_wpid(paired.wpid, paired.codename.as_deref()) + }); ( model.config_key(), asset, @@ -139,11 +152,17 @@ pub(super) fn build_device_list( // timed out. Surface the device anyway using the wpid (or slot // as a last-resort model key) so it appears in the carousel // with a stable display fallback. + // + // Try resolving assets via wpid suffix or codename: many + // Unifying keyboards (K540/K545, K375s, …) are HID++ 1.0 and + // lack feature 0x0003, but their wpid still matches the asset + // registry's modelId. let key = paired.wpid.map_or_else( || format!("slot{}", paired.slot), |w| format!("wpid{w:04x}"), ); - (key, None, None, paired.codename.clone(), None, [0u8; 4]) + let asset = cache.resolve_by_wpid(paired.wpid, paired.codename.as_deref()); + (key, asset, None, paired.codename.clone(), None, [0u8; 4]) }; let stable_id = DeviceStableId::from_parts( route.as_ref(), @@ -160,6 +179,22 @@ pub(super) fn build_device_list( .as_ref() .map(|a| a.display_name.clone()) .or_else(|| paired.codename.as_deref().map(prettify_codename)) + .or_else(|| { + // Transient resolver failure protection: when the asset + // resolver cannot find a match this cycle (e.g. the index + // is being rewritten by the sync task), fall back to the + // persisted identity's display name if it carries a known + // product name and the device model hasn't changed (a kind + // change or model mismatch signals a re-pairing — the stale + // name must not be inherited by the replacement device). + config + .device_identity(&config_key) + .filter(|id| id.kind == paired.kind) + .filter(|id| persisted_model_matches_paired(id, paired)) + .map(|id| &id.display_name) + .filter(|name| !is_fallback_display_name(name)) + .cloned() + }) .unwrap_or_else(|| format!("Slot {}", paired.slot)); let kind = effective_kind(paired.kind, asset.as_ref().map(|a| a.kind)); list.push(DeviceRecord { @@ -182,6 +217,7 @@ pub(super) fn build_device_list( slot: paired.slot, online: paired.online, battery: paired.battery.clone(), + wpid: paired.wpid, }); } } @@ -248,6 +284,7 @@ fn camera_record(camera: &Camera, cache: &AssetResolver) -> DeviceRecord { slot: 0, online: true, battery: None, + wpid: None, } } @@ -321,6 +358,7 @@ fn append_standalone( slot: openlogi_core::hid::DIRECT_DEVICE_INDEX, online: device.online, battery: None, + wpid: None, }); } } @@ -473,6 +511,7 @@ fn offline_record( slot: 0, online: false, battery: None, + wpid: identity.wpid, } } @@ -531,9 +570,9 @@ pub(super) fn adopt_transient_record(known: &DeviceRecord, live: DeviceRecord) - slot: live.slot, online: live.online, battery: live.battery.or_else(|| known.battery.clone()), + wpid: live.wpid.or(known.wpid), } } - /// Order the carousel by physical route. HID enumeration order can change as /// different mice wake, sleep, or are selected; sorting by the stable route /// (not whichever HID node was reported first) keeps the header stable. @@ -585,6 +624,7 @@ fn demo_keyboard() -> DeviceRecord { slot: 0, online: true, battery: None, + wpid: None, } } @@ -633,6 +673,46 @@ pub(super) fn pick_initial_device(list: &[DeviceRecord], saved: Option<&str>) -> /// Tidy a raw HID++ codename for display when no curated asset name exists. /// Logitech reports gaming codenames in ALL CAPS (e.g. `"G513 RGB MECHANICAL +/// Checks whether a persisted [`DeviceIdentity`] plausibly refers to the same +/// product model as the live [`PairedDevice`]. Used to guard against inheriting +/// a stale display name when a different device is re-paired into the same +/// receiver slot. +/// +/// Returns `true` when no available model-level identifier contradicts the +/// persisted identity. When neither side carries a codename or model_info, the +/// WPID is used as a final discriminator before falling back to the +/// kind-only match. +fn persisted_model_matches_paired(id: &DeviceIdentity, paired: &PairedDevice) -> bool { + // Codename is the lightest model discriminator — available even for + // HID++ 1.0 devices that lack feature 0x0003. + if let (Some(persisted_cn), Some(live_cn)) = + (id.codename.as_deref(), paired.codename.as_deref()) + { + return persisted_cn == live_cn; + } + + // model_info config_key: extended_model_id + model_ids[0]. + if let (Some(persisted_mi), Some(live_mi)) = + (id.model_info.as_ref(), paired.model_info.as_ref()) + { + return persisted_mi.config_key() == live_mi.config_key(); + } + + // When the live side lacks both codename and model_info but has a WPID, + // compare it against the persisted identity's WPID. A mismatch means a + // different product now occupies this slot. + if let (Some(persisted_wpid), Some(live_wpid)) = (id.wpid, paired.wpid) + && persisted_wpid != 0 + && live_wpid != 0 + { + return persisted_wpid == live_wpid; + } + + // No discriminator available beyond kind — conservatively allow the + // fallback (matches pre-existing behaviour). + true +} + /// GAMING KEYBOARD"`); title-case each word so it reads like the asset names /// (`"MX Master 3S"`) instead of shouting, while keeping model numbers (tokens /// with a digit, e.g. `G513`) and short acronyms (`RGB`, `TKL`, `SE`) as-is. @@ -740,6 +820,7 @@ mod tests { slot: 1, online: true, battery: None, + wpid: None, } } @@ -762,6 +843,7 @@ mod tests { codename: None, driver_id: None, registry_model_id: None, + wpid: None, } } @@ -858,6 +940,7 @@ mod tests { codename: None, driver_id: Some("litra".into()), registry_model_id: Some("8c900".into()), + wpid: None, }; let record = offline_record( "raw:046d:c900:ff43:0202:serial:known-light", @@ -1184,3 +1267,85 @@ mod tests { assert_ne!(list[0].capture_id, list[1].capture_id); } } + +#[cfg(test)] +mod identity_guard_tests { + use super::*; + use openlogi_core::config::DeviceIdentity; + use openlogi_core::device::{Capabilities, DeviceKind, PairedDevice}; + + fn hidpp1_identity(codename: Option<&str>, wpid: Option) -> DeviceIdentity { + DeviceIdentity { + display_name: codename.map_or_else(|| "Slot 1".to_string(), str::to_string), + kind: DeviceKind::Keyboard, + capabilities: Capabilities::default(), + light_capabilities: None, + model_info: None, + codename: codename.map(str::to_string), + driver_id: None, + registry_model_id: None, + wpid, + } + } + + fn hidpp1_paired(codename: Option<&str>, wpid: Option) -> PairedDevice { + PairedDevice { + slot: 1, + codename: codename.map(str::to_string), + wpid, + kind: DeviceKind::Keyboard, + online: true, + battery: None, + model_info: None, + capabilities: None, + } + } + + #[test] + fn same_wpid_without_codename_allows_fallback() { + let persisted = hidpp1_identity(Some("K540"), Some(0x4074)); + let live = hidpp1_paired(None, Some(0x4074)); + // Same WPID — device hasn't changed, guard should allow preservation + assert!(persisted_model_matches_paired(&persisted, &live)); + } + + #[test] + fn different_wpid_without_codename_blocks_fallback() { + let persisted = hidpp1_identity(Some("K540"), Some(0x4074)); + let live = hidpp1_paired(None, Some(0x4071)); + // Different WPID — re-paired with a different device + assert!(!persisted_model_matches_paired(&persisted, &live)); + } + + #[test] + fn matching_codename_allows_fallback_regardless_of_wpid() { + let persisted = hidpp1_identity(Some("K540"), Some(0x4074)); + let live = hidpp1_paired(Some("K540"), Some(0x4074)); + assert!(persisted_model_matches_paired(&persisted, &live)); + } + + #[test] + fn different_codename_blocks_fallback() { + let persisted = hidpp1_identity(Some("K540"), Some(0x4074)); + let live = hidpp1_paired(Some("K375s"), Some(0x4071)); + assert!(!persisted_model_matches_paired(&persisted, &live)); + } + + #[test] + fn no_identifiers_on_either_side_allows_fallback_conservatively() { + let persisted = hidpp1_identity(None, None); + let live = hidpp1_paired(None, None); + // No evidence of a change — conservatively allow + assert!(persisted_model_matches_paired(&persisted, &live)); + } + + #[test] + fn different_kind_blocks_fallback() { + let mut persisted = hidpp1_identity(Some("K540"), Some(0x4074)); + persisted.kind = DeviceKind::Mouse; + let live = hidpp1_paired(Some("K540"), Some(0x4074)); + // Kind mismatch is checked by the caller (filter), not this function, + // but codename match still holds within same-kind scope + assert!(persisted_model_matches_paired(&persisted, &live)); + } +} diff --git a/crates/openlogi-desktop/src/state/inventory.rs b/crates/openlogi-desktop/src/state/inventory.rs index 048f4d940..ac047e376 100644 --- a/crates/openlogi-desktop/src/state/inventory.rs +++ b/crates/openlogi-desktop/src/state/inventory.rs @@ -353,8 +353,43 @@ pub(super) fn persist_identities(config: &mut Config, list: &[DeviceRecord]) -> codename: record.codename.clone(), driver_id: record.driver_id.clone(), registry_model_id: record.registry_model_id.clone(), + wpid: record.wpid, } .without_unit_identifiers(); + + // Anti-downgrade: do not overwrite a resolved identity with a + // fallback-quality one. A fallback identity has no model_info, no + // codename, and its display_name is a generic placeholder like + // "Slot N" or "Unknown device". If the existing persisted identity + // carries richer information, preserve it — but only when the device + // in this slot hasn't changed. A different device kind signals a + // re-pairing, in which case the stale identity must not be retained. + // Additionally, even within the same kind, a different model (detected + // via codename or model_info config_key) signals slot reuse by a + // different physical product. + if let Some(existing) = config.device_identity(config_key) + && !identity_is_resolved(&identity) + && identity_is_resolved(existing) + && same_model_identity(existing, &identity, record) + { + // Backfill: if the persisted identity lacks a WPID but the live + // record provides one, record it now. This lets future re-pairings + // into the same slot be detected correctly — without it, legacy + // identities (created before WPID persistence existed) would never + // acquire a WPID and the anti-downgrade guard could not distinguish + // a different HID++ 1.0 device occupying the same slot. + if existing.wpid.is_none() + && let Some(live_wpid) = record.wpid + && live_wpid != 0 + { + let mut patched = existing.clone(); + patched.wpid = Some(live_wpid); + config.set_device_identity(config_key, patched); + changed = true; + } + continue; + } + if config.device_identity(config_key) != Some(&identity) { config.set_device_identity(config_key, identity); changed = true; @@ -363,6 +398,90 @@ pub(super) fn persist_identities(config: &mut Config, list: &[DeviceRecord]) -> changed } +/// An identity is considered "resolved" (non-fallback) when it carries at +/// least one piece of model-level information beyond the bare slot number: +/// a real model_info, a codename, or a display_name that doesn't look like +/// a generic placeholder. +fn identity_is_resolved(identity: &DeviceIdentity) -> bool { + if identity.model_info.is_some() { + return true; + } + if identity.codename.is_some() { + return true; + } + // A display_name that is NOT a generic fallback pattern indicates a + // previously resolved asset name (e.g. "K540/K545", "MX Master 3S"). + let name = &identity.display_name; + !is_fallback_display_name(name) +} + +/// Checks whether the persisted identity and the incoming (possibly fallback) +/// identity plausibly refer to the same physical product model in this slot. +/// +/// When the incoming identity is unresolved (fallback-quality), it may lack +/// codename and model_info entirely. In that case we also consult the live +/// [`DeviceRecord`] for its codename and model_key — which may carry the WPID +/// even before full feature probing completes. +/// +/// Returns `true` when no evidence of a model change exists (safe to +/// preserve the old identity). Returns `false` when any available identifier +/// positively contradicts the persisted model — indicating a re-pairing into +/// the same slot by a different product. +fn same_model_identity( + existing: &DeviceIdentity, + incoming: &DeviceIdentity, + record: &DeviceRecord, +) -> bool { + // Different device kind is an immediate disqualifier. + if existing.kind != incoming.kind { + return false; + } + + // If the incoming identity carries a codename (even without model_info), + // compare it against the persisted one. A mismatch means a different + // product now occupies this slot. + let live_codename = incoming.codename.as_deref().or(record.codename.as_deref()); + if let (Some(existing_cn), Some(new_cn)) = (existing.codename.as_deref(), live_codename) { + return existing_cn == new_cn; + } + + // If both carry model_info, compare the model-level config_key (which is + // the extended_model_id + model_ids[0] — identifies the product model but + // not the physical unit). + if let (Some(existing_mi), Some(incoming_mi)) = + (existing.model_info.as_ref(), incoming.model_info.as_ref()) + { + return existing_mi.config_key() == incoming_mi.config_key(); + } + // When the incoming side lacks both codename and model_info (common for + // HID++ 1.0 devices still being probed), compare the persisted WPID + // against the live record's WPID. Both are structural fields now, so + // this works even when model_info is absent on both sides. + if let (Some(existing_wpid), Some(live_wpid)) = (existing.wpid, record.wpid) + && existing_wpid != 0 + && live_wpid != 0 + { + return existing_wpid == live_wpid; + } + + // No discriminating evidence available — conservatively assume it's the + // same device (the guard stays active). This matches the pre-existing + // kind-only behaviour for the corner case where neither side has any + // model-level identifiers at all. + true +} + +/// Returns true if the display name looks like a generic fallback rather than +/// a real product name resolved from the asset registry. +pub(super) fn is_fallback_display_name(name: &str) -> bool { + // "Slot N" pattern + if name.starts_with("Slot ") && name[5..].chars().all(|c| c.is_ascii_digit()) { + return true; + } + // Other known fallback patterns + name == "Unknown device" || name == "Wireless Mouse" || name == "Wireless Keyboard" +} + /// Reset `key`'s consecutive-miss counter — the device was just confirmed /// present (live, adopted, or freshly appeared) or is a kind that never earns /// grace (transient, camera). Leaves the rest of the device's UI row diff --git a/crates/openlogi-desktop/src/state/tests.rs b/crates/openlogi-desktop/src/state/tests.rs index 98d7120ab..6ee1f3061 100644 --- a/crates/openlogi-desktop/src/state/tests.rs +++ b/crates/openlogi-desktop/src/state/tests.rs @@ -460,6 +460,7 @@ fn known_offline_device_is_an_asset_sync_target() { codename: Some("MX Anywhere 3S".to_string()), driver_id: None, registry_model_id: None, + wpid: None, }, ); let (commands, _receiver) = tokio::sync::mpsc::unbounded_channel(); @@ -984,6 +985,7 @@ fn gesture_maps_cover_every_gesture_mode_button() { codename: None, driver_id: None, registry_model_id: None, + wpid: None, }, ); config.set_gesture_mode("2b042", ButtonId::Back, true); @@ -1011,3 +1013,71 @@ fn gesture_maps_cover_every_gesture_mode_button() { "a promoted OS-hook button gets its own menu simultaneously" ); } + +#[test] +fn legacy_identity_without_wpid_gets_backfilled_from_live_device() { + // Regression: a persisted identity created before WPID persistence existed + // (wpid: None) should acquire the live device's WPID on the next inventory + // refresh — even when the anti-downgrade guard fires (incoming identity is + // fallback-quality). Without backfill, same_model_identity can never detect + // a re-pairing because it has no WPID to compare against. + use super::inventory::persist_identities; + use crate::state::devices::DeviceRecord; + + let mut config = Config::default(); + // Seed a legacy resolved identity WITHOUT a WPID. + config.set_device_identity( + "receiver:uni:slot:1", + DeviceIdentity { + display_name: "K540/K545".to_string(), + kind: DeviceKind::Keyboard, + capabilities: Capabilities::presumed_from_kind(DeviceKind::Keyboard), + light_capabilities: None, + model_info: None, + codename: None, + driver_id: None, + registry_model_id: None, + wpid: None, + }, + ); + + // Simulate a live device in the same slot with a known WPID but no + // codename or model_info (typical HID++ 1.0 early-probe state). + let live = DeviceRecord { + config_key: "receiver:uni:slot:1".to_string(), + persistent: true, + model_key: String::new(), + display_name: "Slot 1".to_string(), // fallback-quality name + asset: None, + kind: DeviceKind::Keyboard, + capabilities: Some(Capabilities::presumed_from_kind(DeviceKind::Keyboard)), + light_capabilities: None, + model_info: None, + codename: None, + serial_number: None, + unit_id: [0; 4], + slot: 1, + online: true, + battery: None, + wpid: Some(0x4076), + route: None, + capture_id: None, + driver_id: None, + registry_model_id: None, + }; + + let changed = persist_identities(&mut config, &[live]); + + // The WPID must have been backfilled into the existing identity. + let identity = config + .device_identity("receiver:uni:slot:1") + .expect("identity must still exist"); + assert_eq!( + identity.wpid, + Some(0x4076), + "legacy identity must be backfilled with the live WPID" + ); + // The resolved display_name must NOT be downgraded. + assert_eq!(identity.display_name, "K540/K545"); + assert!(changed, "backfill must mark config as changed"); +}