Просмотр исходного кода

say which HID devices were passed over, and why

`keyboard info` reported "No QMK keyboard found" and stopped, which is
the one case a user cannot read on their own: it looks the same whether
the keyboard is unplugged or its lighting sits on a HID collection this
tool does not address. A board on a GMMK Pro's stock firmware is exactly
that, and nothing in the output said so.

`DiscoverEvery` reports every HID collection the system lists, so a
device that was not taken can be named. `keyboard info` prints them
grouped per device, one line with the usage pages it exposes, and only
when nothing matched: a Mac has 43 collections across 10 devices, none
of them ever a keyboard, and beside a successful listing they bury the
line the user came for. The JSON shape carries them in `otherHidDevices`
on every run, so a consumer asking "which keyboards" and "why not this
one" reads one shape.

Per device rather than per collection because a keyboard over USB has
five — measured on the Impact 80 — and the one that says something is a
vendor page at 0xFF80 where it would have to be 0xFF60. The preamble
names the collection that is looked for and says why a QMK firmware has
it, and says nothing about VIA: nothing is opened here and the filter
asks about a usage page. "No QMK keyboard found" was overstated as
well, since most QMK builds do not enable Raw HID at all.
Paul-Dieter Klumpp 1 неделя назад
Родитель
Сommit
3bc814a030

+ 36 - 0
AGENTS.md

@@ -66,6 +66,42 @@ with neither is still driven, because the channel numbers follow from the QMK
 subsystem they belong to. `keyboards.json` used to supply both and is gone: two
 places to update is how the channel names and the catalog drifted apart.
 
+**An empty discovery has to explain itself, and that is the whole of what
+`DiscoverEvery` is for.** "No keyboard" is what a board that is not connected
+looks like, and it is also what a board that is connected and not reachable looks
+like — a firmware that puts Raw HID on another usage page, a wireless receiver, a
+firmware without Raw HID at all. Nothing in the tool can tell those apart from
+outside, so `keyboard info` prints the HID devices it passed over, one line per
+device with the usage pages it exposes, and the JSON shape carries them in
+`otherHidDevices` on every run.
+
+The two lines above that list say what was looked for and why a board has it, and
+what they may **not** say is that VIA was checked. They did not: this command
+never opens a device, and the filter asks about a usage page, not about VIA. A
+QMK firmware has the collection when Raw HID is enabled and VIA's build cannot be
+compiled without it (`quantum/via.c` errors out), which is a property of the
+firmware and belongs in the sentence as the reason — not as a claim about what
+this command observed. The same goes for "No QMK keyboard found", which is
+overstated anyway: most QMK builds do not enable Raw HID at all, so a plain QMK
+keyboard is a QMK keyboard this tool does not see.
+
+Two decisions there are load-bearing and were measured, not chosen. It is one line
+per **device**, not per collection: a keyboard over USB reports five collections
+(measured on the Impact 80: `0x0001/0x02`, `0x0001/0x01`, `0x0001/0x80`,
+`0x000C/0x01`, `0x0001/0x06`), so per collection buries the one line that says
+something. And in text the list appears **only** when nothing was found: a Mac has
+43 HID collections across 10 devices, none of which is ever a keyboard, and
+printing them beside a successful listing buries the line the user came for. The
+JSON always has them, because a consumer asking "why not this one" needs them
+whether or not the tool found something.
+
+The signature is an exact pair, and it is checked per collection rather than per
+device because macOS reports one HID device per usage pair and Linux one interface
+at a time; the `seen` map is keyed on the path *after* the filter, so a keyboard's
+own keyboard collection cannot consume the raw HID entry. A vendor page near
+`0xFF60` is not a raw HID interface and no guessing is done about intent — the
+firmware picked the page, and saying which page it picked is the answer.
+
 Where a board's own files live is `definitions/` and `profiles/`, both under the
 platform's per-user configuration directory, which `os.UserConfigDir` answers — a
 hardcoded `~/.config` would be wrong on macOS and Windows. `os.UserConfigDir` is

+ 37 - 0
README.md

@@ -296,6 +296,43 @@ HID path, which the operating system reassigns on reboot, and most keyboards
 report no serial number. Re-run `keyboard info` after reconnecting a keyboard
 rather than storing the number.
 
