verify brightness by read-back instead of claiming the requested value

`brightness 200` printed "Brightness set to 200" on every zone, but the
Impact 80 firmware does not store 200. Measured on real hardware:

  logo/side     clamp at 160
  backlight     scales up, saturating at 255 (50->79, 100->159, 200->255)
  speed, logo/side  any value above 0 becomes 4

So a caller scripting this tool was told it had set a value the keyboard
never held, contradicting AGENTS.md rule 6 and rule 8. Set and get are not
the suspect here: both write report[1]=ledType and report[2]=param, and
readResponse verifies the response matches the request, so the transform is
firmware-side.

brightness now reads every selected zone back. Where all zones applied the
request it keeps the exact "Brightness set to N" message. Where any zone
deviates it prints a single line naming what each zone actually holds:

  $ qmk-rgb-tool brightness 200
  Brightness logo 160 backlight 255 side 160 (requested 200)

Exit stays 0: a value the firmware cannot represent is not a failure, but the
line always states what was applied, so a script reading stdout sees the real
value instead of a claim.

Also, to make the warning testable: brightness.go moved from Run +
os.Stderr + os.Exit to RunE with the cobra writers. This also fixes a real
resource leak in the same file — the deferred proto.Close() could never run
because os.Exit skips deferred calls, so every failing brightness invocation
leaked the HID handle. It now uses openRGBProtocol(), the seam the other
commands and their tests already use.

speed has the same defect and a worse shape (all values >0 collapse to 4 on
logo/side) but is deliberately not touched yet: it would warn on nearly every
call. Documented as unverified in README.md and AGENTS.md until it gets the
same treatment. The read-back and reporting helpers are parameter-agnostic, so
applying it to speed is a small change.

AGENTS.md

@@ -20,8 +20,8 @@ qmk-rgb-tool keyboard info
 qmk-rgb-tool effect breathing
 qmk-rgb-tool effect rainbow_moving_chevron
 qmk-rgb-tool effect rainbow_moving_chevron --zone backlight
-qmk-rgb-tool brightness <val>       # 0-255
-qmk-rgb-tool speed <val>            # 0-255
+qmk-rgb-tool brightness <val>       # 0-255, verified by read-back
+qmk-rgb-tool speed <val>            # 0-255, not read back
 qmk-rgb-tool color <hex>            # Six hexadecimal digits
 qmk-rgb-tool mode <index>           # Raw zone-specific effect ID
 qmk-rgb-tool enable
@@ -138,6 +138,20 @@ Documented behavior must also match the code:
 7. **JSON output shape** — field names, types, and whether a field is always present, omitted, or `null`. Agents and scripts parse this.
 8. **Ranges, defaults, and stability** — accepted ranges, default values, boundary behavior, and any ordering or stability guarantee together with the scope it holds over (a device number is stable for one session, not across reboots).
 
+## Firmware Transforms Values
+
+The Impact 80 firmware rescales or clamps brightness and speed per channel, so
+an accepted 0–255 request is not the value the keyboard holds: `logo` and
+`side` cap brightness at 160 and collapse any speed above 0 to 4, while
+`backlight` scales brightness up to 255 and applies speed as given.
+
+`brightness` therefore reads every selected zone back and prints what was
+actually applied; where all zones match it prints `Brightness set to N`, and
+where any zone differs it prints one summary line naming each zone's real value
+and the request. Never let a command report success for a value the keyboard did
+not accept. `speed` does not read back yet and still reports the requested
+value — do not document it as verified.
+
 When you see a name or a behavior in one file, grep for it across the whole repo before deciding if a change is consistent.
 
 ## Code vs Documentation

README.md

@@ -67,7 +67,6 @@ command skips unsupported zones and prints a warning on stderr.
 All output is machine-parseable JSON when applicable.
 
 ## Selecting a Keyboard
-
 `keyboard info` numbers every connected QMK keyboard starting at 1, and
 `--device` takes that number:
 
@@ -86,6 +85,31 @@ 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.
 
