Skip to content

Limit PRO X 2 legacy battery fallback to unsupported features #538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ChrisLauinger77 wants to merge 2 commits into
base: master
Choose a base branch
from
+119 −190
Open

Limit PRO X 2 legacy battery fallback to unsupported features #538

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e942cbf
Use Centurion exclusively for PRO X 2 battery
ChrisLauinger77 Jul 10, 2026
c87c3bf
Limit PRO X 2 battery fallback to unsupported features
ChrisLauinger77 Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 16 additions & 115 deletions lib/devices/logitech_gpro_x2_lightspeed.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ namespace headsetcontrol {
class LogitechGProX2Lightspeed : public protocols::LogitechCenturionProtocol {
public:
static constexpr std::array<uint16_t, 1> SUPPORTED_PRODUCT_IDS { 0x0af7 };
static constexpr size_t PACKET_SIZE = 64;
static constexpr uint8_t REPORT_PREFIX = 0x51;
static constexpr uint8_t SIDETONE_DEVICE_MAX = 100;
static constexpr uint8_t SIDETONE_MIC_ID = 0x01;
static constexpr uint8_t PLAYBACK_DIRECTION = 0x00;
Expand Down Expand Up @@ -122,69 +120,26 @@ class LogitechGProX2Lightspeed : public protocols::LogitechCenturionProtocol {
Result<BatteryResult> getBattery(hid_device* device_handle) override
{
auto centurion_start_time = std::chrono::steady_clock::now();
if (auto centurion_battery = sendCenturionFeatureRequest(
device_handle,
static_cast<uint16_t>(protocols::CenturionFeature::CenturionBatterySoc),
0x00);
centurion_battery) {
auto battery_result = parseCenturionBatteryResponse(*centurion_battery);
if (!battery_result) {
return battery_result.error();
}

battery_result->raw_data = *centurion_battery;
auto centurion_end_time = std::chrono::steady_clock::now();
battery_result->query_duration = std::chrono::duration_cast<std::chrono::milliseconds>(
centurion_end_time - centurion_start_time);
return *battery_result;
auto centurion_battery = sendCenturionFeatureRequest(
device_handle,
static_cast<uint16_t>(protocols::CenturionFeature::CenturionBatterySoc),
0x00);
// 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();

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore the legacy battery fallback

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 👍 / 👎.

@ChrisLauinger77 ChrisLauinger77 Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

auto start_time = std::chrono::steady_clock::now();

std::array<uint8_t, PACKET_SIZE> request = buildBatteryRequest();
if (auto write_result = writeHID(device_handle, request, PACKET_SIZE); !write_result) {
return write_result.error();
auto battery_result = parseCenturionBatteryResponse(*centurion_battery);
if (!battery_result) {
return battery_result.error();
}

std::vector<uint8_t> raw_packets;
raw_packets.reserve(PACKET_SIZE * 4);

for (int attempt = 0; attempt < 4; ++attempt) {
std::array<uint8_t, PACKET_SIZE> response {};
if (auto read_result = readHIDTimeout(device_handle, response, hsc_device_timeout); !read_result) {
return read_result.error();
}

raw_packets.insert(raw_packets.end(), response.begin(), response.end());

if (isPowerOffPacket(response)) {
return DeviceError::deviceOffline("Headset is powered off or not connected");
}

if (isPowerEventPacket(response)) {
continue;
}

if (isAckPacket(response)) {
continue;
}

if (!isBatteryResponsePacket(response)) {
continue;
}

auto battery_result = parseBatteryResponse(response);
if (!battery_result) {
return battery_result.error();
}

battery_result->raw_data = std::move(raw_packets);
auto end_time = std::chrono::steady_clock::now();
battery_result->query_duration = std::chrono::duration_cast<std::chrono::milliseconds>(end_time - start_time);
return *battery_result;
}

return DeviceError::protocolError("Battery response packet not received");
battery_result->raw_data = *centurion_battery;
auto centurion_end_time = std::chrono::steady_clock::now();
battery_result->query_duration = std::chrono::duration_cast<std::chrono::milliseconds>(
centurion_end_time - centurion_start_time);
return *battery_result;
}

Result<SidetoneResult> setSidetone(hid_device* device_handle, uint8_t level) override
Expand Down Expand Up @@ -350,47 +305,6 @@ class LogitechGProX2Lightspeed : public protocols::LogitechCenturionProtocol {
};
}

static constexpr bool isAckPacket(std::span<const uint8_t> packet)
{
return packet.size() >= 2 && packet[0] == REPORT_PREFIX && packet[1] == 0x03;
}

static constexpr bool isPowerOffPacket(std::span<const uint8_t> packet)
{
return packet.size() >= 7 && packet[0] == REPORT_PREFIX && packet[1] == 0x05 && packet[6] == 0x00;
}

static constexpr bool isPowerEventPacket(std::span<const uint8_t> packet)
{
return packet.size() >= 2 && packet[0] == REPORT_PREFIX && packet[1] == 0x05;
}

static constexpr bool isBatteryResponsePacket(std::span<const uint8_t> packet)
{
return packet.size() >= 13 && packet[0] == REPORT_PREFIX && packet[1] == 0x0b && packet[8] == 0x04;
}

static Result<BatteryResult> parseBatteryResponse(std::span<const uint8_t> packet)
{
if (!isBatteryResponsePacket(packet)) {
return DeviceError::protocolError("Unexpected battery response packet");
}

auto level = static_cast<int>(packet[10]);
if (level > 100) {
return DeviceError::protocolError("Battery percentage out of range");
}

auto status = packet[12] == 0x02 ? BATTERY_CHARGING : BATTERY_AVAILABLE;

BatteryResult result {
.level_percent = level,
.status = status,
};

return result;
}

static Result<BatteryResult> parseCenturionBatteryResponse(std::span<const uint8_t> packet)
{
if (packet.empty()) {
Expand Down Expand Up @@ -428,19 +342,6 @@ class LogitechGProX2Lightspeed : public protocols::LogitechCenturionProtocol {
static constexpr std::array<float, 5> PRESET_SHOOTER { -1.0f, -1.0f, 4.0f, 3.0f, 2.0f };
static constexpr std::array<float, 5> PRESET_MOBA { 0.0f, 1.0f, 1.0f, 2.0f, 4.0f };

static constexpr std::array<uint8_t, PACKET_SIZE> buildBatteryRequest()
{
std::array<uint8_t, PACKET_SIZE> request {};
request[0] = REPORT_PREFIX;
request[1] = 0x08;
request[3] = 0x03;
request[4] = 0x1a;
request[6] = 0x03;
request[8] = 0x04;
request[9] = 0x0a;
return request;
}

struct AdvancedEqBand {
uint16_t frequency = 0;
int8_t gain_db = 0;
Expand Down
91 changes: 0 additions & 91 deletions tests/test_protocols.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,94 +248,6 @@ void testHIDPPOfflineDetection()
std::cout << " [OK] HID++ offline detection verified" << std::endl;
}

void testLogitechProX2BatteryPacketParsing()
{
std::cout << " Testing Logitech PRO X2 vendor battery parsing..." << std::endl;

std::array<uint8_t, 64> response {};
response[0] = 0x51;
response[1] = 0x0b;
response[8] = 0x04;
response[10] = 87;
response[12] = 0x02;

ASSERT_TRUE(LogitechGProX2Lightspeed::isBatteryResponsePacket(response), "Should identify battery response packet");

auto result = LogitechGProX2Lightspeed::parseBatteryResponse(response);
ASSERT_TRUE(result.hasValue(), "Battery response should parse successfully");
ASSERT_EQ(87, result->level_percent, "Direct percentage should be parsed from byte 10");
ASSERT_EQ(BATTERY_CHARGING, result->status, "Charging status should map from byte 12");

response[12] = 0x00;
auto discharging_result = LogitechGProX2Lightspeed::parseBatteryResponse(response);
ASSERT_TRUE(discharging_result.hasValue(), "Discharging packet should parse successfully");
ASSERT_EQ(BATTERY_AVAILABLE, discharging_result->status, "Non-0x02 status should be available");

std::cout << " [OK] Logitech PRO X2 battery packet parsing verified" << std::endl;
}

void testLogitechProX2PowerEventDetection()
{
std::cout << " Testing Logitech PRO X2 power event detection..." << std::endl;

// Power-off event: byte[1]==0x05 and byte[6]==0x00
std::array<uint8_t, 64> power_off {};
power_off[0] = 0x51;
power_off[1] = 0x05;
power_off[6] = 0x00;

ASSERT_TRUE(LogitechGProX2Lightspeed::isPowerOffPacket(power_off), "Power-off event should be detected");
ASSERT_TRUE(LogitechGProX2Lightspeed::isPowerEventPacket(power_off), "Power-off should also be a power event");

// Power-on event: byte[1]==0x05 and byte[6]==0x01
std::array<uint8_t, 64> power_on {};
power_on[0] = 0x51;
power_on[1] = 0x05;
power_on[6] = 0x01;

ASSERT_TRUE(!LogitechGProX2Lightspeed::isPowerOffPacket(power_on), "Power-on should not be detected as power-off");
ASSERT_TRUE(LogitechGProX2Lightspeed::isPowerEventPacket(power_on), "Power-on should be detected as power event");

// ACK packet should not be a power event
std::array<uint8_t, 64> ack {};
ack[0] = 0x51;
ack[1] = 0x03;

ASSERT_TRUE(LogitechGProX2Lightspeed::isAckPacket(ack), "ACK packet should be detected");
ASSERT_TRUE(!LogitechGProX2Lightspeed::isPowerEventPacket(ack), "ACK packet should not be treated as power event");

std::cout << " [OK] Logitech PRO X2 power event detection verified" << std::endl;
}

void testLogitechProX2BatteryOutOfRange()
{
std::cout << " Testing Logitech PRO X2 battery out-of-range rejection..." << std::endl;

std::array<uint8_t, 64> response {};
response[0] = 0x51;
response[1] = 0x0b;
response[8] = 0x04;
response[10] = 101; // Out of range
response[12] = 0x00;

auto result = LogitechGProX2Lightspeed::parseBatteryResponse(response);
ASSERT_TRUE(!result.hasValue(), "Battery level 101 should be rejected as out of range");

// Boundary: 100 should be valid
response[10] = 100;
auto valid_result = LogitechGProX2Lightspeed::parseBatteryResponse(response);
ASSERT_TRUE(valid_result.hasValue(), "Battery level 100 should be valid");
ASSERT_EQ(100, valid_result->level_percent, "Battery level should be 100");

// Boundary: 0 should be valid
response[10] = 0;
auto zero_result = LogitechGProX2Lightspeed::parseBatteryResponse(response);
ASSERT_TRUE(zero_result.hasValue(), "Battery level 0 should be valid");
ASSERT_EQ(0, zero_result->level_percent, "Battery level should be 0");

std::cout << " [OK] Logitech PRO X2 battery out-of-range rejection verified" << std::endl;
}

void testLogitechProX2CenturionBatteryParsing()
{
std::cout << " Testing Logitech PRO X2 Centurion battery parsing..." << std::endl;
Expand Down Expand Up @@ -717,9 +629,6 @@ void runAllProtocolTests()
runTest("HID++ Voltage To Percent", testHIDPPVoltageToPercent);
runTest("HID++ Battery Response", testHIDPPBatteryResponseParsing);
runTest("HID++ Offline Detection", testHIDPPOfflineDetection);
runTest("Logitech PRO X2 Battery Parsing", testLogitechProX2BatteryPacketParsing);
runTest("Logitech PRO X2 Power Event", testLogitechProX2PowerEventDetection);
runTest("Logitech PRO X2 Battery Out-of-Range", testLogitechProX2BatteryOutOfRange);
runTest("Logitech PRO X2 Centurion Battery", testLogitechProX2CenturionBatteryParsing);
runTest("Logitech Centurion Frame Building", testCenturionFrameBuilding);
runTest("Logitech Centurion Bridge Parsing", testCenturionBridgeResponseParsing);
Expand Down
Loading