From e942cbf563cc0e497f82f32cb5e2a82e47a32377 Mon Sep 17 00:00:00 2001 From: Christian Lauinger Date: Fri, 10 Jul 2026 16:38:14 +0200 Subject: [PATCH 1/2] Use Centurion exclusively for PRO X 2 battery --- lib/devices/logitech_gpro_x2_lightspeed.hpp | 131 +++----------------- tests/test_protocols.cpp | 91 -------------- 2 files changed, 16 insertions(+), 206 deletions(-) diff --git a/lib/devices/logitech_gpro_x2_lightspeed.hpp b/lib/devices/logitech_gpro_x2_lightspeed.hpp index 6740ed4..9948ff2 100644 --- a/lib/devices/logitech_gpro_x2_lightspeed.hpp +++ b/lib/devices/logitech_gpro_x2_lightspeed.hpp @@ -22,8 +22,6 @@ namespace headsetcontrol { class LogitechGProX2Lightspeed : public protocols::LogitechCenturionProtocol { public: static constexpr std::array 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; @@ -122,69 +120,26 @@ class LogitechGProX2Lightspeed : public protocols::LogitechCenturionProtocol { Result getBattery(hid_device* device_handle) override { auto centurion_start_time = std::chrono::steady_clock::now(); - if (auto centurion_battery = sendCenturionFeatureRequest( - device_handle, - static_cast(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( - centurion_end_time - centurion_start_time); - return *battery_result; + auto centurion_battery = sendCenturionFeatureRequest( + device_handle, + static_cast(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(); } - auto start_time = std::chrono::steady_clock::now(); - - std::array 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 raw_packets; - raw_packets.reserve(PACKET_SIZE * 4); - - for (int attempt = 0; attempt < 4; ++attempt) { - std::array 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(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( + centurion_end_time - centurion_start_time); + return *battery_result; } Result setSidetone(hid_device* device_handle, uint8_t level) override @@ -350,47 +305,6 @@ class LogitechGProX2Lightspeed : public protocols::LogitechCenturionProtocol { }; } - static constexpr bool isAckPacket(std::span packet) - { - return packet.size() >= 2 && packet[0] == REPORT_PREFIX && packet[1] == 0x03; - } - - static constexpr bool isPowerOffPacket(std::span packet) - { - return packet.size() >= 7 && packet[0] == REPORT_PREFIX && packet[1] == 0x05 && packet[6] == 0x00; - } - - static constexpr bool isPowerEventPacket(std::span packet) - { - return packet.size() >= 2 && packet[0] == REPORT_PREFIX && packet[1] == 0x05; - } - - static constexpr bool isBatteryResponsePacket(std::span packet) - { - return packet.size() >= 13 && packet[0] == REPORT_PREFIX && packet[1] == 0x0b && packet[8] == 0x04; - } - - static Result parseBatteryResponse(std::span packet) - { - if (!isBatteryResponsePacket(packet)) { - return DeviceError::protocolError("Unexpected battery response packet"); - } - - auto level = static_cast(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 parseCenturionBatteryResponse(std::span packet) { if (packet.empty()) { @@ -428,19 +342,6 @@ class LogitechGProX2Lightspeed : public protocols::LogitechCenturionProtocol { static constexpr std::array PRESET_SHOOTER { -1.0f, -1.0f, 4.0f, 3.0f, 2.0f }; static constexpr std::array PRESET_MOBA { 0.0f, 1.0f, 1.0f, 2.0f, 4.0f }; - static constexpr std::array buildBatteryRequest() - { - std::array 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; diff --git a/tests/test_protocols.cpp b/tests/test_protocols.cpp index 7ec6e85..074110a 100644 --- a/tests/test_protocols.cpp +++ b/tests/test_protocols.cpp @@ -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 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 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 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 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 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; @@ -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); From c87c3bf46e02835c32932ba397e9523e2e6ca2a4 Mon Sep 17 00:00:00 2001 From: Christian Lauinger Date: Fri, 10 Jul 2026 16:55:23 +0200 Subject: [PATCH 2/2] Limit PRO X 2 battery fallback to unsupported features --- lib/devices/logitech_gpro_x2_lightspeed.hpp | 108 +++++++++++++++++++- tests/test_protocols.cpp | 17 +++ 2 files changed, 122 insertions(+), 3 deletions(-) diff --git a/lib/devices/logitech_gpro_x2_lightspeed.hpp b/lib/devices/logitech_gpro_x2_lightspeed.hpp index 9948ff2..37774e4 100644 --- a/lib/devices/logitech_gpro_x2_lightspeed.hpp +++ b/lib/devices/logitech_gpro_x2_lightspeed.hpp @@ -22,6 +22,8 @@ namespace headsetcontrol { class LogitechGProX2Lightspeed : public protocols::LogitechCenturionProtocol { public: static constexpr std::array 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; @@ -124,10 +126,11 @@ class LogitechGProX2Lightspeed : public protocols::LogitechCenturionProtocol { device_handle, static_cast(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(); + if (!shouldUseLegacyBatteryFallback(centurion_battery.error().code)) { + return centurion_battery.error(); + } + return getLegacyBattery(device_handle, centurion_start_time); } auto battery_result = parseCenturionBatteryResponse(*centurion_battery); @@ -142,6 +145,11 @@ class LogitechGProX2Lightspeed : public protocols::LogitechCenturionProtocol { return *battery_result; } + static constexpr bool shouldUseLegacyBatteryFallback(DeviceError::Code code) + { + return code == DeviceError::Code::NotSupported; + } + Result setSidetone(hid_device* device_handle, uint8_t level) override { uint8_t mapped = map(level, 0, 128, 0, SIDETONE_DEVICE_MAX); @@ -342,6 +350,100 @@ class LogitechGProX2Lightspeed : public protocols::LogitechCenturionProtocol { static constexpr std::array PRESET_SHOOTER { -1.0f, -1.0f, 4.0f, 3.0f, 2.0f }; static constexpr std::array PRESET_MOBA { 0.0f, 1.0f, 1.0f, 2.0f, 4.0f }; + Result 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 raw_packets; + raw_packets.reserve(PACKET_SIZE * 4); + + for (int attempt = 0; attempt < 4; ++attempt) { + std::array 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(end_time - start_time); + return *battery_result; + } + + return DeviceError::protocolError("Battery response packet not received"); + } + + static constexpr bool isLegacyAckPacket(std::span packet) + { + return packet.size() >= 2 && packet[0] == REPORT_PREFIX && packet[1] == 0x03; + } + + static constexpr bool isLegacyPowerOffPacket(std::span packet) + { + return packet.size() >= 7 && packet[0] == REPORT_PREFIX && packet[1] == 0x05 && packet[6] == 0x00; + } + + static constexpr bool isLegacyPowerEventPacket(std::span packet) + { + return packet.size() >= 2 && packet[0] == REPORT_PREFIX && packet[1] == 0x05; + } + + static constexpr bool isLegacyBatteryResponsePacket(std::span packet) + { + return packet.size() >= 13 && packet[0] == REPORT_PREFIX && packet[1] == 0x0b && packet[8] == 0x04; + } + + static Result parseLegacyBatteryResponse(std::span packet) + { + if (!isLegacyBatteryResponsePacket(packet)) { + return DeviceError::protocolError("Unexpected battery response packet"); + } + + auto level = static_cast(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 buildLegacyBatteryRequest() + { + std::array 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; diff --git a/tests/test_protocols.cpp b/tests/test_protocols.cpp index 074110a..9cd24e6 100644 --- a/tests/test_protocols.cpp +++ b/tests/test_protocols.cpp @@ -275,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; @@ -630,6 +646,7 @@ void runAllProtocolTests() runTest("HID++ Battery Response", testHIDPPBatteryResponseParsing); runTest("HID++ Offline Detection", testHIDPPOfflineDetection); 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);