+A keyboard is one HID collection with usage page `0xFF60` and usage `0x61`, the
+QMK Raw HID signature `qmk/qmk_udev` matches on. A QMK firmware has one when Raw
+HID is enabled, and VIA's build cannot be compiled without it — but the two are
+not the same thing, and a board with Raw HID but no VIA is found here and then
+fails the channel probe with a read timeout, because nothing answers a VIA
+command. Nothing else is addressed, and when nothing matched, `keyboard info`
+lists the HID devices that *are* connected with the usage pages they expose —
+because a keyboard that is plugged in and not reachable looks exactly like one
+that is not plugged in:
+
+```console
+$ qmk-rgb-tool keyboard info
+No keyboard with the QMK Raw HID interface found: usage page 0xFF60, usage 0x61.
+A QMK firmware has one when Raw HID is enabled; VIA's build cannot be built without it.
+
+These HID devices are connected, and none of them has that collection:
+  0x320F/0x5044  GMMK Pro              0x0001/0x06 0x000C/0x01 0xFF80/0x61
+  0x046D/0xC041  USB Gaming Mouse      0xFF00/0x01 0x0001/0x02
+  0x0000/0x0000  no product string     0xFF00/0xFF
+```
+
+The first line says what was looked for, the second why a board has it, and
+neither says VIA was checked: nothing is opened here, and the filter asks about
+the collection, not about VIA.
+
+One line per device, not per collection: a keyboard over USB typically has five
+collections (keyboard, consumer control, mouse, vendor), and one line each would
+bury the one that says something — a vendor page that is `0xFF80` where it would
+have to be `0xFF60` is a board this tool cannot address, which is a different
+answer than a board in wireless mode or on a firmware without Raw HID. The
+firmware decides that page, so nothing here is guessed.
+
+The same list is in `keyboard info --json` as `otherHidDevices`, in every run and
+not only an empty one, so a consumer can ask "which keyboards" and "why not this
+one" from one shape. It is never printed beside a successful listing: a Mac has
+around forty HID devices that are never a keyboard.
+
 ## Color Notations
 
 `color` accepts three notations for one operation:

+ 180 - 4
cmd/qmk-rgb-tool/keyboard_info_test.go

@@ -11,15 +11,21 @@ import (
 )
 
 type deviceInfoOutput struct {
-	Devices []keyboardLine `json:"devices"`
-	Total   int            `json:"total"`
+	Devices []keyboardLine        `json:"devices"`
+	Total   int                   `json:"total"`
+	Others  []intdevice.HIDDevice `json:"otherHidDevices"`
 }
 
+// stubDiscovery replaces both enumerations the command makes. Only the
+// keyboards are of interest to most tests, so the devices that were passed over
+// are stubbed empty as well: leaving that one real would make the output depend
+// on what is plugged into the machine running the tests.
 func stubDiscovery(t *testing.T, devices []intdevice.Device) {
 	t.Helper()
-	orig := discoverAll
+	orig, origOther := discoverAll, discoverOther
 	discoverAll = func() ([]intdevice.Device, error) { return devices, nil }
-	t.Cleanup(func() { discoverAll = orig })
+	discoverOther = func() ([]intdevice.HIDDevice, error) { return nil, nil }
+	t.Cleanup(func() { discoverAll, discoverOther = orig, origOther })
 }
 
 // runRealKeyboardInfo executes the shipped command so a re-added
