feat(hid): Unifying receiver support#181
Open
cserby wants to merge 39 commits into
Open
Conversation
Greptile SummaryThis PR adds full Logitech Unifying receiver support across the entire stack — from HID++ 1.0 protocol methods in
Confidence Score: 5/5Safe to merge; the one finding is a theoretical edge case in systemd unit escaping that cannot be triggered by real-world executable paths. The core protocol, caching, routing, and Clone+Drop fixes are all correct and well-tested. The only finding is that escape_systemd_exec leaves a bare double-quote unescaped when the path has no spaces — a path from current_exe() would never contain a double-quote on Linux, so this cannot cause a real failure today. crates/openlogi-agent/src/launch_agent.rs — specifically the escape_systemd_exec helper. Important Files Changed
Reviews (16): Last reviewed commit: "fix(gui,xtask): gate platform permission..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
4 tasks
cserby
force-pushed
the
story/linux-hid/unifying-receiver-support
branch
from
1aa4cd0 to
0cdcc83
Compare
cserby
force-pushed
the
story/linux-hid/unifying-receiver-support
branch
7 times, most recently
from
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 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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")] Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 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" Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 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 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 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() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Added missing translations for the Linux permission row label introduced in this PR. Follows the same pattern as 'Input Monitoring'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… 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 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 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) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cserby
force-pushed
the
story/linux-hid/unifying-receiver-support
branch
3 times, most recently
from
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. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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")] Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 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 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 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 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 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 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Bluetooth HID devices go through the uhid virtual bus, which has no
idVendor sysfs attribute — ATTRS{idVendor}=="046d" doesn't match them.
Add a second rule matching on the HID kernel name format
"bus:VID:PID.iface", whose VID field (046D) covers both BT and USB.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 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)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- 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 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…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 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Added missing translations for the route label introduced in this PR. Placed alongside 'Bolt receiver' following the same pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…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). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cserby
force-pushed
the
story/linux-hid/unifying-receiver-support
branch
from
2b79ca0 to
4ac6458
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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