fix(logi): restore receiver diverts after idle wake - #1012
Conversation
3770cdf to
c0153f2
Compare
Caldis
left a comment
There was a problem hiding this comment.
The problem is real and your evidence chain is solid (idle resume without 0x41, manual Re-Divert restoring events immediately). But the current implementation conflicts with a hard performance constraint this codebase carries from a painful lesson, so I can't take it as-is.
Why unguarded re-diverting is a red line here
We previously shipped (and then removed, in 2a573be / docs/plans/2026-05-03-logi-ble-hidpp-divert-postmortem.md §4.2) a family of "keep the HID++ state alive by re-sending things" mechanisms — notification watchdog, ping pulses, protocol probe matrices. Real-device experience showed they only briefly wake the device, fight with Logitech Options+ over HID++ ownership (both sides re-asserting divert = oscillation), and add constant device load. The standing rule since then: anything that re-sends HID++ requests repeatedly over time is out; discrete one-shot actions on connect/disconnect/user events are fine.
This PR's shape trips that rule in daily use: a mouse crosses the 60s idle threshold many times per hour (reading, video, stepping away). Each wake schedules two unconditional redivertAllControls() passes (1s + 2s) — a full SetControlReporting volley per bound CID, twice, regardless of whether divert was actually lost. That's recurring HID++ write traffic keyed to normal usage patterns, with no "already correct" guard — exactly the keepalive failure mode in a new coat. The [1.0, 2.0] double-shot also reads as compensating for uncertain timing rather than a confirmed causal fix.
Requested change: probe, then repair
On detected idle-wake, send a single GetControlReporting for one representative bound CID (or walk the bound set) and compare against the expected divert state. Only if the readback shows divert lost, re-send — once — and mark the slot repaired until the next disconnect/wake transition. That converts N unconditional writes into 1 read on the common path (divert intact), keeps the fix event-driven, and adds the oscillation guard the red line requires. Bonus: the probe result gives you a definitive log line proving the firmware actually dropped tmpDivert on idle, which would settle the root cause beyond inference.
Please also coordinate with #1013
Both PRs are symptoms of the same device behavior (M720 sleeping without receiver notifications): #1013 handles "never taken over", this one handles "taken over, divert silently lost". Ideally they share one wake-inference mechanism with per-slot state (your lastReceiverReportTime tracking here and the offline→online inference there are two views of the same signal). A combined design would avoid two parallel detectors with different thresholds living in the same report path.
|
Thanks for the detailed context and for pointing me to the earlier postmortem. I agree that the current unconditional two-pass re-divert is the wrong shape for this codebase. Even though it is bounded per wake, normal idle cycles would still turn it into recurring HID++ write traffic, and the interaction risk with Options+ is not worth taking. I’m going to rework this around probe-then-repair: infer a possible wake, read back While re-validating the stock 4.2.1 behavior, I noticed an additional edge case that needs more controlled testing before I can describe it confidently. I don’t want to speculate from incomplete traces, so I’ll finish the baseline and revised-build comparisons first and report back once the behavior is reproducible and understood. The next step is to capture the actual transition with reporting-state readback before and after wake, then validate the conditional repair on the real device. I’ve moved the PR back to draft until that evidence and the revised design are solid. |
|
Moving to draft for evidence-first rework is exactly the right call — no rush on this side. The probe-then-repair direction and the plan to share per-slot activity state with #1013 (now merged) are both approved in principle; looking forward to the readback traces. If the additional edge case you found turns out to be a separate defect, feel free to file it independently so it doesn't block this PR. |
Motivation
Some Logitech devices connected through a receiver can lose their temporary HID++ control diversion after being idle, without emitting the receiver's HID++ 1.0
0x41disconnect/reconnect notifications.This was reproduced with a Logitech M720 Triathlon through a Unifying receiver:
The existing reconnect path already restores diversion when
0x41notifications are present, but it cannot run when the receiver continues to consider the slot connected.Before the fix
Relevant excerpt from the HID++ debug log (unrelated traffic omitted):
Reproduction
What changed
The recovery is event-driven and bounded. It adds no polling, does not use the malformed
SetControlReportingACK returned by the M720 as confirmation, and does not change BLE behavior.Validation
scripts/qa/lint-logi-boundary.shLogiReceiverConnectionStateTests: 33 passed, 0 failed0x41disconnect/reconnect notification occurred;After the fix
Relevant excerpt from the HID++ debug log (unrelated traffic omitted):
The full raw logs contain receiver enumeration and unrelated paired-device traffic, so they are not committed to the repository. They can be provided to reviewers if needed.
Behavior and compatibility
This only affects managed HID++ receiver slots that have an initialized REPROG feature and active diverted bindings. The first report for a slot and normal report traffic do not trigger recovery. Existing receiver reconnect handling and BLE sessions are unchanged.
The main behavior tradeoff is that a managed receiver slot with active diverted bindings will receive a bounded re-divert after at least 60 seconds without peripheral reports, even if the idle period was not caused by sleep.
SetControlReportingis idempotent for the desired state, and no polling or persistent timer is introduced.