Skip to content

fix(agent): reapply volatile settings on device reconnection and retry disagreeing DPI writes - #1270

Merged
davidbudnick merged 2 commits into
AprilNEA:masterfrom
isleofgreg:pr/wake-reapply-volatile
Oct 10, 2026
Merged

davidbudnick merged 2 commits into
AprilNEA:masterfrom
isleofgreg:pr/wake-reapply-volatile

Conversation

@isleofgreg

@isleofgreg isleofgreg commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Two related fixes to the volatile-settings re-apply path, found while instrumenting an MX Master 4 on a Bolt receiver (macOS 15.7) over several days of logged use. Standalone — applies to current master, no dependency on the smooth-scroll series.

  • Reapply volatile settings when a device broadcasts its own reconnection (0x1d4b WirelessDeviceStatus). A mouse waking from a nap re-establishes its link and comes back with firmware-default volatile settings (sensor DPI, SmartShift, wheel mode). The re-apply relied on an offline→online transition between snapshots, but a nap short enough that no reconciliation observed the device offline produced no transition, and the one write a reconnect did get could race the link still stabilising — leaving the wrong DPI live until the next nap. The reconnection broadcast already reaches the inventory watcher; it now survives the settle drain and forces a volatile re-apply on the following snapshot (like a system wake, with a single link-race confirming pass instead of the boot ladder). Forced requests coalesce keeping the larger budget. Verified overnight: after a 12 h nap the DPI was rewritten and read back correct 14.5 s after display resume, with the pre-fix 8705 warnings gone.
  • Retry a DPI write once when the read-back disagrees. Firmware still drowsy from a link re-establishment can ACK setSensorDpi yet keep the old value. The read-back already caught the mismatch but only warned; one re-write now separates the two silent-failure shapes — a dropped write lands on the retry, a value the device clamps deterministically comes back unchanged again and keeps the existing warning.

(An earlier revision also reclaimed a stale diverted wheel route on re-apply. It was withdrawn after review: the agent has no signal to tell an abandoned diversion from a live external consumer, and the identity-free reconnect path touches every online device. It will come back separately with an ownership check.)

