From 2cf81b8e2bbf0b4e11d1482675bf0e04c8f001cc Mon Sep 17 00:00:00 2001 From: snekxs <26660858+snekxs@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:14:48 -0600 Subject: [PATCH] Retry device-index resolution when a direct-connect index has no sensor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolveDeviceIndex() trusted the first HID++2.0-answering index unconditionally for a non-receiver product id, never checking for a DPI/sensor feature the way the receiver-attached path already does. Confirmed on real hardware (a PRO X Superlight reached through its own product id): DEVICE_INDEX_DIRECT (0xff) can be a genuine admin/pass-through endpoint that answers the root feature query with no sensor behind it, while DEVICE_INDEX_RECEIVER (0x01) — the very next candidate — is the mouse itself. The old code latched onto 0xff and readStatus() threw NotAMouseError without ever trying 0x01. resolveDeviceIndex() now accepts an optional excluded-indices set (and a flag carrying forward that something already answered-without-a-sensor, for an accurate final error message); readStatus() calls it a second time, excluding the sensorless index, when its own DPI check comes back empty. The direct-connect fast path (no probe on the very first attempt) is unchanged and still covered by its existing test. Two new tests cover the fallback (mouse found on the second candidate) and the case where nothing has a sensor (still reports NotAMouseError, not the generic 'did not answer' error, since something did answer). --- src/drivers/logitech/hidpp.test.ts | 45 ++++++++++++++++++++++++++++++ src/drivers/logitech/hidpp.ts | 42 +++++++++++++++++++++++----- 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/src/drivers/logitech/hidpp.test.ts b/src/drivers/logitech/hidpp.test.ts index 4a486c3..8f6d61b 100644 --- a/src/drivers/logitech/hidpp.test.ts +++ b/src/drivers/logitech/hidpp.test.ts @@ -280,6 +280,29 @@ async function resolveIndex(client: LogitechHidppClient): Promise return driver.resolvedDeviceIndex; } +/** + * Simulates `readStatus()`'s own retry: clear the already-resolved (and, by + * the time a real caller does this, already proven sensorless) index and + * resolve again excluding it — same as `readStatus()` does when its + * post-resolution DPI check comes back empty. + */ +async function resolveIndexExcluding( + client: LogitechHidppClient, + excluded: ReadonlySet, +): Promise { + const driver = client as unknown as { + open(): Promise; + resolveDeviceIndex(excluded?: ReadonlySet, priorAnsweredWithoutSensor?: boolean): Promise; + resolvedDeviceIndex: number | null; + }; + await driver.open(); + driver.resolvedDeviceIndex = null; + // Mirrors readStatus()'s own call: excluding an index only ever happens + // because that index already answered-without-a-sensor. + await driver.resolveDeviceIndex(excluded, excluded.size > 0); + return driver.resolvedDeviceIndex; +} + test("a merged receiver's mouse is found past the empty first slot", async () => { // The G502 X PLUS moves off slot 0x01 once G HUB merges the keyboard in. const { client, device } = harness(0xc547, { 0x02: "mouse" }); @@ -320,3 +343,25 @@ test("a direct-connect mouse is latched on its own index without a sensor probe" assert.equal(await resolveIndex(client), 0xff); assert.equal(device.probed.some(({ data }) => data[1] === 0x00 && ((data[3] << 8) | data[4]) === 0x2202), false); }); + +test("a direct-connect product falls back to the receiver index when its own index has no sensor", async () => { + // Confirmed on real hardware (a PRO X Superlight): DEVICE_INDEX_DIRECT can + // answer HID++2.0 as a genuine admin/pass-through endpoint with no sensor + // behind it, while DEVICE_INDEX_RECEIVER — the very next candidate — is + // the mouse. `readStatus()` discovers this after the fact and re-resolves + // excluding the sensorless index; this simulates that second call. + const { client } = harness(G402, { 0xff: "keyboard", 0x01: "mouse" }); + assert.equal(await resolveIndex(client), 0xff, "the fast path still trusts the first answer with no probe"); + assert.equal( + await resolveIndexExcluding(client, new Set([0xff])), + 0x01, + "excluding the sensorless index finds the mouse on the next candidate", + ); +}); + +test("a direct-connect product with no sensor anywhere is reported as not a mouse", async () => { + const { client } = harness(G402, { 0xff: "keyboard" }); + assert.equal(await resolveIndex(client), 0xff, "the fast path still trusts the first answer with no probe"); + const error = await resolveIndexExcluding(client, new Set([0xff])).catch((reason) => reason); + assert.equal((error as Error).name, "NotAMouseError"); +}); diff --git a/src/drivers/logitech/hidpp.ts b/src/drivers/logitech/hidpp.ts index 575d774..e0c7ee7 100644 --- a/src/drivers/logitech/hidpp.ts +++ b/src/drivers/logitech/hidpp.ts @@ -499,21 +499,38 @@ export class LogitechHidppClient { * So ask. The root feature query is the cheapest request there is, and the * wrong index simply times out. */ - private async resolveDeviceIndex(): Promise { + private async resolveDeviceIndex( + excluded?: ReadonlySet, + priorAnsweredWithoutSensor = false, + ): Promise { if (this.resolvedDeviceIndex !== null) return; const receiverAttached = KNOWN_RECEIVER_PRODUCT_IDS.has(this.device.productId); - const candidates = hidppDeviceIndexCandidates(receiverAttached); + const candidates = hidppDeviceIndexCandidates(receiverAttached) + .filter((candidate) => !excluded?.has(candidate)); // A merged receiver can carry a keyboard on a lower slot than the mouse. // Keyboards answer the same HID++ queries, so an answering slot only counts - // once it proves it has a sensor; a direct connection has a single endpoint - // and is latched as before, with readStatus deciding whether it is a mouse. - let answeredWithoutSensor = false; + // once it proves it has a sensor; a direct connection is latched on its + // first HID++2.0 answer without that extra round trip — correct for the + // common case, where a "direct connect" product id really does have one + // dedicated endpoint, but not universally true: confirmed on real + // hardware (a PRO X Superlight), DEVICE_INDEX_DIRECT can be an + // admin/pass-through endpoint that answers the root feature query with + // no sensor behind it, while DEVICE_INDEX_RECEIVER — the very next + // candidate — is the mouse itself. `readStatus()` catches that after the + // fact (its own dpiFeature check) and calls this again with the + // sensorless index in `excluded` (and `priorAnsweredWithoutSensor: true`, + // since that index answering-without-a-sensor is *why* it's excluded — + // this call's own loop never revisits it to rediscover that itself); on + // that retry there is no "trust the first answer" shortcut left to take, + // so every remaining candidate gets the same sensor check a + // receiver-attached probe always got. + let answeredWithoutSensor = priorAnsweredWithoutSensor; for (const candidate of candidates) { this.resolvedDeviceIndex = candidate; const outcome = await this.probeHidpp20Root(); if (outcome !== "hidpp20") continue; - if (!receiverAttached) return; + if (!receiverAttached && !excluded) return; answeredWithoutSensor = true; if (await this.hasDpiFeature()) return; } @@ -631,7 +648,18 @@ export class LogitechHidppClient { // the picker cannot filter them out by descriptor. A sensor feature is what // actually distinguishes a mouse, and it is only knowable once connected. if (!dpiFeature.index) { - throw new NotAMouseError(this.device.productName || "That Logitech device"); + // `resolveDeviceIndex()`'s direct-connect fast path trusts the first + // HID++2.0-answering index without a sensor probe — cheap and correct + // for the common case, but confirmed wrong on real hardware (a PRO X + // Superlight whose DEVICE_INDEX_DIRECT is an admin/pass-through + // endpoint with no sensor while DEVICE_INDEX_RECEIVER, the very next + // candidate, is the mouse itself). Retry properly — this time with a + // sensor check on every remaining candidate — before concluding this + // interface really isn't a mouse. + const sensorless = this.resolvedDeviceIndex; + this.resolvedDeviceIndex = null; + await this.resolveDeviceIndex(sensorless === null ? undefined : new Set([sensorless]), sensorless !== null); + return this.readStatus(); } const reportRateFeature = await this.resolveReportRateFeature(); const profilesFeature = await this.getFeature(FEATURE.onboardProfiles);