+## Brightness and Speed Are Not Applied Verbatim
+
+The keyboard's firmware transforms these values per channel, so the accepted
+range is 0–255 but the value the keyboard ends up holding is often different.
+`brightness` reads every zone back and reports what it actually applied:
+
+```console
+$ qmk-rgb-tool brightness 200
+Brightness logo 160 backlight 255 side 160 (requested 200)
+$ qmk-rgb-tool brightness 160 --zone logo
+Brightness set to 160
+```
+
+Observed on the Impact 80:
+
+| Zone | Channel | Brightness | Speed |
+|---|---|---|---|
+| `logo` | `0x02` | clamps at 160 | any value above 0 becomes 4 |
+| `backlight` | `0x03` | scales up, saturating at 255 | applied as given |
+| `side` | `0x04` | clamps at 160 | any value above 0 becomes 4 |
+
+The command exits 0 either way: a value the firmware cannot represent is not a
+failure, but the summary line always states the value that was actually applied.
+`speed` is not yet read back and still reports the requested value.
+
 ## Features
 
 - **Cross-platform** — Linux, macOS, Windows via [hidapi](https://github.com/libusb/hidapi)
@@ -264,7 +288,7 @@ Two models are listed in `keyboards.json`:
 | `qmk-rgb-tool disable`                | Disable selected lighting zones           |
 | `qmk-rgb-tool info`                   | Show per-zone RGB state (JSON)            |
 | `qmk-rgb-tool effect <name>`          | Set a zone-aware effect by name           |
-| `qmk-rgb-tool brightness <val>`       | Set brightness (0–255) on selected zones  |
+| `qmk-rgb-tool brightness <val>`       | Set brightness (0–255) on selected zones, verified by read-back |
 | `qmk-rgb-tool speed <val>`            | Set effect speed (0–255) on selected zones |
 | `qmk-rgb-tool color <hex>`            | Set color (e.g. `ff0000`) on selected zones |
 | `qmk-rgb-tool mode <index>`           | Set a raw zone-specific effect ID         |

cmd/qmk-rgb-tool/brightness.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"fmt"
-	"os"
 
 	"github.com/spf13/cobra"
 )
