fix: WinKeyer 2 sent the dit/dah ratio to the key-compensation command

The byte trace from a real WK2 settled in one line what no amount of reading could: "TX 11 32". Command 0x11 is SET KEY COMPENSATION in milliseconds, not the dit/dah ratio — that is 0x17. So a neutral ratio of 50 asked for 50 ms of extra key-down on EVERY element. At 25 wpm a dit is 48 ms, so elements more than doubled and ran into each other: the reported "it sends one element, then a long pause". The ratio now goes out as 0x17, and the compensation is explicitly set to 0 — merely stopping the wrong command would leave an affected keyer misbehaving, since it keeps the value in EEPROM until something writes over it. The init sequence is now built by a separate function so the BYTES are testable, and a test pins each command number. These numbers are the contract with the hardware, and a wrong one produces a fault that cannot be diagnosed from the UI at all — this one cost the operator weeks and needed a trace to find. This is why the fix waited for the trace rather than being guessed: the plausible guesses (mode register, sidetone, WK1-vs-WK2 differences) were all wrong, and any of them shipped blind would have broken the keyers that work today.
2026-07-29 14:53:18 +02:00
parent c7737aabd6
commit 15fdbce22a
3 changed files with 98 additions and 5 deletions
@@ -0,0 +1,71 @@
+package winkeyer
+
+import "testing"
+
+// The init sequence sent to a K1EL keyer, byte for byte.
+//
+// This exists because of a bug that no amount of reading caught and that a
+// byte-level trace from a real WK2 settled in one line: the dit/dah ratio was
+// sent as command 0x11, which is SET KEY COMPENSATION in milliseconds. A
+// neutral ratio of 50 therefore asked for 50 ms of extra key-down on every
+// element — at 25 wpm a dit is 48 ms — so elements more than doubled and ran
+// into each other. The operator saw "one element, then it stalls".
+//
+// The command NUMBERS are the contract with the hardware. A test on the values
+// is the only thing standing between a typo here and a keyer that misbehaves in
+// a way nobody can debug from the UI.
+func TestApplyConfigCommands(t *testing.T) {
+	cmds := configCommands(Config{
+		WPM: 25, Weight: 50, Ratio: 50, LeadInMs: 0, TailMs: 0, Sidetone: 0,
+	})
+
+	// Every command that must be present, with the value it must carry.
+	want := map[byte][]byte{
+		0x02: {25},   // speed
+		0x03: {50},   // weighting — 50 is neutral, and this one WAS right
+		0x17: {50},   // dit/dah ratio — the corrected opcode
+		0x11: {0x00}, // key compensation explicitly cleared
+	}
+	seen := map[byte][]byte{}
+	for _, c := range cmds {
+		if len(c) < 2 {
+			t.Fatalf("command %X has no argument", c)
+		}
+		seen[c[0]] = c[1:]
+	}
+	for op, args := range want {
+		got, ok := seen[op]
+		if !ok {
+			t.Errorf("command 0x%02X is not sent at all", op)
+			continue
+		}
+		if len(got) != len(args) || (len(args) > 0 && got[0] != args[0]) {
+			t.Errorf("command 0x%02X carries % X, want % X", op, got, args)
+		}
+	}
+
+	// The ratio must NEVER go out as 0x11 again: that is the whole bug.
+	if v, ok := seen[0x11]; ok && len(v) > 0 && v[0] != 0 {
+		t.Errorf("0x11 (key compensation) carries %d — it must be 0, not the ratio", v[0])
+	}
+}
+
+// A keyer left with compensation by an earlier version keeps it in EEPROM, so
+// the fix has to CLEAR it rather than merely stop setting it.
+func TestKeyCompensationIsCleared(t *testing.T) {
+	for _, ratio := range []int{33, 50, 66} {
+		cmds := configCommands(Config{WPM: 20, Weight: 50, Ratio: ratio})
+		found := false
+		for _, c := range cmds {
+			if c[0] == 0x11 {
+				found = true
+				if c[1] != 0 {
+					t.Errorf("ratio %d: key compensation sent as %d, want 0", ratio, c[1])
+				}
+			}
+		}
+		if !found {
+			t.Errorf("ratio %d: key compensation is never cleared", ratio)
+		}
+	}
+}