Limit PRO X 2 legacy battery fallback to unsupported features#538
Limit PRO X 2 legacy battery fallback to unsupported features#538ChrisLauinger77 wants to merge 2 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e942cbf563
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // PID 0x0af7 uses Centurion exclusively. Propagate failures instead of | ||
| // sending the former vendor-specific battery request as a fallback. | ||
| if (!centurion_battery) { | ||
| return centurion_battery.error(); |
There was a problem hiding this comment.
When the Centurion feature lookup/request fails (for example on a G PRO X 2 firmware that still exposes the legacy 0x51/0x0b battery packets but not CenturionBatterySoc, or when discovery times out), this early return makes CAP_BATTERY_STATUS fail even though the previous vendor-specific query path could still return the battery level and detect power-off packets. The class still targets the 0xffa0 vendor protocol, so keeping the fallback for Centurion failures avoids regressing those supported devices.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks, good point regarding a potential firmware variant without CenturionBatterySoc. I updated the implementation to retain the legacy battery query only when the Centurion lookup returns NotSupported. Communication failures such as Timeout, DeviceOffline, and HIDError now propagate immediately, so the offline scenario cannot issue a second battery request. I also added regression coverage for this fallback decision.
ChrisLauinger77
changed the title
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
CENTURION_BATTERY_SOCis genuinely unsupportedMotivation
The Logitech G PRO X 2 LIGHTSPEED (
046d:0af7) is handled as a Centurion device. Its sidetone, inactive-time, equalizer, and primary battery capabilities use the Centurion protocol.Previously, every failed Centurion battery query was followed by a second query using the older vendor-specific
0x51packet format. When the headset was powered off but its USB receiver remained connected, each poll could therefore send two different protocol sequences and consume two HID read timeouts.During long-running Windows testing, repeated polling while the headset was automatically powered off was correlated with the receiver entering a persistent slow state: later HID discovery calls could take about 60 seconds even after the headset was powered on again. Avoiding the second transaction after communication failures makes the offline path shorter and prevents mixing protocols when the device is not responding.
The legacy query remains available for compatibility when Centurion feature discovery succeeds but reports that
CENTURION_BATTERY_SOCis not supported. This preserves support for a potential firmware variant that exposes the older battery packets without the Centurion battery feature.This PR intentionally does not claim that the former fallback is the proven sole cause of the Windows HID slowdown; it removes the redundant request specifically from timeout and error paths so that the scenario can be retested cleanly.
Behavior change
NotSupported: try the legacy vendor-specific battery query.Timeout,DeviceOffline,HIDError, or any other Centurion failure: return the original error without sending a second battery request.Validation
ctest --output-on-failure: 2/2 tests passedNotSupported,Timeout,DeviceOffline, andHIDErrorgit diff --checkheadsetcontrol -o json; battery output works correctlyThe original long-running Windows reproduction scenario will be retested once this change is available to QontrolPanel through the HeadsetControl submodule.