feat(ui): per-device hotspot labels in the mappings list#173
Open
hughesyadaddy wants to merge 12 commits into
Open
feat(ui): per-device hotspot labels in the mappings list#173hughesyadaddy wants to merge 12 commits into
hughesyadaddy wants to merge 12 commits into
Conversation
hughesyadaddy
force-pushed
the
feat/per-device-button-labels
branch
7 times, most recently
from
401fe78 to
680c203
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).
The mappings list used the global BUTTON_NAMES entry for every device,
so an MX Master 4 user binding its thumb-area buttons saw the abstract
"Gesture button" and "Thumb button" labels even though the mouse
diagram directly above the list shows the physical names from the
per-device hotspot catalog ("Sense Panel" and "Top thumb button").
The new `_button_label` helper checks the connected layout's hotspots
first and only falls back to BUTTON_NAMES when no hotspot exists for
the key, so both the mappings list and the per-app profile mappings
match the diagram. On MX Master 4 the user now sees the same labels
end-to-end (Sense Panel / Top thumb button); on older MX Master devices
without per-device overrides the global "Gesture button" / "Horizontal
scroll left" labels continue to render, so existing users see no
regression.
Test coverage covers both directions: a positive test asserts that the
MX Master 4 layout overrides win for `gesture` and `thumb_button`, and
a negative test asserts that MX Master 3S falls back to the global
BUTTON_NAMES entry for `hscroll_right` (which has no per-device
hotspot).
…outs ``_button_label`` previously did three unguarded ``.get()`` calls inside the per-button rebuild loop. Any malformed catalog entry (None layout during disconnect, hotspots stored as ``None``, a non-dict slipping into the array, a present-but-empty label, a duplicate ``buttonKey``) would either throw ``AttributeError`` mid-rebuild or surface an empty button name in the mappings list. Tighten the helper so every shape of bad input degrades cleanly to the global ``BUTTON_NAMES`` fallback: * explicit ``str`` type narrowing on the inputs and return, * tolerate ``self._device_layout is None`` (transient state during reconnect), * skip non-dict hotspot entries instead of crashing, * reject empty-string labels so the user always sees a non-empty name, * return the first match deterministically when duplicate hotspots claim the same ``buttonKey``. Cover each invariant with a focused test in ``BackendDeviceLayoutTests`` so the contract stays nailed down: missing ``hotspots`` array falls back to BUTTON_NAMES, ``None`` layout falls back without raising, empty-string label falls back, non-dict hotspots are ignored without breaking the iteration, and duplicate hotspots return the first match.
hughesyadaddy
force-pushed
the
feat/per-device-button-labels
branch
from
12779f0 to
31ea82f
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
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.
Why
Today the mappings list shows the generic
BUTTON_NAMESentry("Gesture button", "Horizontal scroll left", etc.) for every device.
The MX Master 4 work in #170 adds per-device hotspot labels to the
catalog so the diagram can call out exactly what each button is on
that hardware (Sense Panel, Top thumb button, …), but the
mappings list on the same page is still rendering the abstract
"Gesture button" / "Thumb button" labels.
End result: the diagram says "Sense Panel", the row below it for the
same button says "Gesture button", and users have to mentally map
between the two. On MX Master 4 specifically this is confusing
because Logi Options+ also uses different names again ("Action Ring"
software overlay for the Sense Panel), so we have three names for
the same physical surface.
This PR makes the mappings list and per-app profile list pull their
labels from the same catalog source the diagram uses, so the names
match end-to-end.
What changed
ui/backend.py:_button_label(key, fallback_name)looks up theconnected device layout's hotspot label first and falls back to the
global
BUTTON_NAMESentry when no per-device hotspot exists forthe key.
buttons(mappings list) andprofileButtons(per-appprofile mappings) route through the helper so the displayed label
matches the diagram.
hotspot override for the gesture button, so they continue to
display the global Gesture button entry from
BUTTON_NAMES—zero regression for existing users.
Test plan
BackendDeviceLayoutTests.test_mx_master_4_mappings_use_per_device_hotspot_labels:asserts
gesture→"Sense Panel"andthumb_button→"Top thumb button"(per-device labels win).BackendDeviceLayoutTests.test_mappings_fall_back_to_button_names_when_layout_has_no_hotspot:asserts the fallback path still picks up
"Horizontal scroll right"fromBUTTON_NAMESwhen the layouthas no hotspot for that key.
pytest tests/green (463 tests, 157 subtests).Screenshots
MX Master 4 mappings list -- Sense Panel and Top thumb button
labels now match the diagram above them:
MX Master 3S (and any other device without per-device hotspot
overrides) -- still shows the existing global Gesture button /
Horizontal scroll labels, no regression:
Notes
thumb_buttonBUTTON_NAMESentry from feat(devices): MX Master 4 firmware-first layout metadata #170 / feat(devices): MX Master 4 firmware-first HID++ runtime #171. Until those land, the diffhere includes their content as well.