Test plan

  • cargo fmt --all --check; cargo clippy --workspace --all-targets -- -D warnings on aarch64-apple-darwin and x86_64-pc-windows-gnu for openlogi-device/openlogi-agent-core; RUSTDOCFLAGS=-D warnings cargo doc for those crates; cargo test -p openlogi-device -p openlogi-agent-core -p openlogi-agent — green. New unit tests cover the forced re-apply request coalescing and budget, the reconnection-broadcast path, and the single DPI retry (dropped write vs deterministic clamp). (One pre-existing tray:: test in openlogi-agent is timing-flaky under parallel execution on master — fix(agent): serialize the macOS tray tests that share NSWorkspace's center #1236 addresses it; passes serially.)
  • On hardware: multi-day agent debug logs across naps and overnight sleep.

@isleofgreg
isleofgreg requested a review from AprilNEA as a code owner September 4, 2026 23:46
@greptile-apps

greptile-apps Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Adds device reconnection handling to volatile settings reapply logic.

The PR appears safe to merge; no actionable regression remains in the reconnect re-apply or DPI retry paths.

Fix All in CodexFindings

  1. P1 Live Wheel Route Reclaimed ▶

Summary

This PR improves restoration of volatile device settings after wireless reconnection and retries DPI writes whose first read-back disagrees.

  • Preserves WirelessDeviceStatus reconnect intent while coalescing HID++ lifecycle-event bursts.
  • Forces a bounded re-apply on the next authoritative inventory snapshot and coalesces wake/reconnect confirmation budgets without shrinking them.
  • Adds one DPI rewrite and verification attempt after an ACKed write leaves the old value.
  • Adds focused coverage for reconnect re-application, budget behavior, and dropped DPI writes.
  • The earlier wheel-route reclamation that caused the previous review finding has been removed.
Diagram
sequenceDiagram
    participant Device
    participant InventoryWatcher
    participant Lifecycle
    participant Orchestrator
    participant DPIWriter

    Device->>InventoryWatcher: WirelessDeviceStatus reconnect
    InventoryWatcher->>InventoryWatcher: Settle and coalesce event burst
    InventoryWatcher->>Lifecycle: DeviceWake
    Lifecycle->>Orchestrator: Queue forced re-apply budget
    InventoryWatcher->>Lifecycle: Authoritative snapshot
    Lifecycle->>Orchestrator: Refresh inventory
    Orchestrator->>DPIWriter: Reapply persisted DPI
    DPIWriter->>Device: Set DPI
    DPIWriter->>Device: Read back DPI
    alt Read-back disagrees
        DPIWriter->>Device: Retry set DPI once
        DPIWriter->>Device: Verify again
    end
Loading

Reviews (6) · Last reviewed commit: "fix(hid): retry a DPI write once when th..." · Reviewed by Greptile

Comment thread crates/openlogi-agent-core/src/hardware.rs Outdated
@isleofgreg
isleofgreg force-pushed the pr/wake-reapply-volatile branch from 8c24d22 to da33a42 Compare September 4, 2026 23:53
@isleofgreg

Copy link
Copy Markdown
Contributor Author

Fair point on the reclaim — it treated every diverted route in the forced target set as abandoned, and the identity-free reconnect path makes that set every online device. There's no staleness signal available from the agent's side today (a live external consumer on another device looks identical), so rather than guess I've withdrawn that commit from this PR. It's now the two fixes with an unambiguous owner: the reconnection re-apply and the DPI write retry. I'll bring the wheel-route reclaim back separately once there's a defensible staleness check (or scope it to the agent's own startup pass), if that's of interest.

@isleofgreg isleofgreg changed the title fix(agent): reapply volatile settings on device reconnection, retry disagreeing DPI writes, reclaim stale wheel routes fix(agent): reapply volatile settings on device reconnection and retry disagreeing DPI writes Sep 5, 2026
@isleofgreg
isleofgreg force-pushed the pr/wake-reapply-volatile branch 2 times, most recently from 21e926e to 6cac34c Compare September 5, 2026 00:13
@davidbudnick davidbudnick added type: bug Something is broken or behaves incorrectly platform: all Cross-platform issue labels Sep 8, 2026
@isleofgreg

Copy link
Copy Markdown
Contributor Author

Rebased onto master (post-#1286). The only change: the new replay_tests.rs fixture matches InventoryEvent exhaustively, so the DeviceWake variant this PR adds needed adding to its panic arm — folded into the first commit. No production changes.

@isleofgreg
isleofgreg force-pushed the pr/wake-reapply-volatile branch 2 times, most recently from 0114202 to 61e9c6a Compare September 21, 2026 10:08
@isleofgreg

Copy link
Copy Markdown
Contributor Author

Rebased onto master (a92aa43, v0.8.6) after the orchestrator split. No functional change: the forced re-apply budget, the refresh path and the two entry points stay in orchestrator.rs; the plan_reapply rewrite and RECONNECT_REAPPLY_CONFIRM_RETRIES moved with plan_reapply into orchestrator/devices.rs; the tests into orchestrator/tests/reapply.rs. The DPI retry commit applied unchanged. Previous head for reference: 0114202.

…ection

A mouse waking from a nap re-establishes its receiver link and comes back
with firmware-default volatile settings (sensor DPI, SmartShift, wheel
mode). The re-apply relied on an offline→online transition between
snapshots — but a nap short enough that no reconciliation observed the
device offline produces no transition at all, and the one write a
reconnect did get could race the link still stabilizing, leaving the
wrong DPI live until the next nap.

The firmware's own reconnection broadcast (0x1d4b WirelessDeviceStatus)
already reaches the inventory watcher; it now also survives the settle
drain and forces a volatile re-apply on the snapshot that follows, like
a system wake but with a single link-race confirming pass instead of the
boot ladder. Plain offline→online reconnects queue the same single
confirm, and forced re-apply requests coalesce keeping the larger
budget.
Firmware still drowsy from a link re-establishment can ACK setSensorDpi
yet keep the old value. The read-back already caught the mismatch but
only warned; a single re-write now disambiguates the two silent-failure
shapes: a dropped write lands on the retry, while a value the device
clamps deterministically comes back unchanged again and keeps the
existing warning.
@isleofgreg
isleofgreg force-pushed the pr/wake-reapply-volatile branch from 61e9c6a to 8a62d23 Compare October 8, 2026 17:43
@isleofgreg

Copy link
Copy Markdown
Contributor Author

Rebased onto current master (1505c652). #1649 added an exhaustive InventoryEvent match in replay_tests.rs, so the new DeviceWake variant gets the same ignore arm there as SystemWake. No other changes; workspace fmt/clippy/tests are green.

@davidbudnick
davidbudnick merged commit 0f69d75 into AprilNEA:master Oct 10, 2026
23 checks passed
@aprilnea aprilnea Bot mentioned this pull request Oct 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

platform: all Cross-platform issue type: bug Something is broken or behaves incorrectly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants