fix: restore AVR strip output and avoid late-frame idle time

.github/workflows/firmware.yml

@@ -101,6 +101,11 @@ jobs:
           --clean
           --version "$PCCONTROLLER_VERSION"
 
+      - name: "Verify linked strip output waveform"
+        run: >-
+          node Tools/Firmware/verify-strip-waveform.mjs
+          .build/firmware/PCController.ino_atmega328p_16000000L.lst
+
       - name: "💾 Assert the safe 1 KiB default EEPROM image"
         run: >-
           node .github/scripts/assert-firmware-defaults.mjs

Project/AddressableLeds.cpp

@@ -92,9 +92,12 @@ void show() {
       "nop" "\n\t"
       "sbiw %[count], 1" "\n\t"
       "brne head20%=" "\n"
-      : [port] "+e"(port), [byte] "+r"(byte), [bit] "+r"(bit),
-        [next] "+r"(next), [count] "+w"(count)
-      : [ptr] "e"(ptr), [hi] "r"(hi), [lo] "r"(lo));
+      // The loop overwrites these operands before its final use of hi/lo/port.
+      // Early-clobber prevents next (initially lo) from sharing lo's register.
+      : [byte] "+&r"(byte), [bit] "+&d"(bit), [next] "+&r"(next),
+        [count] "+&w"(count), [ptr] "+&e"(ptr)
+      : [port] "e"(port), [hi] "r"(hi), [lo] "r"(lo)
+      : "memory");
   SREG = savedSreg;
   _delay_us(80);
 #elif defined(PCCONTROLLER_NATIVE)

Tools/Controller/internal/control/strip.go

@@ -298,7 +298,9 @@ func (outputs *OutputScheduler) streamStripRainbow(ctx context.Context, count, f
 func nextStripFrameTime(previous, now time.Time, period time.Duration) time.Time {
 	next := previous.Add(period)
 	if !next.After(now) {
-		next = next.Add((now.Sub(next)/period + 1) * period)
+		// A late transfer already consumed the frame budget. Render current
+		// animation time immediately and rebase, without a catch-up burst.
+		next = now
 	}
 	return next
 }

Tools/Controller/internal/control/strip_test.go

@@ -120,14 +120,21 @@ func TestStripStreamRecoversAfterTransientFrameTimeout(t *testing.T) {
 	}
 }
 
-func TestNextStripFrameTimeSkipsBufferedIntervals(t *testing.T) {
+func TestNextStripFrameTimeRebasesLateTransfers(t *testing.T) {
 	period := 50 * time.Millisecond
 	previous := time.Unix(100, 0)
 	if got, want := nextStripFrameTime(previous, previous.Add(10*time.Millisecond), period), previous.Add(period); !got.Equal(want) {
 		t.Fatalf("ordinary next frame = %s, want %s", got, want)
 	}
-	if got, want := nextStripFrameTime(previous, previous.Add(121*time.Millisecond), period), previous.Add(150*time.Millisecond); !got.Equal(want) {
-		t.Fatalf("delayed next frame = %s, want %s", got, want)
+	for _, delay := range []time.Duration{50, 70, 121} {
+		now := previous.Add(delay * time.Millisecond)
+		got := nextStripFrameTime(previous, now, period)
+		if !got.Equal(now) {
+			t.Fatalf("late transfer %s: next frame = %s, want %s", delay*time.Millisecond, got, now)
+		}
+		if following := nextStripFrameTime(got, now.Add(10*time.Millisecond), period); !following.Equal(now.Add(period)) {
+			t.Fatalf("late transfer must rebase subsequent pacing: got %s", following)
+		}
 	}
 }
 

Tools/Firmware/verify-strip-waveform.mjs

@@ -0,0 +1,94 @@
+#!/usr/bin/env node
+// SPDX-License-Identifier: MIT
+// Exercise the actual linked AVR bit loop, including compiler register allocation.
+import assert from 'node:assert/strict'
+import { readFileSync } from 'node:fs'
+import { loadProjectEnv } from '../Build/env.mjs'
+
+loadProjectEnv()
+
+const listingPath = process.argv[2]
+if (!listingPath) throw new Error('Usage: node Tools/Firmware/verify-strip-waveform.mjs AVR-LISTING.lst')
+const listing = readFileSync(listingPath, 'utf8')
+const start = listing.match(/^([0-9a-f]+) <head20\d+>:/m)
+assert.ok(start, 'linked strip bit-loop label is missing')
+const instructions = []
+for (const line of listing.slice(start.index).split('\n')) {
+  const match = line.trimEnd().match(/^\s*([0-9a-f]+):\s+(?:(?:[0-9a-f]{2})\s+)+\s*([a-z]+)\s*([^;]*)(?:;\s*(.*))?$/)
+  if (!match) continue
+  const [, address, op, operandText, comment = ''] = match
+  const target = comment.match(/^0x([0-9a-f]+)/)
+  instructions.push({ address: parseInt(address, 16), op, args: operandText.trim().split(/\s*,\s*/), target: target && parseInt(target[1], 16) })
+  if (op === 'brne') break
+}
+assert.equal(instructions.at(-1)?.op, 'brne', 'complete strip byte loop is required')
+const byAddress = new Map(instructions.map((instruction, index) => [instruction.address, index]))
+const hi = instructions[0].args[1]
+const byte = instructions.find(i => i.op === 'sbrc').args[0]
+const bit = instructions.find(i => i.op === 'dec').args[0]
+const moves = instructions.filter(i => i.op === 'mov')
+const next = moves[0].args[0]
+const lo = moves[1].args[1]
+const count = instructions.find(i => i.op === 'sbiw').args[0]
+assert.notEqual(next, lo, `compiler aliased changing bit operand and constant low operand (${lo})`)
+assert.equal(new Set([hi, lo, byte, bit, next, count]).size, 6, 'loop operands must have independent registers')
+
+function verify(bytes) {
+  const registers = { [hi]: 64, [lo]: 0, [byte]: bytes[0], [bit]: 8, [next]: 0, [count]: bytes.length }
+  let pointer = 1, pc = 0, cycles = 0, zero = false, carry = 0, level = 0
+  const rises = [], falls = []
+  for (let steps = 0; pc < instructions.length; steps++) {
+    assert.ok(steps < bytes.length * 8 * 30, 'strip loop did not terminate')
+    const instruction = instructions[pc]
+    const [a, b] = instruction.args
+    let advance = 1, elapsed = 1
+    switch (instruction.op) {
+      case 'st': {
+        elapsed = 2
+        const newLevel = registers[b] & 64
+        if (newLevel !== level) (newLevel ? rises : falls).push(cycles + elapsed)
+        level = newLevel
+        break
+      }
+      case 'sbrc': if (!(registers[a] & (1 << Number(b)))) { advance = 2; elapsed = 2 }; break
+      case 'mov': registers[a] = registers[b]; break
+      case 'dec': registers[a] = (registers[a] - 1) & 255; zero = registers[a] === 0; break
+      case 'breq':
+      case 'brne':
+        if (instruction.op === 'breq' ? zero : !zero) {
+          advance = byAddress.get(instruction.target) - pc
+          elapsed = 2
+        }
+        break
+      case 'adc': {
+        const sum = registers[a] + registers[b] + carry
+        registers[a] = sum & 255
+        carry = sum > 255 ? 1 : 0
+        break
+      }
+      case 'rjmp': advance = byAddress.get(instruction.target) - pc; elapsed = 2; break
+      case 'nop': break
+      case 'ldi': registers[a] = Number(b); break
+      case 'ld': registers[a] = bytes[pointer++] ?? 0; elapsed = 2; break
+      case 'sbiw': registers[a]--; zero = registers[a] === 0; elapsed = 2; break
+      default: throw new Error(`Unmodeled AVR instruction ${instruction.op}`)
+    }
+    assert.ok(Number.isFinite(advance), `invalid branch target at ${instruction.address.toString(16)}`)
+    cycles += elapsed
+    pc += advance
+  }
+  assert.equal(level, 0, 'data pin must finish low')
+  assert.equal(rises.length, bytes.length * 8, 'one rising edge per data bit')
+  assert.equal(falls.length, rises.length, 'one falling edge per data bit')
+  for (let i = 0; i < rises.length; i++) {
+    const value = (bytes[i >> 3] >> (7 - (i & 7))) & 1
+    assert.equal(rises[i] - rises[0], i * 20, `bit ${i} period`)
+    assert.equal(falls[i] - rises[i], value ? 13 : 5, `bit ${i} high width`)
+  }
+  assert.equal(pointer, bytes.length + 1, 'byte pointer must advance across the full frame')
+}
+
+// Every byte value, transitions both ways, and full 100-pixel frame length.
+for (let value = 0; value < 256; value++) verify([value, value ^ 255, value])
+verify(Array.from({ length: 300 }, (_, i) => i & 255))
+console.log('PASS: linked AVR strip loop: 256 byte patterns, byte boundaries, 300-byte frame; 20-cycle cells and 5/13-cycle high pulses.')

docs/STRIP-STREAMING.md

@@ -68,3 +68,33 @@ events remain concurrent because they do not write a host request.
 The 100-pixel limit is intentional on the AVR SRAM budget. Higher counts require
 a firmware target/profile with enough memory and a matching advertised limit;
 the UI and native encoder reject counts above this build's maximum.
+
+## Checking the compiled sender
+
+Acknowledged frames do not prove electrical output. The AVR assembly loop must
+declare operands overwritten before the last input use as early-clobber outputs.
+In particular, `next` starts equal to `lo`; without early-clobber, GCC can assign
+both to the same register. The first one bit then replaces the constant low
+value with high, and later writes cannot bring the data pin low. A memory clobber
+alone does not prevent this register alias. The advancing frame pointer must
+also be a read/write operand, and the buffer reads require a memory clobber.
+
+After compiling, exercise the actual linked instructions with:
+
+```console
+node Tools/Firmware/verify-strip-waveform.mjs .build/firmware/PCController.ino_atmega328p_16000000L.lst
+```
+
+The firmware CI job runs this gate. It checks register independence, every byte
+value, byte boundaries, and a 300-byte frame, including 20-cycle cells and the
+existing 5/13-cycle high pulses. The test rejects unrecognized instruction
+sequences instead of assuming their timing. Physical LED acceptance remains a
+separate check using the host's `strip rainbow 100 20` command.
+
+The requested frame rate is a ceiling, not a delivery measurement. A 100-pixel
+frame requires seven acknowledged data chunks plus a show request. On the tested
+serial connection, complete frame commands took 66–76 ms, already exceeding a
+20 FPS frame's 50 ms budget. When a transfer finishes late, the scheduler renders
+the current animation time immediately and rebases its deadline; waiting for the
+next original grid boundary would unnecessarily reduce this case to about 10 FPS.
+Short transfers still wait for their deadline, and no stale frames are queued.