@@ -11,34 +10,40 @@ func NewBrightnessCmd() *cobra.Command {
 	return &cobra.Command{
 		Use:   "brightness <val>",
 		Short: "Set RGB brightness",
-		Long:  "Set the RGB brightness value (0-255).",
-		Args:  cobra.ExactArgs(1),
-		Run: func(cmd *cobra.Command, args []string) {
+		Long: "Set the RGB brightness value (0-255) and read it back, so a value the\n" +
+			"keyboard clamps or rescales is reported instead of silently applied.",
+		Args: cobra.ExactArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
 			val, err := ParseUint8(args[0])
 			if err != nil {
-				fmt.Fprintf(os.Stderr, "Error: %v\n", err)
-				os.Exit(1)
+				return err
 			}
 
 			zones, err := selectedZones()
 			if err != nil {
-				fmt.Fprintf(os.Stderr, "Error: %v\n", err)
-				os.Exit(1)
+				return err
 			}
 
-			proto, err := OpenDevice()
+			proto, err := openRGBProtocol()
 			if err != nil {
-				fmt.Fprintf(os.Stderr, "Error: %v\n", err)
-				os.Exit(1)
+				return err
 			}
 			defer proto.Close()
 
-			if err := setBrightnessOnZones(proto, zones, val); err != nil {
-				fmt.Fprintf(os.Stderr, "Error setting brightness: %v\n", err)
-				os.Exit(1)
+			results, err := setBrightnessVerified(proto, zones, val)
+			if err != nil {
+				return fmt.Errorf("set brightness: %w", err)
+			}
+
+			// Claiming "set to N" when the keyboard holds something else is
+			// the contradiction this read-back exists to prevent.
+			if anyMismatch(results) {
+				fmt.Fprintln(cmd.OutOrStdout(), formatResults("Brightness", results))
+				return nil
 			}
 
-			fmt.Printf("Brightness set to %d\n", val)
+			fmt.Fprintf(cmd.OutOrStdout(), "Brightness set to %d\n", val)
+			return nil
 		},
 	}
 }

cmd/qmk-rgb-tool/brightness_summary_test.go

@@ -0,0 +1,115 @@
+package main
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+
+	"netdome.biz/paul/impact-80/internal/via"
+)
+
+func runBrightness(t *testing.T, applied map[via.LEDType]uint8, zoneFlag string, arg string) (stdout, stderr string) {
+	t.Helper()
+
+	proto := &verifyingProtocol{applied: applied}
+
+	originalOpen := openRGBProtocol
+	originalZone := targetZone
+	t.Cleanup(func() {
+		openRGBProtocol = originalOpen
+		targetZone = originalZone
+	})
+	openRGBProtocol = func() (rgbProtocol, error) { return proto, nil }
+	targetZone = zoneFlag
+
+	var out, errOut bytes.Buffer
+	cmd := NewBrightnessCmd()
+	cmd.SetOut(&out)
+	cmd.SetErr(&errOut)
+	cmd.SetArgs([]string{arg})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatalf("brightness returned error: %v", err)
+	}
+	return out.String(), errOut.String()
+}
+
+// A success message that says "set to 200" while the keyboard holds 160 is a
+// contradiction. Where every zone applied the request, the plain message is
+// accurate and stays.
+func TestBrightnessReportsPlainMessageWhenAllZonesMatch(t *testing.T) {
+	stdout, stderr := runBrightness(t, map[via.LEDType]uint8{via.RGBLight: 200}, "logo", "200")
+
+	if strings.TrimSpace(stdout) != "Brightness set to 200" {
+		t.Errorf("stdout = %q, want the exact success message", stdout)
+	}
+	if stderr != "" {
+		t.Errorf("stderr = %q, want nothing on stderr when the request was met", stderr)
+	}
+}
+
+// Where a zone differs, one line must state what each zone actually holds.
+func TestBrightnessSummarisesAppliedValuesOnMismatch(t *testing.T) {
+	stdout, _ := runBrightness(t, map[via.LEDType]uint8{
+		via.RGBLight:  160,
+		via.RGBMatrix: 255,
+		via.SideLight: 160,
+	}, "", "200")
+
+	for _, want := range []string{"logo 160", "backlight 255", "side 160", "requested 200"} {
+		if !strings.Contains(stdout, want) {
+			t.Errorf("stdout = %q, want it to contain %q", stdout, want)
+		}
+	}
+	if strings.Contains(stdout, "set to 200") {
+		t.Errorf("stdout = %q, must not claim 200 was set when it was not", stdout)
+	}
+	if strings.Count(strings.TrimSpace(stdout), "\n") != 0 {
+		t.Errorf("stdout = %q, want exactly one line", stdout)
+	}
+}
+
+func TestBrightnessSummarisesASingleMismatchingZone(t *testing.T) {
+	// backlight lives on the RGBMatrix channel, not RGBLight.
+	stdout, _ := runBrightness(t, map[via.LEDType]uint8{via.RGBMatrix: 159}, "backlight", "100")
+
+	if !strings.Contains(stdout, "backlight 159") || !strings.Contains(stdout, "requested 100") {
+		t.Errorf("stdout = %q, want the single zone's applied value and the request", stdout)
+	}
+	if strings.Contains(stdout, "logo") || strings.Contains(stdout, "side") {
+		t.Errorf("stdout = %q, want only the selected zone named", stdout)
+	}
+}
+
+// The summary must not invent zones the user did not select.
+func TestBrightnessSummaryNamesOnlySelectedZones(t *testing.T) {
+	proto := &verifyingProtocol{applied: map[via.LEDType]uint8{via.SideLight: 160}}
+
+	originalOpen := openRGBProtocol
+	originalZone := targetZone
+	t.Cleanup(func() {
+		openRGBProtocol = originalOpen
+		targetZone = originalZone
+	})
+	openRGBProtocol = func() (rgbProtocol, error) { return proto, nil }
+	targetZone = "side"
+
+	var out bytes.Buffer
+	cmd := NewBrightnessCmd()
+	cmd.SetOut(&out)
+	cmd.SetErr(&out)
+	cmd.SetArgs([]string{"200"})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatalf("brightness returned error: %v", err)
+	}
+
+	if !strings.Contains(out.String(), "side 160") {
+		t.Errorf("stdout = %q, want side reported", out.String())
+	}
+	for _, unwanted := range []string{"logo", "backlight"} {
+		if strings.Contains(out.String(), unwanted) {
+			t.Errorf("stdout = %q, must not mention %s when only side was selected", out.String(), unwanted)
+		}
+	}
+}

cmd/qmk-rgb-tool/brightness_verify_test.go

