Skip to content

feat(hid): hold-to-repeat for thumb buttons over HID++ - #467

Open
Brumaire wants to merge 4 commits into
AprilNEA:masterfrom
Brumaire:feat/hold-to-repeat
Open

feat(hid): hold-to-repeat for thumb buttons over HID++#467
Brumaire wants to merge 4 commits into
AprilNEA:masterfrom
Brumaire:feat/hold-to-repeat

Conversation

@Brumaire

@Brumaire Brumaire commented Jul 26, 2026

Copy link
Copy Markdown

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:

Input Result
5 quick clicks 5 XBUTTON pairs, DOWNUP in 7-22 ms each
Hold 3 s, then 5 s no mouse event and no keyboard event at all

So there is no hold duration to read on that path. 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. handle_reprog already derives rising edges from this for the DPI button; this surfaces the falling edge too.

Diverting is opt-in per button, on purpose. hold_buttons selects 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-core

  • Action::is_repeatable() — explicit list (volume up/down, the four scroll directions), deliberately not derived from category(): Category::Media holds both VolumeUp (repeatable) and PlayPause (must not be).

openlogi-hid

  • CapturedInput::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 correlating divertedButtonsEvent reports against WM_XBUTTONDOWN timestamps.
  • arm_controls takes hold_buttons, diverts only what the caller asked for, and skips any control the firmware does not report as is_divertable() rather than assuming the CID exists. It builds its ArmedControls in place and disarms 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_reprog tracks the held set and emits both edges once each.
  • ArmedControls::disarm restores the new CIDs.

openlogi-agent-core

  • spawn_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-local HOLD so a synthesized event re-entering the tap cannot double-borrow it.
  • RepeatCmd names 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 a RepeatCycles value split from the thread so it is testable without dispatching real actions.
  • RepeatCmd::StopAll for 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.
  • Accelerating ramp: 400 ms before the second fire, then a gap shrinking 15% per fire from 220 ms to a 45 ms floor (~1.2 s to reach it). Integer maths so the curve is exactly reproducible in tests.
  • watchers::gesturehold_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. dispatch gained a DispatchCtx so the signature stays within the argument limit.

Testing

cargo fmt --all -- --check                                                                          # clean
cargo clippy -p openlogi-core -p openlogi-hid -p openlogi-agent-core --all-targets -- -D warnings    # clean
cargo test -p openlogi-core -p openlogi-hid -p openlogi-agent-core                                  # 237 passed

New tests:

  • repeatable_actions_are_the_increment_style_ones
  • toggles_and_one_shots_never_repeat — guards that MuteVolume / PlayPause never repeat
  • every_repeatable_action_is_in_the_catalog
  • a_hold_tracked_button_emits_both_edges_exactly_once — repeated frames must not re-emit
  • hold_tracking_keeps_the_two_thumb_buttons_independent
  • an_undiverted_button_emits_nothing_even_if_the_device_reports_it
  • the_repeat_ramp_accelerates_and_then_holds_at_the_floor
  • the_repeat_ramp_reaches_the_floor_in_about_a_second

From the review fixes:

  • a_release_stops_only_the_button_that_was_released
  • an_unrelated_buttons_release_leaves_the_hold_running
  • each_button_ramps_on_its_own_clock
  • a_re_press_restarts_that_buttons_delay_and_ramp
  • nothing_fires_before_the_initial_delay_elapses — an ordinary click must never repeat
  • stop_all_ends_every_hold
  • only_the_hold_tracked_buttons_report_both_edges
  • hold_buttons_lists_only_repeatable_bindings_that_can_be_tracked
  • input_queued_before_a_teardown_never_reaches_the_next_session

Runtime-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-hid does not compile there.

Known limitation (pre-existing)

The agent has no graceful-shutdown path — takeover.rs notes "SIGTERM, not a polite RPC: past protocols have no quit method", and the IPC has no quit method — so an abnormal exit skips ArmedControls::disarm and 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

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-apps

greptile-apps Bot commented Jul 26, 2026

Copy link
Copy Markdown

Greptile Summary

The PR adds HID++ hold-to-repeat for repeatable thumb-button actions.

  • Tracks independent repeat cycles by button, with per-button cancellation and acceleration.
  • Diverts only hold-capable controls and emits paired press and release events.
  • Discards stale session input and stops active repeats during capture teardown.
  • Rolls back controls recorded before each asynchronous diversion request.

Confidence Score: 5/5

The 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.

Important Files Changed

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
Loading

Reviews (4): Last reviewed commit: "fix(agent): give each capture session it..." | Re-trigger Greptile

Comment thread crates/openlogi-agent-core/src/hook_runtime.rs Outdated
Comment thread crates/openlogi-hid/src/gesture.rs
Comment thread crates/openlogi-agent-core/src/watchers/gesture.rs Outdated
…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.
@Brumaire

Copy link
Copy Markdown
Author

All four findings are fixed in 310528f — replies are on the three inline threads; this one covers the out-of-diff comment on watchers/gesture.rs.

Capture shutdown leaves repetition active. Right — when the session goes away, no release edge is coming for anything still held. Added RepeatCmd::StopAll, sent from every path where capture ends:

  • deliberate teardown (target or bindings changed, pairing wants the receiver) — the oneshot signal and the stop now go together in a new stop_session helper, so they cannot drift apart;
  • unexpected session death, before re-arming;
  • the OS hook path, when the tap is disabled and events are dropped (this replaced the unkeyed Stop that used to be sent there, which would no longer have covered a second held button).

Guarded by stop_all_ends_every_hold.

Verification after the fixes:

cargo fmt --all -- --check                                                                          # clean
cargo clippy -p openlogi-core -p openlogi-hid -p openlogi-agent-core --all-targets -- -D warnings    # clean
cargo test -p openlogi-core -p openlogi-hid -p openlogi-agent-core                                  # 236 passed

Not re-tested on hardware since these fixes; the earlier hardware run described in the PR body was against the previous commit.

Comment thread crates/openlogi-hid/src/gesture.rs Outdated
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.
Comment thread crates/openlogi-agent-core/src/watchers/gesture.rs Outdated
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Long press (maintain) instead of continuous clicking [Bug]: Volume gesture only changes volume by one step

1 participant