2026-09-27-channel-discovery.md 63 KB

Channel Discovery Implementation Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: The tool discovers which VIA lighting channels the connected keyboard has instead of assuming three fixed zones, names them by QMK subsystem, and keeps the Impact 80's effect catalog as the only board that has one.

Architecture: A new via.Channel type carries the QMK channel space and a probe that asks the keyboard which channels exist; QMK answers a channel it does not compile in with 0xFF. A board's display names come from an optional channels map in keyboards.json, so --zone accepts a display name or a subsystem name. The Impact 80's effect names stay in internal/rgb as a catalog selected by VID/PID; every other board gets raw IDs through mode.

Tech Stack: Go 1.26, github.com/spf13/cobra, github.com/sstallion/go-hid, go test

Spec: docs/superpowers/specs/2026-09-27-channel-discovery-design.md

Global Constraints

  • keyboard info must never open a HID handle. DiscoverAll enumerates only, so board metadata and --zone validation stay device-free.
  • A --zone value that matches neither a display name nor a subsystem name fails before the HID handle is opened. This is documented behaviour in README.md and must survive the change.
  • Read-back reporting is unchanged: brightness, speed and color read back. effect and mode still write without reading back; that gap is deliberately out of scope.
  • Channels are probed from 1 to 15 inclusive. The QMK enum stops at 5; the extra reads exist so a future QMK that claims a higher channel is found without a code change.
  • Absence is the 0xFF response, never a zero payload. An unknown value ID on a known channel is mirrored back with zeros.
  • Probe read errors other than 0xFF abort detection. A short channel list must never be reported as complete.
  • Channels are returned in ascending order, and that order is the default targeting order.
  • Do not add comments that restate the code. Comment only where the reason is not visible in the code.
  • gofmt clean, go vet clean, go test ./... green before every commit.

Review Focus

Five failure modes the spec implies that the task tests must pin down:

  1. Two keyboards, one selected. With --device 2, display names must come from the selected board's keyboards.json entry, not the first enumerated one. Pinned in Task 3.
  2. keyboards.json absent or malformed. README.md promises that an absent or broken file costs names, not the ability to drive the keyboard. Pinned in Task 2 and Task 3.
  3. A display name that shadows a present channel's subsystem name. The command must refuse, not prefer one reading. Pinned in Task 3.
  4. A profile whose keys no longer resolve after a board is renamed. Each unresolved key must produce its own warning naming the key, and load must say on stderr that nothing matched if none did. Pinned in Task 6.
  5. A transport error in the middle of the probe. Detection must fail loudly rather than return the channels it managed to read. Pinned in Task 1.

Two gaps in the spec that this plan closes, both flagged to the human partner:

  • enable on a board without a catalog. enable writes an effect ID, and only the catalog says which ID is a sensible default. The plan makes enable refuse for a board with no catalog and keeps disable working, because disable writes effect 0 and needs no catalog. Rationale: picking an ID blind is the invented data the spec forbids.
  • When display names are validated. The spec says a shadowing name is rejected "when the file is loaded". Presence is only known after the probe, and a static check would reject the Impact 80's own file, because it calls channel 3 backlight while channel 1's subsystem name is also backlight and channel 1 happens to be absent. The plan validates at resolution time, where presence is known, and Task 3 tests both directions.

File Structure

New files

  • internal/via/channel.go — the Channel type, the QMK channel space, Subsystem(), and DetectChannels()
  • internal/via/channel_test.go — detection tests against a queued-response transport
  • internal/rgb/catalog.go — the effect catalog: types, CatalogFor, name and ID resolution
  • internal/rgb/catalog_test.go — catalog and resolution tests
  • cmd/qmk-rgb-tool/channels_test.go — zone name resolution, conflict detection, device selection

Modified files

  • internal/via/protocol.go — an errUnhandled sentinel, and the three value methods change from LEDType to Channel
  • internal/via/protocol_test.go — a queued response list on the existing fake transport
  • internal/device/device.go — Keyboard.Channels, the display name map, and a KeyboardFor lookup
  • internal/device/device_test.go — parsing tests for the new field
  • cmd/qmk-rgb-tool/zones.go — replaced by channel resolution: candidates, conflict check, naming
  • cmd/qmk-rgb-tool/rgb.go — protocol interfaces speak channels; OpenDevice split into prepareTarget and openTarget
  • cmd/qmk-rgb-tool/info.go — the zone field carries a resolved name
  • cmd/qmk-rgb-tool/effect.go — catalog-driven list and set
  • cmd/qmk-rgb-tool/brightness.go, speed.go, color.go, mode.go, enable.go, disable.go — channel plumbing
  • cmd/qmk-rgb-tool/profile.go — profile keys resolve to channels
  • internal/rgb/impact80.go — zone-keyed tables become the catalog's arrays
  • internal/rgb/effects.go — the dead generic Effect enum and State are removed
  • internal/rgb/impact80_test.go — rewritten against the catalog
  • README.md, AGENTS.md — documented behaviour

Existing tests that must be updated, and why: internal/rgb/impact80_test.go and every cmd test that builds intrgb.Zone values assert against a type this change removes. They are updated in the task that removes the type, not earlier, so every commit in between compiles and its tests pass.


Task 1: Channel type and detection

Files:

  • Create: internal/via/channel.go
  • Create: internal/via/channel_test.go
  • Modify: internal/via/protocol.go (unhandled sentinel, LEDType to Channel)
  • Modify: internal/via/protocol_test.go (queued responses, channel arguments)

Interfaces:

  • Consumes: Protocol.handle transport
  • Produces: type Channel uint8; constants ChannelBacklight, ChannelRgblight, ChannelRgbMatrix, ChannelAudio, ChannelLedMatrix; const AssignedChannelMax = 5; func (c Channel) Subsystem() string; func (p *Protocol) DetectChannels() ([]Channel, error); SetValue(Channel, uint8, uint8) error; SetColor(Channel, uint8, uint8) error; GetValue(Channel, uint8) ([]byte, error)

  • [ ] Step 0: Capture the current behaviour as a baseline

The acceptance criterion is that this board does not change, so its output has to be recorded before anything is touched:

mkdir -p /tmp/opencode/baseline
go build -o /tmp/opencode/baseline/qmk-rgb-before ./cmd/qmk-rgb-tool/
/tmp/opencode/baseline/qmk-rgb-before effect --list > /tmp/opencode/baseline/list.json
/tmp/opencode/baseline/qmk-rgb-before info > /tmp/opencode/baseline/info.json

Expected: both commands succeed with the keyboard attached. If the keyboard is not attached, stop and ask, because every acceptance criterion depends on it.

  • Step 1: Extend the fake transport with a response queue

In internal/via/protocol_test.go, add the fields and the queue drain:

type fakeTransport struct {
	reportIDs    []byte
	reports      [][]byte
	response     []byte
	queue        [][]byte
	readCalls    int
	readErr      error
	readErrAfter int
}

func (f *fakeTransport) Read(buf []byte) (int, error) {
	f.readCalls++
	if f.readErr != nil && f.readCalls > f.readErrAfter {
		return 0, f.readErr
	}
	if len(f.queue) > 0 {
		next := f.queue[0]
		f.queue = f.queue[1:]
		return copy(buf, next), nil
	}
	if len(f.response) > 0 {
		return copy(buf, f.response), nil
	}
	if len(f.reports) > 0 {
		return copy(buf, f.reports[len(f.reports)-1]), nil
	}
	return copy(buf, make([]byte, 32)), nil
}
  • Step 2: Run the existing protocol tests

Run: go test ./internal/via/ -count=1 Expected: PASS, unchanged. The queue, the error and the extra fields are zero in every existing test, so behaviour is identical.

  • Step 3: Write the failing detection tests

Create internal/via/channel_test.go:

package via

import (
	"errors"
	"strings"
	"testing"
)

// probeQueue scripts one answer per channel the probe visits, so a test can say
// which channels the keyboard has.
func probeQueue(present ...Channel) [][]byte {
	queue := make([][]byte, 0, probeChannelMax)
	for c := 1; c <= probeChannelMax; c++ {
		buf := make([]byte, 32)
		if containsChannel(present, Channel(c)) {
			buf[0] = byte(CustomGet)
			buf[3] = 160
		} else {
			buf[0] = byte(Unhandled)
		}
		buf[1] = byte(c)
		buf[2] = probeValueID
		queue = append(queue, buf)
	}
	return queue
}

