read effect and mode back, and correct what the catalog claims

Two commands reported success for a value the keyboard never took, and
the catalog claimed a provenance that measurement contradicts.

effect wrote an ID and printed "Effect set to" without asking again. On
the logo and side channels the firmware reads effect ID 0 as "lighting
off" and leaves the mode register where it was, so effect none left the
previous effect running while the command said it was set. mode has the
same shape in the other direction: the backlight channel does not refuse
an index above 46, it clamps it to 46, so mode 99 reported 99 and the
board held 46. Both now read the register back and name what each zone
holds, which is what brightness, speed and color have done since.

The read-back is shared, as AGENTS.md claimed, but not the way it
claimed: setValueVerified writes one value to every channel and cannot
carry an effect, because one name is a different index on each
subsystem. readBackValues is now the shared part and setEffectVerified
is the wrapper that needed.

The catalog's provenance was wrong. README claimed the 46 backlight
names match the QMK RGB Matrix effect catalog exactly. QMK master has 45
entries under other names and other numbering: alpha_mods against
alphas_mods, colorband_sat against band_sat, with pixel_fractal,
typing_heatmap and starlight_smooth among the names QMK has since
added. The list is the QMK catalog of the VIA era, transcribed from the
vendor's VIA JSON, and upstream is not a repair guide for it. The other
two channels are not QMK's at all: the lightweight rgblight subsystem
has about 49 modes that share no name with the seven on logo and side,
so a per-subsystem catalog taken from QMK would be wrong for them.

Writing a name and reading it back proves the ID is one the board takes,
never that the name labels it. All 46 backlight IDs are taken, and so are
logo and side IDs 1 to 6; ID 0 is not. The board takes a 47th ID, 46,
which nothing names, and reports it as unknown rather than inventing a
name for it.

The vendor's Driver & Firmware page is where all three lists come from,
and it settles two things this tool cannot. VIA sees the board in wired
mode only, so finding no keyboard is consistent with the hardware being
on wireless. The vendor also ships a proprietary firmware that disables
VIA entirely, so a board flashed with it is invisible here rather than
unsupported.

AGENTS.md

@@ -98,17 +98,37 @@ animation, 1 is the slowest movement, anything above 1 becomes 4 — while
 tabulates the per-channel behaviour; the rule that follows from it is the part
 that matters when you write code here:
 
-`brightness` and `speed` read every selected zone back and print what was
-actually applied; where all zones match they print `Brightness set to N` or
-`Speed set to N`, and where any zone differs they print 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. Both read back through the shared
-`setValueVerified`, so a future parameter needs no new read-back path.
+`brightness`, `speed`, `color`, `effect` and `mode` read every selected zone
+back and
+print what was actually applied; where all zones match they print the plain
+success line, and where any zone differs they print 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. All of them read back through the shared
+`readBackValues`, so a future parameter needs no new read-back path — but the
+helper above it is not as shared as it looks. `setValueVerified` writes one
+value to every channel, which fits `brightness` and `speed` and fits nothing
+else: `effect` carries a different ID per channel, because one effect name is a
+different index on each subsystem, and so has its own `setEffectVerified`.
+Assume the next parameter needs its own wrapper, and share only the read.
+
+A board can refuse an ID that looks valid. The Impact 80's `logo` and `side`
+channels read effect ID 0 as "lighting off" and leave the mode register alone,
+so `effect none` there does nothing to the effect and is reported as the effect
+still running. `disable` is the command that turns a channel off, because it
+also writes brightness 0. Above the top the behaviour is the opposite: the
+backlight channel does not refuse ID 47 or ID 99 but clamps it to the 46 it
+holds, so `mode` there is reported as the index the keyboard ended up with. Read
+the register back; never report the number that was asked for.
 
 The effect names are a board's catalog in `internal/rgb/catalog.go`, which
 transcribes the arrays `internal/rgb/impact80.go` holds from the two sources its
 comment names. They look like data the tool invented and
-are not; do not edit a list on a hunch, and do not add a second one.
+are not; do not edit a list on a hunch, and do not add a second one. The
+backlight list is the QMK catalog **of the VIA era**, not of current QMK master:
+the two differ in length, naming and numbering, so upstream is not a repair guide
+for it. The `logo` and `side` lists are not QMK's at all. The live register
+proves which IDs a board takes, never which name belongs to one, and the board
+takes one ID more (46 on the backlight) than the catalog names.
 
 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.
 
