feat(hid): hold-to-repeat for thumb buttons over HID++ - #467
Conversation
Holding a button bound to volume (or scroll) only fired once. Repeating needs the release edge, and the OS hook cannot supply it: the device reports the thumb buttons to the OS as a short press/release pulse — measured 7-22 ms on a Lift regardless of how long the button is physically held, with a multi-second hold emitting no mouse or keyboard event at all. The HID++ 0x1b04 divertedButtonsEvent does carry it: the report holds the complete set of currently-held control IDs, so a CID leaving the set is a release. Diverting is opt-in per button and narrow on purpose. `hold_buttons` selects only buttons whose bound action is repeatable, so a thumb button bound to Back or Forward keeps its native OS-hook path and its crash-safety: a hook remap dies with the process, but a divert outlives it and leaves the button dead to the OS until something restores it. `arm_controls` also checks `is_divertable()` before touching a control rather than assuming the CID exists. The repeat itself accelerates: 400 ms before the second fire, then a gap that shrinks 15% per fire from 220 ms down to a floor of 45 ms (~1.2 s to reach it). A short hold nudges a step or two; a long hold sweeps the range. Timing runs on a worker thread, never in the hook callback, and the repeat state stays off the thread-local HOLD so a synthesized event re-entering the tap cannot double-borrow it. `Action::is_repeatable` is an explicit list, not a `category()` test: `Category::Media` holds both VolumeUp (repeatable) and PlayPause (must not be). Known limitation, pre-existing: the agent has no graceful-shutdown path (takeover uses SIGTERM, IPC has no quit method), so an abnormal exit skips the restore in `ArmedControls::disarm` and leaves a diverted button inactive until the agent runs again — restarting it re-diverts and resumes handling, and a reboot recovers via launch-at-login. Power-cycling the mouse is only needed to get the native OS behavior back with no agent running. BACK_CID / FORWARD_CID were reverse-engineered on one device, not read off a spec, and are marked as such.
Greptile SummaryThe PR adds HID++ hold-to-repeat for repeatable thumb-button actions.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; keyed repeat cycles, rising-edge gating, stale-input disposal, teardown cancellation, and pre-recorded diversion rollback address the previously reported paths.
|
| Filename | Overview |
|---|---|
| crates/openlogi-agent-core/src/hook_runtime.rs | Adds independent keyed repeat scheduling, release cancellation, interruption cleanup, and timing tests. |
| crates/openlogi-agent-core/src/watchers/gesture.rs | Integrates HID++ hold events with repetition and safely discards queued input when capture sessions end. |
| crates/openlogi-core/src/binding.rs | Explicitly identifies increment-style actions that are safe to repeat. |
| crates/openlogi-hid/src/gesture.rs | Adds hold-edge capture and pessimistically recorded diversion state for reliable rollback and teardown. |
| crates/openlogi-hid/src/gesture/tests.rs | Covers paired hold edges, independent buttons, and suppression of untracked controls. |
| crates/openlogi-hid/src/reprog_controls.rs | Introduces reverse-engineered Back and Forward control-ID mappings used for selective hold tracking. |
Sequence Diagram
sequenceDiagram
participant Mouse as Logitech mouse
participant HID as HID++ capture
participant Manager as Gesture watcher
participant Repeater as Repeat worker
participant Action as Action dispatcher
Mouse->>HID: Diverted button pressed
HID->>Manager: ButtonPressed(button)
Manager->>Action: Dispatch initial action
Manager->>Repeater: Start(button, action)
loop While button remains held
Repeater->>Action: Dispatch repeated action
end
Mouse->>HID: Diverted button released
HID->>Manager: ButtonReleased(button)
Manager->>Repeater: Stop(button)
opt Capture session ends
Manager->>Manager: Replace input channel
Manager->>Repeater: StopAll
Manager->>HID: Restore diverted controls
end
Reviews (4): Last reviewed commit: "fix(agent): give each capture session it..." | Re-trigger Greptile
…urce Review found four ways a repeat could outlive the hold that started it. `RepeatCmd` now names a button in every variant. One worker still owns the timing, but it keeps a cycle per button, each with its own due time and its own ramp, so two thumb buttons held at once no longer share a single unkeyed cycle that either release could cancel. `RepeatCycles` is split from the worker thread so the scheduling is testable without dispatching real actions: the worker only sleeps and dispatches, `RepeatCycles` decides what is due and when to wake. A cycle also needs a guaranteed release edge, and only the diverted thumb-side buttons have one — the DPI/ModeShift and thumb-wheel captures are rising-edge only. `reports_hold_edges` gates both the divert set and the repeat start on having a hold CID, so binding a repeatable action to DpiToggle no longer starts something nothing can stop. When capture goes away, no release is coming for anything still held, so the new `StopAll` ends every cycle. It goes out on the deliberate teardown (target or bindings changed, pairing wants the receiver) via `stop_session`, on an unexpected session death before re-arming, and on the hook path when the OS disables the tap. Arming is several independent HID++ writes and any one can fail. `arm_controls` now builds an `ArmedControls` in place and hands it to `disarm` on failure, instead of returning early and leaving an already-diverted thumb button captured with no session alive to release it — which for the user meant a dead button until the mouse was power-cycled.
|
All four findings are fixed in 310528f — replies are on the three inline threads; this one covers the out-of-diff comment on Capture shutdown leaves repetition active. Right — when the session goes away, no release edge is coming for anything still held. Added
Guarded by Verification after the fixes: Not re-tested on hardware since these fixes; the earlier hardware run described in the PR body was against the previous commit. |
Rollback recorded a control only after its set_cid_reporting response came back, but the request reaches the device before the response does: a timed-out or lost response can leave the control diverted with nothing in the record to restore it, which for a thumb button means dead to the OS until the mouse is power-cycled. Record first, then await. The record is now deliberately pessimistic — restoring a control that was never actually diverted just hands back a mapping the firmware already owns, while missing one is the failure above.
The manager receiver outlived individual sessions, so a press captured just before a teardown stayed queued and was dispatched after StopAll had run. That started a repeat cycle whose release could never arrive — the session that would have reported it was already gone — and the action kept firing. Teardown now hands the manager a fresh channel, dropping whatever the dead session had queued along with its receiver, on both the deliberate stop and the unexpected-death path. The correctness no longer rests on the order the select loop happens to poll its branches in.
Summary
Holding a thumb button bound to volume only fires once — you have to click repeatedly. Fixes #389 and #365.
The OS hook cannot support this, and the reason is worth recording: the device reports the thumb buttons to the OS as a short press/release pulse regardless of how long the button is held. Measured on a Logitech Lift (BLE, PID
b031) with a low-level mouse hook, nothing else running:XBUTTONpairs,DOWN→UPin 7-22 ms eachSo there is no hold duration to read on that path. HID++
0x1b04divertedButtonsEventdoes carry it — the report holds the complete set of currently-held control IDs, so a CID leaving the set is a release.handle_reprogalready derives rising edges from this for the DPI button; this surfaces the falling edge too.Diverting is opt-in per button, on purpose.
hold_buttonsselects only buttons whose bound action is repeatable and that actually report a release edge. A thumb button bound to Back/Forward keeps its native OS-hook path and its crash-safety: a hook remap dies with the process, but a divert outlives it. That tradeoff is the reason this is not applied to the thumb buttons unconditionally.Changes
openlogi-coreAction::is_repeatable()— explicit list (volume up/down, the four scroll directions), deliberately not derived fromcategory():Category::Mediaholds bothVolumeUp(repeatable) andPlayPause(must not be).openlogi-hidCapturedInput::ButtonReleased(ButtonId), emitted only for hold-tracked buttons.BACK_CID/FORWARD_CID+hold_cid_for_button(). Reverse-engineered on one device, not read off a spec, and marked as such — identified by correlatingdivertedButtonsEventreports againstWM_XBUTTONDOWNtimestamps.arm_controlstakeshold_buttons, diverts only what the caller asked for, and skips any control the firmware does not report asis_divertable()rather than assuming the CID exists. It builds itsArmedControlsin place anddisarms it if any of the several HID++ writes fails part-way, so a failure cannot leave a thumb button diverted with no session alive to release it. Each control is recorded before its divert is awaited, since the request reaches the device before the response comes back — a lost response would otherwise escape the rollback.handle_reprogtracks the held set and emits both edges once each.ArmedControls::disarmrestores the new CIDs.openlogi-agent-corespawn_repeater— worker thread driving the repeat, shared by both input paths. Timing never runs in the hook callback (freeze hazard), and the state stays off the thread-localHOLDso a synthesized event re-entering the tap cannot double-borrow it.RepeatCmdnames a button in every variant, and the worker holds one cycle per button — own due time, own ramp — so two thumb buttons held at once neither share a cycle nor cancel each other on release. The scheduling lives in aRepeatCyclesvalue split from the thread so it is testable without dispatching real actions.RepeatCmd::StopAllfor the cases where no release edge is coming: capture torn down deliberately (target/bindings changed, pairing wants the receiver), a session that died unexpectedly, or the OS disabling the tap. Teardown also swaps in a fresh input channel, so a press the dead session had already queued is dropped rather than dispatched after the stop.watchers::gesture—hold_buttons()derives the divert set from the bindings and joins the session key; press starts the cycle, release stops it.reports_hold_edges()keeps both the divert set and the repeat start to buttons with a hold CID, since the DPI/ModeShift and thumb-wheel captures are rising-edge only and would start a cycle nothing could stop.dispatchgained aDispatchCtxso the signature stays within the argument limit.Testing
New tests:
repeatable_actions_are_the_increment_style_onestoggles_and_one_shots_never_repeat— guards thatMuteVolume/PlayPausenever repeatevery_repeatable_action_is_in_the_cataloga_hold_tracked_button_emits_both_edges_exactly_once— repeated frames must not re-emithold_tracking_keeps_the_two_thumb_buttons_independentan_undiverted_button_emits_nothing_even_if_the_device_reports_itthe_repeat_ramp_accelerates_and_then_holds_at_the_floorthe_repeat_ramp_reaches_the_floor_in_about_a_secondFrom the review fixes:
a_release_stops_only_the_button_that_was_releasedan_unrelated_buttons_release_leaves_the_hold_runningeach_button_ramps_on_its_own_clocka_re_press_restarts_that_buttons_delay_and_rampnothing_fires_before_the_initial_delay_elapses— an ordinary click must never repeatstop_all_ends_every_holdonly_the_hold_tracked_buttons_report_both_edgeshold_buttons_lists_only_repeatable_bindings_that_can_be_trackedinput_queued_before_a_teardown_never_reaches_the_next_sessionRuntime-tested on hardware: yes — Logitech Lift over Bluetooth LE on Windows 11, both thumb buttons bound to volume up/down. Press-and-hold accelerates as described; a single click still moves exactly one step. Only tested on that one device and only on Windows; not tested on macOS or Linux, on a Bolt/Unifying receiver, or on any other model. The hardware run predates the review fixes in 310528f, f6c9396 and 87c41dc, which have not been re-tested on hardware.
Note: building this on Windows also needs #458 (unrelated one-line build fix), otherwise
openlogi-hiddoes not compile there.Known limitation (pre-existing)
The agent has no graceful-shutdown path —
takeover.rsnotes "SIGTERM, not a polite RPC: past protocols have no quit method", and the IPC has no quit method — so an abnormal exit skipsArmedControls::disarmand leaves a diverted button inactive until the agent runs again. Restarting the agent re-diverts and resumes handling it; a reboot recovers via launch-at-login. Power-cycling the mouse is only needed to get the native OS behavior back with no agent running.This is not introduced here, but hold-to-repeat extends its blast radius from the DPI/gesture controls to an ordinary thumb button, so it may be worth a follow-up: on shutdown, signal the capture session and await its restore before exiting. Happy to split that out if you'd prefer it landed first.
Fixes #389
Fixes #365