@@ -0,0 +1,168 @@
+package main
+
+import (
+	"bytes"
+	"errors"
+	"strings"
+	"testing"
+
+	intrgb "netdome.biz/paul/impact-80/internal/rgb"
+	"netdome.biz/paul/impact-80/internal/via"
+)
+
+// verifyingProtocol records writes and reports what the keyboard actually
+// applied, which on real hardware is not always the requested value.
+type verifyingProtocol struct {
+	reports  []commandReport
+	applied  map[via.LEDType]uint8
+	getErr   error
+	getCalls int
+}
+
+func (f *verifyingProtocol) SetValue(channel via.LEDType, param, value uint8) error {
+	f.reports = append(f.reports, commandReport{channel: channel, param: param, value: value})
+	return nil
+}
+
+func (f *verifyingProtocol) SetColor(via.LEDType, uint8, uint8) error { return nil }
+
+func (f *verifyingProtocol) GetValue(channel via.LEDType, _ uint8) ([]byte, error) {
+	f.getCalls++
+	if f.getErr != nil {
+		return nil, f.getErr
+	}
+	return []byte{f.applied[channel]}, nil
+}
+
+func (f *verifyingProtocol) Close() error { return nil }
+
+func appliedFor(p *verifyingProtocol, channel via.LEDType) uint8 { return p.applied[channel] }
+
+func TestSetBrightnessVerifiedReportsAppliedValue(t *testing.T) {
+	tests := []struct {
+		name        string
+		requested   uint8
+		applied     uint8
+		wantWarning bool
+	}{
+		{"keyboard honours the value", 100, 100, false},
+		{"keyboard clamps to 160", 200, 160, true},
+		{"keyboard scales up", 100, 159, true},
+		{"both zones clamp", 255, 160, true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			proto := &verifyingProtocol{
+				applied: map[via.LEDType]uint8{
+					via.RGBLight:  tt.applied,
+					via.RGBMatrix: tt.applied,
+					via.SideLight: tt.applied,
+				},
+			}
+
+			zones := []intrgb.Zone{intrgb.ZoneLogo}
+			results, err := setBrightnessVerified(proto, zones, tt.requested)
+			if err != nil {
+				t.Fatalf("setBrightnessVerified() error = %v", err)
+			}
+			if len(results) != 1 {
+				t.Fatalf("results = %d entries, want 1", len(results))
+			}
+
+			got := results[0]
+			if got.Zone != intrgb.ZoneLogo {
+				t.Errorf("results[0].Zone = %q, want %q", got.Zone, intrgb.ZoneLogo)
+			}
+			if got.Requested != tt.requested {
+				t.Errorf("results[0].Requested = %d, want %d", got.Requested, tt.requested)
+			}
+			if got.Applied != tt.applied {
+				t.Errorf("results[0].Applied = %d, want %d", got.Applied, tt.applied)
+			}
+			if got.Mismatch() != tt.wantWarning {
+				t.Errorf("results[0].Mismatch() = %t, want %t", got.Mismatch(), tt.wantWarning)
+			}
+		})
+	}
+}
+
+// The read-back must happen once per written zone, on the right channel.
+func TestSetBrightnessVerifiedReadsBackEveryZone(t *testing.T) {
+	proto := &verifyingProtocol{
+		applied: map[via.LEDType]uint8{
+			via.RGBLight:  160,
+			via.RGBMatrix: 255,
+			via.SideLight: 160,
+		},
+	}
+
+	results, err := setBrightnessVerified(proto, intrgb.AllZones(), 200)
+	if err != nil {
+		t.Fatalf("setBrightnessVerified() error = %v", err)
+	}
+	if len(results) != 3 {
+		t.Fatalf("results = %d entries, want 3", len(results))
+	}
+	if proto.getCalls != 3 {
+		t.Errorf("GetValue calls = %d, want 3", proto.getCalls)
+	}
+
+	// A set that fails must not be reported as applied.
+	if got := appliedFor(proto, via.RGBLight); got != 160 {
+		t.Errorf("unexpected applied value %d", got)
+	}
+
+	want := map[intrgb.Zone]uint8{
+		intrgb.ZoneLogo:      160,
+		intrgb.ZoneBacklight: 255,
+		intrgb.ZoneSide:      160,
+	}
+	for _, r := range results {
+		if r.Applied != want[r.Zone] {
+			t.Errorf("%s applied = %d, want %d", r.Zone, r.Applied, want[r.Zone])
+		}
+		if !r.Mismatch() {
+			t.Errorf("%s: Mismatch() = false, want true (requested 200)", r.Zone)
+		}
+	}
+}
+
+// A failed read-back must not be silently reported as success.
+func TestSetBrightnessVerifiedPropagatesReadError(t *testing.T) {
+	proto := &verifyingProtocol{getErr: errors.New("read timeout")}
+
+	if _, err := setBrightnessVerified(proto, []intrgb.Zone{intrgb.ZoneLogo}, 100); err == nil {
+		t.Fatal("setBrightnessVerified() expected read error, got nil")
+	}
+}
+
+func TestBrightnessCommandStaysQuietWhenApplied(t *testing.T) {
+	proto := &verifyingProtocol{applied: map[via.LEDType]uint8{via.RGBLight: 200}}
+
+	originalOpen := openRGBProtocol
+	originalZone := targetZone
+	t.Cleanup(func() {
+		openRGBProtocol = originalOpen
+		targetZone = originalZone
+	})
+	openRGBProtocol = func() (rgbProtocol, error) { return proto, nil }
+	targetZone = "logo"
+
+	var out, errOut bytes.Buffer
+	cmd := NewBrightnessCmd()
+	cmd.SetOut(&out)
+	cmd.SetErr(&errOut)
+	cmd.SetArgs([]string{"200"})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatalf("brightness returned error: %v", err)
+	}
+
+	if errOut.Len() != 0 {
+		t.Errorf("stderr = %q, want no warning when the keyboard applied the requested value", errOut.String())
+	}
+	if !strings.Contains(out.String(), "200") {
+		t.Errorf("stdout = %q, want the applied value reported", out.String())
+	}
+}

