feat(hid): Unifying receiver support#181
Conversation
Greptile SummaryThis PR adds full Logi Unifying receiver support to OpenLogi. Previously only Bolt (PID
Confidence Score: 5/5All three issues flagged in the prior review have been addressed; the new Unifying probe path is well-structured, hardware-verified, and covered by 18 new unit tests across 5 crates. The previous blocking findings — Clone/Drop sharing a raw listener handle, cross-receiver cache key collisions, and the timeout fallback discarding good cached data — are each correctly resolved. No new correctness bugs were found in this pass. The timing budget for a Unifying probe (ARRIVAL_DRAIN + concurrent UNIFYING_SLOT_PROBE) sits right at PROBE_BUDGET but is hardware-confirmed to work, and a miss just defers to the next tick. The only remaining note is a cosmetic Option wrapper that is never None. No files require special attention. The sysfs child-node filter (transport.rs) and the ListenerDropGuard pattern (receiver/mod.rs) are the most novel additions and both look correct. Important Files Changed
Sequence DiagramsequenceDiagram
participant E as Enumerator
participant T as Transport
participant U as UnifyingReceiver
participant C as HidppChannel
participant S as probe_unifying_slot
E->>T: enumerate_hidpp_devices()
note over T: filter is_receiver_child_node (Linux)
T-->>E: [hidraw nodes, child nodes excluded]
E->>C: open_hidpp_channel(dev)
E->>U: receiver::detect() → Receiver::Unifying
E->>U: get_unique_id() → serial
E->>U: count_pairings() → count
E->>U: trigger_device_arrival()
loop ARRIVAL_DRAIN (1.5 s per event)
U-->>E: "DeviceConnection { index, kind, wpid, online }"
end
par "concurrent per online slot (UNIFYING_SLOT_PROBE = 3.5 s each)"
E->>S: probe_unifying_slot(conn, receiver_uid)
S->>C: read_codename (0xFF/0xB5/0x6N — receiver register)
S->>C: "probe_or_reuse(slot, online=true)"
C-->>S: "ProbedFeatures { battery, model_info, capabilities }"
S-->>E: PairedDevice + CacheOutcome
end
E-->>E: "DeviceInventory { receiver, paired[] }"
E-->>E: apply CacheOutcome updates
Reviews (18): Last reviewed commit: "Merge remote-tracking branch 'origin/mas..." | Re-trigger Greptile |
1aa4cd0 to
0cdcc83
Compare
96a6e3b to
4aa67cd
Compare
Reconcile the agent's autostart state on Linux by writing/removing a systemd user unit at $XDG_CONFIG_HOME/systemd/user/openlogi-agent.service. Mirrors the macOS LaunchAgent semantics exactly: - Restart=on-failure (crash respawns; clean exit(0) stays stopped) - WantedBy=graphical-session.target (takes effect at next login) - ExecStart escaped for systemd (% doubled, spaces quoted) - Idempotent write/remove — only touches disk when content changes - systemctl --user daemon-reload + enable/disable best-effort
Add a Linux-specific permission probe and settings page row: - probe_uinput(): checks write access to /dev/uinput - probe_logitech_hidraw(): iterates /dev/hidraw*, confirms Logitech vendor (HID_ID sysfs field parsed numerically — 0000046D matches 046d) - classify(uinput_ok, hidraw_ok): pure function → Granted/Denied/Unknown - Settings → Permissions shows one "Input device access" row on Linux with description only when access is not yet granted (no noise when everything works) - macOS permission rows and helpers gated #[cfg(target_os = "macos")]
- Accessibility footer in the main window hidden on Linux - "Open" button in permission rows gated to macOS only - Launch-at-login description no longer says "log in to macOS"
- Remove misplaced sysfs comment above the open() call in probe_logitech_hidraw (the sysfs check is in is_logitech_hidraw) - Remove dead #[cfg(not(target_os = "macos"))] suppressor inside the already-macOS-gated permission_field function - Split Denied/Unknown description text: Unknown means uinput is accessible but no Logitech device is connected, so point the user at the device rather than the udev install guide
- escape_systemd_exec: double $ → $$ to prevent systemd variable substitution in ExecStart paths containing a literal dollar sign - paths: add pub xdg_config_home() that returns the raw XDG config base without the openlogi sub-directory; refactor config_dir() to call it - unit_path: use xdg_config_home() directly instead of relying on the fragile .parent() traversal from config_dir()
Added missing translations for the Linux permission row label introduced in this PR. Follows the same pattern as 'Input Monitoring'.
… scope - Add 'Unifying receiver' and 'Input device access' keys to en.yml (all locale files must match en.yml for the i18n test) - Add 'Ricevitore Unifying' to it.yml for key parity - Move InteractiveElement import outside #[cfg(target_os = "macos")] so on_action() calls for CloseWindow/Minimize/Zoom work on Linux - Fix rustfmt line-wrap in launch_agent::unit_path
- Add StatefulInteractiveElement as _ to unconditional gpui import so on_click() resolves on macOS (fixes E0599) - Gate status_badge with any(macos, linux) since both platforms call it (fixes Windows dead_code warning) - Remove unreachable input_device_access stub for non-mac/non-linux targets; nothing calls it there (fixes Windows dead_code warning)
Upstream added Option<bool> accessibility_granted with a 3-state match. Merge that with our Linux cfg structure: - Take upstream's Some(true)/Some(false)/None match for accessibility - Restore #[cfg(target_os = "macos")] on permission_field, permission_item, Permission import, and StatefulInteractiveElement import — all are macOS-only; gating them unconditionally caused dead_code / unused-import errors on Linux
02a59fa to
2b79ca0
Compare
On Windows, PermissionStatus, classify, rgb, and the permissions module are all dead — they only exist for macOS/Linux permission dialogs. Gate each with the appropriate cfg so Windows clippy -D warnings passes.
Add a Linux-specific permission probe and settings page row: - probe_uinput(): checks write access to /dev/uinput - probe_logitech_hidraw(): iterates /dev/hidraw*, confirms Logitech vendor (HID_ID sysfs field parsed numerically — 0000046D matches 046d) - classify(uinput_ok, hidraw_ok): pure function → Granted/Denied/Unknown - Settings → Permissions shows one "Input device access" row on Linux with description only when access is not yet granted (no noise when everything works) - macOS permission rows and helpers gated #[cfg(target_os = "macos")]
- udev/70-openlogi.rules: TAG+="uaccess" for hidraw (Logitech VID 046d) and uinput; includes non-systemd GROUP="input" fallback instructions - systemd/openlogi-agent.service: packaged user-unit template for /usr/lib/systemd/user/ (complements the per-user unit written by launch_at_login) - desktop/openlogi.desktop: XDG application launcher - install.sh / uninstall.sh: POSIX sh, set -eu; copies binaries + all artifacts, reloads udevadm, best-effort systemd and icon cache updates
docs/INSTALL-linux.md covers: prerequisites, build from source, udev rules (uaccess + non-systemd fallback), install.sh usage, autostart via systemctl, verification steps, and known limitations table. README.md gets a Linux subsection under ## Install with the minimal quick-start (build + udev) and a link to the full guide.
- install.sh: rewrite ExecStart in the systemd unit via sed so the installed service always points to $BINDIR/openlogi-agent, not the hardcoded /usr/bin path (which mismatches the default /usr/local prefix) - uninstall.sh: add udevadm trigger after reload so hidraw and uinput nodes lose the uaccess tag immediately, not only after re-plug/reboot
- install.sh: escape sed replacement string for BINDIR metacharacters (& \ |) so paths like /opt/my&pkg don't corrupt the unit file - install.sh: best-effort daemon-reload via sudo -u $SUDO_USER after writing the unit so reinstalls pick up the new ExecStart immediately - uninstall.sh: use SUDO_USER to target the real user's systemd session when the script is run under sudo, preventing the disable from silently targeting root's session while the user's agent keeps running
sudo -u strips the environment, so systemctl --user cannot locate the user's D-Bus socket without XDG_RUNTIME_DIR. Without it, disable --now in uninstall.sh silently fails (exit code 1, swallowed by || true), leaving the agent enabled even after the binary is removed. daemon-reload in install.sh has the same problem — it would silently skip the reload, requiring a manual reload before the updated unit takes effect. Fix both by computing REAL_UID / INSTALL_USER and passing XDG_RUNTIME_DIR=/run/user/<uid> explicitly to the sudo -u invocation.
Both install.sh (PREFIX/bin) and nfpm postinstall (/usr/bin) now expand the placeholder explicitly, removing the implicit assumption that the template's hardcoded path matches any particular installer.
nfpm.yaml maps the three binaries + udev rules + systemd user unit + desktop entry + icon into .deb/.rpm. VERSION env var is templated in at build time by the xtask. nfpm-scripts/postinstall.sh: reload udevadm, refresh icon and desktop caches, print enable-agent instructions. nfpm-scripts/preremove.sh: reload udevadm to revoke uaccess tags.
xtask/src/linux.rs: PackageLinux command builds release binaries (unless --no-build), then invokes nfpm for both deb and rpm. Reads the workspace version from Cargo.toml and passes it as VERSION to nfpm. .github/workflows/release.yml: new linux-packages job (ubuntu-latest) installs nfpm via the goreleaser apt repo, runs `cargo run -p xtask -- package-linux`, and uploads .deb/.rpm as an artifact. The publish job now depends on both dmg and linux-packages, downloads both artifact sets, and attaches .deb/.rpm to the GitHub Release alongside the DMGs.
- Replace workspace_version() file-I/O + fragile TOML line-scanner with
env!("CARGO_PKG_VERSION") — Cargo bakes the workspace version in at
compile time, zero I/O and always correct
- Merge two separate apt-get update + install steps in the linux-packages
CI job into one (add goreleaser repo first, then single update + install)
- nfpm.yaml: drop v prefix from version field — dpkg/rpm expect bare semver (0.6.0 not v0.6.0); v prefix can cause upgrade ordering issues - nfpm.yaml: fix maintainer to Name <email> format required by Debian - release.yml: replace trusted=yes with a proper GPG key pin for the goreleaser apt repo to prevent supply-chain substitution attacks
Replace the goreleaser apt repo (which has no GPG key and requires trusted=yes) with a direct download of a pinned nfpm release from GitHub. The SHA256 is checked before install, preventing a compromised CDN or MITM from substituting a malicious nfpm binary in the release pipeline.
postinstall.sh / preremove.sh: udevadm trigger is asynchronous — it queues uevent changes but returns before udev finishes processing them. Without udevadm settle, a user who immediately runs `systemctl --user enable --now openlogi-agent` after package install may find /dev/hidraw* and /dev/uinput still denying access because the uaccess tags haven't propagated yet. Add `udevadm settle || true` after the trigger calls in both scripts. release.yml: extend the minisign signing loop from dist/*.dmg to also cover dist/*.deb and dist/*.rpm, so all release artifacts have a .minisig sidecar and SHA256SUMS is consistent across all platforms.
…ring - Upgrade all upload-artifact@v7 → @v8 to match pre-existing download- artifact@v8 in publish job (v7/v8 use incompatible artifact backends) - Add linux-packages to publish job needs[] so download never races upload - Add *.deb *.rpm to sha256sum so Linux packages are integrity-checked - Rename preremove.sh → postremove.sh: dpkg/RPM invoke prerm before removing files, so udevadm trigger re-applied the rules instead of revoking them; postrm runs after deletion so the reload actually revokes
OpenLogi previously only enumerated Logi Bolt receivers (PID 0xC548).
Users with a Unifying receiver (PID 0xC52B / 0xC532) saw no devices.
openlogi-hidpp/receiver/unifying.rs
Add the missing HID++ 1.0 methods needed for device enumeration:
count_pairings(), trigger_device_arrival(), get_device_pairing_information(),
get_unique_id(). Add EventEmitter<Event> + message listener for 0x41
device-connection notifications (same pattern as BoltReceiver). Add
DeviceKind enum (shifted vs Bolt at values 5+: Remote=5, Trackball=6,
Touchpad=7). Update receiver/mod.rs to call get_unique_id() directly.
openlogi-hid/transport.rs
Add is_receiver_child_node() (Linux): the hid-logitech-dj kernel driver
creates a virtual hidraw node per paired Unifying device; these matched
our HID++ filter and caused 5-second probe timeouts every tick. Detect
them by checking whether any known receiver PID appears as a *parent*
directory component in the device's sysfs path, and filter them out
before probing. Extract the path-matching logic to is_receiver_child_sysfs_path()
for testability without filesystem access.
Add BOLT_PIDS and UNIFYING_PIDS as the canonical PID lists; pairing.rs
now derives from them so a new receiver PID needs editing in one place.
openlogi-hid/inventory.rs
probe_one: replace Bolt-only match with explicit Bolt / Unifying / direct
arms. Add probe_unifying_receiver (uses device-arrival events to surface
online paired devices) and probe_unifying_slot (builds PairedDevice from
the event; falls back gracefully on timeout). Add UNIFYING_SLOT_PROBE
(3.5 s) per-slot budget so a slow wireless HID++ 2.0 round-trip on one
device cannot consume the shared PROBE_BUDGET on behalf of others. Add
drain_device_arrival_unifying and map_unifying_kind.
openlogi-hid/route.rs
Add DeviceRoute::Unifying { receiver_uid, slot } — same structure as Bolt
but distinct so the label and write path are correct. Add device_route_for()
constructor that picks Unifying/Bolt/Direct from the receiver's product ID
using the canonical PID lists. Update open_route_channel to handle the
Unifying arm. Add BOLT_PIDS and UNIFYING_PIDS constants.
Consumers of the HID layer all constructed DeviceRoute::Bolt for any receiver-backed device. This caused three problems for Unifying devices: 1. open_route_channel only tried Receiver::Bolt — writes silently failed. 2. The Device tab showed "Bolt receiver" for Unifying devices. 3. The Buttons tab showed the generic mouse hotspot layout for keyboards (K540 has ReprogControls so capabilities.buttons=true, but no asset). 4. Paired devices without HID++ 2.0 model info were skipped entirely by build_device_list, so the K540 never appeared in the carousel. openlogi-agent-core device_order.rs: DeviceStableId::from_parts folds Unifying into the Bolt variant (same slot-based sort key regardless of receiver family, so the GUI carousel and agent agree on "first device"). orchestrator.rs: call DeviceRoute::device_route_for() directly. openlogi-cli/diag Use DeviceRoute::device_route_for() at both diag call sites. openlogi-gui/app.rs route_label: add "Unifying receiver" arm for DeviceRoute::Unifying. tabs_for: gate the Buttons tab on `can_show_mouse_model` — a pointer-type device (Mouse/Trackball) or one with a resolved asset. A keyboard that exposes ReprogControls but has no asset would otherwise show the generic mouse hotspot layout (Middle Click, DPI Toggle, …) which is wrong. Update tabs_follow_capabilities_not_kind test to use kind=Mouse (the 0x0005 kind-correction fix from AprilNEA#127 corrects mislabeled mice at probe time; the test scenario of kind=Keyboard + mouse-caps no longer arises). Add keyboard_without_asset_hides_buttons_tab test. openlogi-gui/state/devices.rs build_device_list: remove the hard model_info guard. Devices without HID++ 2.0 model info (HID++ 1.0 devices, or probes that timed out) are now surfaced with a wpid-based config_key ("wpid{:04x}") so their settings persist across sessions; slot is the last-resort fallback. Call DeviceRoute::device_route_for() directly (one-liner wrapper removed).
inventory.rs — probe_unifying_slot now receives the receiver's unique_id and incorporates its first 3 ASCII bytes into the CacheKey so two Unifying receivers with a device on the same slot number use distinct cache entries. The previous [0,0,0,slot] key was receiver-agnostic and would hand receiver B's slot-1 query the cached ProbedFeatures built for receiver A's slot-1 device (different model, different capabilities). route.rs — device_route_for: make the Bolt-default design explicit in the doc comment and add a tracing::debug for receivers whose PID is in neither BOLT_PIDS nor UNIFYING_PIDS. Behaviour is unchanged — returning None for unknown PIDs would silently drop writes for future Bolt variants with new PIDs, which is worse than routing them as Bolt.
unifying.rs — #[derive(Clone)] copied msg_listener_hdl (u32) verbatim,
so dropping any clone called remove_msg_listener with the shared handle
and silently deregistered the listener for all surviving clones. The
next drain_device_arrival_unifying would return empty, making Unifying
devices vanish from inventory.
Replace the bare u32 with Arc<ListenerDropGuard>: every Receiver clone
shares the Arc; remove_msg_listener is called exactly once, when the
Arc's refcount hits zero (last clone dropped). The manual Drop impl is
removed — ListenerDropGuard::drop owns the cleanup.
inventory.rs — CacheKey::Bolt { unit_id: [uid[0..3], slot] } used only
3 ASCII bytes of the receiver serial as a prefix, so two receivers whose
serials share the same first three characters (common in Logitech batches,
e.g. "DA2699E1" and "DA2604F2") still collided on the same slot.
Add CacheKey::UnifyingSlot { receiver_uid: String, slot: u8 } keyed on
the full serial string + slot. Two Unifying receivers with a device on
the same slot number now have provably distinct cache entries.
Added missing translations for the route label introduced in this PR. Placed alongside 'Bolt receiver' following the same pattern.
BoltReceiver had the same Clone+Drop bug fixed for UnifyingReceiver in the previous commit: #[derive(Clone)] copied msg_listener_hdl (u32) verbatim, so the first drop of any clone deregistered the HID++ listener for all surviving copies — subsequent drain_device_arrival calls would return empty and Bolt devices would vanish from inventory. Move ListenerDropGuard to receiver/mod.rs (pub(super)) so it is shared by both bolt.rs and unifying.rs without duplication. Apply the Arc-wrap fix to BoltReceiver identically: store Arc<ListenerDropGuard> instead of a bare u32, remove the manual impl Drop.
On timeout the fallback returned ProbedFeatures::default() even when a prior successful probe was in the cache, causing battery, model name, and capability flags to disappear from the carousel for that device. Return the cached probe when available and emit Seen so the entry is not counted toward eviction.
The function is only called from a #[cfg(target_os = "linux")] context, making it dead code on macOS/Windows. Gate it with #[cfg(any(target_os = "linux", test))] so the tests remain cross-platform while silencing the dead_code lint in production builds.
Empty string fallback causes CacheKey collisions when two receivers share the same slot and both fail get_unique_id(). Product ID is a weaker but non-empty discriminant; a warning is logged so the degraded isolation is visible.
A manually-disabled unit is not re-enabled on app restart because reconcile exits early when the file content matches. Always call systemctl enable in that arm — it is idempotent and no daemon-reload is needed when the file is unchanged.
…task fn PermissionStatus and classify are only meaningful on platforms where permission-check functions exist (macOS and Linux). Gate them so the Windows clippy job no longer reports dead-code warnings. Gate the test module to Linux-only since the tests exercise Linux-specific classify logic. Remove the unused workspace_version helper from xtask/linux.rs (package_linux uses env! instead).
2b79ca0 to
4ac6458
Compare
# Conflicts: # crates/openlogi-gui/src/windows/settings.rs
4ac6458 to
59fe8f2
Compare
# Conflicts: # crates/openlogi-agent/src/launch_agent.rs
Resolve the IPC poll conflict against the startup-states rework and adapt the diagnostics adapter to master: - call store_agent_snapshot before the status snapshot moves into AgentLink::Ready; the inventory merge keeps the InventoryHealth gate (AprilNEA#213/AprilNEA#215/AprilNEA#220) - read accessibility from the retained AgentStatus; the AppState.accessibility_granted field no longer exists - map DeviceRoute::Unifying (AprilNEA#181) to a new ConnectionKind::UnifyingReceiver
Resolves the inventory.rs conflict with #181 (Unifying receiver support): - probe_one keeps #181's receiver-dispatcher shape but every probe path now returns NodeProbe, feeding the per-node ledger; the Result wrapper is gone (no probe path constructs an error). - Unifying probes get a ledger health verdict: the pairing-count register answering is the health signal — a count/list shortfall is expected while offline Unifying slots aren't enumerable — and a failed arrival trigger (the only Unifying device source) settles as a failed probe so the ledger replays the last snapshot instead of publishing an empty list (#218). - drain_device_arrival_unifying returns Option to distinguish a failed trigger from no devices online. - map_unifying_kind and its test move to mappings.rs with the other pure mappings.
Summary
OpenLogi previously only enumerated Logi Bolt receivers (PID
0xC548). Users with a Unifying receiver (PID0xC52B/0xC532) saw no devices in the GUI or CLI.This PR adds full Unifying receiver support:
openlogi-hidpp): implement the missing HID++ 1.0 methods onUnifyingReceiver—count_pairings,trigger_device_arrival,get_device_pairing_information, device-connection event listener (0x41 sub_id),DeviceKindenum (shifted vs Bolt at values ≥5).openlogi-hid): thehid-logitech-djkernel driver creates a virtual hidraw node per paired Unifying device; these matched the HID++ filter and caused 5-second probe timeouts every enumeration tick. Addedis_receiver_child_node()(Linux) that checks the sysfs path and filters them before probing.openlogi-hid): splitprobe_oneto handleReceiver::Unifyingseparately. Addedprobe_unifying_receiver/probe_unifying_slotthat buildPairedDeviceentries from device-arrival events. AddedUNIFYING_SLOT_PROBE(3.5 s) per-slot budget so a slow wireless HID++ 2.0 round-trip cannot consume the sharedPROBE_BUDGET.openlogi-hid): addDeviceRoute::Unifyingvariant; addDeviceRoute::device_route_for()constructor that picks the right variant from the receiver's product ID; consolidateBOLT_PIDS/UNIFYING_PIDSas the single source of truth (previously duplicated in three places).openlogi-agent-core,openlogi-gui,openlogi-cli): update all routing call sites, fix "Bolt receiver" label, add "Unifying receiver" label, surface devices that lack HID++ 2.0 model info in the carousel (wpid-based config key), hide the mouse-model Buttons tab for keyboards without an asset.Verified on hardware
Known limitations
0xB5/0x5Npairing-info register returnsInvalidValue; the sub-register format differs from Bolt and needs further investigation.CacheKey::Boltsentinel[0,0,0,slot]is reused for Unifying slot caching (no unit_id available without working pairing-info register); documented in the code.Test plan
cargo test --workspacepasses (18 new unit tests across 5 crates)cargo clippy --workspace -- -D warningsclean./target/debug/openlogi-agent+./target/debug/openlogi-gui— all three devices appear in the carousel🤖 Generated with Claude Code