func containsChannel(channels []Channel, want Channel) bool {
	for _, c := range channels {
		if c == want {
			return true
		}
	}
	return false
}

func TestDetectChannelsFindsTheAnsweredChannels(t *testing.T) {
	transport := &fakeTransport{queue: probeQueue(ChannelRgblight, ChannelRgbMatrix, ChannelAudio)}
	protocol := Protocol{handle: transport}

	got, err := protocol.DetectChannels()
	if err != nil {
		t.Fatalf("DetectChannels() error = %v", err)
	}

	want := []Channel{ChannelRgblight, ChannelRgbMatrix, ChannelAudio}
	if len(got) != len(want) {
		t.Fatalf("DetectChannels() = %v, want %v", got, want)
	}
	for i := range want {
		if got[i] != want[i] {
			t.Errorf("DetectChannels()[%d] = %d, want %d", i, got[i], want[i])
		}
	}
}

func TestDetectChannelsAsksBrightnessOnEveryChannelFromOneToFifteen(t *testing.T) {
	transport := &fakeTransport{queue: probeQueue()}
	protocol := Protocol{handle: transport}

	if _, err := protocol.DetectChannels(); err != nil {
		t.Fatalf("DetectChannels() error = %v", err)
	}

	if len(transport.reports) != probeChannelMax {
		t.Fatalf("requests = %d, want %d", len(transport.reports), probeChannelMax)
	}
	for i, report := range transport.reports {
		wantChannel := byte(i + 1)
		if report[0] != byte(CustomGet) {
			t.Errorf("request %d command = 0x%02x, want 0x%02x", i, report[0], CustomGet)
		}
		if report[1] != wantChannel {
			t.Errorf("request %d channel = %d, want %d", i, report[1], wantChannel)
		}
		if report[2] != probeValueID {
			t.Errorf("request %d value = 0x%02x, want 0x%02x (brightness)", i, report[2], probeValueID)
		}
	}
}

// A channel list missing entries is indistinguishable from a complete one, so a
// transport failure has to abort the probe instead of shrinking it.
func TestDetectChannelsFailsOnATransportErrorMidProbe(t *testing.T) {
	transport := &fakeTransport{
		queue:        probeQueue(ChannelRgblight, ChannelRgbMatrix, ChannelAudio),
		readErr:      errors.New("read: interrupted system call"),
		readErrAfter: 3,
	}
	protocol := Protocol{handle: transport}

	_, err := protocol.DetectChannels()
	if err == nil {
		t.Fatal("DetectChannels() expected an error, got nil")
	}
	if !strings.Contains(err.Error(), "channel 4") {
		t.Errorf("error = %q, want it to name the channel that failed", err)
	}
}

func TestDetectChannelsReportsNoChannelsWhenNoneArePresent(t *testing.T) {
	transport := &fakeTransport{queue: probeQueue()}
	protocol := Protocol{handle: transport}

	got, err := protocol.DetectChannels()
	if err != nil {
		t.Fatalf("DetectChannels() error = %v", err)
	}
	if len(got) != 0 {
		t.Errorf("DetectChannels() = %v, want empty", got)
	}
}

