Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 45 additions & 0 deletions src/drivers/logitech/hidpp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,29 @@ async function resolveIndex(client: LogitechHidppClient): Promise<number | null>
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<number>,
): Promise<number | null> {
const driver = client as unknown as {
open(): Promise<void>;
resolveDeviceIndex(excluded?: ReadonlySet<number>, priorAnsweredWithoutSensor?: boolean): Promise<void>;
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" });
Expand Down Expand Up @@ -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");
});
42 changes: 35 additions & 7 deletions src/drivers/logitech/hidpp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
private async resolveDeviceIndex(
excluded?: ReadonlySet<number>,
priorAnsweredWithoutSensor = false,
): Promise<void> {
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;
}
Expand Down Expand Up @@ -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);
Expand Down