diff --git a/crates/openlogi-agent-core/src/watchers/gesture.rs b/crates/openlogi-agent-core/src/watchers/gesture.rs index 80659682..6c79e800 100644 --- a/crates/openlogi-agent-core/src/watchers/gesture.rs +++ b/crates/openlogi-agent-core/src/watchers/gesture.rs @@ -3,7 +3,7 @@ //! Runs [`openlogi_hid::run_capture_session`] on a dedicated thread for whichever //! device the DPI / SmartShift path currently targets //! ([`DpiCycleState::target`]), restarts it when the carousel selection — or the -//! thumb-wheel arming — changes, and dispatches each captured input: +//! thumb-wheel capture mode — changes, and dispatches each captured input: //! //! - a gesture swipe through the gesture binding map, //! - a DPI/ModeShift or thumb-wheel-tap press through the button binding map, @@ -26,7 +26,9 @@ use std::time::{Duration, Instant}; use openlogi_core::binding::{Action, ButtonId, GestureDirection, default_binding}; use openlogi_core::config::DEFAULT_THUMBWHEEL_SENSITIVITY; -use openlogi_hid::{CaptureChannel, CapturedInput, DeviceRoute, run_capture_session}; +use openlogi_hid::{ + CaptureChannel, CapturedInput, DeviceRoute, ThumbwheelCaptureMode, run_capture_session, +}; use tokio::sync::{mpsc, oneshot}; use tracing::{debug, warn}; @@ -42,8 +44,8 @@ pub type GestureBindings = Arc>>; /// event; written only by `AppState::set_thumbwheel_sensitivity`. pub type ThumbwheelSensitivity = Arc; -/// How often to re-read the active device target + thumb-wheel arming so a -/// carousel switch or a binding/sensitivity edit re-points / re-arms capture. +/// How often to re-read the active device target + thumb-wheel capture mode so +/// a carousel switch or a binding/sensitivity edit re-points / re-arms capture. /// It also paces the respawn of a session that ended on its own (see `manage`). const TARGET_POLL: Duration = Duration::from_secs(1); @@ -104,29 +106,37 @@ pub fn spawn( }); } -/// Whether the thumb wheel must be diverted over HID++ (which suppresses native -/// scroll) so we can re-synthesise its scroll or capture its tap. +/// Select how the thumb wheel is captured over HID++. /// -/// We divert when the sensitivity leaves its default (so we can scale scroll -/// ourselves) or when the click or either rotation direction is rebound away -/// from its default; otherwise the OS scrolls the wheel natively. -fn thumbwheel_armed(hook_maps: &SharedHookMaps, sensitivity: i32) -> bool { - if sensitivity != DEFAULT_THUMBWHEEL_SENSITIVITY { - return true; +/// Non-default sensitivity or rotation bindings require diversion so OpenLogi +/// can re-synthesise the rotation. Tap reports are delivered only when the +/// click binding itself differs from its seeded default. +fn thumbwheel_capture_mode(hook_maps: &SharedHookMaps, sensitivity: i32) -> ThumbwheelCaptureMode { + let sensitivity_changed = sensitivity != DEFAULT_THUMBWHEEL_SENSITIVITY; + let Ok(maps) = hook_maps.read() else { + return if sensitivity_changed { + ThumbwheelCaptureMode::DivertedRotation + } else { + ThumbwheelCaptureMode::Native + }; + }; + let binding_changed = |button| { + maps.bindings + .get(&button) + .is_some_and(|action| *action != default_binding(button)) + }; + + if binding_changed(ButtonId::Thumbwheel) { + ThumbwheelCaptureMode::DivertedRotationAndTap + } else if sensitivity_changed + || [ButtonId::ThumbwheelScrollUp, ButtonId::ThumbwheelScrollDown] + .iter() + .any(|&button| binding_changed(button)) + { + ThumbwheelCaptureMode::DivertedRotation + } else { + ThumbwheelCaptureMode::Native } - hook_maps.read().ok().is_some_and(|maps| { - [ - ButtonId::Thumbwheel, - ButtonId::ThumbwheelScrollUp, - ButtonId::ThumbwheelScrollDown, - ] - .iter() - .any(|&button| { - maps.bindings - .get(&button) - .is_some_and(|action| *action != default_binding(button)) - }) - }) } /// Whether a finished capture session should make the manager re-arm. @@ -153,8 +163,8 @@ async fn manage( receiver_access: ReceiverAccess, ) { let (tx, mut rx) = mpsc::unbounded_channel::(); - // (route, capture_thumbwheel, divert_gesture_button) - let mut current: Option<(DeviceRoute, bool, bool)> = None; + // (route, thumbwheel_mode, divert_gesture_button) + let mut current: Option<(DeviceRoute, ThumbwheelCaptureMode, bool)> = None; let mut stop: Option> = None; let mut ticker = tokio::time::interval(TARGET_POLL); let mut accumulators = WheelAccumulators::default(); @@ -200,7 +210,7 @@ async fn manage( target.map(|t| { ( t, - thumbwheel_armed(&hook_maps, sensitivity), + thumbwheel_capture_mode(&hook_maps, sensitivity), divert_gesture, ) }) @@ -208,7 +218,7 @@ async fn manage( if want == current { continue; } - // Target or thumb-wheel arming changed (or first tick): stop the + // Target or thumb-wheel mode changed (or first tick): stop the // old session and start one for the new state. Sending on the // oneshot lets the old session restore the diverted controls. if let Some(stop) = stop.take() { @@ -218,12 +228,12 @@ async fn manage( current = None; continue; } - if let Some((route, capture_thumbwheel, divert_gesture_button)) = want { + if let Some((route, thumbwheel_mode, divert_gesture_button)) = want { let Some(receiver_lease) = receiver_access.try_acquire_for_capture() else { current = None; continue; }; - current = Some((route.clone(), capture_thumbwheel, divert_gesture_button)); + current = Some((route.clone(), thumbwheel_mode, divert_gesture_button)); let (stop_tx, stop_rx) = oneshot::channel(); let sink = tx.clone(); let slot = Arc::clone(&capture_channel); @@ -234,7 +244,7 @@ async fn manage( let _receiver_lease = receiver_lease; if let Err(e) = run_capture_session( route, - capture_thumbwheel, + thumbwheel_mode, divert_gesture_button, sink, stop_rx, @@ -447,6 +457,104 @@ fn advance( mod tests { use super::*; + fn default_thumbwheel_maps() -> SharedHookMaps { + let bindings = [ + ButtonId::Thumbwheel, + ButtonId::ThumbwheelScrollUp, + ButtonId::ThumbwheelScrollDown, + ] + .into_iter() + .map(|button| (button, default_binding(button))) + .collect(); + Arc::new(RwLock::new(hook_runtime::HookMaps { + bindings, + ..hook_runtime::HookMaps::default() + })) + } + + fn set_binding(hook_maps: &SharedHookMaps, button: ButtonId, action: Action) { + let Ok(mut maps) = hook_maps.write() else { + panic!("test hook map should be writable"); + }; + maps.bindings.insert(button, action); + } + + #[test] + fn default_thumbwheel_bindings_use_native_reporting() { + assert_eq!( + thumbwheel_capture_mode(&default_thumbwheel_maps(), DEFAULT_THUMBWHEEL_SENSITIVITY), + ThumbwheelCaptureMode::Native + ); + } + + #[test] + fn swapped_rotation_bindings_divert_without_delivering_taps() { + let hook_maps = default_thumbwheel_maps(); + set_binding( + &hook_maps, + ButtonId::ThumbwheelScrollUp, + default_binding(ButtonId::ThumbwheelScrollDown), + ); + set_binding( + &hook_maps, + ButtonId::ThumbwheelScrollDown, + default_binding(ButtonId::ThumbwheelScrollUp), + ); + + assert_eq!( + thumbwheel_capture_mode(&hook_maps, DEFAULT_THUMBWHEEL_SENSITIVITY), + ThumbwheelCaptureMode::DivertedRotation + ); + } + + #[test] + fn custom_sensitivity_diverts_without_delivering_taps() { + assert_eq!( + thumbwheel_capture_mode( + &default_thumbwheel_maps(), + DEFAULT_THUMBWHEEL_SENSITIVITY + 1 + ), + ThumbwheelCaptureMode::DivertedRotation + ); + } + + #[test] + fn rebound_thumbwheel_click_enables_tap_delivery() { + let hook_maps = default_thumbwheel_maps(); + set_binding(&hook_maps, ButtonId::Thumbwheel, Action::MissionControl); + + assert_eq!( + thumbwheel_capture_mode(&hook_maps, DEFAULT_THUMBWHEEL_SENSITIVITY), + ThumbwheelCaptureMode::DivertedRotationAndTap + ); + } + + #[test] + fn unavailable_or_poisoned_default_bindings_do_not_divert() { + let unavailable = Arc::new(RwLock::new(hook_runtime::HookMaps::default())); + assert_eq!( + thumbwheel_capture_mode(&unavailable, DEFAULT_THUMBWHEEL_SENSITIVITY), + ThumbwheelCaptureMode::Native + ); + + let poisoned = default_thumbwheel_maps(); + let poison_target = Arc::clone(&poisoned); + assert!( + thread::spawn(move || { + let Ok(_guard) = poison_target.write() else { + panic!("test hook map should initially be writable"); + }; + panic!("poison test hook map"); + }) + .join() + .is_err() + ); + assert_eq!( + thumbwheel_capture_mode(&poisoned, DEFAULT_THUMBWHEEL_SENSITIVITY), + ThumbwheelCaptureMode::Native + ); + } + #[test] fn multiplier_is_unity_at_default_sensitivity() { assert!((scroll_multiplier(DEFAULT_THUMBWHEEL_SENSITIVITY) - 1.0).abs() < f32::EPSILON); diff --git a/crates/openlogi-hid/src/gesture.rs b/crates/openlogi-hid/src/gesture.rs index 1783494b..118889d0 100644 --- a/crates/openlogi-hid/src/gesture.rs +++ b/crates/openlogi-hid/src/gesture.rs @@ -12,8 +12,8 @@ //! The GUI maps each [`CapturedInput`] to the user's bound action and dispatches //! it, mirroring how the CGEventTap hook handles the side buttons. The thumb //! wheel is special: diverting it stops native horizontal scroll, so the GUI -//! re-synthesises scroll from the [`CapturedInput::Scroll`] deltas — the wheel -//! is therefore only diverted when its click is actually bound. +//! re-synthesises scroll from the [`CapturedInput::Scroll`] deltas. Its capture +//! mode separately controls whether diverted single-tap reports are delivered. use std::sync::{Arc, Mutex, PoisonError, RwLock}; @@ -26,7 +26,7 @@ use tracing::{debug, info, warn}; use crate::reprog_controls::{self, RawControlEvent, ReprogControlsV4}; use crate::route::{DeviceRoute, open_route_channel}; -use crate::thumbwheel::{self, Thumbwheel}; +use crate::thumbwheel::{self, Thumbwheel, ThumbwheelEvent}; use crate::write::SharedChannel; /// Shared slot holding the active capture session's open channel, so DPI / @@ -34,6 +34,17 @@ use crate::write::SharedChannel; /// whenever no session is connected. pub type CaptureChannel = Arc>>; +/// How a capture session handles the thumb wheel. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ThumbwheelCaptureMode { + /// Leave the wheel in its native HID reporting mode. + Native, + /// Divert the wheel and forward rotation, but suppress single-tap reports. + DivertedRotation, + /// Divert the wheel and forward both rotation and single-tap reports. + DivertedRotationAndTap, +} + /// One input captured from the active device. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub enum CapturedInput { @@ -78,8 +89,8 @@ struct CaptureAccum { } /// Capture the gesture button, DPI/ModeShift button, and (when -/// `capture_thumbwheel`) the thumb wheel on `route` until `shutdown` resolves, -/// forwarding each event to `sink`. +/// `thumbwheel_mode` is diverted) the thumb wheel on `route` until `shutdown` +/// resolves, forwarding each event to `sink`. /// /// The dedicated gesture button (raw-XY) is diverted only when `divert_gesture_button` — /// i.e. it is the device's gesture owner. When the user moves the gesture role @@ -94,7 +105,7 @@ struct CaptureAccum { /// failures to restore on the way out are logged, not propagated. pub async fn run_capture_session( route: DeviceRoute, - capture_thumbwheel: bool, + thumbwheel_mode: ThumbwheelCaptureMode, divert_gesture_button: bool, sink: mpsc::UnboundedSender, shutdown: oneshot::Receiver<()>, @@ -104,13 +115,7 @@ pub async fn run_capture_session( .await? .ok_or(GestureError::DeviceNotFound)?; let device_index = route.device_index(); - let armed = arm_controls( - &chan, - device_index, - capture_thumbwheel, - divert_gesture_button, - ) - .await?; + let armed = arm_controls(&chan, device_index, thumbwheel_mode, divert_gesture_button).await?; // Publish this device's open channel so DPI/SmartShift writes reuse it // instead of opening their own. Cleared on the way out. @@ -142,12 +147,7 @@ pub async fn run_capture_session( if let Some(idx) = thumb_index && let Some(event) = thumbwheel::decode_event(&msg, device_index, idx) { - if event.single_tap { - let _ = sink.send(CapturedInput::ButtonPressed(ButtonId::Thumbwheel)); - } - if event.rotation != 0 { - let _ = sink.send(CapturedInput::Scroll(event.rotation)); - } + forward_thumbwheel_event(event, thumbwheel_mode, &sink); } } }); @@ -170,6 +170,23 @@ pub async fn run_capture_session( Ok(()) } +/// Forward the parts of a decoded thumb-wheel report enabled by `mode`. +fn forward_thumbwheel_event( + event: ThumbwheelEvent, + mode: ThumbwheelCaptureMode, + sink: &mpsc::UnboundedSender, +) { + if mode == ThumbwheelCaptureMode::Native { + return; + } + if event.single_tap && mode == ThumbwheelCaptureMode::DivertedRotationAndTap { + let _ = sink.send(CapturedInput::ButtonPressed(ButtonId::Thumbwheel)); + } + if event.rotation != 0 { + let _ = sink.send(CapturedInput::Scroll(event.rotation)); + } +} + /// The set of controls a session has diverted, kept so they can be handed back /// to the firmware on teardown. struct ArmedControls { @@ -206,13 +223,13 @@ impl ArmedControls { /// Resolve features off the device's root and divert the controls we capture: /// the gesture button (raw-XY) and DPI/ModeShift buttons over `0x1b04`, and — -/// when `capture_thumbwheel` and the wheel reports a single tap — the thumb -/// wheel over `0x2150`. The root-feature lookup mirrors `write::open_feature`, +/// when `thumbwheel_mode` is diverted — the thumb wheel over `0x2150`. The +/// root-feature lookup mirrors `write::open_feature`, /// since hidpp 0.2's registry doesn't carry the features OpenLogi reimplements. async fn arm_controls( chan: &Arc, slot: u8, - capture_thumbwheel: bool, + thumbwheel_mode: ThumbwheelCaptureMode, divert_gesture_button: bool, ) -> Result { let device = Device::new(Arc::clone(chan), slot) @@ -255,7 +272,7 @@ async fn arm_controls( } let mut thumb: Option<(Thumbwheel, u8)> = None; - if capture_thumbwheel + if thumbwheel_mode != ThumbwheelCaptureMode::Native && let Some(info) = device .root() .get_feature(thumbwheel::FEATURE_ID) @@ -263,24 +280,10 @@ async fn arm_controls( .map_err(|e| GestureError::Hidpp(format!("{e:?}")))? { let tw = Thumbwheel::new(Arc::clone(chan), slot, info.index); - // Consume the getInfo error here, before the next await: Hidpp20Error - // isn't Send, so holding it across an await would make this future - // (spawned on tokio) non-Send. - let supports_single_tap = match tw.get_info().await { - Ok(twinfo) => twinfo.supports_single_tap, - Err(e) => { - warn!(error = ?e, "thumb wheel getInfo failed"); - false - } - }; - if supports_single_tap { - tw.set_reporting(true, false) - .await - .map_err(|e| GestureError::Hidpp(format!("{e:?}")))?; - thumb = Some((tw, info.index)); - } else { - debug!("thumb wheel reports no single tap — click not capturable"); - } + tw.set_reporting(true, false) + .await + .map_err(|e| GestureError::Hidpp(format!("{e:?}")))?; + thumb = Some((tw, info.index)); } if !gesture_diverted && dpi_cids.is_empty() && thumb.is_none() { diff --git a/crates/openlogi-hid/src/gesture/tests.rs b/crates/openlogi-hid/src/gesture/tests.rs index 035f2cda..3a10515f 100644 --- a/crates/openlogi-hid/src/gesture/tests.rs +++ b/crates/openlogi-hid/src/gesture/tests.rs @@ -8,6 +8,15 @@ fn release() -> RawControlEvent { RawControlEvent::DivertedButtons([0, 0, 0, 0]) } +fn thumbwheel_event(rotation: i16, single_tap: bool) -> ThumbwheelEvent { + ThumbwheelEvent { + rotation, + single_tap, + touch: single_tap, + proxy: false, + } +} + #[test] fn quick_tap_is_a_click_even_while_the_cursor_moves() { let (tx, mut rx) = mpsc::unbounded_channel(); @@ -102,3 +111,67 @@ fn a_dpi_button_re_presses_after_a_release() { ); assert!(rx.try_recv().is_err()); } + +#[test] +fn rotation_only_capture_suppresses_taps_but_forwards_rotation() { + let (tx, mut rx) = mpsc::unbounded_channel(); + + forward_thumbwheel_event( + thumbwheel_event(7, true), + ThumbwheelCaptureMode::DivertedRotation, + &tx, + ); + + assert_eq!(rx.try_recv(), Ok(CapturedInput::Scroll(7))); + assert!( + rx.try_recv().is_err(), + "rotation-only capture must not deliver the tap" + ); +} + +#[test] +fn rotation_and_tap_capture_forwards_each_input_once() { + let (tx, mut rx) = mpsc::unbounded_channel(); + + forward_thumbwheel_event( + thumbwheel_event(-4, true), + ThumbwheelCaptureMode::DivertedRotationAndTap, + &tx, + ); + + assert_eq!( + rx.try_recv(), + Ok(CapturedInput::ButtonPressed(ButtonId::Thumbwheel)) + ); + assert_eq!(rx.try_recv(), Ok(CapturedInput::Scroll(-4))); + assert!( + rx.try_recv().is_err(), + "one report must emit exactly one tap and one rotation" + ); +} + +#[test] +fn zero_motion_without_a_tap_emits_nothing() { + let (tx, mut rx) = mpsc::unbounded_channel(); + + forward_thumbwheel_event( + thumbwheel_event(0, false), + ThumbwheelCaptureMode::DivertedRotationAndTap, + &tx, + ); + + assert!(rx.try_recv().is_err()); +} + +#[test] +fn native_mode_ignores_a_decoded_thumbwheel_report() { + let (tx, mut rx) = mpsc::unbounded_channel(); + + forward_thumbwheel_event( + thumbwheel_event(3, true), + ThumbwheelCaptureMode::Native, + &tx, + ); + + assert!(rx.try_recv().is_err()); +} diff --git a/crates/openlogi-hid/src/lib.rs b/crates/openlogi-hid/src/lib.rs index b6cf1547..dab08a73 100644 --- a/crates/openlogi-hid/src/lib.rs +++ b/crates/openlogi-hid/src/lib.rs @@ -29,7 +29,9 @@ pub mod smartshift; pub mod thumbwheel; pub mod write; -pub use gesture::{CaptureChannel, CapturedInput, GestureError, run_capture_session}; +pub use gesture::{ + CaptureChannel, CapturedInput, GestureError, ThumbwheelCaptureMode, run_capture_session, +}; pub use hires_wheel::{ ScrollReportingTarget, ScrollResolution, ScrollWheelMode, get_scroll_wheel_mode, get_scroll_wheel_mode_on, set_scroll_inversion, set_scroll_inversion_on, set_scroll_resolution,