func TestChannelSubsystemNames(t *testing.T) {
	tests := []struct {
		channel Channel
		want    string
	}{
		{ChannelBacklight, "backlight"},
		{ChannelRgblight, "rgblight"},
		{ChannelRgbMatrix, "rgb_matrix"},
		{ChannelAudio, "audio"},
		{ChannelLedMatrix, "led_matrix"},
		{Channel(9), ""},
	}

	for _, tt := range tests {
		if got := tt.channel.Subsystem(); got != tt.want {
			t.Errorf("Channel(%d).Subsystem() = %q, want %q", tt.channel, got, tt.want)
	}
}
  • Step 4: Run the tests to verify they fail

Run: go test ./internal/via/ -run 'TestDetectChannels|TestChannelSubsystem' -count=1 Expected: FAIL to compile, undefined: Channel, undefined: probeChannelMax, undefined: probeValueID.

  • Step 5: Add the unhandled sentinel

In internal/via/protocol.go, add after the Message block:

// errUnhandled reports a channel or value ID the firmware does not implement.
// It is a sentinel so a caller can tell "this does not exist here" from a
// transport failure, which the two are otherwise indistinguishable in.
var errUnhandled = errors.New("unhandled response")

and in readResponse, replace

	if buf[0] == byte(Unhandled) {
		return nil, fmt.Errorf("unhandled response")
	}

with

	if buf[0] == byte(Unhandled) {
		return nil, errUnhandled
	}

Add "errors" to the imports.

  • Step 6: Change the value methods to take a Channel

In internal/via/protocol.go, replace the LEDType type and its constants with nothing, and change the three methods that took LEDType to take Channel, packing byte(ch) into the report. SetValue:

func (p *Protocol) SetValue(ch Channel, param, value uint8) error {
	report := make([]byte, 32)
	report[0] = byte(CustomSet)
	report[1] = byte(ch)
	report[2] = param
	report[3] = value

	if _, err := p.handle.SendReport(0x00, report); err != nil {
		return err
	}
	_, err := p.readResponse(CustomSet, byte(ch), param)
	return err
}

SetColor:

func (p *Protocol) SetColor(ch Channel, hue, saturation uint8) error {
	report := make([]byte, 32)
	report[0] = byte(CustomSet)
	report[1] = byte(ch)
	report[2] = 0x04
	report[3] = hue
	report[4] = saturation

	if _, err := p.handle.SendReport(0x00, report); err != nil {
		return err
	}
	_, err := p.readResponse(CustomSet, byte(ch), 0x04)
	return err
}

GetValue:

func (p *Protocol) GetValue(ch Channel, param uint8) ([]byte, error) {
	report := make([]byte, 32)
	report[0] = byte(CustomGet)
	report[1] = byte(ch)
	report[2] = param

	if _, err := p.handle.SendReport(0x00, report); err != nil {
		return nil, fmt.Errorf("send get request: %w", err)
	}

	buf, err := p.readResponse(CustomGet, byte(ch), param)
	if err != nil {
		return nil, err
	}

	valueSize := 1
	if param == 0x04 {
		valueSize = 2
	}
	return append([]byte(nil), buf[3:3+valueSize]...), nil
}

Change readResponse's signature to take the channel as a byte, since it only compares it:

func (p *Protocol) readResponse(command Message, ch, param byte) ([]byte, error) {

Delete the LEDType type, the RGBLight, RGBMatrix and SideLight constants, and the SetLightingColor helper, which had no caller outside SetColor. In internal/via/protocol_test.go, replace every RGBLight, RGBMatrix and SideLight argument with ChannelRgblight, ChannelRgbMatrix and ChannelAudio.

Run: go build ./... 2>&1 | head -20 Expected: compile errors in cmd, which still speaks LEDType. That is fixed in Task 5; the internal/via package itself must compile and pass now.

Run: go test ./internal/via/ -count=1 Expected: PASS.

  • Step 7: Write the channel type and the probe

Create internal/via/channel.go:

package via

import (
	"errors"
	"fmt"
)

// Channel is a VIA lighting channel. The values are QMK's id_qmk_*_channel
// constants from quantum/via.h.
type Channel uint8

const (
	ChannelBacklight Channel = 1
	ChannelRgblight  Channel = 2
	ChannelRgbMatrix Channel = 3
	ChannelAudio     Channel = 4
	ChannelLedMatrix Channel = 5
)

// AssignedChannelMax is the highest channel QMK currently assigns. The probe
// asks beyond it on purpose, see probeChannelMax.
const AssignedChannelMax = ChannelLedMatrix

// probeChannelMax is the highest channel the probe asks about. The QMK enum
// stops at 5; the extra reads cost one round trip each and let a future QMK
// that claims a higher channel be found without a code change.
const probeChannelMax = 15

// probeValueID is the brightness value ID. It is one byte and valid on every
// lighting subsystem, so one request per channel finds any kind of channel.
const probeValueID = 0x01

// Subsystem returns the QMK name of the channel's lighting subsystem. The name
// follows from the channel number, so no board file stores it and it is always
// available for a channel the keyboard has.
func (c Channel) Subsystem() string {
	switch c {
	case ChannelBacklight:
		return "backlight"
	case ChannelRgblight:
		return "rgblight"
	case ChannelRgbMatrix:
		return "rgb_matrix"
	case ChannelAudio:
		return "audio"
	case ChannelLedMatrix:
		return "led_matrix"
	default:
		return ""
	}
}

// DetectChannels asks the keyboard which lighting channels it has, in ascending
// order.
//
// The 0xFF answer is the discriminator, not the payload: QMK rejects a channel
// it does not compile in with id_unhandled, whereas an unknown value ID on a
// known channel is mirrored back with a zero payload. A read that fails for any
// other reason aborts the probe, because a channel list missing entries is
// indistinguishable from a complete one and would silently under-report.
func (p *Protocol) DetectChannels() ([]Channel, error) {
	var present []Channel
	for c := Channel(1); c <= probeChannelMax; c++ {
		if _, err := p.GetValue(c, probeValueID); err != nil {
			if errors.Is(err, errUnhandled) {
				continue
			}
			return nil, fmt.Errorf("probe channel %d: %w", c, err)
		}
		present = append(present, c)
	}
	return present, nil
}
  • Step 8: Run the tests to verify they pass

Run: go test ./internal/via/ -count=1 Expected: PASS, including the mid-probe error test, which fails on the read after the third channel.

  • Step 9: Verify and commit

Run: go test ./internal/via/ -count=1 && gofmt -l internal/ && go vet ./internal/via/ Expected: green, no output from gofmt -l.

git add internal/via/
git commit -m "detect which VIA lighting channels a keyboard has"

Task 2: Display names in keyboards.json

Files:

  • Modify: internal/device/device.go
  • Modify: internal/device/device_test.go

Interfaces:

  • Consumes: nothing from Task 1
  • Produces: device.Keyboard.Channels map[uint16]string; func KeyboardFor(vendorID, productID uint16) (Keyboard, bool, error)

  • [ ] Step 1: Write the failing test

Add to internal/device/device_test.go:

func writeTempKeyboards(t *testing.T, content string) string {
	t.Helper()
	path := filepath.Join(t.TempDir(), "keyboards.json")
	if err := os.WriteFile(path, []byte(content), 0o600); err != nil {
		t.Fatalf("write keyboards.json: %v", err)
	}
	return path
}

func TestLoadKeyboardsReadsChannelDisplayNames(t *testing.T) {
	original := findKeyboardsJSON
	t.Cleanup(func() { findKeyboardsJSON = original })
	findKeyboardsJSON = func() (string, error) {
		return writeTempKeyboards(t, `[
			{"name": "Wobkey Impact 80", "vendorId": 14000, "productId": 12447,
			 "channels": {"2": "logo", "3": "backlight", "4": "side"}}
		]`), nil
	}

	keyboards, err := LoadKeyboards()
	if err != nil {
		t.Fatalf("LoadKeyboards() error = %v", err)
	}
	if len(keyboards) != 1 {
		t.Fatalf("keyboards = %d, want 1", len(keyboards))
	}
	got := keyboards[0].Channels
	if got[2] != "logo" || got[3] != "backlight" || got[4] != "side" {
		t.Errorf("Channels = %v, want 2:logo 3:backlight 4:side", got)
	}
}

func TestLoadKeyboardsAcceptsAnEntryWithoutChannels(t *testing.T) {
	original := findKeyboardsJSON
	t.Cleanup(func() { findKeyboardsJSON = original })
	findKeyboardsJSON = func() (string, error) {
		return writeTempKeyboards(t, `[{"name": "Wobkey Rainy 75", "vendorId": 26214, "productId": 1}]`), nil
	}

	keyboards, err := LoadKeyboards()
	if err != nil {
		t.Fatalf("LoadKeyboards() error = %v", err)
	}
	if len(keyboards[0].Channels) != 0 {
		t.Errorf("Channels = %v, want empty", keyboards[0].Channels)
	}
}

// README.md promises that a missing or broken keyboards.json costs names, not
// the ability to drive the keyboard.
func TestKeyboardForReportsAbsentRatherThanFailing(t *testing.T) {
	original := findKeyboardsJSON
	t.Cleanup(func() { findKeyboardsJSON = original })
	findKeyboardsJSON = func() (string, error) { return "does-not-exist.json", nil }

	if _, known, err := KeyboardFor(0x6666, 0x0001); err != nil {
		t.Errorf("KeyboardFor() error = %v, want nil for a missing file", err)
	} else if known {
		t.Error("KeyboardFor() = known, want unknown for a missing file")
	}
}

Add "os" and "path/filepath" to the test imports if they are missing.

  • Step 2: Run the tests to verify they fail

Run: go test ./internal/device/ -run 'TestLoadKeyboards|TestKeyboardFor' -count=1 Expected: FAIL to compile, keyboards[0].Channels undefined and undefined: KeyboardFor.

  • Step 3: Add the field and the lookup

In internal/device/device.go:

// Keyboard is one entry from keyboards.json.
type Keyboard struct {
	Name      string `json:"name"`
	VendorID  uint16 `json:"vendorId"`
	ProductID uint16 `json:"productId"`
	// Channels maps a VIA channel number to the name this board uses for it.
	// It is optional: a board without it is addressed by its QMK subsystem
	// name, which follows from the channel number.
	Channels map[uint16]string `json:"channels"`
}

// KeyboardFor returns the keyboards.json entry for a device and whether one
// exists. A missing or malformed file yields no entry and no error, because
// the file supplies names only: costing names must not cost the ability to
// drive the keyboard.
func KeyboardFor(vendorID, productID uint16) (Keyboard, bool, error) {
	keyboards, err := LoadKeyboards()
	if err != nil {
		return Keyboard{}, false, nil
	}
	for _, kb := range keyboards {
		if kb.VendorID == vendorID && kb.ProductID == productID {
			return kb, true, nil
		}
	}
	return Keyboard{}, false, nil
}
  • Step 4: Run the tests to verify they pass

Run: go test ./internal/device/ -count=1 Expected: PASS.

  • Step 5: Verify and commit

Run: go test ./internal/device/ -count=1 && gofmt -l internal/ Expected: green.

git add internal/device/
git commit -m "let keyboards.json name a board's channels"

Task 3: Resolve --zone to channels

Files:

  • Modify: cmd/qmk-rgb-tool/zones.go
  • Create: cmd/qmk-rgb-tool/channels_test.go
  • Modify: cmd/qmk-rgb-tool/rgb.go (the loadKeyboards seam is not needed; see step 5)

Interfaces:

  • Consumes: via.Channel, via.AssignedChannelMax, device.Keyboard.Channels, device.KeyboardFor, discoverAll, selectDevice
  • Produces: func resolveZoneName(name string, display map[uint16]string) ([]via.Channel, error); func displayNameConflicts(display map[uint16]string, present []via.Channel) error; func channelName(c via.Channel, display map[uint16]string) string; type targetDeviceData struct; func prepareTarget() (targetDeviceData, error)

  • [ ] Step 1: Write the failing name-resolution tests

Create cmd/qmk-rgb-tool/channels_test.go:

package main

import (
	"strings"
	"testing"

	intdevice "netdome.biz/paul/qmk-rgb/internal/device"
	"netdome.biz/paul/qmk-rgb/internal/via"
)

func TestResolveZoneNameAcceptsASubsystemName(t *testing.T) {
	got, err := resolveZoneName("rgb_matrix", map[uint16]string{2: "logo"})
	if err != nil {
		t.Fatalf("resolveZoneName() error = %v", err)
	}
	if len(got) != 1 || got[0] != via.ChannelRgbMatrix {
		t.Errorf("resolveZoneName() = %v, want [3]", got)
	}
}

func TestResolveZoneNameAcceptsADisplayName(t *testing.T) {
	got, err := resolveZoneName("logo", map[uint16]string{2: "logo", 3: "backlight", 4: "side"})
	if err != nil {
		t.Fatalf("resolveZoneName() error = %v", err)
	}
	if len(got) != 1 || got[0] != via.ChannelRgblight {
		t.Errorf("resolveZoneName() = %v, want [2]", got)
	}
}

func TestResolveZoneNameRejectsAnUnknownName(t *testing.T) {
	_, err := resolveZoneName("nope", map[uint16]string{2: "logo"})
	if err == nil {
		t.Fatal("resolveZoneName() expected an error, got nil")
	}
	if !strings.Contains(err.Error(), "rgb_matrix") {
		t.Errorf("error = %q, want it to name an accepted form", err)
	}
}

// A board that supplies no display names keeps the subsystem vocabulary, so the
// physical names that work on the Impact 80 do not work elsewhere.
func TestResolveZoneNameIgnoresDisplayNamesForAnotherBoard(t *testing.T) {
	if _, err := resolveZoneName("logo", nil); err == nil {
		t.Fatal("resolveZoneName(\"logo\", nil) expected an error, got nil")
	}
}

// A display name always wins over a subsystem name, and the tool reports the
// ambiguity rather than picking one.
func TestResolveZoneNameReportsAnAmbiguousName(t *testing.T) {
	_, err := resolveZoneName("backlight", map[uint16]string{2: "logo", 3: "backlight"})
	if err == nil {
		t.Fatal("resolveZoneName() expected an error for a name that means two channels")
	}
	if !strings.Contains(err.Error(), "backlight") {
		t.Errorf("error = %q, want it to name the conflicting name", err)
	}
}

func TestDisplayNameConflictsRejectsAShadowedSubsystemName(t *testing.T) {
	// Channel 1 is present and its subsystem is "backlight", while the file
	// also calls channel 3 "backlight": two channels, one name.
	err := displayNameConflicts(
		map[uint16]string{3: "backlight"},
		[]via.Channel{via.ChannelBacklight, via.ChannelRgbMatrix},
	)
	if err == nil {
		t.Fatal("displayNameConflicts() expected an error, got nil")
	}
	if !strings.Contains(err.Error(), "backlight") {
		t.Errorf("error = %q, want it to name the conflicting name", err)
	}
}

func TestDisplayNameConflictsAllowsTheImpact80Naming(t *testing.T) {
	// The Impact 80 calls channel 3 "backlight" and has no channel 1, so
	// nothing shadows anything.
	err := displayNameConflicts(
		map[uint16]string{2: "logo", 3: "backlight", 4: "side"},
		[]via.Channel{via.ChannelRgblight, via.ChannelRgbMatrix, via.ChannelAudio},
	)
	if err != nil {
		t.Fatalf("displayNameConflicts() error = %v, want nil", err)
	}
}

func TestChannelNamePrefersTheDisplayName(t *testing.T) {
	display := map[uint16]string{2: "logo"}
	if got := channelName(via.ChannelRgblight, display); got != "logo" {
		t.Errorf("channelName(2) = %q, want %q", got, "logo")
	}
	if got := channelName(via.ChannelRgbMatrix, display); got != "rgb_matrix" {
		t.Errorf("channelName(3) = %q, want %q", got, "rgb_matrix")
	}
}
  • Step 2: Run the tests to verify they fail

Run: go test ./cmd/qmk-rgb-tool/ -run 'TestResolveZoneName|TestDisplayNameConflicts|TestChannelName' -count=1 Expected: FAIL to compile, undefined: resolveZoneName.

  • Step 3: Write the resolution code

Replace cmd/qmk-rgb-tool/zones.go entirely with:

package main

import (
	"fmt"
	"sort"
	"strings"

	"netdome.biz/paul/qmk-rgb/internal/via"
)

// resolveZoneName maps a --zone value to the channels it may mean, without
// opening the keyboard. A display name of the connected board wins over a
// subsystem name, and a value that would mean two channels is refused rather
// than resolved to one of them.
//
// An empty name means every channel and is reported as a nil slice.
func resolveZoneName(name string, display map[uint16]string) ([]via.Channel, error) {
	if name == "" {
		return nil, nil
	}

	if channels := displayNameChannels(display, name); len(channels) > 1 {
		return nil, fmt.Errorf("zone %q matches several channels of this keyboard; keyboards.json gives the same name to more than one", name)
	} else if len(channels) == 1 {
		return channels, nil
	}

	for c := via.Channel(1); c <= via.AssignedChannelMax; c++ {
		if c.Subsystem() == name {
			return []via.Channel{c}, nil
		}
	}

	return nil, fmt.Errorf("unknown zone %q: use a channel name such as rgb_matrix, or a name from keyboards.json", name)
}

// displayNameChannels returns every channel the board names displayName.
func displayNameChannels(display map[uint16]string, displayName string) []via.Channel {
	var channels []via.Channel
	for number, name := range display {
		if name == displayName {
			channels = append(channels, via.Channel(number))
		}
	}
	sort.Slice(channels, func(i, j int) bool { return channels[i] < channels[j] })
	return channels
}

// displayNameConflicts rejects a board whose display name is also the subsystem
// name of a different channel the keyboard actually has. A name that means two
// channels is a silent retarget, so the tool refuses it.
//
// Presence is only known after the probe, which is why this is checked here
// rather than when the file is read: a static check would reject the Impact 80,
// which calls channel 3 "backlight" while channel 1 is absent.
func displayNameConflicts(display map[uint16]string, present []via.Channel) error {
	presentSet := make(map[via.Channel]bool, len(present))
	for _, c := range present {
		presentSet[c] = true
	}

	names := make([]string, 0, len(display))
	for name := range display {
		names = append(names, name)
	}
	sort.Strings(names)

	for _, name := range names {
		named := via.Channel(0)
		for _, ch := range displayNameChannels(display, name) {
			named = ch
			break
		}
		for _, other := range present {
			if other != named && presentSet[other] && other.Subsystem() == name {
				return fmt.Errorf("keyboards.json: channel %d is named %q, which is also the subsystem name of channel %d", named, name, other)
			}
		}
	}
	return nil
}

// channelName is how a channel is printed: the board's display name when it has
// one, otherwise the QMK subsystem name.
func channelName(c via.Channel, display map[uint16]string) string {
	if name, ok := display[uint8(c)]; ok {
		return name
	}
	return c.Subsystem()
}
  • Step 4: Run the resolution tests to verify they pass

Run: go test ./cmd/qmk-rgb-tool/ -run 'TestResolveZoneName|TestDisplayNameConflicts|TestChannelName' -count=1 Expected: PASS.

  • Step 5: Write the failing device-selection test

Append to cmd/qmk-rgb-tool/channels_test.go:

// The display names must come from the keyboard the command targets, not from
// whichever one enumeration returned first.
func TestPrepareTargetUsesTheSelectedKeyboard(t *testing.T) {
	devices := []intdevice.Device{
		{VendorID: 0x6666, ProductID: 0x0001},
		{VendorID: 0x36B0, ProductID: 0x309F, Name: "Wobkey Impact 80"},
	}

	originalDiscover := discoverAll
	originalKeyboardFor := keyboardFor
	originalTarget := targetDevice
	originalZone := targetZone
	t.Cleanup(func() {
		discoverAll = originalDiscover
		keyboardFor = originalKeyboardFor
		targetDevice = originalTarget
		targetZone = originalZone
	})

	discoverAll = func() ([]intdevice.Device, error) { return devices, nil }
	keyboardFor = func(vendorID, productID uint16) (intdevice.Keyboard, bool, error) {
		switch {
		case vendorID == 0x6666:
			return intdevice.Keyboard{Name: "Wobkey Rainy 75", Channels: map[uint16]string{2: "deck"}}, true, nil
		case vendorID == 0x36B0:
			return intdevice.Keyboard{Name: "Wobkey Impact 80", Channels: map[uint16]string{2: "logo"}}, true, nil
		}
		return intdevice.Keyboard{}, false, nil
	}
	targetDevice = "2"
	targetZone = "logo"

	got, err := prepareTarget()
	if err != nil {
		t.Fatalf("prepareTarget() error = %v", err)
	}
	if got.Display[2] != "logo" {
		t.Errorf("display names = %v, want the Impact 80's", got.Display)
	}
}
  • Step 6: Run it to verify it fails

Run: go test ./cmd/qmk-rgb-tool/ -run TestPrepareTarget -count=1 Expected: FAIL to compile, undefined: keyboardFor, undefined: prepareTarget.

  • Step 7: Add the lookup seam and prepareTarget

In cmd/qmk-rgb-tool/rgb.go, add near the existing seams:

// keyboardFor is a seam for tests; it reads the optional keyboards.json.
var keyboardFor = intdevice.KeyboardFor

and add:

// targetDeviceData is everything a command needs before it opens the keyboard.
type targetDeviceData struct {
	Device  intdevice.Device
	Display map[uint16]string
	// Requested is nil when no --zone was given, which means every channel
	// the keyboard has.
	Requested []via.Channel
}

// prepareTarget resolves the keyboard, its display names and the requested
// channels. Enumeration does not open a HID handle, so an unusable --zone value
// is still rejected before the device is opened.
func prepareTarget() (targetDeviceData, error) {
	devices, err := discoverAll()
	if err != nil {
		return targetDeviceData{}, fmt.Errorf("discover: %w", err)
	}

	dev, err := selectDevice(devices, targetDevice)
	if err != nil {
		return targetDeviceData{}, err
	}

	keyboard, _, err := keyboardFor(dev.VendorID, dev.ProductID)
	if err != nil {
		return targetDeviceData{}, err
	}

	requested, err := resolveZoneName(targetZone, keyboard.Channels)
	if err != nil {
		return targetDeviceData{}, err
	}

	return targetDeviceData{Device: dev, Display: keyboard.Channels, Requested: requested}, nil
}
  • Step 8: Run the test to verify it passes

Run: go test ./cmd/qmk-rgb-tool/ -run TestPrepareTarget -count=1 Expected: PASS.

  • Step 9: Commit

Run: gofmt -l . && go test ./cmd/qmk-rgb-tool/ -run 'TestPrepareTarget|TestResolveZoneName|TestDisplayNameConflicts|TestChannelName' -count=1 Expected: green, no output from gofmt -l.

git add cmd/qmk-rgb-tool/zones.go cmd/qmk-rgb-tool/channels_test.go cmd/qmk-rgb-tool/rgb.go
git commit -m "resolve --zone to channels before the device is opened"

Task 4: Effect catalog per board

Files:

  • Create: internal/rgb/catalog.go
  • Create: internal/rgb/catalog_test.go
  • Modify: internal/rgb/impact80.go (tables stay, zone API goes)
  • Modify: internal/rgb/effects.go (remove the dead generic enum and State)
  • Modify: internal/rgb/impact80_test.go

Interfaces:

  • Consumes: via.Channel
  • Produces: type EffectTarget struct { Channel via.Channel; ID uint8 }; type Catalog struct; func CatalogFor(vendorID, productID uint16) (*Catalog, bool); func (c *Catalog) Names(ch via.Channel) []string; func (c *Catalog) Name(ch via.Channel, id uint8) string; func (c *Catalog) ID(ch via.Channel, name string) (uint8, bool); func (c *Catalog) DefaultEffect(ch via.Channel) (uint8, bool); func ResolveEffect(catalog *Catalog, name string, channels []via.Channel) ([]EffectTarget, []string, error)

  • [ ] Step 1: Write the failing catalog tests

Create internal/rgb/catalog_test.go:

package rgb

import (
	"strings"
	"testing"

	"netdome.biz/paul/qmk-rgb/internal/via"
)

func impact80(t *testing.T) *Catalog {
	t.Helper()
	catalog, ok := CatalogFor(0x36B0, 0x309F)
	if !ok {
		t.Fatal("CatalogFor(0x36b0, 0x309f) = not found, want the Impact 80 catalog")
	}
	return catalog
}

func TestCatalogHasTheVendorEffectFamilies(t *testing.T) {
	catalog := impact80(t)

	if got := len(catalog.Names(via.ChannelRgbMatrix)); got != 46 {
		t.Errorf("backlight effects = %d, want 46", got)
	}
	if got := len(catalog.Names(via.ChannelRgblight)); got != 7 {
		t.Errorf("logo effects = %d, want 7", got)
	}
	if got := len(catalog.Names(via.ChannelAudio)); got != 7 {
		t.Errorf("side effects = %d, want 7", got)
	}
}

func TestCatalogIsEmptyForAnUnknownBoard(t *testing.T) {
	if _, ok := CatalogFor(0x6666, 0x0001); ok {
		t.Error("CatalogFor(0x6666, 0x0001) = found, want none")
	}
}

func TestCatalogIDResolvesNamesAndAliases(t *testing.T) {
	catalog := impact80(t)

	tests := []struct {
		channel via.Channel
		name    string
		want    uint8
	}{
		{via.ChannelRgblight, "light", 5},
		{via.ChannelRgblight, "solid", 5},
		{via.ChannelRgblight, "breathe", 4},
		{via.ChannelRgbMatrix, "solid_color", 1},
		{via.ChannelRgbMatrix, "rainbow_moving_chevron", 17},
	}

	for _, tt := range tests {
		got, ok := catalog.ID(tt.channel, tt.name)
		if !ok {
			t.Errorf("ID(%d, %q) not found", tt.channel, tt.name)
			continue
		}
		if got != tt.want {
			t.Errorf("ID(%d, %q) = %d, want %d", tt.channel, tt.name, got, tt.want)
		}
	}
}

func TestCatalogNameReportsUnknownForAnUnknownID(t *testing.T) {
	catalog := impact80(t)
	if got := catalog.Name(via.ChannelRgbMatrix, 200); got != "unknown" {
		t.Errorf("Name(3, 200) = %q, want %q", got, "unknown")
	}
}

func TestCatalogDefaultEffectPerChannel(t *testing.T) {
	catalog := impact80(t)

	if got, ok := catalog.DefaultEffect(via.ChannelRgblight); !ok || got != 4 {
		t.Errorf("DefaultEffect(2) = %d, %t, want 4, true", got, ok)
	}
	if got, ok := catalog.DefaultEffect(via.ChannelRgbMatrix); !ok || got != 5 {
		t.Errorf("DefaultEffect(3) = %d, %t, want 5, true", got, ok)
	}
}

func TestResolveEffectWithoutACatalogRefuses(t *testing.T) {
	_, _, err := ResolveEffect(nil, "wave", []via.Channel{via.ChannelRgblight})
	if err == nil {
		t.Fatal("ResolveEffect(nil, ...) expected an error, got nil")
	}
	if !strings.Contains(err.Error(), "mode") {
		t.Errorf("error = %q, want it to point at mode", err)
	}
}

// ID 5 is light on channel 2 and rainbow_beacon on channel 3, so the same name
// cannot be applied to both.
func TestResolveEffectSkipsAChannelThatDoesNotSupportTheName(t *testing.T) {
	catalog := impact80(t)

	targets, skipped, err := ResolveEffect(catalog, "light", []via.Channel{via.ChannelRgblight, via.ChannelRgbMatrix})
	if err != nil {
		t.Fatalf("ResolveEffect() error = %v", err)
	}
	if len(targets) != 1 || targets[0].Channel != via.ChannelRgblight || targets[0].ID != 5 {
		t.Errorf("targets = %+v, want one target on channel 2 with id 5", targets)
	}
	if len(skipped) != 1 || skipped[0] != "rgb_matrix" {
		t.Errorf("skipped = %v, want [rgb_matrix]", skipped)
	}
}

// Asking for the unsupported channel by name is an error, not a skip: a command
// that quietly did nothing would look like a command that worked.
func TestResolveEffectRejectsTheOnlyChannelWhenItDoesNotSupportTheName(t *testing.T) {
	catalog := impact80(t)

	_, _, err := ResolveEffect(catalog, "light", []via.Channel{via.ChannelRgbMatrix})
	if err == nil {
		t.Fatal("ResolveEffect() expected an error, want light rejected on the backlight")
	}
}

func TestResolveEffectRejectsANameNoChannelKnows(t *testing.T) {
	catalog := impact80(t)

	if _, _, err := ResolveEffect(catalog, "nope", []via.Channel{via.ChannelRgblight}); err == nil {
		t.Fatal("ResolveEffect() expected an error for an unknown name")
	}
}
  • Step 2: Run the tests to verify they fail

Run: go test ./internal/rgb/ -run 'TestCatalog|TestResolveEffect' -count=1 Expected: FAIL to compile, undefined: CatalogFor.

  • Step 3: Write the catalog

Create internal/rgb/catalog.go:

package rgb

import (
	"fmt"

	"netdome.biz/paul/qmk-rgb/internal/via"
)

// EffectTarget is one effect ID to write to one channel.
type EffectTarget struct {
	Channel via.Channel
	ID      uint8
}

const unknownEffectName = "unknown"

// Catalog is one board's effect names, per channel. The keyboard holds numbers,
// not names, so `effect <name>` needs a catalog and a board without one is
// driven through raw IDs.
type Catalog struct {
	board   string
	names   map[via.Channel][]string
	aliases map[via.Channel]map[string]string
	default map[via.Channel]uint8
}

// CatalogFor returns the effect catalog of a board, and whether one exists.
//
// Provenance, because these lists look invented and are not:
//
//   - The 46 backlight names are QMK's rgb_matrix_effects.inc in order, each ID
//     verified against the live register of an Impact 80.
//   - The 7 logo and 7 side names are this board's vendor VIA definition, whose
//     dropdowns read "fixed wave" and "breathe". The tool spells them
//     fixed_wave and breathing and accepts the vendor's spellings as aliases.
//   - The brightness and speed transforms documented in README.md were measured
//     on the unit, not read from anywhere.
func CatalogFor(vendorID, productID uint16) (*Catalog, bool) {
	if vendorID != 0x36B0 || productID != 0x309F {
		return nil, false
	}
	return &Catalog{
		board: "impact80",
		names: map[via.Channel][]string{
			via.ChannelRgblight:  impact80LogoEffects[:],
			via.ChannelRgbMatrix: impact80BacklightEffects[:],
			via.ChannelAudio:     impact80SideEffects[:],
		},
		aliases: map[via.Channel]map[string]string{
			via.ChannelRgblight: {
				"off":          "none",
				"breathe":      "breathing",
				"rainbow":      "spectrum",
				"rainbow_wave": "wave",
				"solid":        "light",
				"static":       "solid",
			},
			via.ChannelRgbMatrix: {
				"off":     "none",
				"breathe": "breathing",
				"rainbow": "rainbow_moving_chevron",
				"solid":   "solid_color",
				"static":  "solid",
			},
			via.ChannelAudio: {
				"off":          "none",
				"breathe":      "breathing",
				"rainbow":      "spectrum",
				"rainbow_wave": "wave",
				"solid":        "light",
				"static":       "solid",
			},
		},
		default: map[via.Channel]uint8{
			via.ChannelRgblight:  4,
			via.ChannelRgbMatrix: 5,
			via.ChannelAudio:     4,
		},
	}, true
}

// Name returns the catalog's board name, which `effect --list` reports.
func (c *Catalog) Name() string {
	if c == nil {
		return ""
	}
	return c.board
}

// Names returns the effect names of a channel, or nil when the catalog says
// nothing about it.
func (c *Catalog) Names(ch via.Channel) []string {
	if c == nil {
		return nil
	}
	return c.names[ch]
}

// EffectName returns the name of an effect ID, or "unknown" when the catalog has
// no entry for it.
func (c *Catalog) EffectName(ch via.Channel, id uint8) string {
	if c == nil {
		return unknownEffectName
	}
	names := c.names[ch]
	if int(id) >= len(names) {
		return unknownEffectName
	}
	return names[id]
}

// EffectID resolves an effect name on a channel, following one level of alias.
func (c *Catalog) EffectID(ch via.Channel, name string) (uint8, bool) {
	if c == nil {
		return 0, false
	}
	for id, candidate := range c.names[ch] {
		if candidate == name {
			return uint8(id), true
		}
	}
	if canonical, ok := c.aliases[ch][name]; ok {
		for id, candidate := range c.names[ch] {
			if candidate == canonical {
				return uint8(id), true
			}
		}
	}
	return 0, false
}

// DefaultEffect returns the effect ID enable writes when turning a channel on.
func (c *Catalog) DefaultEffect(ch via.Channel) (uint8, bool) {
	if c == nil {
		return 0, false
	}
	id, ok := c.default[ch]
	return id, ok
}

// ResolveEffect turns an effect name into one target per channel that supports
// it, plus the subsystem names of those that do not. The skips are an error
// rather than a warning when a single channel was asked for, because a command
// that silently did nothing looks like a command that worked.
func ResolveEffect(catalog *Catalog, name string, channels []via.Channel) ([]EffectTarget, []string, error) {
	if catalog == nil {
		return nil, nil, fmt.Errorf("no effect catalog for this keyboard; set an effect by number with `mode <index>`")
	}

	known := false
	for _, ch := range channels {
		if _, ok := catalog.EffectID(ch, name); ok {
			known = true
			break
		}
	}
	if !known {
		return nil, nil, fmt.Errorf("unknown effect: %s", name)
	}

	explicit := len(channels) == 1
	var targets []EffectTarget
	var skipped []string

	for _, ch := range channels {
		id, ok := catalog.EffectID(ch, name)
		if !ok {
			if explicit {
				return nil, nil, fmt.Errorf("effect %s is not supported on %s", name, ch.Subsystem())
			}
			skipped = append(skipped, ch.Subsystem())
			continue
		}
		targets = append(targets, EffectTarget{Channel: ch, ID: id})
	}
	return targets, skipped, nil
}

The tests in step 1 call catalog.Name(...) for the board name and catalog.EffectName(...) for an effect ID, which is why the two are separate methods here.

  • Step 4: Run the catalog tests to verify they pass

Run: go test ./internal/rgb/ -run 'TestCatalog|TestResolveEffect' -count=1 Expected: PASS.

  • Step 5: Strip impact80.go to the tables

In internal/rgb/impact80.go, keep the three name arrays with a comment naming their source, and delete everything else: Zone, ZoneLogo, ZoneBacklight, ZoneSide, AllZones, ParseZone, Channel, EffectName, ResolveEffect, resolveEffectID, DefaultEffect, impact80EffectNames, impact80EffectAliases, EffectTarget, Impact80EffectName, ParseImpact80Effect, AllLogoEffects, AllSideEffects, AllBacklightEffects and ZoneEffects.

  • Step 6: Remove the dead generic enum and State

In internal/rgb/effects.go, delete the Effect type with its constants, the String method and ParseEffect, and the State struct. Keep LEDParam and its constants, Color, ParseHexColor and HSV, which are in use. Drop any import left unused.

Run: go build ./... 2>&1 | head -20 Expected: compile errors, all of them in cmd and in the internal/rgb tests that still reference the removed zone API. Both are fixed in Task 5 and in step

  1. Do not paper over them here.
  • Step 7: Rewrite the Impact 80 tests against the catalog

Replace internal/rgb/impact80_test.go with:

package rgb

import (
	"testing"

	"netdome.biz/paul/qmk-rgb/internal/via"
)

func TestImpact80CatalogHasTheVendorEffectFamilies(t *testing.T) {
	catalog, ok := CatalogFor(0x36B0, 0x309F)
	if !ok {
		t.Fatal("CatalogFor() = not found")
	}

	logoWant := []string{"none", "wave", "fixed_wave", "spectrum", "breathing", "light", "shutdown"}
	for id, want := range logoWant {
		if got := catalog.EffectName(via.ChannelRgblight, uint8(id)); got != want {
			t.Errorf("logo effect %d = %q, want %q", id, got, want)
		}
	}

	backlightWant := map[int]string{
		0: "none", 1: "solid_color", 5: "breathing", 17: "rainbow_moving_chevron",
		29: "pixel_flow", 37: "solid_reactive_multinexus", 45: "riverflow",
	}
	for id, want := range backlightWant {
		if got := catalog.EffectName(via.ChannelRgbMatrix, uint8(id)); got != want {
			t.Errorf("backlight effect %d = %q, want %q", id, got, want)
		}
	}
}

func TestImpact80LogoAndSideShareTheirEffectFamily(t *testing.T) {
	catalog, _ := CatalogFor(0x36B0, 0x309F)

	logo := catalog.Names(via.ChannelRgblight)
	side := catalog.Names(via.ChannelAudio)
	if len(logo) != len(side) {
		t.Fatalf("logo has %d effects, side has %d", len(logo), len(side))
	}
	for i := range logo {
		if logo[i] != side[i] {
			t.Errorf("effect %d: logo %q, side %q", i, logo[i], side[i])
		}
	}
}

Delete the other tests in that file, which assert on the removed zone API. Also delete the tests of the removed ParseEffect and String in internal/rgb/effects_test.go, keeping TestQMKValueIDs and the colour tests.

Run: go test ./internal/rgb/ -count=1 Expected: PASS.

  • Step 8: Commit
git add internal/rgb/
git commit -m "make the effect catalog a property of the board, not the zone"

Task 5: Address channels in the commands

Files:

  • Modify: cmd/qmk-rgb-tool/rgb.go
  • Modify: cmd/qmk-rgb-tool/info.go, effect.go, brightness.go, speed.go, color.go, mode.go, enable.go, disable.go
  • Modify: the cmd tests that build intrgb.Zone

Interfaces:

  • Consumes: via.Channel, DetectChannels, rgb.Catalog, CatalogFor, prepareTarget, channelName, displayNameConflicts
  • Produces: zoneProtocol and rgbProtocol speaking via.Channel; zoneResult.Name and colorResult.Name holding a string; func openTarget() (rgbProtocol, targetDeviceData, []via.Channel, error)

  • [ ] Step 1: Change the protocol interfaces and the device opening

In cmd/qmk-rgb-tool/rgb.go:

type zoneProtocol interface {
	SetValue(via.Channel, uint8, uint8) error
	SetColor(via.Channel, uint8, uint8) error
}

type rgbProtocol interface {
	zoneProtocol
	GetValue(via.Channel, uint8) ([]byte, error)
	DetectChannels() ([]via.Channel, error)
	Close() error
}

Replace OpenDevice with the pair the commands call, so the device-free part stays device-free:

// openTarget opens the keyboard and resolves the requested channels against the
// ones it actually has.
func openTarget() (rgbProtocol, targetDeviceData, []via.Channel, error) {
	target, err := prepareTarget()
	if err != nil {
		return nil, targetDeviceData{}, nil, err
	}

	proto, err := via.New(target.Device)
	if err != nil {
		return nil, targetDeviceData{}, nil, fmt.Errorf("open protocol: %w", err)
	}

	channels, err := resolveChannels(proto, target)
	if err != nil {
		proto.Close()
		return nil, targetDeviceData{}, nil, err
	}
	return proto, target, channels, nil
}

// resolveChannels intersects the requested channels with the detected ones, and
// refuses a name that resolves to a channel this keyboard does not have.
func resolveChannels(proto rgbProtocol, target targetDeviceData) ([]via.Channel, error) {
	present, err := proto.DetectChannels()
	if err != nil {
		return nil, err
	}

	if err := displayNameConflicts(target.Display, present); err != nil {
		return nil, err
	}

	if target.Requested == nil {
		if len(present) == 0 {
			return nil, fmt.Errorf("this keyboard exposes no VIA lighting channels")
		}
		return present, nil
	}

	presentSet := make(map[via.Channel]bool, len(present))
	for _, ch := range present {
		presentSet[ch] = true
	}

	var found []via.Channel
	for _, ch := range target.Requested {
		if presentSet[ch] {
			found = append(found, ch)
		}
	}
	if len(found) == 0 {
		return nil, fmt.Errorf("channel %s is not present on this keyboard", target.Requested[0].Subsystem())
	}
	return found, nil
}
  • Step 2: Replace the zone plumbing

In cmd/qmk-rgb-tool/rgb.go, replace forEachSelectedZone, zoneChannels and set*OnZones with channel plumbing:

func forEachChannel(channels []via.Channel, fn func(via.Channel) error) error {
	for _, ch := range channels {
		if err := fn(ch); err != nil {
			return err
		}
	}
	return nil
}

func setValueOnChannels(proto zoneProtocol, channels []via.Channel, param, value uint8) error {
	return forEachChannel(channels, func(ch via.Channel) error {
		return proto.SetValue(ch, param, value)
	})
}

The result types carry a name, because a name is what a user reads:

type zoneResult struct {
	Name      string
	Requested uint8
	Applied   uint8
}

type colorResult struct {
	Name                 string
	RequestedHue         uint8
	RequestedSaturation  uint8
	Hue                  uint8
	Saturation           uint8
}

Every set*Verified helper takes channels []via.Channel and display map[uint16]string, fills Name: channelName(ch, display), and formatResults prints r.Name.

  • Step 3: Rewrite the commands

Each follows the same shape. brightness.go:

		RunE: func(cmd *cobra.Command, args []string) error {
			val, err := ParseUint8(args[0])
			if err != nil {
				return err
			}

			proto, target, channels, err := openTarget()
			if err != nil {
				return err
			}
			defer proto.Close()

			results, err := setBrightnessVerified(proto, channels, target.Display, val)
			if err != nil {
				return fmt.Errorf("set brightness: %w", err)
			}

			if anyMismatch(results) {
				fmt.Fprintln(cmd.OutOrStdout(), formatResults("Brightness", results))
				return nil
			}
			fmt.Fprintf(cmd.OutOrStdout(), "Brightness set to %d\n", val)
			return nil
		},

speed.go and color.go follow it exactly, with their own value. mode.go takes the same shape, keeps its raw index and prints Mode set to index %d. disable.go writes effect 0 and brightness 0 to the resolved channels and needs no catalog. enable.go refuses a board with no catalog:

			catalog, _ := rgb.CatalogFor(target.Device.VendorID, target.Device.ProductID)
			for _, ch := range channels {
				if _, ok := catalog.DefaultEffect(ch); !ok {
					return fmt.Errorf("this keyboard has no effect catalog, so `enable` cannot choose an effect; set one with `mode <index>`")
				}
			}

info.go changes zoneInfo.Zone to string, keeps Channel as uint8, and readInfo takes channels []via.Channel plus display map[uint16]string:

func readInfo(proto infoGetter, channels []via.Channel, display map[uint16]string) (infoOutput, error) {
	out := infoOutput{Zones: make([]zoneInfo, 0, len(channels))}
	var queryErrors []error
	for _, ch := range channels {
		record, err := readZoneInfo(proto, ch, display)
		out.Zones = append(out.Zones, record)
		if err != nil {
			queryErrors = append(queryErrors, err)
		}
	}
	...
}

readZoneInfo sets record.Zone = channelName(ch, display) and reads with ch.

effect.go becomes catalog-driven. listAllEffects needs the catalog and the channel list, both of which come from the target and the probe, so the command opens the device only to detect and never writes:

func listAllEffects(cmd *cobra.Command, catalog *rgb.Catalog, channels []via.Channel, display map[uint16]string) error {
	type ZoneEffectList struct {
		Zone      string `json:"zone"`
		Channel   uint8  `json:"channel"`
		Subsystem string `json:"subsystem"`
		Effect    string `json:"effect"`
		ID        uint8  `json:"id"`
	}
	type EffectList struct {
		Catalog string           `json:"catalog"`
		Zones   []ZoneEffectList `json:"zones"`
	}

	list := EffectList{Catalog: catalog.Name(), Zones: []ZoneEffectList{}}
	for _, ch := range channels {
		for id, name := range catalog.Names(ch) {
			list.Zones = append(list.Zones, ZoneEffectList{
				Zone:      channelName(ch, display),
				Channel:   uint8(ch),
				Subsystem: ch.Subsystem(),
				Effect:    name,
				ID:        uint8(id),
			})
		}
	}
	return encodeJSON(cmd.OutOrStdout(), list)
}

For setting, resolveEffectTargets becomes:

func resolveEffectTargets(catalog *rgb.Catalog, name string, channels []via.Channel) ([]rgb.EffectTarget, []string, error) {
	if name == "static" {
		name = "solid"
	}
	return rgb.ResolveEffect(catalog, name, channels)
}
  • Step 4: Update the command tests

Every test that builds intrgb.Zone values must build via.Channel values and a display map. Give the existing fake the new method and a field for the channels it should report:

func (f *verifyingProtocol) DetectChannels() ([]via.Channel, error) {
	if f.channels == nil {
		return []via.Channel{via.ChannelRgblight, via.ChannelRgbMatrix, via.ChannelAudio}, nil
	}
	return f.channels, nil
}

and add channels []via.Channel to verifyingProtocol. Change the applied maps in brightness_verify_test.go, brightness_summary_test.go and color_verify_test.go to be keyed by via.Channel instead of via.LEDType. Change the helpers in brightness_summary_test.go and color_verify_test.go to pass map[uint16]string{2: "logo", 3: "backlight", 4: "side"} as the display map, so the expected output strings stay exactly as they are today.

  • Step 5: Run the suite until it is green

Run: go test ./... -count=1 Expected: PASS with no remaining references to intrgb.Zone.

Run: grep -rn "intrgb.Zone\|rgb.Zone\|LEDType" --include="*.go" . Expected: no output.

  • Step 6: Compare against the baseline
go build -o /tmp/opencode/baseline/qmk-rgb-after ./cmd/qmk-rgb-tool/
/tmp/opencode/baseline/qmk-rgb-after effect --list > /tmp/opencode/list-after.json
/tmp/opencode/baseline/qmk-rgb-after info > /tmp/opencode/info-after.json
diff /tmp/opencode/baseline/info.json /tmp/opencode/info-after.json
diff /tmp/opencode/baseline/list.json /tmp/opencode/list-after.json

Expected: info is byte-identical. effect --list differs only by the three added fields catalog, channel and subsystem, one per entry, and by the new top-level catalog. Record the exact diff in the commit message; if info differs in anything, stop and report rather than committing.

  • Step 7: Commit
git add cmd/ internal/via/
git commit -m "address detected channels instead of assumed zones"

Task 6: Profiles resolve zone names to channels

Files:

  • Modify: cmd/qmk-rgb-tool/profile.go
  • Modify: cmd/qmk-rgb-tool/profile_load_test.go

Interfaces:

  • Consumes: prepareTarget, openTarget, resolveZoneName, rgb.Catalog, rgb.ResolveEffect
  • Produces: unchanged profile file format; keys are display or subsystem names

  • [ ] Step 1: Write the failing test

Add to cmd/qmk-rgb-tool/profile_load_test.go:

// A board renamed after a profile was written leaves keys that resolve to
// nothing. Each one must be reported by name, not swallowed.
func TestLoadWarnsForEveryUnresolvableZoneKey(t *testing.T) {
	dir := t.TempDir()
	profile := Profile{
		Name:    "renamed",
		Version: 1,
		Zones: map[string]*ZoneSettings{
			"old-logo":      {Enabled: true, Effect: "light", Brightness: 100, Speed: 1, Color: "00ff"},
			"old-backlight": {Enabled: true, Effect: "wave", Brightness: 100, Speed: 1, Color: "00ff"},
		},
	}
	if err := writeProfileForTest(dir, "renamed", profile); err != nil {
		t.Fatalf("write profile: %v", err)
	}
	originalProfileDir := profileDirectory
	t.Cleanup(func() { profileDirectory = originalProfileDir })
	profileDirectory = func() (string, error) { return dir, nil }

	proto := &verifyingProtocol{applied: map[via.Channel]uint8{}}
	originalOpen := openRGBProtocol
	originalTarget := targetDevice
	originalZone := targetZone
	t.Cleanup(func() {
		openRGBProtocol = originalOpen
		targetDevice = originalTarget
		targetZone = originalZone
	})
	openRGBProtocol = func() (rgbProtocol, error) { return proto, nil }
	targetDevice = ""
	targetZone = ""

	var stderr bytes.Buffer
	cmd := NewProfileLoadCmd()
	cmd.SetOut(&bytes.Buffer{})
	cmd.SetErr(&stderr)
	cmd.SetArgs([]string{"renamed"})

	if err := cmd.Execute(); err != nil {
		t.Fatalf("load returned error: %v", err)
	}

	for _, want := range []string{"old-logo", "old-backlight"} {
		if !strings.Contains(stderr.String(), want) {
			t.Errorf("stderr = %q, want it to name the unresolved key %q", stderr, want)
		}
	}
	if len(proto.reports) != 0 {
		t.Errorf("reports = %v, want nothing written for unresolvable keys", proto.reports)
	}
}

Add the helper it needs, using whatever seam the file already has for the profile directory; if none exists, add one:

func writeProfileForTest(dir, name string, p Profile) error {
	data, err := json.MarshalIndent(p, "", "  ")
	if err != nil {
		return err
	}
	return os.WriteFile(filepath.Join(dir, name+".json"), data, 0o600)
}
  • Step 2: Run it to verify it fails

Run: go test ./cmd/qmk-rgb-tool/ -run TestLoadWarnsForEveryUnresolvable -count=1 Expected: FAIL, because the current code keys the profile by the zone type and never sees an unresolvable key.

  • Step 3: Make profile keys resolve

In cmd/qmk-rgb-tool/profile.go:

  • Profile.Zones becomes map[string]*ZoneSettings, keyed by the name as written in the file
  • on load, for each key call resolveZoneName(key, target.Display); a key that resolves to nothing produces Warning: profile %q names zone %q, which this keyboard does not have; skipping on stderr
  • a key that resolves to several channels applies to each of them
  • the iteration order is the sorted key order, which is what the current sort.Slice on zones already does
  • effect resolution uses rgb.ResolveEffect(catalog, settings.Effect, channels) and warns per channel when a name is not supported there
  • save writes channelName(ch, display) as the key, so a saved profile on the Impact 80 keeps the keys logo, backlight and side its existing files use

  • [ ] Step 4: Run the profile tests

Run: go test ./cmd/qmk-rgb-tool/ -run 'TestProfile|TestLoad|TestSave' -count=1 Expected: PASS.

  • Step 5: Verify a real round trip
./qmk-rgb-tool save roundtrip
./qmk-rgb-tool brightness 100 --zone backlight
./qmk-rgb-tool load roundtrip
./qmk-rgb-tool info

Expected: the brightness is back at the value in the profile, and profiles/roundtrip.json uses the keys logo, backlight and side. Delete the file afterwards.

  • Step 6: Commit
git add cmd/qmk-rgb-tool/profile.go cmd/qmk-rgb-tool/profile_load_test.go
git commit -m "resolve profile zone names against the connected keyboard"

Task 7: Documented behaviour

Files:

  • Modify: README.md
  • Modify: AGENTS.md

Interfaces:

  • Consumes: everything above
  • Produces: no code

  • [ ] Step 1: Rewrite the --zone documentation

In README.md, replace the paragraph beginning --zone accepts logo, backlight, or side with:

`--zone` accepts a VIA lighting channel: `backlight`, `rgblight`, `rgb_matrix`,
`audio` or `led_matrix`. The name follows from the channel number, so it works on
every QMK keyboard. A board listed in `keyboards.json` may give a channel a
display name, and that name is accepted too — the Impact 80 calls channels 2, 3
and 4 `logo`, `backlight` and `side`. Where a display name would be the subsystem
name of a different channel the keyboard has, the board is refused rather than
the command being sent to the wrong channel.

Without `--zone`, commands target every channel the keyboard reports, in channel
order. The keyboard is asked which channels it has: a channel its firmware does
not implement answers as unhandled and is skipped.
  • Step 2: Document the effect catalog

Add a section after "Color Notations":

## Effect Names Are Per Board

The keyboard holds effect numbers, not names. `effect <name>` therefore needs a
catalog, and the tool has one for the Impact 80: the 46 backlight names are
QMK's `rgb_matrix_effects.inc`, the 7 `logo` and 7 `side` names are that board's
vendor VIA definition, and the compatibility aliases are the tool's own.

A keyboard without a catalog is still driven: `brightness`, `speed`, `color`,
`mode <index>` and `info` all work, because none of them needs a name. Only
`effect <name>` and `effect --list` need the catalog, and they say so rather than
guessing.
  • Step 3: Correct the protocol section

In README.md's "Protocol" section, replace the lighting channel line with:

- Lighting channels are QMK's `id_qmk_*_channel` values: `0x01` backlight,
  `0x02` rgblight, `0x03` rgb_matrix, `0x04` audio, `0x05` led_matrix

and add that RGB values 0x01–0x04 are identical for every lighting subsystem, which is why the tool has no per-subsystem code path.

  • Step 4: Fix AGENTS.md

Replace the sentence "--zone accepts logo, backlight, or side" in the Device Selection section with a pointer to README.md, and replace the claim that the three zones are fixed with the fact that channels are probed. Add one line to the Firmware Transforms Values section: the effect names are a board's catalog in internal/rgb/catalog.go, transcribed from the two sources named in its comment, so nobody edits a list on a hunch.

  • Step 5: Check the documentation tests still pass

Run: go test ./cmd/qmk-rgb-tool/ -run TestAgentsDoc -count=1 Expected: PASS. The new AGENTS.md text must not name effects from the list the test forbids.

  • Step 6: Commit
git add README.md AGENTS.md
git commit -m "document channel discovery and the per-board effect catalog"

Task 8: Live verification against the Impact 80

Files:

  • none

Interfaces:

  • Consumes: everything above
  • Produces: the evidence that the change did not alter this board

  • [ ] Step 1: Confirm detection finds the three channels

qmk-rgb-tool info

Expected: three zones named logo, backlight and side, with channel 2, 3 and 4.

  • Step 2: Confirm both spellings reach the same channel
qmk-rgb-tool brightness 120 --zone backlight
qmk-rgb-tool info | grep -A 3 '"zone": "backlight"'
qmk-rgb-tool brightness 120 --zone rgb_matrix
qmk-rgb-tool info | grep -A 3 '"zone": "backlight"'

Expected: both reach channel 3.

  • Step 3: Confirm an unknown name fails before the device is opened
qmk-rgb-tool brightness 120 --zone nope; echo "exit=$?"

Expected: Error: unknown zone "nope": use a channel name such as rgb_matrix, or a name from keyboards.json and exit=1.

  • Step 4: Confirm the read-back reporting is unchanged
qmk-rgb-tool speed 60
qmk-rgb-tool brightness 200
qmk-rgb-tool color hsv:0,255,200

Expected: the summary lines in the same shape as before, each naming the zones and the request.

  • Step 5: Report the outcome

State in the final message which of the five Review Focus items were verified on the hardware and which were only covered by unit tests.