cmd/qmk-rgb-tool/rgb.go

@@ -3,6 +3,7 @@ package main
 import (
 	"fmt"
 	"strconv"
+	"strings"
 
 	intdevice "netdome.biz/paul/impact-80/internal/device"
 	intrgb "netdome.biz/paul/impact-80/internal/rgb"
@@ -44,6 +45,66 @@ func setBrightnessOnZones(proto zoneProtocol, zones []intrgb.Zone, value uint8)
 	return setValueOnZones(proto, zones, uint8(intrgb.Brightness), value)
 }
 
+// zoneResult records what was requested for a zone and what the keyboard
+// reported afterwards. The two differ on real hardware: the Impact 80 clamps
+// brightness at 160 on the logo and side channels and scales it up to 255 on
+// the backlight channel.
+type zoneResult struct {
+	Zone      intrgb.Zone
+	Requested uint8
+	Applied   uint8
+}
+
+// Mismatch reports whether the keyboard applied something other than the
+// requested value.
+func (r zoneResult) Mismatch() bool { return r.Applied != r.Requested }
+
+// anyMismatch reports whether any zone deviated from the request.
+func anyMismatch(results []zoneResult) bool {
+	for _, r := range results {
+		if r.Mismatch() {
+			return true
+		}
+	}
+	return false
+}
+
+// formatResults renders one line stating what each selected zone actually
+// holds, so a partial application is visible instead of being summarised as
+// the value that was asked for.
+func formatResults(label string, results []zoneResult) string {
+	var b strings.Builder
+	b.WriteString(label)
+	for _, r := range results {
+		fmt.Fprintf(&b, " %s %d", r.Zone, r.Applied)
+	}
+	if len(results) > 0 {
+		fmt.Fprintf(&b, " (requested %d)", results[0].Requested)
+	}
+	return b.String()
+}
+
+// setBrightnessVerified writes the brightness and reads every zone back, so a
+// clamped or rescaled value is reported instead of silently claimed as set.
+func setBrightnessVerified(proto rgbProtocol, zones []intrgb.Zone, value uint8) ([]zoneResult, error) {
+	if err := setBrightnessOnZones(proto, zones, value); err != nil {
+		return nil, err
+	}
+
+	results := make([]zoneResult, 0, len(zones))
+	for _, zone := range zones {
+		raw, err := proto.GetValue(via.LEDType(zone.Channel()), uint8(intrgb.Brightness))
+		if err != nil {
+			return nil, fmt.Errorf("read back brightness for %s: %w", zone, err)
+		}
+		if len(raw) == 0 {
+			return nil, fmt.Errorf("read back brightness for %s: empty response", zone)
+		}
+		results = append(results, zoneResult{Zone: zone, Requested: value, Applied: raw[0]})
+	}
+	return results, nil
+}
+
 func setSpeedOnZones(proto zoneProtocol, zones []intrgb.Zone, value uint8) error {
 	return setValueOnZones(proto, zones, uint8(intrgb.Speed), value)
 }