fix(hooks): pass through OS mouse events when no Logitech is connected#185
Open
hughesyadaddy wants to merge 1 commit into
Open
fix(hooks): pass through OS mouse events when no Logitech is connected#185hughesyadaddy wants to merge 1 commit into
hughesyadaddy wants to merge 1 commit into
Conversation
Mouser is a Logitech-mouse remapper. The macOS CGEventTap and the Windows WH_MOUSE_LL hook are both *global* -- they see events from every input device the OS knows about -- so without a "is there even a Logitech attached?" gate Mouser would happily intercept and remap an xbutton click from a generic USB mouse, swallow a trackpad scroll, or run a swipe through its gesture detector when the user switched their KVM to another host. That last failure mode is the practical bug: with the KVM pointing elsewhere the Logitech is fully disconnected from this machine, but Mouser keeps remapping whatever other mouse the user has plugged in. Add ``BaseMouseHook._should_intercept_events()`` that returns True only when ``self._connected_device is not None`` -- the same flag that flips under HID++ connect / disconnect (and under Linux evdev attach / release). Both event-tap callbacks early-return the original event when the gate is closed, immediately after the existing injected-event filter and before any blocking / remapping / dispatching runs. The Linux hook is naturally gated because its evdev hook only attaches once a Logitech source device has been resolved. Behavior under the four states: * No Logitech ever connected -> pass-through. * KVM points away from this host -> HID disconnects -> next event passes through. * KVM points back to this host -> HID reconnects -> next event is intercepted. * Logitech connected normally -> unchanged. Tests ----- - ``test_should_intercept_events_defaults_to_false`` pins the cold-start behavior on a fresh BaseMouseHook. - ``test_should_intercept_events_flips_on_hid_connect`` / ``..._flips_off_on_hid_disconnect`` pin the transition contract. - ``MacOSPassthroughWhenNoDeviceTests`` covers the user-visible failure modes end-to-end against the real ``MouseHook._event_tap_callback``: scroll passes through when no device, xbutton passes through when no device, and intercept resumes on the very next event after ``_on_hid_connect`` fires. - ``MacOSTrackpadScrollFilterTests`` updates its ``_make_hook`` helper to pin a stub ``_connected_device`` so the existing trackpad-filter cases exercise the path they actually mean to, instead of falling through the new gate.
hughesyadaddy
added a commit
to hughesyadaddy/Mouser
that referenced
this pull request
May 25, 2026
…cept_events The OS-layer scroll-invert fallbacks `_apply_vscroll_invert_fallback` and `_apply_hscroll_invert_fallback` each inlined a `self._connected_device is not None` check. The same predicate is the sole condition of the top-level CGEventTap / WH_MOUSE_LL early-return gate added in TomBadash#185 (`_should_intercept_events`). Two independent copies of the same boolean drift the moment one of them grows a new clause. Hoist the predicate into `_should_intercept_events` on BaseMouseHook and route the two scroll-invert fallbacks through it. Behavior preserved (49 mouse-hook tests still pass); the only change is that the "no Logitech bound" semantics now live in one place. This duplicates the helper definition that TomBadash#185 also adds, so whichever PR lands second resolves a trivial 4-line dedup conflict; both branches remain independently self-contained against master in the meantime.
hughesyadaddy
added a commit
to hughesyadaddy/Mouser
that referenced
this pull request
May 25, 2026
…cept_events The OS-layer scroll-invert fallbacks `_apply_vscroll_invert_fallback` and `_apply_hscroll_invert_fallback` each inlined a `self._connected_device is not None` check. The same predicate is the sole condition of the top-level CGEventTap / WH_MOUSE_LL early-return gate added in TomBadash#185 (`_should_intercept_events`). Two independent copies of the same boolean drift the moment one of them grows a new clause. Hoist the predicate into `_should_intercept_events` on BaseMouseHook and route the two scroll-invert fallbacks through it. Behavior preserved (49 mouse-hook tests still pass); the only change is that the "no Logitech bound" semantics now live in one place. This duplicates the helper definition that TomBadash#185 also adds, so whichever PR lands second resolves a trivial 4-line dedup conflict; both branches remain independently self-contained against master in the meantime.
hughesyadaddy
added a commit
to hughesyadaddy/Mouser
that referenced
this pull request
May 25, 2026
…cept_events The OS-layer scroll-invert fallbacks `_apply_vscroll_invert_fallback` and `_apply_hscroll_invert_fallback` each inlined a `self._connected_device is not None` check. The same predicate is the sole condition of the top-level CGEventTap / WH_MOUSE_LL early-return gate added in TomBadash#185 (`_should_intercept_events`). Two independent copies of the same boolean drift the moment one of them grows a new clause. Hoist the predicate into `_should_intercept_events` on BaseMouseHook and route the two scroll-invert fallbacks through it. Behavior preserved (49 mouse-hook tests still pass); the only change is that the "no Logitech bound" semantics now live in one place. This duplicates the helper definition that TomBadash#185 also adds, so whichever PR lands second resolves a trivial 4-line dedup conflict; both branches remain independently self-contained against master in the meantime.
hughesyadaddy
added a commit
to hughesyadaddy/Mouser
that referenced
this pull request
May 25, 2026
…cept_events The OS-layer scroll-invert fallbacks `_apply_vscroll_invert_fallback` and `_apply_hscroll_invert_fallback` each inlined a `self._connected_device is not None` check. The same predicate is the sole condition of the top-level CGEventTap / WH_MOUSE_LL early-return gate added in TomBadash#185 (`_should_intercept_events`). Two independent copies of the same boolean drift the moment one of them grows a new clause. Hoist the predicate into `_should_intercept_events` on BaseMouseHook and route the two scroll-invert fallbacks through it. Behavior preserved (49 mouse-hook tests still pass); the only change is that the "no Logitech bound" semantics now live in one place. This duplicates the helper definition that TomBadash#185 also adds, so whichever PR lands second resolves a trivial 4-line dedup conflict; both branches remain independently self-contained against master in the meantime.
hughesyadaddy
added a commit
to hughesyadaddy/Mouser
that referenced
this pull request
May 25, 2026
…dash#185 integration ``MacOSThumbButtonTests._make_hook`` previously left ``hook._connected_device`` unset. The thumb-button dispatch path runs unconditionally on this branch, so the test passes; but TomBadash#185 adds an early-return at the top of the CGEventTap callback gated on ``_should_intercept_events`` (which is ``self._connected_device is not None``), and once both PRs are integrated that gate fires before ``btn=6`` reaches the thumb-button handler and the assertions about ``THUMB_BUTTON_DOWN`` / ``THUMB_BUTTON_UP`` / ``XBUTTON1_DOWN`` start failing. Pin a stub ``SimpleNamespace`` device on the hook in ``_make_hook``. This is also semantically more truthful: btn=6 / btn=3 only originate from a connected MX Master mouse in production, so the test should model that state rather than the "no Logitech bound" cold-start state the dispatch path was never designed for. No-op against current master (the early-return does not yet exist); becomes load-bearing as soon as TomBadash#185 lands.
hughesyadaddy
added a commit
to hughesyadaddy/Mouser
that referenced
this pull request
May 25, 2026
…dash#185 integration ``MacOSThumbButtonTests._make_hook`` previously left ``hook._connected_device`` unset. The thumb-button dispatch path runs unconditionally on this branch, so the test passes; but TomBadash#185 adds an early-return at the top of the CGEventTap callback gated on ``_should_intercept_events`` (which is ``self._connected_device is not None``), and once both PRs are integrated that gate fires before ``btn=6`` reaches the thumb-button handler and the assertions about ``THUMB_BUTTON_DOWN`` / ``THUMB_BUTTON_UP`` / ``XBUTTON1_DOWN`` start failing. Pin a stub ``SimpleNamespace`` device on the hook in ``_make_hook``. This is also semantically more truthful: btn=6 / btn=3 only originate from a connected MX Master mouse in production, so the test should model that state rather than the "no Logitech bound" cold-start state the dispatch path was never designed for. No-op against current master (the early-return does not yet exist); becomes load-bearing as soon as TomBadash#185 lands.
hughesyadaddy
added a commit
to hughesyadaddy/Mouser
that referenced
this pull request
May 25, 2026
…dash#185 integration ``MacOSThumbButtonTests._make_hook`` previously left ``hook._connected_device`` unset. The thumb-button dispatch path runs unconditionally on this branch, so the test passes; but TomBadash#185 adds an early-return at the top of the CGEventTap callback gated on ``_should_intercept_events`` (which is ``self._connected_device is not None``), and once both PRs are integrated that gate fires before ``btn=6`` reaches the thumb-button handler and the assertions about ``THUMB_BUTTON_DOWN`` / ``THUMB_BUTTON_UP`` / ``XBUTTON1_DOWN`` start failing. Pin a stub ``SimpleNamespace`` device on the hook in ``_make_hook``. This is also semantically more truthful: btn=6 / btn=3 only originate from a connected MX Master mouse in production, so the test should model that state rather than the "no Logitech bound" cold-start state the dispatch path was never designed for. No-op against current master (the early-return does not yet exist); becomes load-bearing as soon as TomBadash#185 lands.
hughesyadaddy
added a commit
to hughesyadaddy/Mouser
that referenced
this pull request
May 25, 2026
…dash#185 integration ``MacOSThumbButtonTests._make_hook`` previously left ``hook._connected_device`` unset. The thumb-button dispatch path runs unconditionally on this branch, so the test passes; but TomBadash#185 adds an early-return at the top of the CGEventTap callback gated on ``_should_intercept_events`` (which is ``self._connected_device is not None``), and once both PRs are integrated that gate fires before ``btn=6`` reaches the thumb-button handler and the assertions about ``THUMB_BUTTON_DOWN`` / ``THUMB_BUTTON_UP`` / ``XBUTTON1_DOWN`` start failing. Pin a stub ``SimpleNamespace`` device on the hook in ``_make_hook``. This is also semantically more truthful: btn=6 / btn=3 only originate from a connected MX Master mouse in production, so the test should model that state rather than the "no Logitech bound" cold-start state the dispatch path was never designed for. No-op against current master (the early-return does not yet exist); becomes load-bearing as soon as TomBadash#185 lands.
hughesyadaddy
added a commit
to hughesyadaddy/Mouser
that referenced
this pull request
May 25, 2026
…cept_events The OS-layer scroll-invert fallbacks `_apply_vscroll_invert_fallback` and `_apply_hscroll_invert_fallback` each inlined a `self._connected_device is not None` check. The same predicate is the sole condition of the top-level CGEventTap / WH_MOUSE_LL early-return gate added in TomBadash#185 (`_should_intercept_events`). Two independent copies of the same boolean drift the moment one of them grows a new clause. Hoist the predicate into `_should_intercept_events` on BaseMouseHook and route the two scroll-invert fallbacks through it. Behavior preserved (49 mouse-hook tests still pass); the only change is that the "no Logitech bound" semantics now live in one place. This duplicates the helper definition that TomBadash#185 also adds, so whichever PR lands second resolves a trivial 4-line dedup conflict; both branches remain independently self-contained against master in the meantime.
6 tasks
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
Mouser is a Logitech-mouse remapper, but the macOS
CGEventTapand the WindowsWH_MOUSE_LLhook are both global — they see events from every input device the OS knows about. Without a "is there even a Logitech attached?" gate Mouser will happily:xbuttonclick from a generic USB mouse and route it through whatever action the user mapped (e.g.browser_back),invert_vscrolltoggle for their Logitech wheel,The practical user-facing failure is the KVM scenario: the user switches their KVM to another host, the Logitech is fully detached from this machine, but Mouser keeps running on this side and silently remaps whatever other mouse is plugged in. The same failure mode bites users in the disconnect window between sleep and HID++ reconnect.
Fix
Add
BaseMouseHook._should_intercept_events()that returnsTrueonly whenself._connected_device is not None. That flag flips under both HID++ connect / disconnect on macOS / Windows and under Linux evdev attach / release, so the contract is platform-uniform.Both the macOS event-tap callback and the Windows LL-hook callback early-return the original event when the gate is closed — immediately after the existing injected-event filter and before any blocking / remapping / dispatching code. The Linux hook is already gated by construction (its evdev hook only attaches once a Logitech source device has been resolved), but consults the same helper defensively.
Behavior under each state
The gate is evaluated per-event against live state, so transitions are instant — no event-loop tick required.
Test plan
BaseMouseHookRuntimeStateTestsgains three cases pinning the defaultFalse, the flip on_on_hid_connect, and the flip back on_on_hid_disconnect.MacOSPassthroughWhenNoDeviceTestsexercises the realMouseHook._event_tap_callbackand pins the user-visible failure modes:_on_hid_connect.MacOSTrackpadScrollFilterTests._make_hookupdated to assign a stub_connected_deviceso the existing cases still exercise the path they mean to.pytest tests/ -q: 501 passed, 1 skipped, 170 subtests passed.Why no opt-out?
Mouser's purpose statement in the README is unambiguously about Logitech mice. A user without a Logitech bound to the host gets no value from the OS-level remap pipeline running anyway, and the previous behavior was a silent correctness bug for every KVM / multi-host user. If a future use case needs "remap any mouse" the gate can be relaxed behind an explicit setting, but the default has to be "stay out of the way".
Cross-PR coordination with #171
#171 (
feat(devices): MX Master 4 firmware-first HID++ runtime) also definesBaseMouseHook._should_intercept_events— identically — and routes its own OS-layer scroll-invert fallbacks (_apply_vscroll_invert_fallback/_apply_hscroll_invert_fallback) through that single helper instead of duplicating theself._connected_device is not Nonepredicate.Whichever of #171 / #185 merges second resolves a trivial 21-line duplicate-add on the same method definition. The method body is identical between the two PRs; behavior is preserved either way. #171 also pre-positions
MacOSThumbButtonTests._make_hookto set_connected_deviceso this PR's top-level early-return does not short-circuit that test class once both PRs are integrated.