@@ -107,6 +113,176 @@ func TestKeyboardInfoEmitsEmptyArrayNotNull(t *testing.T) {
 	if !strings.Contains(stdout, `"devices": []`) {
 		t.Errorf("stdout = %q, want an empty array so consumers can iterate unconditionally", stdout)
 	}
+	if !strings.Contains(stdout, `"otherHidDevices": []`) {
+		t.Errorf("stdout = %q, want the passed-over devices as an empty array, not null", stdout)
+	}
+}
+
+// An empty result is the one case a user cannot read on their own: it looks the
+// same whether the keyboard is unplugged or its lighting is on a HID collection
+// this tool does not address. So the listing names the devices that are there,
+// with the usage pages they do expose — the missing 0xFF60/0x61 is the finding.
+func TestKeyboardInfoEmptyResultNamesTheDevicesPassedOver(t *testing.T) {
+	stubDiscovery(t, nil)
+	stubOtherDiscovery(t, []intdevice.HIDDevice{
+		{
+			VendorID:  0x320f,
+			ProductID: 0x5044,
+			Name:      "GMMK Pro",
+			UsagePairs: []intdevice.UsagePair{
+				{UsagePage: 0x0001, Usage: 0x06},
+				{UsagePage: 0xff80, Usage: 0x61},
+			},
+		},
+	})
+
+	stdout, _, err := runRealKeyboardInfo(t)
+	if err != nil {
+		t.Fatalf("keyboard info returned error: %v", err)
+	}
+
+	for _, want := range []string{
+		"No keyboard with the QMK Raw HID interface found",
+		"usage page 0xFF60, usage 0x61",
+		"0x320F/0x5044",
+		"GMMK Pro",
+		"0xFF80/0x61",
+	} {
+		if !strings.Contains(stdout, want) {
+			t.Errorf("stdout = %q, want it to contain %q", stdout, want)
+		}
+	}
+}
+
+// The preamble says what was looked for and why a board has it, and it must not
+// claim VIA was checked: nothing is opened here, and VIA is never what the
+// collection filter asks about. A sentence that says it did would send a user
+// looking for a probe that does not exist.
+func TestKeyboardInfoEmptyResultDoesNotClaimToHaveCheckedVIA(t *testing.T) {
+	stubDiscovery(t, nil)
+	stubOtherDiscovery(t, nil)
+
+	stdout, _, err := runRealKeyboardInfo(t)
+	if err != nil {
+		t.Fatalf("keyboard info returned error: %v", err)
+	}
+
+	if !strings.Contains(stdout, "VIA's build cannot be built without it") {
+		t.Errorf("stdout = %q, want it to say why a QMK firmware has the collection", stdout)
+	}
+	if strings.Contains(stdout, "no VIA") || strings.Contains(stdout, "does not support VIA") {
+		t.Errorf("stdout = %q, want no claim about VIA: this command never opened the board", stdout)
+	}
+}
+
+// And the opposite: a keyboard was found, so the devices that were passed over
+// are only ever a dock or a mouse, and a listing beside the line the user came
+// for buries it.
+func TestKeyboardInfoListsOtherDevicesOnlyWhenNoneWasFound(t *testing.T) {
+	stubDiscovery(t, []intdevice.Device{
+		{Index: 1, Path: "/dev/hidraw7", VendorID: 0x36b0, ProductID: 0x309f, Name: "Impact 80"},
+	})
+	stubOtherDiscovery(t, []intdevice.HIDDevice{
+		{VendorID: 0x046d, ProductID: 0xc041, Name: "USB Gaming Mouse",
+			UsagePairs: []intdevice.UsagePair{{UsagePage: 0xff00, Usage: 0x01}}},
+	})
+
+	stdout, _, err := runRealKeyboardInfo(t)
+	if err != nil {
+		t.Fatalf("keyboard info returned error: %v", err)
+	}
+
+	if !strings.Contains(stdout, "Impact 80") {
+		t.Errorf("stdout = %q, want the keyboard that was found", stdout)
+	}
+	if strings.Contains(stdout, "USB Gaming Mouse") {
+		t.Errorf("stdout = %q, want no listing of passed-over devices when a keyboard was found", stdout)
+	}
+}
+
+// With nothing found and nothing else connected, the sentence has to say that
+// rather than print a heading with nothing under it.
+func TestKeyboardInfoEmptyResultWithNoHIDDevicesAtAll(t *testing.T) {
+	stubDiscovery(t, nil)
+	stubOtherDiscovery(t, nil)
+
+	stdout, _, err := runRealKeyboardInfo(t)
+	if err != nil {
+		t.Fatalf("keyboard info returned error: %v", err)
+	}
+
+	if !strings.Contains(stdout, "No other HID device is connected") {
+		t.Errorf("stdout = %q, want it to say that no HID device is connected", stdout)
+	}
+	if strings.Contains(stdout, "These HID devices are connected") {
+		t.Errorf("stdout = %q, want no heading for a list that is not there", stdout)
+	}
+}
+
+// A consumer asking "why not this one" reads the JSON, so the devices that were
+// passed over are in it whether or not a keyboard was found.
+func TestKeyboardInfoJSONCarriesTheOtherHIDDevicesWhateverItFinds(t *testing.T) {
+	withJSON(t)
+	stubDiscovery(t, []intdevice.Device{
+		{Index: 1, Path: "/dev/hidraw7", VendorID: 0x36b0, ProductID: 0x309f, Name: "Impact 80"},
+	})
+	stubOtherDiscovery(t, []intdevice.HIDDevice{
+		{VendorID: 0x320f, ProductID: 0x5044, Name: "GMMK Pro",
+			UsagePairs: []intdevice.UsagePair{{UsagePage: 0xff80, Usage: 0x61}}},
+	})
+
+	stdout, _, err := runRealKeyboardInfo(t)
+	if err != nil {
+		t.Fatalf("keyboard info returned error: %v", err)
+	}
+
+	var got deviceInfoOutput
+	if err := json.Unmarshal([]byte(stdout), &got); err != nil {
+		t.Fatalf("stdout is not valid JSON: %v (output %q)", err, stdout)
+	}
+	if len(got.Devices) != 1 {
+		t.Fatalf("devices = %d entries, want 1", len(got.Devices))
+	}
+	if len(got.Others) != 1 {
+		t.Fatalf("otherHidDevices = %d entries, want 1", len(got.Others))
+	}
+	if got.Others[0].VendorID != 0x320f {
+		t.Errorf("otherHidDevices[0].vendorId = %04X, want 320F", got.Others[0].VendorID)
+	}
+	if len(got.Others[0].UsagePairs) != 1 || got.Others[0].UsagePairs[0].UsagePage != 0xff80 {
+		t.Errorf("otherHidDevices[0].usagePages = %+v, want the page it was passed over for", got.Others[0].UsagePairs)
+	}
+}
+
+// The passed-over devices are never selectable, so they carry no index and none
+// is printed: a number there would be one --device does not take.
+func TestKeyboardInfoGivesNoNumberToADeviceThatCannotBeSelected(t *testing.T) {
+	stubDiscovery(t, nil)
+	stubOtherDiscovery(t, []intdevice.HIDDevice{
+		{VendorID: 0x320f, ProductID: 0x5044, Name: "GMMK Pro",
+			UsagePairs: []intdevice.UsagePair{{UsagePage: 0xff80, Usage: 0x61}}},
+	})
+
+	stdout, _, err := runRealKeyboardInfo(t)
+	if err != nil {
+		t.Fatalf("keyboard info returned error: %v", err)
+	}
+
+	for _, line := range strings.Split(stdout, "\n") {
+		if !strings.Contains(line, "0x320F/0x5044") {
+			continue
+		}
+		if strings.HasPrefix(strings.TrimSpace(line), "1 ") {
+			t.Errorf("stdout = %q, want no number on a device --device cannot select", stdout)
+		}
+	}
+}
+
+func stubOtherDiscovery(t *testing.T, others []intdevice.HIDDevice) {
+	t.Helper()
+	orig := discoverOther
+	discoverOther = func() ([]intdevice.HIDDevice, error) { return others, nil }
+	t.Cleanup(func() { discoverOther = orig })
 }
 
 func TestKeyboardInfoPropagatesDiscoveryError(t *testing.T) {

+ 34 - 6
cmd/qmk-rgb-tool/main.go

@@ -95,11 +95,24 @@ func inGroup(cmd *cobra.Command, group string) *cobra.Command {
 // discoverAll is a seam for tests.
 var discoverAll = device.DiscoverAll
 
+// discoverOther is a seam for tests, and the reason an empty result explains
+// itself: a keyboard that is connected but not reachable is only distinguishable
+// from a keyboard that is not there by the HID devices that were seen and passed
+// over.
+var discoverOther = device.DiscoverOther
+
 // runKeyboardInfo writes the connected QMK keyboards, as text unless --json
 // asks otherwise. The 1-based index that --device accepts is in the "index"
 // field, so the machine shape emits no banner alongside it. Whether a keyboard
 // has effect names is reported per keyboard, because it differs: one board may
 // have a definition and the next may not.
+//
+// The HID devices that were passed over travel along in the JSON shape whatever
+// it finds, because a consumer asking "which keyboards" and a consumer asking
+// "why not this one" want the same answer. The text form only prints them when
+// there is no keyboard, which is when they explain something; a Mac has dozens of
+// HID devices that are never a keyboard, and a successful listing next to thirty
+// of them buries the line the user came for.
 func runKeyboardInfo(out io.Writer) error {
 	devices, err := discoverAll()
 	if err != nil {
@@ -110,23 +123,38 @@ func runKeyboardInfo(out io.Writer) error {
 		devices = []device.Device{}
 	}
 
+	others, err := discoverOther()
+	if err != nil {
+		return err
+	}
+	if others == nil {
+		others = []device.HIDDevice{}
+	}
+
 	lines := describeDevices(devices)
 	if !jsonOutput {
-		return printKeyboardInfoText(out, lines)
+		return printKeyboardInfoText(out, lines, others)
 	}
 
 	type info struct {
-		Devices []keyboardLine `json:"devices"`
-		Total   int            `json:"total"`
+		Devices []keyboardLine     `json:"devices"`
+		Total   int                `json:"total"`
+		Others  []device.HIDDevice `json:"otherHidDevices"`
 	}
-	return encodeJSON(out, info{Devices: lines, Total: len(lines)})
+	return encodeJSON(out, info{Devices: lines, Total: len(lines), Others: others})
 }
 
 var keyboardInfoCmd = &cobra.Command{
 	Use:   "info",
 	Short: "Discover connected QMK keyboards",
-	Long: "Scan for connected QMK keyboards and print device info as JSON.\n" +
-		"Each device carries the 1-based index that --device accepts.",
+	Long: "Scan for connected QMK keyboards and print device info as text, or as JSON\n" +
+		"with --json.\n" +
+		"Each device carries the 1-based index that --device accepts.\n" +
+		"\n" +
+		"A keyboard is one whose HID collection is the QMK Raw HID signature,\n" +
+		"0xFF60/0x61. Nothing else is addressed, and the HID devices that were\n" +
+		"passed over are reported, so a keyboard that is connected but unreachable\n" +
+		"does not look like a keyboard that is not connected.",
 	Args: cobra.NoArgs,
 	RunE: func(cmd *cobra.Command, args []string) error {
 		return runKeyboardInfo(cmd.OutOrStdout())

+ 42 - 2
cmd/qmk-rgb-tool/output.go

@@ -3,6 +3,7 @@ package main
 import (
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/spf13/cobra"
 	"netdome.biz/paul/qmk-rgb/internal/device"
@@ -67,11 +68,33 @@ func formatInfoColor(c infoColor) string {
 
 // printKeyboardInfoText writes one line per keyboard, with the index --device
 // takes, because the index is the only part a user has to act on.
-func printKeyboardInfoText(out io.Writer, devices []keyboardLine) error {
+//
+// An empty result explains itself, because it is the one case a user cannot read
+// on their own: "no keyboard" is what a board that is not connected looks like,
+// and it is also what a board that is connected but not reachable looks like.
+// The HID devices that were passed over are what tells the two apart — a keyboard
+// over USB has a keyboard collection, a consumer control and usually a vendor
+// page, and it is the vendor page at 0xFF60/0x61 that is missing.
+//
+// Those devices are listed only when there is no keyboard. On a Mac there are
+// around forty HID devices that are never a keyboard, and putting them beside a
+// successful listing buries the line the user came for; a consumer that wants
+// them regardless reads them from the JSON shape, which always carries them.
+func printKeyboardInfoText(out io.Writer, devices []keyboardLine, others []device.HIDDevice) error {
 	if len(devices) == 0 {
-		fmt.Fprintln(out, "No QMK keyboard found.")
+		fmt.Fprintln(out, "No keyboard with the QMK Raw HID interface found: usage page 0xFF60, usage 0x61.")
+		fmt.Fprintln(out, "A QMK firmware has one when Raw HID is enabled; VIA's build cannot be built without it.")
+		if len(others) == 0 {
+			fmt.Fprintln(out, "No other HID device is connected, so no keyboard is plugged in at all.")
+			return nil
+		}
+		fmt.Fprintln(out, "\nThese HID devices are connected, and none of them has that collection:")
+		for _, o := range others {
+			fmt.Fprintf(out, "  0x%04X/0x%04X  %s\n", o.VendorID, o.ProductID, hidDeviceName(o))
+		}
 		return nil
 	}
+
 	for _, d := range devices {
 		line := fmt.Sprintf("%d  %s  0x%04X/0x%04X", d.Index, d.Name, d.VendorID, d.ProductID)
 		if d.Effects {
@@ -82,6 +105,23 @@ func printKeyboardInfoText(out io.Writer, devices []keyboardLine) error {
 	return nil
 }
 
+// hidDeviceName is what a passed-over HID device is called in the listing, with
+// the usage pages it does expose. The pages are the finding: one of them being
+// 0xFF80/0x61 rather than 0xFF60/0x61 is a board that put its lighting on a
+// vendor page this tool does not address, and a reader should not have to run a
+// program to learn that.
+func hidDeviceName(o device.HIDDevice) string {
+	name := o.Name
+	if name == "" {
+		name = "no product string"
+	}
+	pages := make([]string, 0, len(o.UsagePairs))
+	for _, p := range o.UsagePairs {
+		pages = append(pages, fmt.Sprintf("0x%04X/0x%02X", p.UsagePage, p.Usage))
+	}
+	return fmt.Sprintf("%-28s %s", name, strings.Join(pages, " "))
+}
+
 // keyboardLine is one keyboard as the info command reports it. Whether it has
 // effect names is a fact about the tool, not about a data file: it is true when
 // a definition covers the board or a catalog is compiled in for it. Its channels

+ 87 - 0
internal/device/device.go

@@ -39,6 +39,93 @@ func DiscoverAll() ([]Device, error) {
 	return indexDevices(devices), nil
 }
 
+// UsagePair is one HID collection of a device: its usage page and its usage.
+// Both are needed to describe a collection, because a keyboard reports several
+// on one device and the page alone does not tell them apart.
+type UsagePair struct {
+	UsagePage uint16 `json:"usagePage"`
+	Usage     uint16 `json:"usage"`
+}
+
+// HIDDevice is a connected device whose HID collections are not the QMK Raw HID
+// one, which is what a discovery failure has to name: a board that is connected
+// but not reachable is a different fact from a board that is not connected, and
+// the usage pages are the only thing that tells the two apart.
+//
+// It is per device rather than per collection, because a device that is not
+// reachable usually has several collections and one line per collection buries
+// the one line that says something. A keyboard over USB typically has five: a
+// keyboard page, two consumer-control pages, mouse and a vendor page. What is
+// missing is the vendor page at 0xFF60.
+//
+// There is deliberately no Index and no Path. A number would look like something
+// --device accepts, and nothing accepts these.
+type HIDDevice struct {
+	VendorID   uint16      `json:"vendorId"`
+	ProductID  uint16      `json:"productId"`
+	Name       string      `json:"name,omitempty"`
+	UsagePairs []UsagePair `json:"usagePages"`
+}
+
+// DiscoverOther returns every connected HID device that is not a QMK Raw HID
+// keyboard, each with the usage pages it does expose. Devices this tool drives
+// are left out: they are what DiscoverAll numbers, and a list repeating them
+// would suggest a second way to select a keyboard.
+//
+// Order is the order the HID layer reported the devices in, and the pairs within
+// a device are in the order they were reported. Neither is sorted, because
+// neither carries a number a user could act on and a keyboard's collections mean
+// nothing in an order.
+func DiscoverOther() ([]HIDDevice, error) {
+	infos, err := hid.DiscoverEvery()
+	if err != nil {
+		return nil, fmt.Errorf("discover hid collections: %w", err)
+	}
+	return groupOther(infos), nil
+}
+
+// groupOther turns HID collections into the per-device list the commands report.
+// It is separate from DiscoverOther because the grouping is the whole of the
+// decision and nothing about it needs a keyboard plugged in to be checked.
+func groupOther(infos []hid.DeviceInfo) []HIDDevice {
+	var devices []HIDDevice
+	index := make(map[string]int)
+	for _, info := range infos {
+		if info.RawHID {
+			continue
+		}
+		key := fmt.Sprintf("%04X/%04X/%s", info.VendorID, info.ProductID, info.ProductString)
+		at, seen := index[key]
+		if !seen {
+			devices = append(devices, HIDDevice{
+				VendorID:  info.VendorID,
+				ProductID: info.ProductID,
+				Name:      info.ProductString,
+			})
+			at = len(devices) - 1
+			index[key] = at
+		}
+		pair := UsagePair{UsagePage: info.UsagePage, Usage: info.Usage}
+		if !containsUsagePair(devices[at].UsagePairs, pair) {
+			devices[at].UsagePairs = append(devices[at].UsagePairs, pair)
+		}
+	}
+
+	return devices
+}
+
+// containsUsagePair reports whether a device already lists a collection. The HID
+// layer reports some devices once per interface, so the same pair arrives twice
+// for one device and a line that says it twice is a line about nothing.
+func containsUsagePair(pairs []UsagePair, want UsagePair) bool {
+	for _, p := range pairs {
+		if p == want {
+			return true
+		}
+	}
+	return false
+}
+
 // indexDevices sorts devices deterministically and assigns 1-based indexes.
 // The input slice is left untouched.
 func indexDevices(devices []Device) []Device {

+ 61 - 0
internal/device/device_test.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"strings"
 	"testing"
+
+	"netdome.biz/paul/qmk-rgb/internal/hid"
 )
 
 func TestIndexDevices(t *testing.T) {
@@ -134,3 +136,62 @@ func TestDeviceTakesTheNameFromTheProductString(t *testing.T) {
 		t.Errorf("Name = %q, want the product string the keyboard reports", d.Name)
 	}
 }
+
+// The passed-over devices are what an empty discovery result has to explain, so
+// the grouping is the whole of the decision: one line per device rather than one
+// per collection, the raw HID collections left out, and a device that reports the
+// same collection twice listed once.
+func TestGroupOtherPerDevice(t *testing.T) {
+	infos := []hid.DeviceInfo{
+		{Path: "a", VendorID: 0x320f, ProductID: 0x5044, ProductString: "GMMK Pro",
+			UsagePage: 0x0001, Usage: 0x06},
+		{Path: "a", VendorID: 0x320f, ProductID: 0x5044, ProductString: "GMMK Pro",
+			UsagePage: 0x000c, Usage: 0x01},
+		// The same pair a second time, as the HID layer reports when a device
+		// has several interfaces: one line about it says nothing.
+		{Path: "a2", VendorID: 0x320f, ProductID: 0x5044, ProductString: "GMMK Pro",
+			UsagePage: 0x0001, Usage: 0x06},
+		// A board that put its lighting on a vendor page this tool does not
+		// address. The page is the finding, so it has to survive the grouping.
+		{Path: "b", VendorID: 0x320f, ProductID: 0x5044, ProductString: "GMMK Pro",
+			UsagePage: 0xff80, Usage: 0x61},
+		// The collection this tool drives, which belongs to DiscoverAll.
+		{Path: "c", VendorID: 0x36b0, ProductID: 0x309f, ProductString: "Impact 80",
+			UsagePage: 0xff60, Usage: 0x61, RawHID: true},
+	}
+
+	got := groupOther(infos)
+
+	if len(got) != 1 {
+		t.Fatalf("groupOther() = %d devices, want 1: %+v", len(got), got)
+	}
+	if got[0].VendorID != 0x320f || got[0].ProductID != 0x5044 {
+		t.Errorf("vendor/product = %04X/%04X, want 320F/5044", got[0].VendorID, got[0].ProductID)
+	}
+	if got[0].Name != "GMMK Pro" {
+		t.Errorf("name = %q, want %q", got[0].Name, "GMMK Pro")
+	}
+
+	want := []UsagePair{{0x0001, 0x06}, {0x000c, 0x01}, {0xff80, 0x61}}
+	if len(got[0].UsagePairs) != len(want) {
+		t.Fatalf("usage pages = %+v, want %+v", got[0].UsagePairs, want)
+	}
+	for i, w := range want {
+		if got[0].UsagePairs[i] != w {
+			t.Errorf("usagePages[%d] = %+v, want %+v", i, got[0].UsagePairs[i], w)
+		}
+	}
+}
+
+// A device with no product string still has to be listed, or the one board that
+// states nothing about itself is the one that goes missing from the explanation.
+func TestGroupOtherKeepsADeviceWithoutAName(t *testing.T) {
+	got := groupOther([]hid.DeviceInfo{{VendorID: 0x0000, ProductID: 0x0000, UsagePage: 0xff00, Usage: 0xff}})
+
+	if len(got) != 1 {
+		t.Fatalf("groupOther() = %d devices, want 1", len(got))
+	}
+	if got[0].Name != "" {
+		t.Errorf("name = %q, want it empty so the command can say so", got[0].Name)
+	}
+}

+ 73 - 7
internal/hid/hid.go

@@ -20,11 +20,29 @@ type DeviceInfo struct {
 	VendorID  uint16
 	ProductID uint16
 	RawHID    bool
+	// UsagePage and Usage are the HID collection this entry describes. A
+	// keyboard reports itself as several collections over one path, one per
+	// top-level collection in its descriptor: a keyboard, a consumer control, a
+	// vendor page. macOS reports every one of them separately, Linux one
+	// interface at a time, and the QMK Raw HID collection is the one with this
+	// pair. They travel with the entry because an entry that was not taken is
+	// only explainable by the pair it was rejected for.
+	UsagePage uint16
+	Usage     uint16
 	// ProductString is the USB product string, which is where a keyboard states
 	// its own model name. Firmware chooses it, so it can be empty.
 	ProductString string
 }
 
+// rawHIDUsagePage and rawHIDUsage are the QMK Raw HID collection: the signature
+// qmk/qmk_udev matches on, and the only one this tool drives. A board that
+// declares a different pair is a board this tool cannot address, and saying so
+// is the point of reporting the pair a rejected device came with.
+const (
+	rawHIDUsagePage uint16 = 0xff60
+	rawHIDUsage     uint16 = 0x61
+)
+
 // DevicesInfo lists all connected HID devices.
 type DevicesInfo struct {
 	Devices []DeviceInfo
@@ -64,6 +82,53 @@ func DiscoverAll() ([]DeviceInfo, error) {
 			ProductID:     info.ProductID,
 			ProductString: info.ProductStr,
 			RawHID:        true,
+			UsagePage:     info.UsagePage,
+			Usage:         info.Usage,
+		})
+		return nil
+	})
+	if err != nil {
+		return nil, fmt.Errorf("enumerate: %w", err)
+	}
+
+	return devices, nil
+}
+
+// DiscoverEvery returns every HID collection the system reports, the one this
+// tool drives included. It exists so that a device which was not taken can be
+// explained: no keyboard found and a keyboard whose collection is not the QMK
+// Raw HID one look identical from outside, and only the second is worth telling
+// a user about.
+//
+// A collection is the unit here, not a device. One path appears once per usage
+// pair, so a keyboard with a keyboard collection and a vendor collection is two
+// entries, and each carries the pair it was rejected for. Order is the
+// enumeration order, which nothing may sort: these entries carry no number a user
+// could pass to a command, so they have no order to promise.
+func DiscoverEvery() ([]DeviceInfo, error) {
+	if err := ensureInit(); err != nil {
+		return nil, fmt.Errorf("init hid: %w", err)
+	}
+
+	seen := make(map[string]bool)
+	var devices []DeviceInfo
+	err := hid.Enumerate(hid.VendorIDAny, hid.ProductIDAny, func(info *hid.DeviceInfo) error {
+		// The pair belongs in the key as well as the path: a keyboard reports
+		// several collections on one path, and the same collection listed twice
+		// is one entry.
+		key := fmt.Sprintf("%s/%04X/%04X", info.Path, info.UsagePage, info.Usage)
+		if seen[key] {
+			return nil
+		}
+		seen[key] = true
+		devices = append(devices, DeviceInfo{
+			Path:          info.Path,
+			VendorID:      info.VendorID,
+			ProductID:     info.ProductID,
+			ProductString: info.ProductStr,
+			RawHID:        isRawHID(info),
+			UsagePage:     info.UsagePage,
+			Usage:         info.Usage,
 		})
 		return nil
 	})
@@ -93,6 +158,8 @@ func Enumerate(vendorID, productID uint16, fn func(info *DeviceInfo) error) erro
 			ProductID:     info.ProductID,
 			ProductString: info.ProductStr,
 			RawHID:        true,
+			UsagePage:     info.UsagePage,
+			Usage:         info.Usage,
 		})
 	})
 	if err != nil {
@@ -101,14 +168,13 @@ func Enumerate(vendorID, productID uint16, fn func(info *DeviceInfo) error) erro
 	return nil
 }
 
+// isRawHID reports whether a HID collection is the QMK Raw HID one, the signature
+// qmk/qmk_udev matches on. It is an exact pair on purpose: a vendor page that
+// happens to be near it is not a raw HID interface, and a board that puts its
+// lighting on another page is a board this tool cannot address rather than one it
+// may guess at.
 func isRawHID(info *hid.DeviceInfo) bool {
-	if info.UsagePage != 0xff60 {
-		return false
-	}
-	if info.Usage != 0x61 {
-		return false
-	}
-	return true
+	return info.UsagePage == rawHIDUsagePage && info.Usage == rawHIDUsage
 }
 
 // Open opens a HID device by VID/PID.

+ 31 - 0
internal/hid/hid_test.go

@@ -2,6 +2,8 @@ package hid
 
 import (
 	"testing"
+
+	"github.com/sstallion/go-hid"
 )
 
 func TestReadTimesOutWithoutData(t *testing.T) {
@@ -9,3 +11,32 @@ func TestReadTimesOutWithoutData(t *testing.T) {
 	// so we skip this test without a real HID device.
 	t.Skip("requires real HID device")
 }
+
+// The signature is an exact pair, and the two halves are load-bearing in both
+// directions: a firmware that puts Raw HID on a neighbouring page is a board this
+// tool cannot address, and taking it anyway would mean writing lighting commands
+// to a collection that does not answer them. A board that does so has to show up
+// in the passed-over listing instead, which is what tells a user why.
+func TestRawHIDIsAnExactPair(t *testing.T) {
+	tests := []struct {
+		name      string
+		usagePage uint16
+		usage     uint16
+		want      bool
+	}{
+		{name: "the QMK Raw HID signature", usagePage: 0xff60, usage: 0x61, want: true},
+		{name: "the Impact 80's own vendor interface", usagePage: 0xff80, usage: 0x61, want: false},
+		{name: "another usage on the same page", usagePage: 0xff60, usage: 0x62, want: false},
+		{name: "the keyboard collection of a keyboard", usagePage: 0x0001, usage: 0x06, want: false},
+		{name: "the page with no usage", usagePage: 0xff60, usage: 0x00, want: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			info := &hid.DeviceInfo{UsagePage: tt.usagePage, Usage: tt.usage}
+			if got := isRawHID(info); got != tt.want {
+				t.Errorf("isRawHID(%04X/%02X) = %t, want %t", tt.usagePage, tt.usage, got, tt.want)
+			}
+		})
+	}
+}