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

Merged
Sapd merged 2 commits into from
Jul 23, 2026
+119 −190
Merged

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

Show file tree
Hide file tree
Changes from all commits
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
201 changes: 102 additions & 99 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 @@ -122,69 +122,32 @@ 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();
auto centurion_battery = sendCenturionFeatureRequest(
device_handle,
static_cast<uint16_t>(protocols::CenturionFeature::CenturionBatterySoc),
0x00);
if (!centurion_battery) {
if (!shouldUseLegacyBatteryFallback(centurion_battery.error().code)) {
return centurion_battery.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;
return getLegacyBattery(device_handle, centurion_start_time);
}

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;
}
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;
}

return DeviceError::protocolError("Battery response packet not received");
static constexpr bool shouldUseLegacyBatteryFallback(DeviceError::Code code)
{
return code == DeviceError::Code::NotSupported;
}

Result<SidetoneResult> setSidetone(hid_device* device_handle, uint8_t level) override
Expand Down Expand Up @@ -350,47 +313,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,7 +350,88 @@ 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()
Result<BatteryResult> getLegacyBattery(
hid_device* device_handle,
std::chrono::steady_clock::time_point start_time)
{
auto request = buildLegacyBatteryRequest();
if (auto write_result = writeHID(device_handle, request, PACKET_SIZE); !write_result) {
return write_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 (isLegacyPowerOffPacket(response)) {
return DeviceError::deviceOffline("Headset is powered off or not connected");
}
if (isLegacyPowerEventPacket(response) || isLegacyAckPacket(response)) {
continue;
}
if (!isLegacyBatteryResponsePacket(response)) {
continue;
}

auto battery_result = parseLegacyBatteryResponse(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");
}

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

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

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

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

static Result<BatteryResult> parseLegacyBatteryResponse(std::span<const uint8_t> packet)
{
if (!isLegacyBatteryResponsePacket(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");
}

return BatteryResult {
.level_percent = level,
.status = packet[12] == 0x02 ? BATTERY_CHARGING : BATTERY_AVAILABLE,
};
}

static constexpr std::array<uint8_t, PACKET_SIZE> buildLegacyBatteryRequest()
{
std::array<uint8_t, PACKET_SIZE> request {};
request[0] = REPORT_PREFIX;
Expand Down
108 changes: 17 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 @@ -363,6 +275,22 @@ void testLogitechProX2CenturionBatteryParsing()
std::cout << " [OK] Logitech PRO X2 Centurion battery parsing verified" << std::endl;
}

void testLogitechProX2LegacyBatteryFallbackDecision()
{
std::cout << " Testing Logitech PRO X2 legacy battery fallback decision..." << std::endl;

ASSERT_TRUE(LogitechGProX2Lightspeed::shouldUseLegacyBatteryFallback(DeviceError::Code::NotSupported),
"A missing Centurion battery feature should use the legacy fallback");
ASSERT_TRUE(!LogitechGProX2Lightspeed::shouldUseLegacyBatteryFallback(DeviceError::Code::Timeout),
"A Centurion timeout should not trigger a second battery request");
ASSERT_TRUE(!LogitechGProX2Lightspeed::shouldUseLegacyBatteryFallback(DeviceError::Code::DeviceOffline),
"An offline device should not trigger a second battery request");
ASSERT_TRUE(!LogitechGProX2Lightspeed::shouldUseLegacyBatteryFallback(DeviceError::Code::HIDError),
"A HID error should not trigger a second battery request");

std::cout << " [OK] Logitech PRO X2 legacy battery fallback decision verified" << std::endl;
}

void testCenturionFrameBuilding()
{
std::cout << " Testing Logitech Centurion frame building..." << std::endl;
Expand Down Expand Up @@ -717,10 +645,8 @@ 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 PRO X2 Legacy Battery Fallback", testLogitechProX2LegacyBatteryFallbackDecision);
runTest("Logitech Centurion Frame Building", testCenturionFrameBuilding);
runTest("Logitech Centurion Bridge Parsing", testCenturionBridgeResponseParsing);
runTest("Logitech Centurion Bridge Size Limit", testCenturionBridgeMessageSizeLimit);
Expand Down
Loading