Skip to content

fix(smartshift): stop runaway free-spin scroll and SmartShift control snap-back#333

Open
AprilNEA wants to merge 2 commits into
masterfrom
fix/smartshift-scroll-runaway
Open

fix(smartshift): stop runaway free-spin scroll and SmartShift control snap-back#333
AprilNEA wants to merge 2 commits into
masterfrom
fix/smartshift-scroll-runaway

Conversation

@AprilNEA

Copy link
Copy Markdown
Owner

Summary

Three independent defects made the wheel scroll "insanely fast / unusable" and the SmartShift GUI controls (Permanent Ratchet, sensitivity slider) snap back to defaults on MX Master 3 / 3s and MX Anywhere 3s.

Root causes & fixes

1. SmartShift writes rejected the 0 "do not change" sentinel → silent failure / snap-back

set_status validated auto_disengage / tunable_torque as non-zero and bailed out before the HID++ call. Per the x2110 / x2111 setRatchetControlMode spec, 0 means "do not change" (real values are 0x01..=0xFF). On a device that reports tunable_torque == 0 (no tunable-torque hardware, e.g. MX Anywhere 3s) every SmartShift write failed silently — the panel showed the change optimistically, then the confirm read snapped it back. Now 0 is sent as preserve; the redundant set_mode_preserving_status / nonzero_smartshift_value helpers are removed and the toggle shares set_status.

2. A 0 readback clamped to auto_disengage = 1 (≈0.25 turn/s) → constant free-spin

That threshold releases the ratchet into free-spin on the gentlest scroll, which reads as "insanely fast." The sensitivity slider floor is raised from 1 to a usable 8 (≈2 turn/s); sub-floor / sentinel values normalise to the 16 default. The floor and default are single-sourced in openlogi-core, and a deserialize-time heal repairs already-persisted low thresholds on load — so a config.toml that already holds a runaway value is fixed automatically on the next agent start (the values live in volatile RAM and the agent re-applies config on reconnect, #189).

3. Native scroll inversion (0x2121) rewritten on every config reload

reload_config fires on every DPI / SmartShift / binding edit, and each one re-wrote the wheel mode — needless HID++ traffic that could race other writes. The write now short-circuits when the wheel already holds the desired (native, invert) state.

Notes

  • User-visible: the sensitivity slider minimum moves from 1 to 8; persisted thresholds below 8 are healed to 16 on load. 0xFF (permanent ratchet) and real thresholds ≥ 8 are untouched.
  • An adversarial multi-agent review flagged that gating the inversion re-apply on "changed only" would drop the incidental recovery when a wake-time write times out; since the 0x2121 write is now idempotent, the re-apply stays unconditional and just skips at the HID++ layer.

Testing

  • Unit tests added: the config heal (low_auto_disengage_heals_to_default_on_load) and the panel threshold clamp.
  • cargo test and cargo clippy --all-targets -- -D warnings pass across the changed crates.
  • Hardware verification still recommended on MX Master 3/3s (0x2111, tunable torque) and MX Anywhere 3s (no torque) over Bluetooth/Bolt: confirm controls no longer snap back, the wheel ratchets normally at every slider position, and a hand-edited auto_disengage = 1 config heals to 16 on restart.

Fixes #317

…317)

Three independent defects made the wheel scroll "insanely fast" and the
SmartShift GUI controls snap back to defaults on MX Master 3s / 3 and
MX Anywhere 3s.

- set_status rejected a 0-valued auto_disengage / tunable_torque instead of
  treating 0 as the firmware's documented "do not change" sentinel (x2110 /
  x2111). On a device that reports tunable_torque == 0 (no tunable-torque
  hardware) every SmartShift write failed silently, so the panel optimistically
  showed the change then snapped back. 0 is now sent as preserve; the redundant
  set_mode_preserving_status / nonzero_smartshift_value helpers are gone and the
  toggle shares set_status.

- The panel clamped a 0 readback to auto_disengage = 1 (~0.25 turn/s), which
  releases the ratchet into free-spin on the gentlest scroll. The friendly
  slider floor is raised to a usable 8 (~2 turn/s) and sub-floor / sentinel
  values normalise to the 16 default; the floor and default are single-sourced
  in openlogi-core, and a deserialize heal repairs already-persisted low
  thresholds on load so reapply pushes the good value on reconnect.

- Native scroll inversion (0x2121) was re-written on every config reload,
  needless traffic that could race other writes. The write now short-circuits
  when the wheel already holds the desired (native, invert) state.

Adds unit tests for the threshold clamp and the config heal.
@greptile-apps

greptile-apps Bot commented Jun 29, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes three independent bugs that caused runaway free-spin scrolling and SmartShift GUI controls snapping back to defaults on MX Master 3/3s and MX Anywhere 3s devices.

  • Config heal on load: Adds deserialize_auto_disengage with SMARTSHIFT_MIN_AUTO_DISENGAGE = 8 floor and SMARTSHIFT_AUTO_DISENGAGE_DEFAULT = 16; persisted thresholds below the floor are healed to the default at deserialization time, with a tracing::warn!.
  • GUI threshold floor raised: THRESHOLD_MIN in the panel is raised from 1 to SMARTSHIFT_MIN_AUTO_DISENGAGE; clamp_threshold now normalises sub-floor values (including the firmware 0 sentinel) to DEFAULT_THRESHOLD instead of the runaway 1.
  • Silent write failure removed: nonzero_smartshift_value (which rejected 0 before the HID++ call) is deleted; set_status now passes NonZeroU8::new(0) = None for Enhanced (firmware "do not change" sentinel) and Some(0) for Legacy (wire-level preserve), fixing writes on devices with no tunable-torque hardware.

