diff --git a/Controllers/LogitechController/LogitechControllerDetect.cpp b/Controllers/LogitechController/LogitechControllerDetect.cpp index 522d3079a..e4b1504a6 100644 --- a/Controllers/LogitechController/LogitechControllerDetect.cpp +++ b/Controllers/LogitechController/LogitechControllerDetect.cpp @@ -1153,7 +1153,9 @@ static std::vector HIDPP20Enumerate(hid_device_info* info) } { - LogitechHIDPP20Controller probe(dev, info->path, LOGITECH_DEFAULT_DEVICE_INDEX, false, nullptr, info->usage_page); + LogitechHIDPP20Controller probe(dev, info->path, LOGITECH_DEFAULT_DEVICE_INDEX, false, nullptr, + info->usage_page, nullptr, + info->bus_type == HID_API_BUS_BLUETOOTH); std::string unit_id = probe.ProbeIdentity(); @@ -1303,6 +1305,7 @@ public: uint16_t vendor_id = 0; uint16_t product_id = 0; bool behind_receiver = false; + bool bluetooth = false; std::shared_ptr node_mutex; std::string pairing_name; }; @@ -1432,7 +1435,8 @@ static RGBController_LogitechHIDPP20* HIDPP20BuildController(const HIDPP20BuildT LogitechHIDPP20Controller* controller = new LogitechHIDPP20Controller(dev, target.node_path.c_str(), target.index, target.behind_receiver, target.node_mutex, - target.usage_page, perkey_vl); + target.usage_page, perkey_vl, + target.bluetooth); if(target.behind_receiver) { @@ -1561,6 +1565,7 @@ static DetectedControllers HIDPP20Create(hid_device_info* info, const std::strin target.product_id = (uint16_t)info->product_id; target.node_mutex = slot.node_mutex; target.pairing_name = slot.pairing_name; + target.bluetooth = (info->bus_type == HID_API_BUS_BLUETOOTH); HIDPP20RecordTarget(target); @@ -1687,10 +1692,10 @@ DetectedControllers DetectLogitechHIDPP20(hid_device_info* info, const std::stri | The registrations cover every legacy transport signature: | | any interface, 0xFF00 usage 2 standard HID++ long report (modern keyboards/mice, receivers, G915 family, wired Lightspeed mice). | | Usage 2 is the collection we write to, on Windows, the only one that accepts our writes. | -| interface 1, 0xFF43 any usage keyboards (G213/G512/G610/G810/G813/G815/G910/G Pro); usage varies by model | +| any interface, 0xFF43 any usage keyboards (G213/G512/G610/G810/G813/G815/G910/G Pro), the G560 speaker, the | +| G933 headset, and Bluetooth nodes. Not interface-keyed: a Bluetooth node | +| reports interface -1, which is also HID_INTERFACE_ANY, so it can never match. | | interface 1, 0xFF00 any usage older mice whose HID++ collection is not the usage-2 one | -| interface 2, 0xFF43 usage 514 G560 speaker | -| interface 3, 0xFF43 usage 514 G933 headset | | any interface, 0xFFA0 usage 1 Centurion (G522, PRO X 2) | | | | Not covered, deliberately: the G600 (page 0xFF80) and the X56 (own VID) are not HID++ 2.0. A matching non-HID++ node costs one failed | @@ -1702,10 +1707,8 @@ DetectedControllers DetectLogitechHIDPP20(hid_device_info* info, const std::stri | DUMMY_DEVICE_DETECTOR("Logitech G560 Lightsync Speaker", DetectLogitechHIDPP20, 0x046D, 0x0A78 ) | \*-------------------------------------------------------------------------------------------------------------------------------------*/ REGISTER_HID_DETECTOR_PU_ONLY ("Logitech HID++ 2.0", DetectLogitechHIDPP20, 0xFF00, 2); -REGISTER_HID_DETECTOR_IP_ONLY ("Logitech HID++ 2.0", DetectLogitechHIDPP20, 1, 0xFF43); +REGISTER_HID_DETECTOR_P_ONLY ("Logitech HID++ 2.0", DetectLogitechHIDPP20, 0xFF43); REGISTER_HID_DETECTOR_IP_ONLY ("Logitech HID++ 2.0", DetectLogitechHIDPP20, 1, 0xFF00); -REGISTER_HID_DETECTOR_IPU_ONLY("Logitech HID++ 2.0", DetectLogitechHIDPP20, 2, 0xFF43, 514); -REGISTER_HID_DETECTOR_IPU_ONLY("Logitech HID++ 2.0", DetectLogitechHIDPP20, 3, 0xFF43, 514); REGISTER_HID_DETECTOR_PU_ONLY ("Logitech HID++ 2.0", DetectLogitechHIDPP20, 0xFFA0, 1); /*-------------------------------------------------------------------------------------------------------------------------------------------------*\ diff --git a/Controllers/LogitechController/LogitechHIDPP20Controller/LogitechHIDPP20Controller.cpp b/Controllers/LogitechController/LogitechHIDPP20Controller/LogitechHIDPP20Controller.cpp index 12ac55216..30a74274a 100644 --- a/Controllers/LogitechController/LogitechHIDPP20Controller/LogitechHIDPP20Controller.cpp +++ b/Controllers/LogitechController/LogitechHIDPP20Controller/LogitechHIDPP20Controller.cpp @@ -165,7 +165,8 @@ LogitechHIDPP20Controller::LogitechHIDPP20Controller bool wireless, std::shared_ptr mutex_ptr, uint16_t usage_page, - hid_device* perkey_vl_dev + hid_device* perkey_vl_dev, + bool bluetooth ) { this->dev = dev; @@ -173,6 +174,7 @@ LogitechHIDPP20Controller::LogitechHIDPP20Controller this->location = path; this->device_index = device_index; this->wireless = wireless; + this->transport.bluetooth = bluetooth; this->mutex = mutex_ptr; this->long_only = false; @@ -403,6 +405,58 @@ int LogitechHIDPP20Controller::ReadFromQueue return msg.result; } +/*---------------------------------------------------------*\ +| An answer to a send that had already timed out arrives | +| after the retry has been answered. Nothing in an IRoot | +| reply says which feature it was asked about, so one left | +| in the pipe becomes the next request's answer and the | +| feature map takes a wrong index. Read them off before the | +| next command goes out. | +\*---------------------------------------------------------*/ +void LogitechHIDPP20Controller::DrainLateAnswers(uint8_t feat_idx, uint8_t function, int expected) +{ + std::chrono::steady_clock::time_point deadline = std::chrono::steady_clock::now() + + std::chrono::milliseconds(HIDPP20_LATE_ANSWER_GRACE_MS); + int drained = 0; + + while(drained < expected) + { + std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now(); + + if(now >= deadline) + { + break; + } + + int remaining = (int)std::chrono::duration_cast( + deadline - now).count(); + + uint8_t resp_feat = 0; + uint8_t resp_func = 0; + uint8_t resp_data[60] = {}; + + int rd = ReadMessage(&resp_feat, &resp_func, resp_data, sizeof(resp_data), remaining); + + if(rd <= 0) + { + break; + } + + if(resp_feat == feat_idx + && (resp_func & 0xF0) == (function & 0xF0) + && (resp_func & 0x0F) == HIDPP20_SW_ID) + { + drained++; + } + } + + if(drained > 0) + { + LOG_DEBUG("%s Discarded %d late answer(s) for feat=0x%02X func=0x%02X", + LOG_TAG, drained, feat_idx, function); + } +} + /*---------------------------------------------------------*\ | Sleep delay_ms in slices, waking early when the link is | | about to change or the device went offline. Returns false | @@ -500,7 +554,9 @@ int LogitechHIDPP20Controller::SendAcked int last_result = 0; uint8_t last_error = 0; - for(uint8_t attempt = 0; attempt < policy.attempts; attempt++) + bool long_latch_retry = false; + + for(int attempt = 0; attempt < (int)policy.attempts; attempt++) { /*-------------------------------------------------*\ | Backoff before each attempt (0 on first). | @@ -534,6 +590,9 @@ int LogitechHIDPP20Controller::SendAcked return 0; } + const bool sent_short = (transport.type == HIDPP20_TRANSPORT_STANDARD) + && PrefersShortFrame(send_len); + int send_result = SendMessage(feat_idx, function, send_data, send_len); if(send_result < 0) @@ -693,6 +752,8 @@ int LogitechHIDPP20Controller::SendAcked LOG_DEBUG("%s SendAcked[%s] succeeded on attempt %d " "feat=0x%02X func=0x%02X", LOG_TAG, policy.name, attempt, feat_idx, function); + + DrainLateAnswers(feat_idx, function, attempt); } consecutive_timeouts.store(0); @@ -701,6 +762,32 @@ int LogitechHIDPP20Controller::SendAcked /* Non-matching, non-error: stale unrelated frame, keep reading */ } + + /*-------------------------------------------------*\ + | A collection with no short report answers nothing | + | rather than rejecting the write: Linux hidraw | + | takes the 0x10 frame and drops it. Silence to a | + | short frame is the same evidence as a rejected | + | write, so latch long and let the next attempt | + | resend. | + \*-------------------------------------------------*/ + if(!need_resend && sent_short && !long_only.load()) + { + LOG_DEBUG("%s Short report went unanswered, using long frames", LOG_TAG); + long_only.store(true); + + /*---------------------------------------------*\ + | The frame the device could not receive says | + | nothing about whether it answers, so repeat | + | this attempt as long rather than spend one on | + | the discovery. Once per call. | + \*---------------------------------------------*/ + if(!long_latch_retry) + { + long_latch_retry = true; + attempt--; + } + } } LOG_DEBUG("%s SendAcked[%s] exhausted %d attempts feat=0x%02X func=0x%02X " @@ -802,6 +889,39 @@ int LogitechHIDPP20Controller::SendAckedIntoFAP | Report IDs 0x10 (7 bytes) / 0x11 (20 bytes) | \*---------------------------------------------------------*/ +/*---------------------------------------------------------*\ +| Budget for the first exchange with a node: wider for a | +| device a receiver already named and for a Bluetooth link, | +| whose connection interval puts the first answer hundreds | +| of ms out. Everything else fails fast. | +\*---------------------------------------------------------*/ +const HIDPP20RetryPolicy& LogitechHIDPP20Controller::FirstContactPolicy() const +{ + if(transport.bluetooth) + { + return HIDPP20_POLICY_BLUETOOTH; + } + + return wireless ? HIDPP20_POLICY_FIRST_CONTACT + : HIDPP20_POLICY_PROBE; +} + +/*---------------------------------------------------------*\ +| Frame choice for standard HID++: short (0x10) carries 3 | +| payload bytes, long (0x11) carries 16. Windows opens the | +| long-message collection only, and long_only latches a | +| collection that has no short report at all. | +\*---------------------------------------------------------*/ +bool LogitechHIDPP20Controller::PrefersShortFrame(size_t len) const +{ +#if defined(_WIN32) + (void)len; + return false; +#else + return (len <= 3) && !long_only.load(); +#endif +} + /*---------------------------------------------------------*\ | Outgoing frame as hex, for trace-level wire comparison. | \*---------------------------------------------------------*/ @@ -845,11 +965,7 @@ int LogitechHIDPP20Controller::SendStandard uint8_t buf[LOGITECH_LONG_MESSAGE_LEN]; size_t msg_len; -#if defined(_WIN32) - const bool prefer_short = false; -#else - const bool prefer_short = (len <= 3) && !long_only.load(); -#endif + const bool prefer_short = PrefersShortFrame(len); if(prefer_short) { @@ -3432,14 +3548,7 @@ bool LogitechHIDPP20Controller::Probe() \*-----------------------------------------------------*/ uint8_t test_idx = 0; - /*-----------------------------------------------------*\ - | A receiver-paired device is known to speak HID++, so | - | it gets the wider first-contact budget; an unknown | - | node keeps the tight probe and fails fast. | - \*-----------------------------------------------------*/ - const HIDPP20RetryPolicy& first_contact = wireless - ? HIDPP20_POLICY_FIRST_CONTACT - : HIDPP20_POLICY_PROBE; + const HIDPP20RetryPolicy& first_contact = FirstContactPolicy(); if(transport.type == HIDPP20_TRANSPORT_CENTURION) { @@ -3691,7 +3800,7 @@ std::string LogitechHIDPP20Controller::ProbeIdentity() /*-----------------------------------------------------*\ | Nothing else is worth asking until IRoot answers. | \*-----------------------------------------------------*/ - if(GetFeatureIndex(HIDPP20_FEAT_FEATURE_SET, HIDPP20_POLICY_PROBE) == 0) + if(GetFeatureIndex(HIDPP20_FEAT_FEATURE_SET, FirstContactPolicy()) == 0) { return ""; } @@ -6276,12 +6385,24 @@ void LogitechHIDPP20Controller::RediscoverFeatures() std::string LogitechHIDPP20Controller::CurrentLinkKey() const { /*-----------------------------------------------------*\ - | Key the link: rx#slot over the dongle, usb#idx | - | direct. hidraw paths are reused by the kernel so | - | aren't used. A slot collision across dongles is | - | caught by the reclaim self-heal. | + | Key the link: rx#page#slot over the dongle, | + | bt#page#idx over a Bluetooth radio, usb#page#idx on a | + | cable or a dongle of the device's own. hidraw paths | + | are reused by the kernel so aren't used. Every direct | + | link is device index 0xFF, so the page is what | + | separates two of them, a cable at 0xFF00 from a | + | Centurion dongle at 0xFFA0. A slot collision across | + | dongles is caught by the reclaim self-heal. | \*-----------------------------------------------------*/ - return std::string(wireless ? "rx#" : "usb#") + std::to_string((int)device_index); + const char* link = wireless ? "rx#" + : transport.bluetooth ? "bt#" + : "usb#"; + + char key[32]; + + snprintf(key, sizeof(key), "%s%04X#%d", link, transport.usage_page, (int)device_index); + + return std::string(key); } HIDPP20LinkIndexMap LogitechHIDPP20Controller::SnapshotLinkIndexMap() const diff --git a/Controllers/LogitechController/LogitechHIDPP20Controller/LogitechHIDPP20Controller.h b/Controllers/LogitechController/LogitechHIDPP20Controller/LogitechHIDPP20Controller.h index 416f018f7..59f0e6e1f 100644 --- a/Controllers/LogitechController/LogitechHIDPP20Controller/LogitechHIDPP20Controller.h +++ b/Controllers/LogitechController/LogitechHIDPP20Controller/LogitechHIDPP20Controller.h @@ -372,6 +372,7 @@ struct HIDPP20Transport { HIDPP20TransportType type; uint16_t usage_page; // 0xFF00, 0xFF43, or 0xFFA0 + bool bluetooth; // link is a Bluetooth radio, not USB uint8_t report_id; // 0x10/0x11 for standard, 0x51/0x50 for Centurion bool addressed; // Centurion 0x50 has device address byte uint8_t device_address; // Centurion 0x50: device address (e.g., 0x23) @@ -462,6 +463,8 @@ static constexpr uint16_t HIDPP20_BACKOFF_PROBE[] = { 0, 100 }; static constexpr uint16_t HIDPP20_BACKOFF_FIRST_CONTACT[] = { 0, 100, 250, 500 }; +static constexpr uint16_t HIDPP20_BACKOFF_BLUETOOTH[] = + { 0, 200, 500 }; /*---------------------------------------------------------*\ | SW-control reclaim backoff. Used by ReconnectDevice | @@ -579,6 +582,30 @@ static constexpr HIDPP20RetryPolicy HIDPP20_POLICY_FIRST_CONTACT = { "first-contact" }; +/*---------------------------------------------------------*\ +| Bluetooth policy: a BLE link answers its first exchange | +| in ~300ms cold and ~40ms once awake, so the first-contact | +| window is sized for the cold case rather than retried | +| into it. Worst case ~2.1s for a node that never answers. | +\*---------------------------------------------------------*/ +static constexpr HIDPP20RetryPolicy HIDPP20_POLICY_BLUETOOTH = { + HIDPP20_BACKOFF_BLUETOOTH, + sizeof(HIDPP20_BACKOFF_BLUETOOTH) / sizeof(uint16_t), + 700, // read window + true, // flush_before + true, // retry_on_busy + "bluetooth" +}; + +/*---------------------------------------------------------*\ +| Grace for an answer to a send that already timed out. It | +| arrives after the retry has been answered, and IRoot | +| replies carry nothing that ties them to the feature they | +| were asked about, so one left in the pipe is read as the | +| next request's answer. | +\*---------------------------------------------------------*/ +#define HIDPP20_LATE_ANSWER_GRACE_MS 150 + /*---------------------------------------------------------*\ | Rolling resync. The per-key stream is a delta and takes | | an ACK as proof of paint, so a write the device answers | @@ -602,7 +629,8 @@ public: uint8_t device_index, bool wireless, std::shared_ptr mutex_ptr, uint16_t usage_page = 0xFF00, - hid_device* perkey_vl_dev = nullptr); + hid_device* perkey_vl_dev = nullptr, + bool bluetooth = false); ~LogitechHIDPP20Controller(); /*-----------------------------------------------------*\ @@ -948,6 +976,29 @@ private: const uint8_t* send_data, size_t send_len, uint8_t* recv_data, size_t recv_max); + /*-----------------------------------------------------*\ + | True when a payload of len fits the short (0x10) | + | frame and this collection still accepts one. | + \*-----------------------------------------------------*/ + bool PrefersShortFrame(size_t len) const; + + /*-----------------------------------------------------*\ + | Discard up to `expected` answers to sends that had | + | already timed out when the reply to the retry came | + | in. | + \*-----------------------------------------------------*/ + void DrainLateAnswers(uint8_t feat_idx, uint8_t function, int expected); + + /*-----------------------------------------------------*\ + | Budget for the first exchange with a node. A device | + | named by a receiver's pairing table is known to speak | + | HID++, and a Bluetooth link can take several | + | connection intervals to answer; both get the wider | + | window. An unknown wired node keeps the tight probe | + | and fails fast. | + \*-----------------------------------------------------*/ + const HIDPP20RetryPolicy& FirstContactPolicy() const; + /*-----------------------------------------------------*\ | Unified send-and-ack primitive with retry policy. | | All command paths converge here. Returns: |