diff --git a/lib/devices/logitech_gpro_x2_lightspeed.hpp b/lib/devices/logitech_gpro_x2_lightspeed.hpp index 6740ed4..37774e4 100644 --- a/lib/devices/logitech_gpro_x2_lightspeed.hpp +++ b/lib/devices/logitech_gpro_x2_lightspeed.hpp @@ -122,69 +122,32 @@ 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(); + auto centurion_battery = sendCenturionFeatureRequest( + device_handle, + static_cast(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( - 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 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; - } + 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; + } - return DeviceError::protocolError("Battery response packet not received"); + static constexpr bool shouldUseLegacyBatteryFallback(DeviceError::Code code) + { + return code == DeviceError::Code::NotSupported; } Result setSidetone(hid_device* device_handle, uint8_t level) override @@ -350,47 +313,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,7 +350,88 @@ 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() + 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; diff --git a/tests/test_protocols.cpp b/tests/test_protocols.cpp index 7ec6e85..9cd24e6 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; @@ -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; @@ -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);