diff --git a/crates/openlogi-agent-core/src/device_order.rs b/crates/openlogi-agent-core/src/device_order.rs index 3330e4fa..aa9da053 100644 --- a/crates/openlogi-agent-core/src/device_order.rs +++ b/crates/openlogi-agent-core/src/device_order.rs @@ -59,7 +59,10 @@ impl DeviceStableId { unit_id: [u8; 4], ) -> Self { match route { - Some(DeviceRoute::Bolt { receiver_uid, slot }) => Self::Bolt { + Some( + DeviceRoute::Bolt { receiver_uid, slot } + | DeviceRoute::Unifying { receiver_uid, slot }, + ) => Self::Bolt { receiver_uid: receiver_uid.to_ascii_lowercase(), slot: *slot, }, @@ -78,3 +81,42 @@ impl DeviceStableId { } } } + +#[cfg(test)] +mod tests { + use openlogi_hid::DeviceRoute; + + use super::DeviceStableId; + + #[test] + fn unifying_route_maps_to_bolt_stable_id() { + let route = DeviceRoute::Unifying { + receiver_uid: "DA2699E1".into(), + slot: 2, + }; + let id = DeviceStableId::from_parts(Some(&route), 2, None, [0; 4]); + // Unifying and Bolt share the same stable-id variant so the GUI and + // agent agree on carousel order regardless of receiver family. + assert!( + matches!(id, DeviceStableId::Bolt { ref receiver_uid, slot: 2 } + if receiver_uid == "da2699e1"), + "Unifying route should map to DeviceStableId::Bolt with case-folded uid" + ); + } + + #[test] + fn bolt_and_unifying_same_uid_slot_produce_identical_stable_id() { + let bolt = DeviceRoute::Bolt { + receiver_uid: "AABB".into(), + slot: 1, + }; + let unifying = DeviceRoute::Unifying { + receiver_uid: "AABB".into(), + slot: 1, + }; + assert_eq!( + DeviceStableId::from_parts(Some(&bolt), 1, None, [0; 4]), + DeviceStableId::from_parts(Some(&unifying), 1, None, [0; 4]), + ); + } +} diff --git a/crates/openlogi-agent-core/src/orchestrator.rs b/crates/openlogi-agent-core/src/orchestrator.rs index d250e09d..60d56726 100644 --- a/crates/openlogi-agent-core/src/orchestrator.rs +++ b/crates/openlogi-agent-core/src/orchestrator.rs @@ -16,7 +16,7 @@ use std::sync::{Arc, RwLock}; use openlogi_core::config::Config; use openlogi_core::device::DeviceInventory; -use openlogi_hid::{CaptureChannel, DIRECT_DEVICE_INDEX, DeviceRoute}; +use openlogi_hid::{CaptureChannel, DeviceRoute}; use tracing::warn; use crate::DpiCycleState; @@ -296,7 +296,7 @@ fn build_devices(inventories: &[DeviceInventory]) -> Vec { }; devices.push(AgentDevice { config_key: model.config_key(), - route: device_route(inv, paired.slot), + route: DeviceRoute::device_route_for(inv, paired.slot), slot: paired.slot, serial: model.serial_number.clone(), unit_id: model.unit_id, @@ -330,25 +330,6 @@ fn pick_current(devices: &[AgentDevice], saved: Option<&str>) -> usize { .unwrap_or(0) } -/// Build the [`DeviceRoute`] HID++ writes use to reach a device. A Bolt-paired -/// device routes through its receiver UID + slot; a directly attached one -/// (USB / Bluetooth) carries no receiver UID and sits at [`DIRECT_DEVICE_INDEX`], -/// routing by vendor/product id. A Bolt device whose receiver UID is unknown -/// gets no route, so writes are skipped rather than mis-routed. -fn device_route(inv: &DeviceInventory, slot: u8) -> Option { - match &inv.receiver.unique_id { - Some(receiver_uid) => Some(DeviceRoute::Bolt { - receiver_uid: receiver_uid.clone(), - slot, - }), - None if slot == DIRECT_DEVICE_INDEX => Some(DeviceRoute::Direct { - vendor_id: inv.receiver.vendor_id, - product_id: inv.receiver.product_id, - }), - None => None, - } -} - /// Replace the value behind an `RwLock`, logging (not panicking) on poison so a /// background thread that paniced while holding the lock can't take the agent /// down — it just keeps the stale value until the next successful rebuild. diff --git a/crates/openlogi-agent/src/launch_agent.rs b/crates/openlogi-agent/src/launch_agent.rs index e4e6bef1..7b05fbd7 100644 --- a/crates/openlogi-agent/src/launch_agent.rs +++ b/crates/openlogi-agent/src/launch_agent.rs @@ -234,6 +234,9 @@ fn reconcile_linux(enabled: bool) -> io::Result<()> { match (desired.as_deref(), current.as_deref()) { (Some(want), Some(have)) if want == have => { debug!(path = %path.display(), "systemd user unit already current"); + // Re-enable unconditionally: the unit file is current but the user + // may have manually disabled the service since the last reconcile. + run_systemctl(&["enable", UNIT_NAME]); } (Some(want), _) => { if let Some(parent) = path.parent() { diff --git a/crates/openlogi-cli/src/cmd/diag/features.rs b/crates/openlogi-cli/src/cmd/diag/features.rs index e059672d..a53a8a98 100644 --- a/crates/openlogi-cli/src/cmd/diag/features.rs +++ b/crates/openlogi-cli/src/cmd/diag/features.rs @@ -17,16 +17,11 @@ pub async fn run(_args: FeaturesArgs) -> Result<()> { for inv in &inventories { for paired in inv.paired.iter().filter(|p| p.online) { any = true; - let route = match &inv.receiver.unique_id { - Some(uid) => DeviceRoute::Bolt { - receiver_uid: uid.clone(), - slot: paired.slot, - }, - None => DeviceRoute::Direct { + let route = + DeviceRoute::device_route_for(inv, paired.slot).unwrap_or(DeviceRoute::Direct { vendor_id: inv.receiver.vendor_id, product_id: inv.receiver.product_id, - }, - }; + }); let name = paired .codename .clone() diff --git a/crates/openlogi-cli/src/cmd/diag/mod.rs b/crates/openlogi-cli/src/cmd/diag/mod.rs index 974cecb2..d84f17ac 100644 --- a/crates/openlogi-cli/src/cmd/diag/mod.rs +++ b/crates/openlogi-cli/src/cmd/diag/mod.rs @@ -51,19 +51,15 @@ async fn online_devices() -> Result> { let inventories = openlogi_hid::enumerate().await?; let mut out = Vec::new(); for inv in inventories { - for paired in inv.paired.into_iter().filter(|p| p.online) { - let route = match &inv.receiver.unique_id { - Some(uid) => DeviceRoute::Bolt { - receiver_uid: uid.clone(), - slot: paired.slot, - }, - None => DeviceRoute::Direct { + for paired in inv.paired.iter().filter(|p| p.online) { + let route = + DeviceRoute::device_route_for(&inv, paired.slot).unwrap_or(DeviceRoute::Direct { vendor_id: inv.receiver.vendor_id, product_id: inv.receiver.product_id, - }, - }; + }); let name = paired .codename + .clone() .unwrap_or_else(|| format!("Slot {}", paired.slot)); out.push(Candidate { route, name }); } diff --git a/crates/openlogi-gui/locales/da.yml b/crates/openlogi-gui/locales/da.yml index f00323ff..16a2f836 100644 --- a/crates/openlogi-gui/locales/da.yml +++ b/crates/openlogi-gui/locales/da.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Fuldt" "Battery error": "Batterifejl" "Bolt receiver": "Bolt-modtager" +"Unifying receiver": "Unifying receiver" "Direct connection": "Direkte forbindelse" "Unavailable": "Ikke tilgængelig" "Mouse": "Mus" diff --git a/crates/openlogi-gui/locales/de.yml b/crates/openlogi-gui/locales/de.yml index 131c35b5..c52d1650 100644 --- a/crates/openlogi-gui/locales/de.yml +++ b/crates/openlogi-gui/locales/de.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Voll" "Battery error": "Akkufehler" "Bolt receiver": "Bolt-Empfänger" +"Unifying receiver": "Unifying receiver" "Direct connection": "Direktverbindung" "Unavailable": "Nicht verfügbar" "Mouse": "Maus" diff --git a/crates/openlogi-gui/locales/el.yml b/crates/openlogi-gui/locales/el.yml index 0da15402..fe6ef4dc 100644 --- a/crates/openlogi-gui/locales/el.yml +++ b/crates/openlogi-gui/locales/el.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Πλήρης" "Battery error": "Σφάλμα μπαταρίας" "Bolt receiver": "Δέκτης Bolt" +"Unifying receiver": "Unifying receiver" "Direct connection": "Απευθείας σύνδεση" "Unavailable": "Μη διαθέσιμο" "Mouse": "Ποντίκι" diff --git a/crates/openlogi-gui/locales/en.yml b/crates/openlogi-gui/locales/en.yml index e4fc4de9..82f79799 100644 --- a/crates/openlogi-gui/locales/en.yml +++ b/crates/openlogi-gui/locales/en.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Full" "Battery error": "Battery error" "Bolt receiver": "Bolt receiver" +"Unifying receiver": "Unifying receiver" "Direct connection": "Direct connection" "Unavailable": "Unavailable" "Mouse": "Mouse" diff --git a/crates/openlogi-gui/locales/es.yml b/crates/openlogi-gui/locales/es.yml index 196de416..c97f0297 100644 --- a/crates/openlogi-gui/locales/es.yml +++ b/crates/openlogi-gui/locales/es.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Completa" "Battery error": "Error de batería" "Bolt receiver": "Receptor Bolt" +"Unifying receiver": "Unifying receiver" "Direct connection": "Conexión directa" "Unavailable": "No disponible" "Mouse": "Ratón" diff --git a/crates/openlogi-gui/locales/fi.yml b/crates/openlogi-gui/locales/fi.yml index 76f04f52..aee514ea 100644 --- a/crates/openlogi-gui/locales/fi.yml +++ b/crates/openlogi-gui/locales/fi.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Täynnä" "Battery error": "Akkuvirhe" "Bolt receiver": "Bolt-vastaanotin" +"Unifying receiver": "Unifying receiver" "Direct connection": "Suora yhteys" "Unavailable": "Ei käytettävissä" "Mouse": "Hiiri" diff --git a/crates/openlogi-gui/locales/fr.yml b/crates/openlogi-gui/locales/fr.yml index 1b1efbdf..63e6ee69 100644 --- a/crates/openlogi-gui/locales/fr.yml +++ b/crates/openlogi-gui/locales/fr.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Pleine" "Battery error": "Erreur de batterie" "Bolt receiver": "Récepteur Bolt" +"Unifying receiver": "Unifying receiver" "Direct connection": "Connexion directe" "Unavailable": "Indisponible" "Mouse": "Souris" diff --git a/crates/openlogi-gui/locales/it.yml b/crates/openlogi-gui/locales/it.yml index 96b45c5d..a0caffa4 100644 --- a/crates/openlogi-gui/locales/it.yml +++ b/crates/openlogi-gui/locales/it.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Carica completa" "Battery error": "Errore batteria" "Bolt receiver": "Ricevitore Bolt" +"Unifying receiver": "Ricevitore Unifying" "Direct connection": "Connessione diretta" "Unavailable": "Non disponibile" "Mouse": "Mouse" diff --git a/crates/openlogi-gui/locales/ja.yml b/crates/openlogi-gui/locales/ja.yml index 67d11fb4..ff7136f9 100644 --- a/crates/openlogi-gui/locales/ja.yml +++ b/crates/openlogi-gui/locales/ja.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "フル充電" "Battery error": "バッテリーエラー" "Bolt receiver": "Bolt レシーバー" +"Unifying receiver": "Unifying レシーバー" "Direct connection": "直接接続" "Unavailable": "利用不可" "Mouse": "マウス" diff --git a/crates/openlogi-gui/locales/ko.yml b/crates/openlogi-gui/locales/ko.yml index 877515f8..210cf7d7 100644 --- a/crates/openlogi-gui/locales/ko.yml +++ b/crates/openlogi-gui/locales/ko.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "완충" "Battery error": "배터리 오류" "Bolt receiver": "Bolt 수신기" +"Unifying receiver": "Unifying receiver" "Direct connection": "직접 연결" "Unavailable": "사용 불가" "Mouse": "마우스" diff --git a/crates/openlogi-gui/locales/nb.yml b/crates/openlogi-gui/locales/nb.yml index 4529baed..8f16a083 100644 --- a/crates/openlogi-gui/locales/nb.yml +++ b/crates/openlogi-gui/locales/nb.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Fullt" "Battery error": "Batterifeil" "Bolt receiver": "Bolt-mottaker" +"Unifying receiver": "Unifying receiver" "Direct connection": "Direkte tilkobling" "Unavailable": "Utilgjengelig" "Mouse": "Mus" diff --git a/crates/openlogi-gui/locales/nl.yml b/crates/openlogi-gui/locales/nl.yml index 90326a6e..5d339551 100644 --- a/crates/openlogi-gui/locales/nl.yml +++ b/crates/openlogi-gui/locales/nl.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Vol" "Battery error": "Batterijfout" "Bolt receiver": "Bolt-ontvanger" +"Unifying receiver": "Unifying receiver" "Direct connection": "Directe verbinding" "Unavailable": "Niet beschikbaar" "Mouse": "Muis" diff --git a/crates/openlogi-gui/locales/pl.yml b/crates/openlogi-gui/locales/pl.yml index 9809816d..763a86c5 100644 --- a/crates/openlogi-gui/locales/pl.yml +++ b/crates/openlogi-gui/locales/pl.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Pełna" "Battery error": "Błąd baterii" "Bolt receiver": "Odbiornik Bolt" +"Unifying receiver": "Unifying receiver" "Direct connection": "Połączenie bezpośrednie" "Unavailable": "Niedostępne" "Mouse": "Mysz" diff --git a/crates/openlogi-gui/locales/pt-BR.yml b/crates/openlogi-gui/locales/pt-BR.yml index 74d37f37..8d88fcbe 100644 --- a/crates/openlogi-gui/locales/pt-BR.yml +++ b/crates/openlogi-gui/locales/pt-BR.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Cheia" "Battery error": "Erro de bateria" "Bolt receiver": "Receptor Bolt" +"Unifying receiver": "Unifying receiver" "Direct connection": "Conexão direta" "Unavailable": "Indisponível" "Mouse": "Mouse" diff --git a/crates/openlogi-gui/locales/pt-PT.yml b/crates/openlogi-gui/locales/pt-PT.yml index 4bd8167a..e7c2c5b9 100644 --- a/crates/openlogi-gui/locales/pt-PT.yml +++ b/crates/openlogi-gui/locales/pt-PT.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Completa" "Battery error": "Erro de bateria" "Bolt receiver": "Recetor Bolt" +"Unifying receiver": "Unifying receiver" "Direct connection": "Ligação direta" "Unavailable": "Indisponível" "Mouse": "Rato" diff --git a/crates/openlogi-gui/locales/ru.yml b/crates/openlogi-gui/locales/ru.yml index 6629bbd1..8807172f 100644 --- a/crates/openlogi-gui/locales/ru.yml +++ b/crates/openlogi-gui/locales/ru.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Полный заряд" "Battery error": "Ошибка батареи" "Bolt receiver": "Приёмник Bolt" +"Unifying receiver": "Приёмник Unifying" "Direct connection": "Прямое подключение" "Unavailable": "Недоступно" "Mouse": "Мышь" diff --git a/crates/openlogi-gui/locales/sv.yml b/crates/openlogi-gui/locales/sv.yml index 2915a4be..52331d7a 100644 --- a/crates/openlogi-gui/locales/sv.yml +++ b/crates/openlogi-gui/locales/sv.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "Fulladdat" "Battery error": "Batterifel" "Bolt receiver": "Bolt-mottagare" +"Unifying receiver": "Unifying receiver" "Direct connection": "Direktanslutning" "Unavailable": "Otillgänglig" "Mouse": "Mus" diff --git a/crates/openlogi-gui/locales/zh-CN.yml b/crates/openlogi-gui/locales/zh-CN.yml index 91072bd6..24771ab1 100644 --- a/crates/openlogi-gui/locales/zh-CN.yml +++ b/crates/openlogi-gui/locales/zh-CN.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "已充满" "Battery error": "电池错误" "Bolt receiver": "Bolt 接收器" +"Unifying receiver": "Unifying 接收器" "Direct connection": "直接连接" "Unavailable": "不可用" "Mouse": "鼠标" diff --git a/crates/openlogi-gui/locales/zh-HK.yml b/crates/openlogi-gui/locales/zh-HK.yml index 3c3371e3..5c5c3cc5 100644 --- a/crates/openlogi-gui/locales/zh-HK.yml +++ b/crates/openlogi-gui/locales/zh-HK.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "已充滿" "Battery error": "電池錯誤" "Bolt receiver": "Bolt 接收器" +"Unifying receiver": "Unifying 接收器" "Direct connection": "直接連線" "Unavailable": "不可用" "Mouse": "滑鼠" diff --git a/crates/openlogi-gui/locales/zh-TW.yml b/crates/openlogi-gui/locales/zh-TW.yml index 5e919cca..7dbc7fb3 100644 --- a/crates/openlogi-gui/locales/zh-TW.yml +++ b/crates/openlogi-gui/locales/zh-TW.yml @@ -33,6 +33,7 @@ _version: 1 "Full": "已充滿" "Battery error": "電池錯誤" "Bolt receiver": "Bolt 接收器" +"Unifying receiver": "Unifying 接收器" "Direct connection": "直接連線" "Unavailable": "無法使用" "Mouse": "滑鼠" diff --git a/crates/openlogi-gui/src/app.rs b/crates/openlogi-gui/src/app.rs index 007b4642..a7209501 100644 --- a/crates/openlogi-gui/src/app.rs +++ b/crates/openlogi-gui/src/app.rs @@ -80,22 +80,24 @@ impl DetailTab { /// Each panel is gated on the device's actual [`Capabilities`] — the HID++ /// features it announced — not on its [`DeviceKind`]. A panel shows iff the /// device can do that thing, so a misclassified device can't lose its - /// panels (issue #127) and a keyboard's future button config won't be hidden - /// by a kind check. Devices we never probed (offline at startup) have no + /// panels (issue #127). Devices we never probed (offline at startup) have no /// measured capabilities; we presume a set from their kind so a sleeping /// mouse still shows its (host-side) button bindings. + /// + /// The Buttons panel renders a *mouse-model* silhouette with hotspots. It is + /// only useful for pointer-type devices (Mouse / Trackball) or when the device + /// has a resolved asset that provides its own correct layout. A keyboard that + /// exposes ReprogControls via HID++ but has no asset would get the generic + /// mouse fallback hotspots — confusing and wrong. Suppress the Buttons tab for + /// such devices until a proper keyboard-layout UI is available. fn tabs_for(record: &DeviceRecord) -> Vec { let caps = record .capabilities .unwrap_or_else(|| Capabilities::presumed_from_kind(record.kind)); - // A real keyboard exposes reprogrammable controls (media / G-keys) that - // the HID++ probe reports as `buttons`, but those aren't the mouse remap - // section — so a keyboard only earns the Buttons tab when it's also a - // pointing device (i.e. actually a mouse the registry mislabelled, the - // #127 case). Non-keyboards keep the capability-driven behaviour. - let pointer_device = caps.pointer || record.kind != DeviceKind::Keyboard; + let can_show_mouse_model = record.asset.is_some() + || matches!(record.kind, DeviceKind::Mouse | DeviceKind::Trackball); let mut tabs = Vec::new(); - if caps.buttons && pointer_device { + if caps.buttons && can_show_mouse_model { tabs.push(Self::Buttons); } if caps.pointer { @@ -1259,6 +1261,7 @@ fn sidebar_action( fn route_label(route: Option<&DeviceRoute>) -> String { match route { Some(DeviceRoute::Bolt { .. }) => tr!("Bolt receiver").to_string(), + Some(DeviceRoute::Unifying { .. }) => tr!("Unifying receiver").to_string(), Some(DeviceRoute::Direct { .. }) => tr!("Direct connection").to_string(), None => tr!("Unavailable").to_string(), } @@ -1586,8 +1589,8 @@ mod tests { } /// Tabs follow measured capabilities, not kind — the core of the #127 fix. - /// A device the registry mislabels (kind=Keyboard) but that exposes button + - /// pointer features still gets its Buttons/Pointer tabs and no lighting. + /// A device the Bolt register mislabels as Keyboard but whose 0x0005 probe + /// returns Mouse ends up with kind=Mouse; measured caps drive the tabs. #[test] fn tabs_follow_capabilities_not_kind() { let caps = Some(Capabilities { @@ -1595,12 +1598,31 @@ mod tests { pointer: true, lighting: false, }); - let tabs = DetailTab::tabs_for(&record(DeviceKind::Keyboard, caps)); + // After 0x0005 kind-correction the record has kind=Mouse, not Keyboard. + let tabs = DetailTab::tabs_for(&record(DeviceKind::Mouse, caps)); assert!(tabs.contains(&DetailTab::Buttons)); assert!(tabs.contains(&DetailTab::Pointer)); assert!(!tabs.contains(&DetailTab::Lighting)); } + /// A keyboard that exposes ReprogControls (buttons=true) but has no resolved + /// asset should not get the mouse-model Buttons panel — the generic mouse + /// hotspot layout (Middle Click, DPI Toggle, …) is wrong for a keyboard. + #[test] + fn keyboard_without_asset_hides_buttons_tab() { + let caps = Some(Capabilities { + buttons: true, + pointer: false, + lighting: true, + }); + let tabs = DetailTab::tabs_for(&record(DeviceKind::Keyboard, caps)); + assert!( + !tabs.contains(&DetailTab::Buttons), + "mouse model shown for keyboard" + ); + assert!(tabs.contains(&DetailTab::Lighting)); + } + /// Each panel is independent: a lighting-only device (e.g. a keyboard with /// RGB but no remappable keys yet) shows only Lighting + Device. #[test] diff --git a/crates/openlogi-gui/src/state/devices.rs b/crates/openlogi-gui/src/state/devices.rs index 73b07cea..c2318787 100644 --- a/crates/openlogi-gui/src/state/devices.rs +++ b/crates/openlogi-gui/src/state/devices.rs @@ -2,7 +2,7 @@ use openlogi_agent_core::device_order::DeviceStableId; use openlogi_core::device::{BatteryInfo, Capabilities, DeviceInventory, DeviceKind}; -use openlogi_hid::{DIRECT_DEVICE_INDEX, DeviceRoute}; +use openlogi_hid::DeviceRoute; use tracing::debug; use crate::asset::{AssetResolver, ResolvedAsset}; @@ -44,11 +44,27 @@ pub(super) fn build_device_list( let mut list = Vec::new(); for inv in inventories { for paired in &inv.paired { - let Some(model) = paired.model_info.as_ref() else { - continue; - }; - let config_key = model.config_key(); - let asset = cache.resolve(model, paired.codename.as_deref()); + let (config_key, asset, serial_number, unit_id) = + if let Some(model) = paired.model_info.as_ref() { + let asset = cache.resolve(model, paired.codename.as_deref()); + ( + model.config_key(), + asset, + model.serial_number.clone(), + model.unit_id, + ) + } else { + // No HID++ 2.0 model info — HID++ 1.0 device or feature walk + // timed out. Surface the device anyway using the wpid (or slot + // as a last resort) as a stable config key so it appears in the + // carousel and its settings survive across sessions. + let key = paired.wpid.map_or_else( + || format!("slot{}", paired.slot), + |w| format!("wpid{w:04x}"), + ); + (key, None, None, [0u8; 4]) + }; + let display_name = asset .as_ref() .map(|a| a.display_name.clone()) @@ -59,9 +75,9 @@ pub(super) fn build_device_list( config_key, display_name, asset, - serial_number: model.serial_number.clone(), - unit_id: model.unit_id, - route: device_route(inv, paired.slot), + serial_number, + unit_id, + route: DeviceRoute::device_route_for(inv, paired.slot), kind, capabilities: paired.capabilities, slot: paired.slot, @@ -124,28 +140,6 @@ fn demo_keyboard() -> DeviceRecord { } } -/// Build the [`DeviceRoute`] HID++ writes use to reach a device. -/// -/// A Bolt-paired device routes through its receiver UID + slot. A directly -/// attached one (USB cable / Bluetooth) carries no receiver UID and sits at -/// [`DIRECT_DEVICE_INDEX`] — it routes by the HID node's vendor/product id -/// instead. A Bolt device whose receiver UID couldn't be read gets no route -/// (`None`), so hardware writes are skipped rather than mis-routed to the -/// receiver's own pid. -fn device_route(inv: &DeviceInventory, slot: u8) -> Option { - match &inv.receiver.unique_id { - Some(receiver_uid) => Some(DeviceRoute::Bolt { - receiver_uid: receiver_uid.clone(), - slot, - }), - None if slot == DIRECT_DEVICE_INDEX => Some(DeviceRoute::Direct { - vendor_id: inv.receiver.vendor_id, - product_id: inv.receiver.product_id, - }), - None => None, - } -} - /// Last step of the device-kind precedence chain: /// /// > **asset registry** > HID++ `0x0005` > Bolt pairing register @@ -213,7 +207,64 @@ fn prettify_codename(raw: &str) -> String { #[cfg(test)] mod tests { - use super::{DeviceKind, effective_kind}; + use openlogi_core::device::{DeviceInventory, DeviceKind, PairedDevice, ReceiverInfo}; + + use crate::asset::AssetResolver; + + use super::{build_device_list, effective_kind}; + + fn paired_device_no_model_info(slot: u8, wpid: Option) -> PairedDevice { + PairedDevice { + slot, + codename: None, + wpid, + kind: DeviceKind::Keyboard, + online: true, + battery: None, + model_info: None, + capabilities: None, + } + } + + fn inventory_with(devices: Vec) -> DeviceInventory { + DeviceInventory { + receiver: ReceiverInfo { + name: "Unifying Receiver".into(), + vendor_id: 0x046d, + product_id: 0xc52b, + unique_id: Some("DA2699E1".into()), + }, + paired: devices, + } + } + + #[test] + fn no_model_info_uses_wpid_as_config_key() { + let inv = inventory_with(vec![paired_device_no_model_info(1, Some(0x4076))]); + let cache = AssetResolver::new(); + let list = build_device_list(&[inv], &cache); + assert_eq!(list.len(), 1); + assert_eq!(list[0].config_key, "wpid4076"); + assert!(list[0].serial_number.is_none()); + assert_eq!(list[0].unit_id, [0u8; 4]); + } + + #[test] + fn no_model_info_falls_back_to_slot_when_no_wpid() { + let inv = inventory_with(vec![paired_device_no_model_info(3, None)]); + let cache = AssetResolver::new(); + let list = build_device_list(&[inv], &cache); + assert_eq!(list.len(), 1); + assert_eq!(list[0].config_key, "slot3"); + } + + #[test] + fn no_model_info_display_name_falls_back_to_slot() { + let inv = inventory_with(vec![paired_device_no_model_info(2, Some(0x4051))]); + let cache = AssetResolver::new(); + let list = build_device_list(&[inv], &cache); + assert_eq!(list[0].display_name, "Slot 2"); + } #[test] fn asset_kind_overrides_a_misreporting_hid_kind() { diff --git a/crates/openlogi-hid/src/inventory.rs b/crates/openlogi-hid/src/inventory.rs index 4e01181e..e0f6efc0 100644 --- a/crates/openlogi-hid/src/inventory.rs +++ b/crates/openlogi-hid/src/inventory.rs @@ -25,6 +25,10 @@ use hidpp::{ DeviceConnection as BoltDeviceConnection, DeviceKind as BoltDeviceKind, Event as BoltEvent, Receiver as BoltReceiver, }, + unifying::{ + DeviceConnection as UnifyingDeviceConnection, DeviceKind as UnifyingDeviceKind, + Event as UnifyingEvent, Receiver as UnifyingReceiver, + }, }, }; use openlogi_core::device::{ @@ -60,6 +64,18 @@ const MAX_BOLT_SLOTS: u8 = 6; /// trip it. const PROBE_BUDGET: Duration = Duration::from_secs(5); +/// Per-slot budget for the HID++ 2.0 feature walk on a Unifying paired device. +/// +/// Unifying wireless round-trips are slower than Bolt BTLE: some devices (e.g. +/// K540) take ~3 s for the version ping to return. Running multiple slow slots +/// concurrently can still consume the full PROBE_BUDGET and get cancelled +/// mid-walk — the probe returns nothing rather than partial features. A +/// per-slot cap ensures each slot's feature walk is bounded independently of +/// how many other slots are being probed at the same time. A timed-out slot +/// still surfaces in the inventory (kind + wpid from the arrival event) — it +/// just lacks capabilities / battery until the next tick. +const UNIFYING_SLOT_PROBE: Duration = Duration::from_millis(3500); + #[derive(Debug, Error)] pub enum InventoryError { #[error("HID transport error")] @@ -83,6 +99,11 @@ const REFRESH_TICKS: u64 = 15; enum CacheKey { /// Bolt: the unit id from the pairing register (cheap, read every tick). Bolt { unit_id: [u8; 4] }, + /// Unifying: keyed on the full receiver serial number + pairing slot. + /// Using the complete serial (not just a prefix) avoids collisions between + /// two receivers whose serials share a common prefix (e.g. "DA2699E1" and + /// "DA2604F2" share "DA2"). + UnifyingSlot { receiver_uid: String, slot: u8 }, /// Direct (Bluetooth/USB): the OS-assigned HID node id (macOS registry-entry /// id, Linux dev path, Windows interface path). Unique *per node*, so two /// units of the same model never collide, and stable while connected so the @@ -298,10 +319,13 @@ impl Enumerator { let mut outcomes = Vec::new(); for result in results { match result { - Ok((inv, mut probed)) => { + Ok(Ok((inv, mut probed))) => { inventories.extend(inv); outcomes.append(&mut probed); } + Ok(Err(e)) => { + warn!(error = ?e, "device probe failed — skipping"); + } Err(_) => { warn!(budget = ?PROBE_BUDGET, "device probe timed out — skipping (asleep/unresponsive)"); } @@ -357,16 +381,30 @@ async fn probe_one( channel: Arc, cache: &HashMap, tick: u64, -) -> (Option, Vec) { - let Some(Receiver::Bolt(bolt)) = receiver::detect(Arc::clone(&channel)) else { - // No receiver detected — this might be a directly-paired device - // (Bluetooth-direct, USB-C cable). HID++ at device-index 0xff - // addresses the device's own features. Probe in case it answers. - // P2.4 — verified path; no Bolt-pairing slot indirection needed. - let (inventory, outcome) = probe_direct(channel, &info, cache, tick).await; - return (inventory, vec![outcome]); - }; +) -> Result<(Option, Vec), InventoryError> { + match receiver::detect(Arc::clone(&channel)) { + Some(Receiver::Bolt(bolt)) => probe_bolt_receiver(channel, info, bolt, cache, tick).await, + Some(Receiver::Unifying(unifying)) => { + probe_unifying_receiver(channel, info, unifying, cache, tick).await + } + None | Some(_) => { + // No recognised receiver — this might be a directly-paired device + // (Bluetooth-direct, USB-C cable). HID++ at device-index 0xff + // addresses the device's own features. Probe in case it answers. + // P2.4 — verified path; no Bolt-pairing slot indirection needed. + let (inventory, outcome) = probe_direct(channel, &info, cache, tick).await; + Ok((inventory, vec![outcome])) + } + } +} +async fn probe_bolt_receiver( + channel: Arc, + info: async_hid::DeviceInfo, + bolt: BoltReceiver, + cache: &HashMap, + tick: u64, +) -> Result<(Option, Vec), InventoryError> { let unique_id = bolt.get_unique_id().await.ok(); let pairing_count = bolt.count_pairings().await.ok(); debug!(?pairing_count, "receiver reports pairing count"); @@ -397,7 +435,7 @@ async fn probe_one( ); } - ( + Ok(( Some(DeviceInventory { receiver: ReceiverInfo { name: "Logi Bolt Receiver".to_string(), @@ -408,7 +446,78 @@ async fn probe_one( paired, }), outcomes, - ) + )) +} + +async fn probe_unifying_receiver( + channel: Arc, + info: async_hid::DeviceInfo, + unifying: UnifyingReceiver, + cache: &HashMap, + tick: u64, +) -> Result<(Option, Vec), InventoryError> { + let unique_id = unifying.get_unique_id().await.ok(); + let pairing_count = unifying.count_pairings().await.ok(); + debug!(?pairing_count, "receiver reports pairing count"); + + // Trigger device-arrival events and collect one event per online device. + // Each event carries the slot index, kind, wpid, and online flag — enough + // to build a PairedDevice entry for every currently-connected device. + // + // Note: the Unifying `0xB5/0x5N` pairing-info register uses a different + // sub-register base than Bolt, so we don't yet poll offline paired slots. + // Online devices are covered by the arrival drain; offline device support + // requires resolving the correct sub-register format. + let connections = drain_device_arrival_unifying(&unifying).await; + debug!(events = connections.len(), "drained device-arrival events"); + + // Probe all online slots concurrently so a slow HID++ 2.0 feature walk on + // one device doesn't push the next slot past the PROBE_BUDGET deadline. + // Pass the receiver UID so each slot's cache key is scoped to this specific + // receiver — two Unifying receivers sharing a slot number must not share a + // cache entry (different devices, different capabilities). + let receiver_uid_fallback; + let receiver_uid = if let Some(uid) = unique_id.as_deref() { + uid + } else { + // UID fetch failed — use the product ID as a weaker discriminant so + // two receivers with the same PID still collide, but a receiver and a + // direct device never share a cache entry. + tracing::warn!("Unifying receiver UID unavailable; cache isolation may be degraded"); + receiver_uid_fallback = format!("pid:{:04x}", info.product_id); + &receiver_uid_fallback + }; + let slot_results = connections + .iter() + .map(|conn| probe_unifying_slot(&channel, conn, receiver_uid, cache, tick)) + .collect::>() + .join() + .await; + + let (paired, outcomes): (Vec<_>, Vec<_>) = slot_results.into_iter().flatten().unzip(); + + if let Some(count) = pairing_count + && paired.len() != usize::from(count) + { + debug!( + expected = count, + found = paired.len(), + "online devices differ from pairing count; offline devices not yet surfaced for Unifying" + ); + } + + Ok(( + Some(DeviceInventory { + receiver: ReceiverInfo { + name: "Unifying Receiver".to_string(), + vendor_id: info.vendor_id, + product_id: info.product_id, + unique_id, + }, + paired, + }), + outcomes, + )) } /// Probe a single Bolt pairing slot. Returns `None` when the slot is empty or @@ -574,6 +683,88 @@ async fn drain_device_arrival(bolt: &BoltReceiver) -> Vec out } +async fn drain_device_arrival_unifying( + unifying: &UnifyingReceiver, +) -> Vec { + let rx = unifying.listen(); + if let Err(e) = unifying.trigger_device_arrival().await { + debug!(error = ?e, "trigger_device_arrival failed; receiver may report no devices"); + return Vec::new(); + } + + let mut out = Vec::new(); + loop { + match timeout(ARRIVAL_DRAIN, rx.recv()).await { + Ok(Ok(UnifyingEvent::DeviceConnection(c))) => out.push(c), + Ok(Ok(_)) => {} + Ok(Err(_)) | Err(_) => break, + } + } + out +} + +/// Probe a Unifying slot from a live device-connection event. +/// +/// Device-arrival events carry the slot index, kind, wpid, and online status — +/// enough to surface an entry for every currently-connected device. The +/// unit_id (needed for stable caching across ticks) is not available without a +/// working `get_device_pairing_information` call; we derive a stable cache key +/// from the receiver UID + slot so the feature-table walk is amortised at ~30s +/// and two receivers sharing a slot number don't collide in the cache. +async fn probe_unifying_slot( + channel: &Arc, + event: &UnifyingDeviceConnection, + receiver_uid: &str, + cache: &HashMap, + tick: u64, +) -> Option<(PairedDevice, CacheOutcome)> { + let slot = event.index; + let codename = read_codename(channel, slot).await; + debug!( + slot, + online = event.online, + wpid = format_args!("{:04x}", event.wpid), + kind = ?event.kind, + codename = ?codename, + "unifying paired slot" + ); + + // Cache key: full receiver serial + slot so two Unifying receivers with + // a device on the same slot number never share a cache entry. + let id = CacheKey::UnifyingSlot { + receiver_uid: receiver_uid.to_string(), + slot, + }; + let cached = cache.get(&id); + let register_kind = map_unifying_kind(event.kind); + + let probe_result = timeout( + UNIFYING_SLOT_PROBE, + probe_or_reuse(channel, slot, Some(id.clone()), cached, event.online, tick), + ) + .await; + let (probe, outcome) = if let Ok(r) = probe_result { + r + } else { + debug!(slot, budget = ?UNIFYING_SLOT_PROBE, + "Unifying slot probe timed out; using cached data if available"); + let probe = cached.map_or_else(ProbedFeatures::default, |c| c.probe.clone()); + (probe, CacheOutcome::Seen(id)) + }; + + let device = PairedDevice { + slot, + codename, + wpid: Some(event.wpid), + kind: resolve_device_kind(probe.kind, register_kind), + online: event.online, + battery: probe.battery, + model_info: probe.model_info, + capabilities: probe.capabilities, + }; + Some((device, outcome)) +} + /// Reads a paired device's codename, working around a slicing bug in /// `hidpp 0.2`'s `BoltReceiver::get_device_codename` that truncates names /// longer than 8 characters (it treats `response[2]` as an end-index when it @@ -763,6 +954,19 @@ fn map_kind(k: BoltDeviceKind) -> DeviceKind { } } +fn map_unifying_kind(k: UnifyingDeviceKind) -> DeviceKind { + match k { + UnifyingDeviceKind::Keyboard => DeviceKind::Keyboard, + UnifyingDeviceKind::Mouse => DeviceKind::Mouse, + UnifyingDeviceKind::Numpad => DeviceKind::Numpad, + UnifyingDeviceKind::Presenter => DeviceKind::Presenter, + UnifyingDeviceKind::Remote => DeviceKind::Remote, + UnifyingDeviceKind::Trackball => DeviceKind::Trackball, + UnifyingDeviceKind::Touchpad => DeviceKind::Touchpad, + _ => DeviceKind::Unknown, + } +} + /// Map the HID++ `0x0005` marketing device type to our [`DeviceKind`]. Types we /// don't model (receiver, webcam, dock, …) fall back to [`DeviceKind::Unknown`]. fn map_device_type(ty: HidppDeviceType) -> DeviceKind { @@ -831,7 +1035,8 @@ mod tests { use super::{ CACHE_MISS_GRACE, CacheKey, Cached, DeviceKind, Enumerator, ProbedFeatures, REFRESH_TICKS, - UnifiedBatteryFeature, battery_feature_index, is_stale, resolve_device_kind, + UnifiedBatteryFeature, UnifyingDeviceKind, battery_feature_index, is_stale, + map_unifying_kind, resolve_device_kind, }; use hidpp::feature::CreatableFeature as _; @@ -960,4 +1165,25 @@ mod tests { DeviceKind::Unknown ); } + + #[test] + fn unifying_kind_maps_all_variants() { + let cases = [ + (UnifyingDeviceKind::Unknown, DeviceKind::Unknown), + (UnifyingDeviceKind::Keyboard, DeviceKind::Keyboard), + (UnifyingDeviceKind::Mouse, DeviceKind::Mouse), + (UnifyingDeviceKind::Numpad, DeviceKind::Numpad), + (UnifyingDeviceKind::Presenter, DeviceKind::Presenter), + (UnifyingDeviceKind::Remote, DeviceKind::Remote), + (UnifyingDeviceKind::Trackball, DeviceKind::Trackball), + (UnifyingDeviceKind::Touchpad, DeviceKind::Touchpad), + ]; + for (input, expected) in cases { + assert_eq!( + map_unifying_kind(input), + expected, + "kind {input:?} mapped incorrectly" + ); + } + } } diff --git a/crates/openlogi-hid/src/lib.rs b/crates/openlogi-hid/src/lib.rs index 8e38e1d1..c6de4ae7 100644 --- a/crates/openlogi-hid/src/lib.rs +++ b/crates/openlogi-hid/src/lib.rs @@ -27,7 +27,7 @@ pub use pairing::{ Click, DiscoveredDevice, PairingCommand, PairingError, PairingEvent, PairingReceiver, PasskeyMethod, ReceiverFamily, ReceiverSelector, list_pairing_receivers, run_pairing, unpair, }; -pub use route::{DIRECT_DEVICE_INDEX, DeviceRoute}; +pub use route::{BOLT_PIDS, DIRECT_DEVICE_INDEX, DeviceRoute, UNIFYING_PIDS}; pub use smartshift::{AUTO_DISENGAGE_PERMANENT, SmartShiftMode, SmartShiftStatus}; pub use write::{ DpiCapabilities, DpiInfo, FeatureEntry, SharedChannel, WriteError, dump_features, get_dpi, diff --git a/crates/openlogi-hid/src/pairing.rs b/crates/openlogi-hid/src/pairing.rs index 87401058..2d7220f5 100644 --- a/crates/openlogi-hid/src/pairing.rs +++ b/crates/openlogi-hid/src/pairing.rs @@ -67,10 +67,6 @@ mod notif { /// big-endian. Both must be set for the receiver to stream pairing events. const NOTIF_FLAGS: [u8; 3] = [0x00, 0x09, 0x00]; -/// Product IDs (vendor `0x046d`) of supported pairing-capable receivers. -const BOLT_PRODUCT_IDS: &[u16] = &[0xc548]; -const UNIFYING_PRODUCT_IDS: &[u16] = &[0xc52b, 0xc532]; - /// Receiver pairing family. Each uses a different register flow. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum ReceiverFamily { @@ -79,9 +75,9 @@ pub enum ReceiverFamily { } fn family_for(product_id: u16) -> Option { - if BOLT_PRODUCT_IDS.contains(&product_id) { + if crate::BOLT_PIDS.contains(&product_id) { Some(ReceiverFamily::Bolt) - } else if UNIFYING_PRODUCT_IDS.contains(&product_id) { + } else if crate::UNIFYING_PIDS.contains(&product_id) { Some(ReceiverFamily::Unifying) } else { None diff --git a/crates/openlogi-hid/src/route.rs b/crates/openlogi-hid/src/route.rs index 95ab588b..8f78ce62 100644 --- a/crates/openlogi-hid/src/route.rs +++ b/crates/openlogi-hid/src/route.rs @@ -21,6 +21,7 @@ use hidpp::{ channel::HidppChannel, receiver::{self, Receiver}, }; +use openlogi_core::device::DeviceInventory; use serde::{Deserialize, Serialize}; use crate::transport::{enumerate_hidpp_devices, open_hidpp_channel}; @@ -39,6 +40,9 @@ pub enum DeviceRoute { /// Paired to a Logi Bolt receiver. `receiver_uid` disambiguates multiple /// plugged-in receivers; `slot` is the device's pairing slot (1..=6). Bolt { receiver_uid: String, slot: u8 }, + /// Paired to a Logi Unifying receiver. Same addressing structure as Bolt + /// (receiver channel + pairing slot) but the receiver speaks HID++ 1.0. + Unifying { receiver_uid: String, slot: u8 }, /// Attached straight to the host over USB cable or Bluetooth, addressed at /// the HID++ self-index. Re-found by matching the HID node's vendor/product /// id — two identical mice on one host are indistinguishable here, so the @@ -46,22 +50,70 @@ pub enum DeviceRoute { Direct { vendor_id: u16, product_id: u16 }, } +/// USB product IDs that identify Logi Bolt receivers. +pub const BOLT_PIDS: &[u16] = &[0xc548]; + +/// USB product IDs that identify Logi Unifying receivers. Used by callers that +/// need to construct the correct [`DeviceRoute`] variant from a raw inventory. +pub const UNIFYING_PIDS: &[u16] = &[0xc52b, 0xc532]; + impl DeviceRoute { /// The HID++ device index features are addressed at for this route: the /// pairing slot for a Bolt device, the self-index for a direct one. #[must_use] pub fn device_index(&self) -> u8 { match self { - Self::Bolt { slot, .. } => *slot, + Self::Bolt { slot, .. } | Self::Unifying { slot, .. } => *slot, Self::Direct { .. } => DIRECT_DEVICE_INDEX, } } + + /// Build the route that reaches a paired device from a receiver inventory. + /// + /// Picks [`DeviceRoute::Unifying`] or [`DeviceRoute::Bolt`] based on the + /// receiver's product ID using the canonical `UNIFYING_PIDS` / `BOLT_PIDS` + /// lists. Any receiver PID not in `UNIFYING_PIDS` — including future Bolt + /// variants whose PID isn't yet in `BOLT_PIDS` — defaults to + /// [`DeviceRoute::Bolt`] so writes keep working rather than silently + /// dropping. [`DeviceRoute::Direct`] is used for directly-attached devices + /// (slot == [`DIRECT_DEVICE_INDEX`] with no receiver UID). Returns `None` + /// when the receiver UID is unknown (writes are skipped, not mis-routed). + #[must_use] + pub fn device_route_for(inv: &DeviceInventory, slot: u8) -> Option { + match &inv.receiver.unique_id { + Some(uid) if UNIFYING_PIDS.contains(&inv.receiver.product_id) => Some(Self::Unifying { + receiver_uid: uid.clone(), + slot, + }), + Some(uid) => { + // Default to Bolt for any receiver whose PID is not in + // UNIFYING_PIDS. This covers both known Bolt PIDs (BOLT_PIDS) + // and any future Bolt-compatible receiver with a new PID — + // returning None would silently drop writes for such receivers. + if !BOLT_PIDS.contains(&inv.receiver.product_id) { + tracing::debug!( + pid = format_args!("{:04x}", inv.receiver.product_id), + "unknown receiver PID — routing as Bolt" + ); + } + Some(Self::Bolt { + receiver_uid: uid.clone(), + slot, + }) + } + None if slot == DIRECT_DEVICE_INDEX => Some(Self::Direct { + vendor_id: inv.receiver.vendor_id, + product_id: inv.receiver.product_id, + }), + None => None, + } + } } impl fmt::Display for DeviceRoute { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::Bolt { receiver_uid, slot } => { + Self::Bolt { receiver_uid, slot } | Self::Unifying { receiver_uid, slot } => { write!(f, "slot {slot} on receiver {receiver_uid}") } Self::Direct { @@ -110,8 +162,96 @@ pub(crate) async fn open_route_channel( return Ok(Some(channel)); } } + DeviceRoute::Unifying { receiver_uid, .. } => { + let Some(Receiver::Unifying(unifying)) = receiver::detect(Arc::clone(&channel)) + else { + continue; + }; + if let Ok(uid) = unifying.get_unique_id().await + && uid.eq_ignore_ascii_case(receiver_uid) + { + return Ok(Some(channel)); + } + } DeviceRoute::Direct { .. } => return Ok(Some(channel)), } } Ok(None) } + +#[cfg(test)] +mod tests { + use openlogi_core::device::{DeviceInventory, ReceiverInfo}; + + use super::{DIRECT_DEVICE_INDEX, DeviceRoute, UNIFYING_PIDS}; + + fn inv(product_id: u16, unique_id: Option<&str>) -> DeviceInventory { + DeviceInventory { + receiver: ReceiverInfo { + name: "test".into(), + vendor_id: 0x046d, + product_id, + unique_id: unique_id.map(str::to_string), + }, + paired: vec![], + } + } + + #[test] + fn device_route_for_unifying_pids_create_unifying_route() { + for &pid in UNIFYING_PIDS { + let route = DeviceRoute::device_route_for(&inv(pid, Some("A1B2")), 2); + assert!( + matches!(route, Some(DeviceRoute::Unifying { ref receiver_uid, slot: 2 }) if receiver_uid == "A1B2"), + "pid {pid:#06x} should produce Unifying route" + ); + } + } + + #[test] + fn device_route_for_bolt_pid_creates_bolt_route() { + // 0xC548 is Bolt; anything not in UNIFYING_PIDS defaults to Bolt so + // future Bolt variants with unknown PIDs still work. + let route = DeviceRoute::device_route_for(&inv(0xc548, Some("UID")), 1); + assert!(matches!( + route, + Some(DeviceRoute::Bolt { ref receiver_uid, slot: 1 }) if receiver_uid == "UID" + )); + } + + #[test] + fn device_route_for_direct_when_no_uid_and_direct_slot() { + let route = DeviceRoute::device_route_for(&inv(0xb025, None), DIRECT_DEVICE_INDEX); + assert!(matches!( + route, + Some(DeviceRoute::Direct { + vendor_id: 0x046d, + product_id: 0xb025 + }) + )); + } + + #[test] + fn device_route_for_none_when_no_uid_and_non_direct_slot() { + let route = DeviceRoute::device_route_for(&inv(0xc52b, None), 1); + assert!(route.is_none()); + } + + #[test] + fn unifying_device_index_is_the_slot() { + let route = DeviceRoute::Unifying { + receiver_uid: "X".into(), + slot: 4, + }; + assert_eq!(route.device_index(), 4); + } + + #[test] + fn unifying_display_matches_bolt_format() { + let r = DeviceRoute::Unifying { + receiver_uid: "AABBCC".into(), + slot: 3, + }; + assert_eq!(r.to_string(), "slot 3 on receiver AABBCC"); + } +} diff --git a/crates/openlogi-hid/src/transport.rs b/crates/openlogi-hid/src/transport.rs index 05e598a7..97ac6f89 100644 --- a/crates/openlogi-hid/src/transport.rs +++ b/crates/openlogi-hid/src/transport.rs @@ -126,11 +126,67 @@ pub(crate) async fn enumerate_hidpp_devices() -> Result, Ok(all .into_iter() .filter(|d| { - d.vendor_id == LOGITECH_VID && is_hidpp_long_collection(d.usage_page, d.usage_id) + d.vendor_id == LOGITECH_VID + && is_hidpp_long_collection(d.usage_page, d.usage_id) + && !is_receiver_child_node(&d.id) }) .collect()) } +/// Returns `true` when a HID++ node is a virtual per-device interface created by +/// the `hid-logitech-dj` kernel driver as a child of a Unifying or Bolt receiver. +/// +/// On Linux, each device paired to a Unifying receiver gets its own hidraw node +/// whose sysfs path is a subdirectory of the receiver's HID device path. These +/// nodes expose the same HID++ long-report collection as the receiver, but HID++ +/// communication must go through the receiver node, not these child nodes. +/// Probing them directly causes long timeouts and produces no useful inventory. +/// +/// Detection: the sysfs path of a child node looks like +/// `.../0003:046D:C52B.0009/0003:046D:4076.000A` +/// while the receiver itself ends at `…/0003:046D:C52B.0009`. We check whether +/// any known receiver PID appears as a *parent directory* component in the path. +#[cfg(target_os = "linux")] +fn is_receiver_child_node(id: &async_hid::DeviceId) -> bool { + use async_hid::DeviceId; + let DeviceId::DevPath(dev_path) = id else { + return false; + }; + let Some(node_name) = dev_path.file_name().and_then(|n| n.to_str()) else { + return false; + }; + let sysfs_link = format!("/sys/class/hidraw/{node_name}/device"); + let Ok(real_path) = std::fs::canonicalize(&sysfs_link) else { + return false; + }; + is_receiver_child_sysfs_path(&real_path.to_string_lossy()) +} + +/// Determines whether a resolved sysfs path belongs to a device that is a +/// child of a known receiver. Separated from `is_receiver_child_node` so it +/// can be unit-tested without filesystem access. +#[cfg(any(target_os = "linux", test))] +fn is_receiver_child_sysfs_path(path: &str) -> bool { + // Build parent-component markers from the canonical PID lists so adding a + // new receiver PID only needs to be done in one place (route.rs). + // The kernel HID device name format is "BUS:VID:PID.IFACE" with uppercase hex. + crate::BOLT_PIDS + .iter() + .chain(crate::UNIFYING_PIDS.iter()) + .any(|&pid| { + let marker = format!(":{LOGITECH_VID:04X}:{pid:04X}."); + // A parent component contains the marker followed by at least one + // more "/" — it is not the terminal component of the path. + path.find(&marker) + .is_some_and(|idx| path[idx + marker.len()..].contains('/')) + }) +} + +#[cfg(not(target_os = "linux"))] +fn is_receiver_child_node(_id: &async_hid::DeviceId) -> bool { + false +} + /// Open the raw HID writer for a directly-attached (USB) device, for sending /// reports the HID++ wrapper can't model — e.g. the 64-byte `0x12` lighting /// frames G-series keyboards use. Returns `None` for Bolt routes or when no @@ -602,4 +658,36 @@ mod tests { ); assert_ne!(bolt, unifying); } + + // Sysfs path: child of Unifying receiver + const UNIFYING_CHILD: &str = "/sys/devices/pci0000:00/0000:00:14.0/usb3/3-5/3-5.4/3-5.4.3/\ + 3-5.4.3:1.2/0003:046D:C52B.0009/0003:046D:4076.000A"; + // Sysfs path: the Unifying receiver node itself (terminal component has C52B) + const UNIFYING_RECEIVER: &str = "/sys/devices/pci0000:00/0000:00:14.0/usb3/3-5/3-5.4/3-5.4.3/\ + 3-5.4.3:1.2/0003:046D:C52B.0009"; + // Sysfs path: child of Bolt receiver + const BOLT_CHILD: &str = "/sys/devices/pci0000:00/0000:00:14.0/usb3/3-5/\ + 0003:046D:C548.0001/0003:046D:B037.0002"; + // Sysfs path: unrelated non-Logitech device + const UNRELATED: &str = "/sys/devices/pci0000:00/0000:00:15.0/i2c-0/0018:06CB:CE67.0001"; + + #[test] + fn child_of_unifying_receiver_is_detected() { + assert!(is_receiver_child_sysfs_path(UNIFYING_CHILD)); + } + + #[test] + fn unifying_receiver_itself_is_not_a_child() { + assert!(!is_receiver_child_sysfs_path(UNIFYING_RECEIVER)); + } + + #[test] + fn child_of_bolt_receiver_is_detected() { + assert!(is_receiver_child_sysfs_path(BOLT_CHILD)); + } + + #[test] + fn unrelated_device_is_not_a_child() { + assert!(!is_receiver_child_sysfs_path(UNRELATED)); + } } diff --git a/crates/openlogi-hidpp/src/receiver/bolt.rs b/crates/openlogi-hidpp/src/receiver/bolt.rs index 1b9e8c50..e7bcc49a 100755 --- a/crates/openlogi-hidpp/src/receiver/bolt.rs +++ b/crates/openlogi-hidpp/src/receiver/bolt.rs @@ -12,6 +12,7 @@ use std::sync::Arc; +use crate::receiver::ListenerDropGuard; use derive_builder::Builder; use futures::{FutureExt, pin_mut, select}; use num_enum::{IntoPrimitive, TryFromPrimitive}; @@ -92,7 +93,7 @@ pub enum InfoSubRegister { pub struct Receiver { chan: Arc, emitter: Arc>, - msg_listener_hdl: u32, + _listener: Arc, } impl Receiver { @@ -229,9 +230,12 @@ impl Receiver { }); Ok(Receiver { + _listener: Arc::new(ListenerDropGuard { + chan: Arc::clone(&chan), + hdl, + }), chan, emitter, - msg_listener_hdl: hdl, }) } @@ -518,12 +522,6 @@ fn parse_codename(response: &[u8; 16]) -> Option<&str> { str::from_utf8(raw).ok() } -impl Drop for Receiver { - fn drop(&mut self) { - self.chan.remove_msg_listener(self.msg_listener_hdl); - } -} - /// Indicates which notifications are enabled and thus sent by the receiver. /// /// This information can be queried using [`Receiver::get_notification_state`]. diff --git a/crates/openlogi-hidpp/src/receiver/mod.rs b/crates/openlogi-hidpp/src/receiver/mod.rs index 2be3f08d..eba44ef7 100755 --- a/crates/openlogi-hidpp/src/receiver/mod.rs +++ b/crates/openlogi-hidpp/src/receiver/mod.rs @@ -18,6 +18,23 @@ use thiserror::Error; use crate::{channel::HidppChannel, protocol::v10::Hidpp10Error}; +/// Removes a HID++ message listener when the last receiver clone is dropped. +/// +/// Storing this inside an `Arc` on the receiver struct prevents the +/// `#[derive(Clone)]` copy from sharing the raw `u32` handle: cloning a +/// receiver increments the Arc refcount, and `remove_msg_listener` is called +/// exactly once — when the last clone's Arc refcount reaches zero. +pub(super) struct ListenerDropGuard { + pub(super) chan: Arc, + pub(super) hdl: u32, +} + +impl Drop for ListenerDropGuard { + fn drop(&mut self) { + self.chan.remove_msg_listener(self.hdl); + } +} + pub mod bolt; pub mod unifying; @@ -63,7 +80,7 @@ impl Receiver { pub async fn get_unique_id(&self) -> Result { match self { Self::Bolt(bolt) => bolt.get_unique_id().await, - Self::Unifying(unifying) => unifying.get_receiver_info().await.map(|x| x.serial_number), + Self::Unifying(unifying) => unifying.get_unique_id().await, } } } diff --git a/crates/openlogi-hidpp/src/receiver/unifying.rs b/crates/openlogi-hidpp/src/receiver/unifying.rs index b43689cc..c086d33d 100755 --- a/crates/openlogi-hidpp/src/receiver/unifying.rs +++ b/crates/openlogi-hidpp/src/receiver/unifying.rs @@ -1,7 +1,12 @@ //! Implements the Unifying Receiver. //! -//! Unifying is a very versatile receiver that can pair up to 6 supported -//! devices. +//! Unifying is a versatile receiver that can pair up to 6 devices using the +//! 2.4 GHz eQuad radio protocol. It uses HID++ 1.0 registers for receiver +//! control; paired devices speak HID++ 2.0 once addressed via their slot index. +//! +//! The register layout for device enumeration (`0xB5/0x5N`, `0xB5/0x6N`) is +//! identical to Bolt's. The device-kind encoding differs from Bolt at values 5+ +//! (see [`DeviceKind`]). use std::sync::Arc; @@ -9,7 +14,9 @@ use num_enum::{IntoPrimitive, TryFromPrimitive}; use crate::{ channel::HidppChannel, - receiver::{RECEIVER_DEVICE_INDEX, ReceiverError}, + event::EventEmitter, + protocol::v10::{self, Hidpp10Error}, + receiver::{ListenerDropGuard, RECEIVER_DEVICE_INDEX, ReceiverError}, }; /// All USB vendor & product ID pairs that are known to identify Unifying @@ -17,53 +24,139 @@ use crate::{ pub const VPID_PAIRS: &[(u16, u16)] = &[(0x046d, 0xc52b), (0x046d, 0xc532)]; /// All known registers of the Unifying receiver. -/// -/// In most cases you should not need to access these manually, as [`Receiver`] -/// implements many features. #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, IntoPrimitive, TryFromPrimitive)] #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[non_exhaustive] #[repr(u8)] pub enum Register { + /// Enables or disables wireless device-connection notifications; also used + /// to read the pairing count and to trigger device-arrival events. + Connections = 0x02, + /// Provides information about the receiver and paired devices. It uses - /// sub-registers, as defined in [`UnifyingInfoSubRegister`], to - /// differentiate between different kinds of information. + /// sub-registers, as defined in [`InfoSubRegister`], to differentiate + /// between different kinds of information. ReceiverInfo = 0xb5, } -/// Represents the known sub-registers of the [`UnifyingRegister::ReceiverInfo`] +/// Represents the known sub-registers of the [`Register::ReceiverInfo`] /// register. #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, IntoPrimitive, TryFromPrimitive)] #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[non_exhaustive] #[repr(u8)] pub enum InfoSubRegister { - /// Provides general information about the receiver. + /// Provides general information about the receiver (serial number, pairing + /// slot count). ReceiverInfo = 0x03, + + /// Provides information about a specific paired device. The device index + /// (4 bits) must be added to this base address to form the actual + /// sub-register: `0x50 | (device_index & 0x0f)`. + DevicePairingInformation = 0x50, + + /// Provides the codename of a specific paired device. The device index (4 + /// bits) must be added: `0x60 | (device_index & 0x0f)`. + DeviceCodename = 0x60, } /// Implements the Unifying wireless receiver. #[derive(Clone)] pub struct Receiver { - /// The underlying HID++ channel. chan: Arc, + emitter: Arc>, + _listener: Arc, } impl Receiver { - /// Tries to initialize a new [`UnifyingReceiver`] from a raw HID++ channel. + /// Tries to initialize a new [`Receiver`] from a raw HID++ channel. /// - /// If no receiver could be found, or if the vendor and product IDs don't - /// match the ones of any known Unifying receiver, this function will return - /// [`ReceiverError::UnknownReceiver`]. + /// Returns [`ReceiverError::UnknownReceiver`] when the channel's VID/PID + /// doesn't match any known Unifying receiver. pub fn new(chan: Arc) -> Result { if !VPID_PAIRS.contains(&(chan.vendor_id, chan.product_id)) { return Err(ReceiverError::UnknownReceiver); } - Ok(Receiver { chan }) + let emitter = Arc::new(EventEmitter::new()); + + let hdl = chan.add_msg_listener({ + let emitter = Arc::clone(&emitter); + move |raw, matched| { + if matched { + return; + } + + let parsed = v10::Message::from(raw); + let header = parsed.header(); + let payload = parsed.extend_payload(); + + // Device-connection notifications are directed at a specific slot + // (header.device_index = slot) with sub_id 0x41. + if header.sub_id != 0x41 { + return; + } + + let Ok(kind) = DeviceKind::try_from(payload[1] & 0x0f) else { + return; + }; + + emitter.emit(Event::DeviceConnection(DeviceConnection { + index: header.device_index, + kind, + encrypted: payload[1] & (1 << 4) != 0, + online: payload[1] & (1 << 6) == 0, + wpid: u16::from_le_bytes(payload[2..=3].try_into().unwrap()), + })); + } + }); + + Ok(Receiver { + _listener: Arc::new(ListenerDropGuard { + chan: Arc::clone(&chan), + hdl, + }), + chan, + emitter, + }) } - /// Provides general information about the receiver. + /// Creates a new listener for receiving receiver events. + pub fn listen(&self) -> async_channel::Receiver { + self.emitter.create_receiver() + } + + /// Counts the number of devices currently paired to this receiver. + /// Offline (sleeping) devices are included since pairings are persistent. + pub async fn count_pairings(&self) -> Result { + let response = self + .chan + .read_register( + RECEIVER_DEVICE_INDEX, + Register::Connections.into(), + [0u8; 3], + ) + .await?; + + Ok(response[1]) + } + + /// Triggers device-arrival notifications for all currently connected + /// devices. Used to enumerate online devices at startup. + pub async fn trigger_device_arrival(&self) -> Result<(), ReceiverError> { + self.chan + .write_register( + RECEIVER_DEVICE_INDEX, + Register::Connections.into(), + [0x02, 0x00, 0x00], + ) + .await?; + + Ok(()) + } + + /// Provides general information about the receiver (serial number and + /// pairing slot count). pub async fn get_receiver_info(&self) -> Result { let response = self .chan @@ -79,6 +172,40 @@ impl Receiver { pairing_slots: response[6], }) } + + /// Retrieves the pairing information for the device at `device_index` + /// (1-based slot number). + pub async fn get_device_pairing_information( + &self, + device_index: u8, + ) -> Result { + let response = self + .chan + .read_long_register( + RECEIVER_DEVICE_INDEX, + Register::ReceiverInfo.into(), + [ + u8::from(InfoSubRegister::DevicePairingInformation) | (device_index & 0x0f), + 0x00, + 0x00, + ], + ) + .await?; + + Ok(DevicePairingInformation { + wpid: u16::from_le_bytes(response[2..=3].try_into().unwrap()), + kind: DeviceKind::try_from(response[1] & 0x0f) + .map_err(|_| Hidpp10Error::UnsupportedResponse)?, + encrypted: response[1] & (1 << 4) != 0, + online: response[1] & (1 << 6) == 0, + unit_id: response[4..=7].try_into().unwrap(), + }) + } + + /// Provides the unique ID of the receiver (serial number). + pub async fn get_unique_id(&self) -> Result { + self.get_receiver_info().await.map(|i| i.serial_number) + } } /// Represents some general information about a Unifying receiver. @@ -89,3 +216,61 @@ pub struct ReceiverInfo { pub serial_number: String, pub pairing_slots: u8, } + +/// Represents information about a paired device as read from the pairing +/// register. +#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)] +#[cfg_attr(feature = "serde", derive(serde::Serialize))] +#[non_exhaustive] +pub struct DevicePairingInformation { + pub wpid: u16, + pub kind: DeviceKind, + pub encrypted: bool, + pub online: bool, + pub unit_id: [u8; 4], +} + +/// Represents the kind of a device paired to a Unifying receiver. +/// +/// The encoding matches Bolt for values 1–4; from 5 onwards Unifying uses a +/// shifted table (Remote=5, Trackball=6, Touchpad=7) while Bolt reserves those +/// values and places them at 7–9. +#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, IntoPrimitive, TryFromPrimitive)] +#[cfg_attr(feature = "serde", derive(serde::Serialize))] +#[non_exhaustive] +#[repr(u8)] +pub enum DeviceKind { + Unknown = 0x00, + Keyboard = 0x01, + Mouse = 0x02, + Numpad = 0x03, + Presenter = 0x04, + Remote = 0x05, + Trackball = 0x06, + Touchpad = 0x07, +} + +/// Represents a device-connection event fired by the receiver when a paired +/// device comes online (or in response to [`Receiver::trigger_device_arrival`]). +#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)] +#[cfg_attr(feature = "serde", derive(serde::Serialize))] +#[non_exhaustive] +pub struct DeviceConnection { + /// Slot index (1-based) of the device. + pub index: u8, + pub kind: DeviceKind, + pub encrypted: bool, + pub online: bool, + /// Wireless product ID of the device. + pub wpid: u16, +} + +/// Represents an event emitted by the Unifying receiver. +#[derive(Clone, PartialEq, Eq, Hash, Debug)] +#[cfg_attr(feature = "serde", derive(serde::Serialize))] +#[non_exhaustive] +pub enum Event { + /// Fired whenever a paired device connects or reconnects, and for all + /// online devices in response to [`Receiver::trigger_device_arrival`]. + DeviceConnection(DeviceConnection), +}