@@ -124,7 +144,14 @@ saturation and value. The vendor's VIA definition JSON for the board settles
 what the interface exposes at all — which custom value IDs exist, which ranges
 they take, and for which effects a color control is offered. It is the authority
 on the wire format, more than upstream is, because the ID mapping is the
-vendor's.
+vendor's. For the Impact 80 that JSON is
+`https://drive.wobkey.com/f/d/6BtO/Impact_80.JSON`, reached from the vendor's
+[Driver & Firmware page](https://wiki.wobkey.com/en/Products/PMOKEY-Impact-80/Driver-Firmware);
+the page is the place to re-read, because it also carries what a wrong flash does
+to this tool: the vendor ships a proprietary firmware alongside the VIA one, it
+disables VIA completely, and a board on it is invisible rather than unsupported.
+It also documents that VIA only sees the board in wired mode. A keyboard the
+tool cannot find is therefore not yet evidence of a bug here.
 
 The board's own firmware is a vendor binary with no public source, so both
 sources describe the family, not the unit on the desk. Confirm against the

README.md

@@ -193,12 +193,28 @@ a hex triple.
 ## Effect Names Are Per Board
 
 The keyboard holds effect numbers, not names, so `effect <name>` needs a catalog
-and the tool has one for the Impact 80. Its 46 backlight names are QMK's
-`rgb_matrix_effects.inc` in order, verified against the keyboard's register; the
-7 `logo` and 7 `side` names are that board's vendor VIA definition, whose
+and the tool has one for the Impact 80. Its 46 backlight names are the QMK
+`rgb_matrix_effects.inc` of the VIA era, transcribed from that board's vendor VIA
+definition, and the 7 `logo` and 7 `side` names are that same definition, whose
 dropdowns read `fixed wave` and `breathe`; the compatibility aliases are the
 tool's own.
 
+`effect` writes the ID and reads the register back, because a board can refuse
+one. All 46 backlight IDs are taken, and so are the `logo` and `side` IDs from 1
+to 6. ID 0 is not: on those two channels the firmware reads it as "lighting
+off" and leaves the mode register where it was, so `effect none --zone logo`
+leaves the previous effect running and says so.
+
+```
+$ qmk-rgb-tool effect none --zone logo
+Effect logo "wave" (1) (requested "none" (0))
+```
+
+The read-back proves an ID is one the keyboard takes, not that the name labels
+it correctly; the names come from the vendor definition. The board takes 47 IDs
+on the backlight channel, 0 to 46, of which this catalog names 46 — ID 46 exists,
+is reported as `unknown`, and is set with `mode 46`.
+
 A keyboard without a catalog is still driven: `brightness`, `speed`, `color`,
 `mode <index>` and `info` all work, because none of them needs a name. Four
 commands need the catalog and say so rather than guessing:
@@ -279,7 +295,7 @@ failure, but the summary line always states the value that was actually applied.
 - **Cross-platform** — Linux, macOS, Windows via [hidapi](https://github.com/libusb/hidapi)
 - **Channel-aware effects** — per-channel effect control with per-board effect catalogs, so a board without one is still driven
 - **Profile system** — save, load, list, and delete RGB presets as JSON files in `profiles/`
-- **46 backlight effects** — complete Impact 80 effect family mapped to zone-aware effect names
+- **46 backlight effect names** — the Impact 80's VIA-era QMK family, mapped to zone-aware names; the board takes 47 IDs, the highest has no name
 - **Compatibility aliases** — `off`, `breathe`, `rainbow`, `rainbow_wave`, `solid`, `static` resolve to correct effect IDs per channel
 - **Reactive & splash effects** — honor `color` and `speed` for key-press illumination
 - **Machine-parseable output** — JSON for `keyboard info`, `info`, `list`, and `effect --list`
@@ -365,14 +381,22 @@ for Logo/Side and `rainbow_moving_chevron` for Backlight, `rainbow_wave` →
 for Backlight. The legacy `static` alias remains accepted as a compatibility
 alias for `solid`.
 
-The effect catalog is consistent with the QMK RGB Matrix firmware. The
-keyboard implements the VIA protocol (version 3) and its 46 backlight effect
-names match the QMK RGB Matrix effect catalog exactly — verified through live
-testing. The animations are defined in
+The effect catalog follows the QMK RGB Matrix firmware of the VIA era, not
+current QMK master: the two lists differ in length, in naming and in numbering
+(`alpha_mods` against `alphas_mods`, `colorband_sat` against `band_sat`, and
+`pixel_fractal`, `typing_heatmap` and `starlight_smooth` among the names QMK has
+since added), so the catalog must not be repaired against upstream. The
+keyboard implements the VIA protocol (version 3). The animations are defined in
 [QMK `rgb_matrix/animations/`](https://github.com/qmk/qmk_firmware/tree/master/quantum/rgb_matrix/animations),
 where each effect has its own header file (e.g.
 `digital_rain_anim.h`, `solid_reactive_anim.h`, `riverflow_anim.h`).
 
+Its other two channels are not QMK's at all. QMK's lightweight `rgblight`
+subsystem has about 49 modes named `STATIC_LIGHT`, `RAINBOW_MOOD`, `SNAKE`,
+`KNIGHT`, `CHRISTMAS` and `TWINKLE`, and shares no name with the seven the Impact
+80 offers on `logo` and `side`. A per-subsystem catalog taken from QMK would
+therefore be wrong for those channels.
+
 ID 39 is `multisplash`; ID 41 is the distinct `solid_multisplash` name.
 
 `info` reports a `zones` array with each zone's channel, enabled state,
@@ -463,6 +487,36 @@ Two models are listed in `keyboards.json`:
 | Wobkey Rainy 75 | 0x6666 | 0x0001 |
 | Wobkey Impact 80 | 0x36B0 | 0x309F |
 
+### Impact 80 Sources
+
+The vendor's
+[Driver & Firmware page](https://wiki.wobkey.com/en/Products/PMOKEY-Impact-80/Driver-Firmware)
+is where the effect catalog in `internal/rgb/impact80.go` comes from. It is the
+authority on the wire format, and it settles two things this tool cannot:
+
+- The board must be in **wired mode** for VIA to see it. On 2.4G or Bluetooth
+  the keyboard is not detected at all, so a tool that finds no keyboard is
+  consistent with the hardware being on wireless. Changes made in wired mode are
+  saved to onboard memory and still apply in wireless mode.
+- The vendor ships **two firmware variants**. The VIA variant is what this tool
+  drives. The proprietary variant offers the advanced lighting (music rhythm,
+  SyncLight) but **disables VIA**, so a board flashed with it exposes no QMK Raw
+  HID interface and is invisible to the tool, not unsupported by it. The audio
+  channel this board reports as `side` is where that lighting lives.
+
+The page links the sources used here directly:
+
+| What | Where |
+|------|-------|
+| VIA definition JSON, the origin of all three effect lists | [`Impact_80.JSON`](https://drive.wobkey.com/f/d/6BtO/Impact_80.JSON) |
+| VIA firmware image | [`impact_80.bin`](https://drive.wobkey.com/f/d/9yH8/impact_80.bin) |
+| Update instructions, vendor's warning against updating a working board | [Driver & Firmware page](https://wiki.wobkey.com/en/Products/PMOKEY-Impact-80/Driver-Firmware) |
+
+Stock VIA exposes no way to ask a keyboard which effect IDs it implements, so
+the names come from that JSON and not from the board. The counts the board
+actually takes were measured here: 47 on the backlight channel, 6 on `logo` and
+`side`.
+
 ## CLI Reference
 
 | Command                         | Description                              |
@@ -471,7 +525,7 @@ Two models are listed in `keyboards.json`:
 | `qmk-rgb-tool enable`                 | Enable selected lighting zones            |
 | `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 effect <name>`          | Set a zone-aware effect by name, verified by read-back |
 | `qmk-rgb-tool effect`                 | With no argument, list every effect per channel (JSON) |
 | `qmk-rgb-tool effect --list`          | The same list, as a flag                   |
 | `qmk-rgb-tool brightness <val>`       | Set brightness (0–255) on selected zones, verified by read-back |
@@ -479,7 +533,7 @@ Two models are listed in `keyboards.json`:
 | `qmk-rgb-tool color <hex>`            | Set color (e.g. `ff0000`) on selected zones |
 | `qmk-rgb-tool color rgb:<hex>`        | The same hex color, written out |
 | `qmk-rgb-tool color hsv:<h>,<s>,<v>`  | Set hue and saturation (0–255) and write `v` to the brightness of the same zones |
-| `qmk-rgb-tool mode <index>`           | Set a raw zone-specific effect ID         |
+| `qmk-rgb-tool mode <index>`           | Set a raw zone-specific effect ID, verified by read-back; a board that does not implement the index clamps it to the highest it does |
 | `qmk-rgb-tool --zone <channel> ...`   | Target one channel: `backlight`, `rgblight`, `rgb_matrix`, `audio` or `led_matrix`, or a name from `keyboards.json` |
 | `qmk-rgb-tool --device <n> ...`      | Target keyboard by number (see `keyboard info`) |
 | `qmk-rgb-tool -v`, `--version`       | Print the version                          |

cmd/qmk-rgb-tool/commands_test.go

@@ -57,7 +57,15 @@ func (f *fakeZoneProtocol) SetColor(channel via.Channel, hue, saturation uint8)
 	return nil
 }
 
-func (f *fakeZoneProtocol) GetValue(via.Channel, uint8) ([]byte, error) {
+// GetValue reports the last value written to a channel, so a command that reads
+// back what it set sees a keyboard that accepted it. A channel that was never
+// written to still fails, so a read-back without a preceding set stays visible.
+func (f *fakeZoneProtocol) GetValue(channel via.Channel, param uint8) ([]byte, error) {
+	for i := len(f.reports) - 1; i >= 0; i-- {
+		if f.reports[i].channel == channel && f.reports[i].param == param {
+			return []byte{f.reports[i].value}, nil
+		}
+	}
 	return nil, errors.New("unexpected GetValue call")
 }
 

cmd/qmk-rgb-tool/effect.go

@@ -93,10 +93,17 @@ func runEffectSet(cmd *cobra.Command, args []string) error {
 		fmt.Fprintf(cmd.ErrOrStderr(), "Warning: effect not supported on channel(s): %s\n", strings.Join(skipped, ", "))
 	}
 
-	for _, t := range targets {
-		if err := proto.SetValue(t.Channel, uint8(intrgb.EffectID), t.ID); err != nil {
-			return fmt.Errorf("set effect on %s: %w", channelName(t.Channel, target.Display), err)
-		}
+	results, err := setEffectVerified(proto, targets, target.Display, catalog)
+	if err != nil {
+		return err
+	}
+
+	// Claiming the requested effect when the keyboard kept another one is the
+	// contradiction this read-back exists to prevent: on the logo and side
+	// channels an effect of ID 0 leaves the previous effect running.
+	if anyEffectMismatch(results) {
+		fmt.Fprintln(cmd.OutOrStdout(), formatEffectResults(args[0], results))
+		return nil
 	}
 
 	fmt.Fprintf(cmd.OutOrStdout(), "Effect set to %q\n", args[0])

cmd/qmk-rgb-tool/effect_test.go

@@ -115,7 +115,7 @@ func TestResolveEffectTargetsStaticCompatibility(t *testing.T) {
 	}
 }
 
-func executeEffectCommand(t *testing.T, protocol *fakeZoneProtocol, target string, args ...string) (string, string, error) {
+func executeEffectCommand(t *testing.T, protocol rgbProtocol, target string, args ...string) (string, string, error) {
 	t.Helper()
 	originalZone := targetZone
 	t.Cleanup(func() { targetZone = originalZone })

cmd/qmk-rgb-tool/effect_verify_test.go

@@ -0,0 +1,133 @@
+package main
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	intrgb "netdome.biz/paul/qmk-rgb/internal/rgb"
+	"netdome.biz/paul/qmk-rgb/internal/via"
+)
+
+// The Impact 80's logo and side channels do not accept effect ID 0: the
+// firmware reads it as "switch the lighting off" and leaves the mode register
+// on its previous value. A set of "none" therefore leaves the effect running
+// while the command reports success, which is the contradiction this read-back
+// exists to prevent.
+func TestSetEffectVerifiedReportsHeldEffect(t *testing.T) {
+	proto := &verifyingProtocol{applied: map[via.Channel]uint8{via.ChannelRgblight: 1}}
+
+	targets := []intrgb.EffectTarget{{Channel: via.ChannelRgblight, ID: 0}}
+	results, err := setEffectVerified(proto, targets, impact80Display(), impact80Catalog(t))
+	if err != nil {
+		t.Fatalf("setEffectVerified() error = %v", err)
+	}
+	if len(results) != 1 {
+		t.Fatalf("results = %d entries, want 1", len(results))
+	}
+
+	got := results[0]
+	if got.Name != "logo" {
+		t.Errorf("results[0].Name = %q, want %q", got.Name, "logo")
+	}
+	if got.RequestedID != 0 || got.RequestedName != "none" {
+		t.Errorf("results[0].Requested = %d/%q, want 0/%q", got.RequestedID, got.RequestedName, "none")
+	}
+	if got.AppliedID != 1 || got.AppliedName != "wave" {
+		t.Errorf("results[0].Applied = %d/%q, want 1/%q", got.AppliedID, got.AppliedName, "wave")
+	}
+	if !got.Mismatch() {
+		t.Error("results[0].Mismatch() = false, want true (keyboard kept effect 1)")
+	}
+}
+
+func TestSetEffectVerifiedMatchesWhenApplied(t *testing.T) {
+	proto := &verifyingProtocol{
+		applied: map[via.Channel]uint8{
+			via.ChannelRgblight:  1,
+			via.ChannelRgbMatrix: 28,
+			via.ChannelAudio:     1,
+		},
+	}
+
+	// "wave" is a different ID on every channel of this board, so the
+	// read-back has to compare per channel, not one value for all.
+	targets := []intrgb.EffectTarget{
+		{Channel: via.ChannelRgblight, ID: 1},
+		{Channel: via.ChannelRgbMatrix, ID: 28},
+		{Channel: via.ChannelAudio, ID: 1},
+	}
+	results, err := setEffectVerified(proto, targets, impact80Display(), impact80Catalog(t))
+	if err != nil {
+		t.Fatalf("setEffectVerified() error = %v", err)
+	}
+	if proto.getCalls != 3 {
+		t.Errorf("GetValue calls = %d, want 3", proto.getCalls)
+	}
+	for _, r := range results {
+		if r.Mismatch() {
+			t.Errorf("%s: Mismatch() = true, want false (requested %d, board holds %d)", r.Name, r.RequestedID, r.AppliedID)
+		}
+	}
+}
+
+// An effect the catalog does not name must be reported as unknown rather than
+// dropped, so a set to a raw index the catalog does not cover stays visible.
+func TestSetEffectVerifiedNamesUncataloguedEffect(t *testing.T) {
+	proto := &verifyingProtocol{applied: map[via.Channel]uint8{via.ChannelRgbMatrix: 46}}
+
+	targets := []intrgb.EffectTarget{{Channel: via.ChannelRgbMatrix, ID: 46}}
+	results, err := setEffectVerified(proto, targets, impact80Display(), impact80Catalog(t))
+	if err != nil {
+		t.Fatalf("setEffectVerified() error = %v", err)
+	}
+	if got := results[0].AppliedName; got != "unknown" {
+		t.Errorf("results[0].AppliedName = %q, want %q", got, "unknown")
+	}
+}
+
+func TestSetEffectVerifiedPropagatesReadError(t *testing.T) {
+	proto := &verifyingProtocol{getErr: errors.New("read timeout")}
+
+	targets := []intrgb.EffectTarget{{Channel: via.ChannelRgblight, ID: 1}}
+	if _, err := setEffectVerified(proto, targets, impact80Display(), impact80Catalog(t)); err == nil {
+		t.Fatal("setEffectVerified() expected read error, got nil")
+	}
+}
+
+// A keyboard that ignored the set must not be reported as having taken the
+// effect.
+func TestEffectCommandReportsHeldEffect(t *testing.T) {
+	proto := &verifyingProtocol{applied: map[via.Channel]uint8{via.ChannelRgblight: 1}}
+
+	stdout, _, err := executeEffectCommand(t, proto, "logo", "none")
+	if err != nil {
+		t.Fatalf("effect returned error: %v", err)
+	}
+	if strings.Contains(stdout, `Effect set to "none"`) {
+		t.Errorf("stdout = %q, want no success claim for an effect the keyboard kept", stdout)
+	}
+	for _, want := range []string{"logo", "wave", "none"} {
+		if !strings.Contains(stdout, want) {
+			t.Errorf("stdout = %q, want it to name %q", stdout, want)
+		}
+	}
+}
+
+func TestEffectCommandStaysQuietWhenApplied(t *testing.T) {
+	proto := &verifyingProtocol{
+		applied: map[via.Channel]uint8{
+			via.ChannelRgblight:  1,
+			via.ChannelRgbMatrix: 28,
+			via.ChannelAudio:     1,
+		},
+	}
+
+	stdout, _, err := executeEffectCommand(t, proto, "", "wave")
+	if err != nil {
+		t.Fatalf("effect returned error: %v", err)
+	}
+	if got, want := stdout, "Effect set to \"wave\"\n"; got != want {
+		t.Errorf("stdout = %q, want %q", got, want)
+	}
+}

cmd/qmk-rgb-tool/mode.go

@@ -12,7 +12,9 @@ func NewModeCmd() *cobra.Command {
 		Short: "Set effect mode by index",
 		Long: "Set the effect mode by numeric index (0-255). The index is the\n" +
 			"channel's own: 5 is light on rgblight and rainbow_beacon on rgb_matrix.\n" +
-			"Use `effect <name>` where a catalog exists, which is unambiguous.",
+			"Use `effect <name>` where a catalog exists, which is unambiguous.\n" +
+			"The index is read back, because a board that does not implement it\n" +
+			"clamps it to the highest it does instead of refusing it.",
 		Args: cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			val, err := ParseUint8(args[0])
@@ -20,16 +22,24 @@ func NewModeCmd() *cobra.Command {
 				return err
 			}
 
-			proto, _, channels, err := openTarget()
+			proto, target, channels, err := openTarget()
 			if err != nil {
 				return err
 			}
 			defer proto.Close()
 
-			if err := setModeOnChannels(proto, channels, val); err != nil {
+			results, err := setModeVerified(proto, channels, target.Display, val)
+			if err != nil {
 				return fmt.Errorf("set mode: %w", err)
 			}
 
+			// Claiming an index the keyboard clamped away is the same
+			// contradiction the other verified sets avoid.
+			if anyMismatch(results) {
+				fmt.Fprintln(cmd.OutOrStdout(), formatResults("Mode", results))
+				return nil
+			}
+
 			fmt.Fprintf(cmd.OutOrStdout(), "Mode set to index %d\n", val)
 			return nil
 		},

cmd/qmk-rgb-tool/mode_verify_test.go

@@ -0,0 +1,93 @@
+package main
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	intrgb "netdome.biz/paul/qmk-rgb/internal/rgb"
+	"netdome.biz/paul/qmk-rgb/internal/via"
+)
+
+// A raw index above what the board implements is not refused: the Impact 80
+// clamps it to the highest ID it holds, so the command would report an index the
+// keyboard never took. mode is the escape hatch for a board without a catalog,
+// which is exactly where an unverified success would hurt most.
+func TestModeCommandReportsClampedIndex(t *testing.T) {
+	proto := &verifyingProtocol{applied: map[via.Channel]uint8{via.ChannelRgbMatrix: 46}}
+
+	t.Cleanup(impact80Target(t, proto))
+
+	var out, errOut strings.Builder
+	cmd := NewModeCmd()
+	cmd.SetOut(&out)
+	cmd.SetErr(&errOut)
+	cmd.SetArgs([]string{"99"})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatalf("mode returned error: %v", err)
+	}
+
+	stdout := out.String()
+	if strings.Contains(stdout, "Mode set to index 99") {
+		t.Errorf("stdout = %q, want no success claim for an index the keyboard clamped", stdout)
+	}
+	for _, want := range []string{"backlight", "46", "99"} {
+		if !strings.Contains(stdout, want) {
+			t.Errorf("stdout = %q, want it to name %q", stdout, want)
+		}
+	}
+}
+
+// An index the board takes must stay a plain success line, so the summary is
+// reserved for a real divergence.
+func TestModeCommandStaysQuietWhenApplied(t *testing.T) {
+	proto := &verifyingProtocol{
+		applied: map[via.Channel]uint8{
+			via.ChannelRgblight:  5,
+			via.ChannelRgbMatrix: 20,
+			via.ChannelAudio:     5,
+		},
+	}
+
+	t.Cleanup(impact80Target(t, proto))
+
+	var out, errOut strings.Builder
+	cmd := NewModeCmd()
+	cmd.SetOut(&out)
+	cmd.SetErr(&errOut)
+	cmd.SetArgs([]string{"5"})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatalf("mode returned error: %v", err)
+	}
+	// Only the backlight stores 20, so two of three zones deviate and the
+	// summary is expected; a silent run would mean the read-back is skipped.
+	if out.Len() == 0 {
+		t.Error("stdout = \"\", want a divergence summary")
+	}
+}
+
+func TestSetModeVerifiedPropagatesReadError(t *testing.T) {
+	proto := &verifyingProtocol{getErr: errors.New("read timeout")}
+
+	_, err := setModeVerified(proto, []via.Channel{via.ChannelRgbMatrix}, impact80Display(), 46)
+	if err == nil {
+		t.Fatal("setModeVerified() expected read error, got nil")
+	}
+}
+
+// The mode command writes the effect value ID, the same one effect writes.
+func TestSetModeVerifiedUsesTheEffectValueID(t *testing.T) {
+	proto := &verifyingProtocol{applied: map[via.Channel]uint8{via.ChannelRgbMatrix: 46}}
+
+	if _, err := setModeVerified(proto, []via.Channel{via.ChannelRgbMatrix}, impact80Display(), 46); err != nil {
+		t.Fatalf("setModeVerified() error = %v", err)
+	}
+	if len(proto.reports) != 1 {
+		t.Fatalf("reports = %d, want 1", len(proto.reports))
+	}
+	if got := proto.reports[0].param; got != uint8(intrgb.EffectID) {
+		t.Errorf("param = 0x%02x, want 0x%02x", got, uint8(intrgb.EffectID))
+	}
+}

cmd/qmk-rgb-tool/rgb.go

@@ -115,6 +115,24 @@ func formatResults(label string, results []zoneResult) string {
 	return b.String()
 }
 
+// readBackValues reads one value ID from every channel, in the order given.
+// Every verified set reads back through this, so a parameter that needs no new
+// read-back path reuses it.
+func readBackValues(proto rgbProtocol, channels []via.Channel, display map[uint16]string, param uint8) ([]uint8, error) {
+	values := make([]uint8, 0, len(channels))
+	for _, ch := range channels {
+		raw, err := proto.GetValue(ch, param)
+		if err != nil {
+			return nil, fmt.Errorf("read back value 0x%02x for %s: %w", param, channelName(ch, display), err)
+		}
+		if len(raw) == 0 {
+			return nil, fmt.Errorf("read back value 0x%02x for %s: empty response", param, channelName(ch, display))
+		}
+		values = append(values, raw[0])
+	}
+	return values, nil
+}
+
 // setValueVerified writes one value ID to every zone and reads each back, so a
 // clamped or rescaled value is reported instead of silently claimed as set.
 // The firmware transform differs per channel and per value ID, so the read-back
@@ -124,16 +142,89 @@ func setValueVerified(proto rgbProtocol, channels []via.Channel, display map[uin
 		return nil, err
 	}
 
+	applied, err := readBackValues(proto, channels, display, param)
+	if err != nil {
+		return nil, err
+	}
+
 	results := make([]zoneResult, 0, len(channels))
-	for _, ch := range channels {
-		raw, err := proto.GetValue(ch, param)
-		if err != nil {
-			return nil, fmt.Errorf("read back value 0x%02x for %s: %w", param, channelName(ch, display), err)
+	for i, ch := range channels {
+		results = append(results, zoneResult{Name: channelName(ch, display), Requested: value, Applied: applied[i]})
+	}
+	return results, nil
+}
+
+// effectResult records the effect a zone was asked for and the one the keyboard
+// holds afterwards. The Impact 80's logo and side channels do not take effect ID
+// 0: the firmware reads it as "lighting off" and leaves the mode register where
+// it was, so the effect that is still running has to be named rather than
+// reported as the requested one.
+type effectResult struct {
+	Name          string
+	RequestedName string
+	RequestedID   uint8
+	AppliedName   string
+	AppliedID     uint8
+}
+
+// Mismatch reports whether the keyboard holds an effect other than the one
+// requested.
+func (r effectResult) Mismatch() bool { return r.AppliedID != r.RequestedID }
+
+// anyEffectMismatch reports whether any zone kept an effect of its own.
+func anyEffectMismatch(results []effectResult) bool {
+	for _, r := range results {
+		if r.Mismatch() {
+			return true
 		}
-		if len(raw) == 0 {
-			return nil, fmt.Errorf("read back value 0x%02x for %s: empty response", param, channelName(ch, display))
+	}
+	return false
+}
+
+// formatEffectResults names the effect each selected zone actually holds, so an
+// effect the keyboard refused is visible instead of being summarised as the one
+// that was asked for.
+func formatEffectResults(requestedName string, results []effectResult) string {
+	var b strings.Builder
+	b.WriteString("Effect")
+	for _, r := range results {
+		fmt.Fprintf(&b, " %s %q (%d)", r.Name, r.AppliedName, r.AppliedID)
+	}
+	if len(results) > 0 {
+		fmt.Fprintf(&b, " (requested %q (%d))", requestedName, results[0].RequestedID)
+	}
+	return b.String()
+}
+
+// setEffectVerified writes one effect ID per channel and reads every channel
+// back. It cannot reuse setValueVerified, because one effect name is a
+// different index on each subsystem, so the requested value is carried per
+// target while the read-back stays shared.
+func setEffectVerified(proto rgbProtocol, targets []intrgb.EffectTarget, display map[uint16]string, catalog *intrgb.Catalog) ([]effectResult, error) {
+	channels := make([]via.Channel, 0, len(targets))
+	requested := make(map[via.Channel]uint8, len(targets))
+	for _, t := range targets {
+		if err := proto.SetValue(t.Channel, uint8(intrgb.EffectID), t.ID); err != nil {
+			return nil, fmt.Errorf("set effect on %s: %w", channelName(t.Channel, display), err)
 		}
-		results = append(results, zoneResult{Name: channelName(ch, display), Requested: value, Applied: raw[0]})
+		channels = append(channels, t.Channel)
+		requested[t.Channel] = t.ID
+	}
+
+	applied, err := readBackValues(proto, channels, display, uint8(intrgb.EffectID))
+	if err != nil {
+		return nil, err
+	}
+
+	results := make([]effectResult, 0, len(channels))
+	for i, ch := range channels {
+		results = append(results, effectResult{
+			Name:          channelName(ch, display),
+			RequestedName: catalog.EffectName(ch, requested[ch]),
+			RequestedID:   requested[ch],
+			AppliedName:   catalog.EffectName(ch, applied[i]),
+			AppliedID:     applied[i],
+		})
 	}
 	return results, nil
 }
@@ -148,6 +239,14 @@ func setSpeedVerified(proto rgbProtocol, channels []via.Channel, display map[uin
 	return setValueVerified(proto, channels, display, uint8(intrgb.Speed), value)
 }
 
+// setModeVerified writes a raw effect index and reads every channel back. A
+// board that does not implement the index clamps it to the highest ID it holds
+// rather than refusing it, so the index the keyboard ended up with is the only
+// honest thing to print.
+func setModeVerified(proto rgbProtocol, channels []via.Channel, display map[uint16]string, value uint8) ([]zoneResult, error) {
+	return setValueVerified(proto, channels, display, uint8(intrgb.EffectID), value)
+}
+
 // colorResult records the hue and saturation one channel holds after a color was
 // written to it. The color value ID carries two bytes, so both components are
 // read back: a keyboard that stored another saturation must not be reported as

internal/rgb/catalog.go

@@ -28,11 +28,24 @@ type Catalog struct {
 //
 // 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 46 backlight names are the QMK rgb_matrix_effects.inc of the VIA era,
+//     in order, transcribed from this board's vendor VIA definition. They are not
+//     current QMK master, which has 45 entries under other names and other IDs,
+//     so the list must not be repaired against upstream. Each of the 46 IDs was
+//     taken by the keyboard when written; the board also takes ID 46, which no
+//     name in this list covers.
 //   - 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 keyboard takes these IDs only from 1 to 6: ID 0 means "lighting off"
+//     and leaves the mode register untouched.
+//
+// Both lists come from the VIA JSON the vendor's Driver & Firmware page links at
+// https://wiki.wobkey.com/en/Products/PMOKEY-Impact-80/Driver-Firmware
+// (the file itself: https://drive.wobkey.com/f/d/6BtO/Impact_80.JSON). That page
+// is also where the firmware variants are: the proprietary one offers the
+// advanced lighting but disables VIA, so a board running it has no Raw HID
+// interface and is invisible here rather than unsupported.
 //   - 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) {