Confidence Score: 5/5

Safe to merge — all three bug fixes are narrowly scoped, well-tested, and the logic flows correctly through the protocol boundary.

The config heal and GUI threshold clamp are both tested with full boundary coverage. Removing nonzero_smartshift_value correctly aligns with the HID++ 0x2110/0x2111 spec where 0x00 means "do not change". The idempotency guard in the wheel-mode write is safe: it only short-circuits when the device already holds the exact desired state, and the write still fires on any mismatch to recover from a timed-out earlier attempt.

No files require special attention.

Important Files Changed

Filename Overview
crates/openlogi-core/src/config.rs Adds SMARTSHIFT_AUTO_DISENGAGE_DEFAULT, SMARTSHIFT_MIN_AUTO_DISENGAGE constants and deserialize_auto_disengage custom deserializer with tracing::warn!; well-tested with comprehensive boundary assertions including 0, sub-floor, floor, default, and 0xFF.
crates/openlogi-gui/src/components/smartshift_panel.rs Raises THRESHOLD_MIN from 1 to 8 (shared with core constants) and updates clamp_threshold to normalise sub-floor values to DEFAULT_THRESHOLD; adds unit tests; the split between clamped committed for Ratchet pill and raw cur_auto for Free pill is intentional and correct.
crates/openlogi-hid/src/hires_wheel.rs Adds idempotency guard before set_wheel_mode: skips the write when wheel is already Native with the desired invert flag, eliminating needless HID++ traffic on every config reload while still recovering a device whose earlier write timed out.
crates/openlogi-hid/src/write/smartshift.rs Removes nonzero_smartshift_value and set_mode_preserving_status; set_status now uses NonZeroU8::new (Enhanced) and Some(0) wire-level preserve (Legacy) to handle zero fields as firmware "do not change" sentinels, fixing silent write failures on no-tunable-torque devices.
crates/openlogi-agent-core/src/orchestrator.rs Doc comment update only — clarifies the idempotent-at-HID++-layer behavior for apply_native_scroll_inversions; no logic change.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant GUI as SmartShift Panel
    participant State as AppState
    participant IPC as IPC / Agent
    participant HID as HID++ Layer
    participant Device as Device

    Note over GUI,Device: Bug 1 fix - zero tunable_torque no longer blocks writes
    GUI->>State: "commit_smartshift(mode, auto_disengage, torque=0)"
    State->>IPC: SetSmartShift(route, mode, auto_disengage, 0)
    IPC->>HID: set_status SmartShiftStatus
    Note over HID: NonZeroU8::new(0) = None = 0x00 do not change sentinel
    HID->>Device: setRatchetControlMode(mode, auto_disengage, 0x00)
    Device-->>HID: OK

    Note over GUI,Device: Bug 2 fix - sub-floor threshold healed before reaching device
    Note over State: Config load deserialize_auto_disengage(1) heals to 16
    Note over GUI: clamp_threshold(0) returns DEFAULT_THRESHOLD
    GUI->>State: commit_smartshift(Ratchet, 16, torque)
    State->>IPC: SetSmartShift(route, Ratchet, 16, torque)
    IPC->>Device: setRatchetControlMode(Ratchet, 16, torque)

    Note over GUI,Device: Bug 3 fix - scroll inversion write skipped when already set
    IPC->>HID: set_scroll_inversion(inverted)
    HID->>Device: getWheelMode()
    Device-->>HID: target Native inverted true
    alt already desired state
        Note over HID: Skip write return Ok
    else state differs
        HID->>Device: setWheelMode(Native, resolution, inverted)
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant GUI as SmartShift Panel
    participant State as AppState
    participant IPC as IPC / Agent
    participant HID as HID++ Layer
    participant Device as Device

    Note over GUI,Device: Bug 1 fix - zero tunable_torque no longer blocks writes
    GUI->>State: "commit_smartshift(mode, auto_disengage, torque=0)"
    State->>IPC: SetSmartShift(route, mode, auto_disengage, 0)
    IPC->>HID: set_status SmartShiftStatus
    Note over HID: NonZeroU8::new(0) = None = 0x00 do not change sentinel
    HID->>Device: setRatchetControlMode(mode, auto_disengage, 0x00)
    Device-->>HID: OK

    Note over GUI,Device: Bug 2 fix - sub-floor threshold healed before reaching device
    Note over State: Config load deserialize_auto_disengage(1) heals to 16
    Note over GUI: clamp_threshold(0) returns DEFAULT_THRESHOLD
    GUI->>State: commit_smartshift(Ratchet, 16, torque)
    State->>IPC: SetSmartShift(route, Ratchet, 16, torque)
    IPC->>Device: setRatchetControlMode(Ratchet, 16, torque)

    Note over GUI,Device: Bug 3 fix - scroll inversion write skipped when already set
    IPC->>HID: set_scroll_inversion(inverted)
    HID->>Device: getWheelMode()
    Device-->>HID: target Native inverted true
    alt already desired state
        Note over HID: Skip write return Ok
    else state differs
        HID->>Device: setWheelMode(Native, resolution, inverted)
    end
Loading

Reviews (2): Last reviewed commit: "docs(smartshift): document zero preserve..." | Re-trigger Greptile

Comment thread crates/openlogi-core/src/config.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]/[Bug]: Modify scrolling speed, unusable now!!

1 participant