Repository navigation
Conversation
The 160 ms hold required before gesture travel may commit to a swipe is a compile-time constant. Users who flick the gesture button faster than that reach a release with no committed direction, so the press is read as a plain click — the binding fires instead of the gesture, on every swipe. Add a per-device `gesture_hold` (40-1000 ms, default 160) and plumb it to both capture paths: the HID++ gesture sources and the OS-hook gesture buttons. - `GestureHold` is a validated newtype, omitted from config.toml when unset. - `SwipeAccumulator::begin_with(hold)` carries the gate per hold; `begin()` keeps the shared default so existing call sites and tests are unchanged. - `CaptureSpec` and `HookMaps` carry the configured value to the accumulators. Config schema: `DeviceConfig` denies unknown fields, so a file written with `gesture_hold` is rejected by a build that predates this field. The schema bump is left to the maintainer per docs/DEVELOPMENT.md.
[Medium impact] The change affects gesture-button behavior across both supported capture paths. Fix the repeat switch before merging; users can still trigger multiple actions with repeats disabled.
|
| /// 160 ms) and is omitted from `config.toml`. Lowering it lets a quick | ||
| /// hold-and-flick register as a swipe instead of a plain click on release. | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub gesture_hold: Option<GestureHold>, |
There was a problem hiding this comment.
Chosen delay disappears on reconnect
config::identity::fold does not carry over gesture_hold. If a user adds this setting to an existing route-keyed entry while the device is offline, the next online adoption removes that entry and drops the value. The device then uses 160 ms again.
Add fold_option_field!(gesture_hold) alongside the other optional settings and cover adoption with a test.
Knowledge Base Used: Core domain and configuration
| #[test] | ||
| fn a_hold_gate_of_zero_commits_on_the_first_report() { | ||
| let mut acc = SwipeAccumulator::default(); | ||
| acc.begin_with(Duration::ZERO); |
There was a problem hiding this comment.
Custom delay lacks capture tests
The only custom-delay test calls SwipeAccumulator::begin_with directly with zero, which the config cannot accept. The hook and device tests still pass GestureHold::DEFAULT, so they can stay green if either capture path drops the user's setting.
Add tests using an allowed non-default value through both paths, including reset_input_state.
AprilNEA
left a comment
There was a problem hiding this comment.
Thanks for the PR.
Whether OpenLogi adds this setting is a maintainer decision. This PR stays open until then. Three open PRs target the symptom in #1016:
- #1048 (2026-08-26): per-control hold and travel presets, GUI and TOML.
- #1996 (2026-10-23): a long, straight flick commits before the gate. No new setting.
- This PR: a per-device hold gate, TOML only. The default stays 160 ms, so #1016 still happens for users who do not edit
config.toml.
Findings that apply to any version of this change:
crates/openlogi-core/src/config.rs: master is atSCHEMA_VERSION = 10since #993. After a rebase, a newDeviceConfigfield needs v11 and a doc paragraph, as v8 (smooth_wheel) and v10 (disabled_keys) did.crates/openlogi-core/src/config/identity.rs:247fold(): addfold_option_field!(gesture_hold);and an adoption test. This is the Greptile thread ondevice.rs:286.crates/openlogi-agent-core/src/capture_plan.rs:326andorchestrator.rs:668repeat one lookup. AddConfig::gesture_hold(&self, device_key) -> GestureHoldinconfig/per_device.rs, next tosmooth_wheel()(.agents/rules/rust.md: "Policy is an API").crates/openlogi-device/src/session/gesture.rs:196:CaptureTargetequality is the firmware rearm contract (capture_plan.rs:25-28). With the gate inCaptureSpec, each edit restores and re-diverts the controls.thumbwheel_sensitivity, also a software value, lives in the hot-replaceableDispatchPlan. Hot-update the gate inGestureCapture, or state the trade-off on the field.config/settings.rs:385GESTURE_HOLD_DEFAULT_MSandbinding/swipe.rs:35GESTURE_HOLD_FOR_SWIPEare two copies of 160 ms (AGENTS.md: "Single source of truth"). Keep one and derive the other from it.binding/swipe.rs:155begin_with(Duration)accepts anyDurationand skips the validated type. Use onebegin(hold: GestureHold)and store theGestureHold. ItsDefaultthen keeps#[derive(Default)]onSwipeAccumulator.binding/swipe.rs:295backdate_hold_for_testbackdates byGESTURE_HOLD_FOR_SWIPE * 2(320 ms). A configured gate above 320 ms stays closed. Useself.hold_for_swipe.- Tests: drive an allowed non-default value through both capture paths, including
reset_input_state. This is the Greptile thread onswipe.rs:385. docs/CONFIGURATION.md, "Common device fields": addgesture_holdwith its range and default.
Carrying a gesture on past its first commit fires that direction again every 600 raw units (12x the commit threshold), which is Options+' behaviour for stepping through several desktops. A user who holds a long stroke instead gets two or three actions from one press, with no setting to stop it. Add a per-device `gesture_repeat` (default on, so nothing changes for anyone who does not set it). Off limits one press to one swipe: the accumulator stops testing for the repeat distance at all. `Option<bool>` rather than a bare bool keeps the config serialization honest (unset is omitted, not written as `true`) and DeviceConfig under clippy's bool-count lint.
| let straight = | ||
| detect_swipe(self.repeat, self.repeat_cross) == Some(GestureDirection::Right); | ||
| if self.repeat >= GESTURE_REPEAT_DISTANCE && straight { | ||
| if self.repeat_enabled && self.repeat >= GESTURE_REPEAT_DISTANCE && straight { |
There was a problem hiding this comment.
With gesture_repeat = false, this check only stops repeats during continuous travel. A pause of at least 250 ms clears last, so continuing in the same direction fires another action through the unchecked commit path. Reversing direction can also fire another swipe. One press can therefore run multiple actions despite the setting's promise of a single swipe.
Stop accepting movement after fired becomes true when repeats are disabled, and add tests for pauses and reversals.
|
One practical question about validating this on real hardware. The gesture path can't be exercised by a locally built agent: without the Developer ID signature, Would you be open to publishing a throwaway artifact of this branch (even unsigned-for-release, just notarised/Developer ID signed, arm64)? I have a macOS arm64 machine with an MX Master 3S where the current 160 ms gate and the 600-unit repeat both reproduce on every gesture, and I can report back with No pressure if that's not part of your release flow — happy to wait for a release build instead. |
AprilNEA
left a comment
There was a problem hiding this comment.
On the signed build: a Developer ID build of an external branch runs that branch's code where the signing secrets live. Whether to do that is the maintainer's call, so I have left it to them. Two facts for testing in the meantime:
- This PR changes swipe detection, not dispatch. The HID++ path logs
gesture committedat debug level (accum.rs:257), so a local build shows whether the gate and the repeat setting behave. - A local build loses only the virtual HID keyboard; shortcut keys fall back to
CGEventPost(docs/DECISIONS.md, "macOS key synthesis types through a virtual HID keyboard when entitled"). Only some shortcuts ignore posted keys (#1653, #744). #1996 reports a hardware test on an MX Master 3S withcargo run -p openlogi-desktop.
Changes needed on bdb0cb8e:
- Scope:
gesture_repeatis a second new setting with its own product decision (#1652 added repeats on purpose). Move it to a separate PR. binding/swipe.rs:193-203and:276: with repeats off, a rest ofGESTURE_REARM_PAUSEclearslast, and the normal commit path fires the same direction again. A reversal also commits. The doc onDeviceConfig::gesture_repeat("one press to a single swipe") does not hold. This is the Greptile P1 onswipe.rs:230. Add tests for a pause and for a reversal.boolparameters:accum.rs:49begin_holdnow takes three, andHoldState::begin,GestureCapture::newandwith_gesture_settingstake one each (.agents/rules/rust.md: "Aboolparameter is boolean-blind"). Pass one named type, for exampleGestureTuning { hold: GestureHold, repeat: GestureRepeat }withenum GestureRepeat { Chain, Once }, into a singleSwipeAccumulator::begin. That also replacesset_repeat_enabled.capture_plan.rs:310andorchestrator.rs:675both write the default asunwrap_or(true). Use oneConfigaccessor, as forgesture_hold.config/identity.rsfold()needsgesture_repeatas well.
The 9 findings from my review of e375538c are still open, and so are the Greptile threads on device.rs:286 and swipe.rs:395.
What and why
Two gesture behaviours were compile-time constants, and both are the reason a gesture button misbehaves for someone who swipes fast:
GESTURE_HOLD_FOR_SWIPE(160 ms). Travel may only commit to a direction once the button has been held past it. Flicking faster than that reaches the release with no committed direction, soSwipeAccumulator::endreports a plain click and the button'sClickbinding fires instead of the gesture — reproducibly, on every swipe.GESTURE_REPEAT_DISTANCE(600 raw units, 12× the commit threshold). Carrying the stroke on fires the same direction again, the way Options+ steps through several desktops. A long hold-and-drag therefore fires two or three actions from one press, with no way to stop it.Both become per-device settings, alongside
smooth_wheelandscroll_resolution, which already tune per-device wheel feel.Changes
GestureHold(crates/openlogi-core/src/config/settings.rs): validatednutype, 40–1000 ms,DEFAULT = 160.DeviceConfig::gesture_hold: Option<GestureHold>— omitted fromconfig.tomlwhen unset.DeviceConfig::gesture_repeat: Option<bool>—Noneandtruekeep the current repeat behaviour;falselimits one press to one swipe.Option(not a bare bool) keeps unset values out of the file andDeviceConfigunder clippy's bool-count lint.SwipeAccumulator::begin_with(hold)sets the gate for that hold;set_repeat_enabled(bool)gates the repeat branch at all (no distance test when off).begin()keeps the shared defaults, so existing callers and tests are unchanged.CaptureSpec::gesture_hold/gesture_repeat→GestureCapture→CaptureAccum(both preserved acrossreset_input_state).HookMaps::gesture_hold/gesture_repeat, published per selected device, applied inHoldState::begin.Tests
a_hold_gate_of_zero_commits_on_the_first_reportthe_default_hold_gate_still_withholds_a_quick_swipean_abandoned_repeat_keeps_one_swipe_per_pressgesture_hold_round_trips_and_rejects_out_of_rangegesture_repeat_defaults_on_and_can_be_disabledcargo test -p openlogi-core -p openlogi-device -p openlogi-agent-core: 1600 passed, 0 failed.cargo clippy --all-targetson the same three crates: no warnings.cargo fmt --check: clean.Schema note
DeviceConfigisdeny_unknown_fields, so a config written with either field makes a pre-change build refuse the whole file. I did not bumpschema_version, since the versioning policy is yours — say the word and I'll add the v10 bump (plus the load-side note) to this PR.Scope
Backend + TOML only; no GUI control is added here. Happy to wire both into the device settings pane (next to the SmartShift/scroll rows) if you want it in the same PR.