feat(ui): wheel-divert state badge on Scroll page#172
Open
hughesyadaddy wants to merge 10 commits into
Open
Conversation
05f1b0c to
0d1763b
Compare
Extend the per-device catalog with the metadata follow-up PRs need to drive the MX Master 4's two thumb-area buttons. None of this changes runtime behavior yet; consumers land in the HID listener, engine, and mouse-hook PRs. MX_MASTER_4_BUTTONS adds `thumb_button` to the standard MX Master button set so the relocated Mouse Gesture Button (CID 0x00C3) has its own UI mapping target. LogiDeviceSpec gains `has_hires_wheel`, `has_thumbwheel`, `gesture_via_sense_panel`, and `thumb_button_cid` so device entries declare HID++ feature presence and CID roles up front. ConnectedDeviceInfo mirrors those four plus runtime-state siblings (`hires_wheel_active`, `thumbwheel_active`, `active_gesture_cid`, `thumb_button_via_hid`) so platform mouse hooks read state without re-walking the capability inventory inline. `build_connected_device_info` accepts the new state kwargs, folds in spec hints only when the caller did not provide a value, and normalizes `active_gesture_cid` to int. Naming follows Logitech's firmware/marketing terminology (verified against Solaar `special_keys.py`, the MX Master 4 reprog-controls dump in pwr-Solaar/Solaar#2964, and Logitech's own MX Master 4 marketing): 0x01A0 is the haptic Sense Panel, 0x00C3 is the relocated Mouse Gesture Button. "Action Ring" is a Logi Options+ software overlay invoked by pressing the Sense Panel, not a hardware button, so internal symbols use `thumb_button` / `gesture_via_sense_panel` instead. The MX Master 4 catalog entry lists `gesture_cids` with 0x01A0 first so the listener prefers diverting the larger Sense Panel with rawXY, points `thumb_button_cid` at 0x00C3 so the relocated Mouse Gesture Button is wired as a button-only extra, and sets `gesture_via_sense_panel = True` so the device is eligible for an OS-level fallback swap when firmware rejects the panel divert. Both CIDs are confirmed divertable + rawXY- capable on real MX Master 4 firmware (Solaar issue #2964). `tests/test_logi_devices.py` updates the haptic-control test to assert `thumb_button` is in `supported_buttons` while the Sense Panel CID still produces no UI button entry, and adds tests for spec-to-runtime mirroring and `active_gesture_cid` int normalization.
…back tests - Replace ``bool`` capability defaults in ``build_connected_device_info`` with ``bool | None``. ``None`` means "no probe performed" and defers to the catalog hint; explicit ``True``/``False`` is definitive runtime evidence. Prior behavior silently dropped a definitive ``False`` runtime probe under an optimistic catalog ``True``, which would let the listener try to divert a feature the firmware did not actually expose. - Extract ``_coerce_cid`` so caller-supplied CIDs (int, ``"0x01A0"`` hex string, ``None``) all funnel through one normalizer; downstream consumers no longer compare ``int`` against ``str`` and silently miss matches. - Stop letting an empty ``gesture_cids=()`` collapse into the spec default via short-circuit ``or``. An empty tuple is the runtime's explicit "I saw none"; treat it as truth. New tests cover: hex-string CID normalization, malformed CID resolves to None, unknown PID falls back to generic + no MX-family buttons, explicit runtime ``False`` overrides catalog ``True``, runtime ``True`` upgrades an unknown device, and empty ``gesture_cids`` is respected on both spec and fallback paths.
clamp_dpi used a bare int(value) cast against whatever flowed in from config.json, QML bindings, or HID reads. A user-edited config that quoted the DPI as a string, a stale None from a partial HID report, or a leaked bool flag would raise ValueError/TypeError and abort the engine's DPI path -- the cursor freezes until restart. Coerce defensively instead. Accept ints, floats, decimal/hex strings, and fall back to the device's dpi_min on anything else. Reject bool explicitly so a truthy flag never silently clamps to 0 or 1 DPI via int(True). The clamp range itself is unchanged.
Two MX Master 4 hardware affordances need firmware-level wiring to work without freezing the cursor or fighting external KVM forwarding. This change wires both through the HID++ vendor channel, with an OS-level fallback path for cases where the firmware divert is rejected. The first is the small thumb button (CID 0x00c3, Solaar's `Mouse_Gesture_Button` with the `Thumb_Button` alias on MX Master 4). MX Master 4 also ships the Sense Panel (CID 0x01a0, Solaar's `Haptic`), which is what Logitech markets as "Haptic Sense" and what Logi Options+ binds the "Action Ring" software overlay to. The Sense Panel also surfaces as OS-level mouse btn=6 (BTN_TASK on Linux evdev) when not diverted, and without divert that report stays high for the entire press-and-drag, which freezes the cursor in many apps. HidGestureListener lists 0x01a0 first in gesture_cids so it diverts the Sense Panel with rawXY; gesture press / release / motion flow over the HID++ vendor channel and the OS never sees btn=6. The small button (0x00c3) is diverted as a button-only extra in the same setCidReporting round so its press / release fire on_thumb_button_down/up. Adds an OS-level fallback swap for cases where firmware rejects the Sense Panel divert: the platform mouse hook treats btn=6 / BTN_TASK as the gesture button and the small HID++ button becomes the thumb_button trigger. The second is wheel scroll inversion. OS-layer post-injection is unreliable when Synergy / DeskFlow / KVM forwards events to a remote machine because the inversion only applies on the host. Drive inversion at the device instead. HidGestureListener discovers FEAT_HIRES_WHEEL_ENHANCED (0x2121) and FEAT_THUMB_WHEEL (0x2150) and exposes them as has_hires_wheel / has_thumbwheel on ConnectedDeviceInfo. request_wheel_native_invert is a cross-thread API: the engine calls it from the Qt thread, the listener loop applies the device write on the HID thread via a pending-request slot, and the caller blocks on a result event with a 3 s timeout. _abort_pending_wheel_divert wakes the caller with result=False if the connection drops mid-request so callers never strand. The engine drives this via _apply_wheel_invert_setting on every profile or device change. When the device acknowledges, the engine and the platform mouse hook both flip wheel_native_invert_active and the hook suppresses its OS-layer inversion path. Cross-cutting changes: core/config.py adds a thumb_button entry to BUTTON_NAMES and BUTTON_TO_EVENTS, bumps DEFAULT_CONFIG version 9 to 11, and adds migrations v10 (wheel_divert default) and v11 (thumb_button mapping default). core/mouse_hook_contract.py adds wheel_native_invert_active to the Protocol so the engine reads it without an isinstance check. core/mouse_hook_macos.py routes btn=6 OtherMouseDown / Up to THUMB_BUTTON events and to _begin_gesture_capture / _end_gesture_capture (HID-first swallow, OS-fallback gesture capture). State mutations protected by a new _gesture_lock that covers both the CGEventTap main-thread callback and the HID listener background thread. core/mouse_hook_linux.py adds a BTN_TASK branch to _handle_button with the same routing, and core/mouse_hook_windows.py guards WM_MOUSEWHEEL / WM_MOUSEHWHEEL on wheel_native_invert_active. tests cover MX4 thumb-button dispatch, the wheel native-invert request / timeout / replay machinery, and the new config migrations.
…vice The wheel-invert toggle was designed to flip Logitech scroll across KVM / Synergy by asking the firmware to invert at the source. The OS-layer fallback (event-tap negate on macOS, uinput sign flip on Linux, raw-input inject on Windows) was a safety net for devices that cannot do the firmware path -- but it ran any time ``wheel_native_invert_active`` was False, including when no Logitech was connected at all. The practical bug: turning the toggle on while only a trackpad or generic USB mouse is attached inverted every scroll event flowing through Mouser, even though the toggle was never meant to apply to those devices. The fix centralises the gate as ``BaseMouseHook._apply_v/hscroll_invert_fallback``, which now requires ``self._connected_device is not None`` (a Logitech is currently bound) in addition to ``not wheel_native_invert_active``. All three platform hooks route through the new helpers so future ports inherit the same contract. Also fixes a stack of FANG-level defects in the firmware-runtime layer: core/hid_gesture.py - Gate ``_install_thumb_button_extra`` on the live REPROG_V4 controls list. We never queue a divert for a CID the firmware does not actually advertise on the current connection -- previously the static catalog was the sole source of truth, which let setCidReporting hammer the device for controls that did not exist. - Track per-CID divert acknowledgments in ``_extra_divert_acks`` and recompute ``thumb_button_via_hid`` against that set. Previously the property flipped to True immediately on install, before the setCidReporting call ran; the hook layer trusted that to suppress the OS BTN_TASK fallback, eating presses whenever the divert was rejected. - Drop failed CIDs from ``_extra_diverts`` so the OS-fallback path stays in charge of buttons the firmware refused. - Replace bare ``except Exception: pass`` in ``_atexit_stop_listeners`` and ``stop()`` with narrowed handlers that log the failure. A stuck divert state is a user-visible bug; the next session needs the breadcrumb to diagnose it. - Move ``self._wheel_divert_target = target`` inside ``_wheel_divert_call_lock`` in ``request_wheel_native_invert`` so two concurrent callers cannot interleave updates to the reconnect-replay cache. core/mouse_hook_macos.py - Resolve ``kCGScrollWheelEventIsContinuous`` through the Quartz symbol when available; fall back to the integer literal so the event-tap path no longer carries a naked magic number and picks up future SDK renumbering automatically. Tests - New: ``test_install_thumb_button_extra_skipped_when_cid_absent_from_reprog`` and ``test_divert_extras_clears_acks_and_drops_failed_cids`` lock the capability-gating + ack-tracking invariants. - New: ``test_os_inversion_skipped_when_no_logitech_connected`` and ``test_os_inversion_resumes_when_logitech_reconnects`` pin the connected-device gate on the macOS path. - Updated: existing ``MacOSSuppressionTests`` cases now set ``hook._connected_device`` explicitly so they exercise the fallback path the gate intends.
Two FANG-grade cleanups that turn stringly-typed config + magic protocol constants into named, validated surfaces: core/config.py - Introduce ``WHEEL_DIVERT_AUTO`` / ``WHEEL_DIVERT_OFF`` / ``WHEEL_DIVERT_VALID_VALUES`` / ``WHEEL_DIVERT_DEFAULT`` so call sites compare against named constants instead of bare strings. - ``coerce_wheel_divert_setting`` normalizes case + whitespace, rejects unknowns into the safe default, and logs one warning per distinct typo so a misconfigured ``config.json`` produces one actionable message instead of a per-event flood. - ``load_config`` now routes ``settings.wheel_divert`` through the coercer so the on-disk value is the sealed value before any engine branch sees it. core/engine.py - ``_apply_wheel_invert_setting`` and ``_run_saved_settings_replay`` both pull the kill-switch decision through ``coerce_wheel_divert_setting``, removing the two direct string comparisons that previously trusted the config file's value. core/hid_gesture.py - Replace the bare ``0x33`` / ``0x03`` / ``0x22`` / ``0x02`` REPROG_V4 ``setCidReporting`` mode bytes with named constants ``_DIVERT_RAW_XY``, ``_DIVERT_BUTTON_ONLY``, ``_UNDIVERT_RAW_XY``, ``_UNDIVERT_BUTTON``. Drop a single block-comment explaining the bit layout so the next reader does not have to spelunk the HID++ spec to understand what ``0x33`` means. - ``_undivert`` now logs each teardown failure rather than swallowing the exception; teardown still completes regardless because callers (disconnect, atexit) depend on it never raising. tests/test_config.py - New ``WheelDivertCoercionTests`` (10 cases) pinning: canonical values round-trip, case + whitespace normalize, unknown strings fall back to the default, warnings are deduped per distinct value, ``None`` resolves silently, non-string types warn once per type, end-to-end ``load_config`` normalizes case on disk, and an unknown typo persisted to disk surfaces as the sealed default.
A handful of adjacent FANG-grade tightenings against the same code paths the HID++ runtime exercises. Each one stands alone but they share the same theme: never swallow a callback failure, never mutate state after teardown, never read shared state without the lock that protects its writes. core/mouse_hook_base.py - ``_set_device_connected`` and the three ``_emit_*`` helpers used to swallow callback exceptions with ``except Exception: pass``. Replace with narrow handlers that log the offending callback by name; nothing propagates because the call sites are inside hot paths and must continue, but the breadcrumb tells you which integration is broken. core/mouse_hook_windows.py - Add ``_gesture_lock`` and serialize the begin / end transitions of ``_gesture_active`` / ``_gesture_triggered`` across the LL hook thread (XBUTTON path), the Raw Input window-proc thread, and the HID listener thread. macOS and Linux already had this lock; Windows did not, so a side-button press racing with a HID gesture-down could leave the state machine half-flipped. - Move ``_dispatch`` and ``_emit_*`` out of the lock so an engine callback that re-enters the hook can never deadlock. core/mouse_hook_macos.py - Drop the event-tap callback early when ``_running`` is False. The CGEventTap keeps firing briefly after ``stop()`` -- macOS does not drain in-flight callbacks before disabling the tap, so without the guard we still enqueue events into a torn-down dispatch worker and apply scroll inversion after the connection has been released. - Log the previously silent Quartz user-data probe failure so a borked binding cannot make the injected-event marker silently misfire for the rest of the session. core/engine.py - ``stop()`` now cancels every outstanding entry in ``_mouse_release_timers``. The safety auto-release timer scheduled by ``execute_action`` could otherwise fire after the hook had already been torn down and call ``inject_mouse_up`` against nothing. - ``_battery_poll_loop`` reads ``_replay_inflight`` under ``_replay_lock``. Without it the Smart Shift poll can sneak in partway through a replay round-trip and the firmware queues conflicting HID++ writes.
When the engine diverts scroll inversion to the device firmware via HID++ (0x2121 / 0x2150), the OS-level inversion path is suppressed. Without any UI feedback this made "scroll is inverted but KVM forwarding isn't" awkward to debug, so the Scroll page now shows a small badge next to the invert toggles indicating which path is live. The backend exposes wheelDivertActive as a Property and routes the engine-thread change callback through a queued slot before emitting the notification on the Qt main thread. ScrollPage.qml renders the badge with a tooltip explaining the Synergy / DeskFlow / KVM forwarding implication, and three locale keys cover English, Simplified Chinese, and Traditional Chinese.
The badge previously had two states ("Applied on device (HID++)" /
"Applied by Mouser"). With the platform-hook gate from TomBadash#171, the toggle
also has a third runtime state: on but inactive because no Logitech is
currently connected. Without the third state, the badge would say
"Applied by Mouser" while the gate actually suppresses inversion -- the
exact UX failure mode the gate is meant to eliminate.
* InvertScopeBadge resolves its scope from (mouseConnected,
wheelDivertActive): "device" when firmware owns it, "mouser" when the
OS-layer fallback owns it, "inactive" when neither is in effect.
* Add ``Accessible.role`` / ``name`` / ``description`` so screen readers
surface the same information as the hover tooltip.
* Add ``scroll.wheel_invert_inactive`` + ``..._inactive_tooltip`` strings
in English, Simplified Chinese, and Traditional Chinese.
Cover the cross-thread Backend plumbing in
``BackendWheelDivertSignalTests``:
* engine callback registration (gated on attribute presence),
* default ``wheelDivertActive`` state is False,
* callback flip + signal emission via the Qt event loop,
* redundant callbacks do not churn the signal,
* non-bool truthy values coerce to strict bool so QML's
``Property(bool, ...)`` binding never sees an int.
The Backend class defined ``_handleStatusMessage`` twice with identical bodies (~lines 1641 and 1854 before this change). Python keeps the second definition, so the earlier copy was dead code -- but any future edit to the upper block would silently no-op. Delete the first copy; the second one already carries the @slot(str) decorator and the same body, and it is the one the cross-thread connection in __init__ already binds against (the bound-method ``self._handleStatusMessage`` resolves to the second definition at connect time regardless).
ec8446c to
d7decdb
Compare
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
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.
Why
#171 makes Mouser flip the scroll-wheel sign at the firmware layer
(HID++ feature
0x2121HiRes Wheel Enhanced and0x2150Thumbwheel)so wheel inversion survives Synergy / DeskFlow / KVM / Barrier
forwarding to remote machines. That's a big deal for the
multi-monitor / hackintosh / KVM crowd that today has to give up
wheel inversion the moment they forward events through Synergy.
The runtime works. The problem is: from the user's seat there's
no way to tell whether the inversion they just toggled is the new
device-level path or the legacy local-injection path, and the
practical consequence is huge:
any machine seeing those events (over Synergy, DeskFlow, KVM,
Barrier, even a USB-over-IP forwarding chain) gets the inverted
direction natively.
host. The moment events leave the host, they revert to their
un-inverted direction. The user toggles inversion on, scrolls on
their secondary machine, and it scrolls the wrong way. Mouser
looks broken. The forwarder looks broken. Nobody finds the bug.
The fallback path can kick in for three different reasons (device
doesn't expose
0x2121/0x2150, firmware rejects the divert,wheel_divert: "off"kill switch), so the user has no way topredict which one they'll get. This PR puts a small pill inside
each invert toggle row, visible only when that toggle is on, that
states unambiguously which path is currently applying the
inversion.
What changed
ui/qml/ScrollPage.qml:InvertScopeBadgecomponent placed inside both the"Invert vertical scroll" and "Invert horizontal scroll" rows,
between the label and the
Switch.visibleis bound to the row's own toggle (vscrollSwitch.checked/
hscrollSwitch.checked) so the badge never claims to describeinactive inversion.
activeis bound tobackend.wheelDivertActive. Active stateshows a filled accent pill labelled "Applied on device (HID++)";
inactive shows a muted outlined pill labelled "Applied by
Mouser".
0x2121 / 0x2150). The scroll wheel reports the inverted
direction natively, so any remote machine receiving these
events over Synergy / DeskFlow / KVM / Barrier sees them
already inverted."
OS dispatches the event. A remote machine receiving these
scroll events over Synergy / DeskFlow / KVM / Barrier will see
the original (un-inverted) direction."
ui/backend.py: surfaces a newwheelDivertActiveProperty thatmirrors the engine's
wheel_native_invert_activefield, withwheelDivertActiveChangednotifications so each badge updates livewhen a profile / device change re-evaluates the divert path.
ui/locale_manager.py: four locale keys(
scroll.wheel_invert_native,scroll.wheel_invert_os,scroll.wheel_invert_native_tooltip,scroll.wheel_invert_os_tooltip) with Simplified and TraditionalChinese translations.
Test plan
wheel_divert: "auto":toggle each invert on; badges show Applied on device (HID++).
Verified end-to-end via
Engine wheel native-invert -> ON (HID++) … ack=Truelog."wheel_divert": "off"inmouser.json, reload profile:badges flip to Applied by Mouser live (no restart).
no inversion is being applied).
a 400 ms delay.
visible:gating sincethe toggle stays off when no device is wired.
pytest tests/green (461 tests, 157 subtests).Screenshots
Both invert toggles on, device-level inversion active -- each
toggle row shows its own Applied on device (HID++) pill so the
user can see at a glance that scroll inversion will survive Synergy
/ DeskFlow / KVM forwarding:
Same toggles with
wheel_divert: "off"kill switch -- badges flipto Applied by Mouser, signalling that scroll inversion will only
work on this host and will revert to its original direction the
moment events leave the machine through forwarding: