Add boot self-test and idle indicator; target the Waveshare RP2350-Plus

The board arriving is a Waveshare RP2350-Plus (4MB, USB-C), which is now the default BOARD. Its pico-sdk header defines neither PICO_DEFAULT_LED_PIN nor PICO_DEFAULT_WS2812_PIN - it has no onboard indicator at all - so a bare board gives no sign of life. The status-LED support added for boards that do have one (zero, one, tiny, usb_a on GPIO16; eth on GPIO25) is kept and compiles out here. That makes feedback on the strip itself the useful path, and it turns out to be the better one anyway: - Boot self-test sweeps one pixel from index 0 to the far end once at startup. It answers in a single glance whether the firmware runs, PIO drives the line, the strip is the length LED_COUNT claims, the far end holds voltage, and - because you see which end it starts from - whether STRIP_REVERSED is right. It runs before USB, so the first test needs nothing but 5V. - Idle indicator holds one dim pixel lit while no host is connected, separating "powered and waiting" from "no power" and from "crashed". Both are platform-independent, so the Circle build gets them too. Verified: tests pass across seventeen configurations, now including the self-test and idle paths on and off. Both platforms build clean with no warnings from project sources. Note on the previous commit's verification: a filtered build log hid a real compile error in the Pico target (sleep_ms takes uint32_t, which is unsigned long here, and did not match the portable void(*)(unsigned) delay callback). The build script now gets an explicit success check rather than a grep. Claude-Session: https://claude.ai/code/session_01TVCB25LBsmeteWvaSMz4Ne
2026-08-27 23:22:56 -07:00
parent 202adacf7a
commit 59bd3b4cea
10 changed files with 307 additions and 14 deletions
@@ -57,9 +57,20 @@ PICO_SDK_PATH=/path/to/pico-sdk ./pico/build.sh
 Produces `pico/build/pianoled.uf2`. Hold BOOTSEL while plugging the board in
 and copy the `.uf2` onto the drive that appears.

-`BOARD` selects the target (`pico`, `pico2`, `pico_w`, `pico2_w`; default
-`pico2`). All four run this firmware unchanged — the radio variants simply
-leave their radio unused.
+`BOARD` selects the target; the default is `waveshare_rp2350_plus_4mb`
+(Waveshare RP2350-Plus, 4MB, USB-C). pico-sdk 2.3.0 also ships definitions for
+eleven other Waveshare RP2350 boards and the stock `pico`/`pico2`:
+
+```sh
+ls "$PICO_SDK_PATH"/src/boards/include/boards/ | grep rp2350
+BOARD=waveshare_rp2350_zero ./pico/build.sh
+```
+
+Note the RP2350-Plus has **no onboard LED** — its board header defines neither
+`PICO_DEFAULT_LED_PIN` nor `PICO_DEFAULT_WS2812_PIN`, so the status-LED code
+compiles out. Boards that do have one (`zero`, `one`, `tiny`, `usb_a` on
+GPIO16; `eth` on GPIO25) get it automatically. On boards without, the boot
+self-test and idle indicator on the strip itself are the feedback.

 The strip data pin is `WS2812_PIN` in `src/config.h`, default GPIO2. PIO can
 drive it from any GPIO, so this is a free choice.
@@ -165,6 +176,25 @@ the semitone pitch (~1.7 LEDs) is narrower than `LEDS_PER_KEY`. That is
 expected, and the renderer paints only lit keys so a neighbour cannot erase
 them.

+## First power-up
+
+Two things run before any PC is involved, so a bare board on a bench still
+tells you something:
+
+- **Boot self-test** — one pixel sweeps from index 0 to the far end, once, then
+  clears. Watching it answers, in a single glance: the firmware runs, PIO
+  drives the data line, the strip is the length `LED_COUNT` claims, the far end
+  still has voltage, and — because you see which end it starts from — whether
+  `STRIP_REVERSED` is the right way round.
+- **Idle indicator** — while no USB host is connected, one dim pixel stays lit
+  at the strip's start. This distinguishes "powered and waiting for the PC"
+  from "no power" and from "crashed".
+
+Both are on by default (`BOOT_SELF_TEST`, `IDLE_INDICATOR` in `src/config.h`).
+
+So the very first test needs nothing but 5V: power the strip and the board, and
+watch for the sweep.
+
 ## Calibration

 This is a headless appliance, so calibration runs over MIDI — the one channel
@@ -16,6 +16,11 @@

 static const char FromKernel[] = "kernel";

+static void SelfTestDelay (unsigned nMilliSeconds)
+{
+	CTimer::Get ()->MsDelay (nMilliSeconds);
+}
+
 void CKernel::MIDIPacketHandler (unsigned nCable, u8 *pPacket, unsigned nLength,
 				 unsigned nDevice, void *pParam)
 {
@@ -108,6 +113,7 @@ void CKernel::UpdateMIDIDevice (void)
 		// The gadget builds a new device object on each enumeration, so
 		// clear anything held at the moment the host went away.
 		m_PianoLEDs.AllOff ();
+		m_PianoLEDs.SetHostConnected (true);

 		m_pMIDIDevice->RegisterPacketHandler (MIDIPacketHandler, this);

@@ -117,6 +123,7 @@ void CKernel::UpdateMIDIDevice (void)
 	{
 		// Host went away mid-chord. Do not leave keys lit.
 		m_PianoLEDs.AllOff ();
+		m_PianoLEDs.SetHostConnected (false);

 		m_Logger.Write (FromKernel, LogNotice, "USB MIDI gadget disconnected");
 	}
@@ -135,6 +142,9 @@ TShutdownMode CKernel::Run (void)
 			(unsigned) GLOBAL_BRIGHTNESS, (unsigned) MAX_LIT_KEYS);
 	m_Logger.Write (FromKernel, LogNotice, "Waiting for USB host");

+	// Runs on power alone, before any host is present.
+	m_PianoLEDs.RunSelfTest (SelfTestDelay);
+
 	for (;;)
 	{
 		UpdateMIDIDevice ();
@@ -1,8 +1,7 @@
 cmake_minimum_required(VERSION 3.13)

-# Board: pico, pico2, pico_w, pico2_w. All run this firmware unchanged; the
-# radio variants simply leave their radio unused.
-set(PICO_BOARD pico2 CACHE STRING "Target board")
+# Board. Defaults to the Waveshare RP2350-Plus (4MB, USB-C).
+set(PICO_BOARD waveshare_rp2350_plus_4mb CACHE STRING "Target board")

 include(pico_sdk_import.cmake)

@@ -8,9 +8,11 @@
 set -e
 cd "$(dirname "$0")"

-# Board: pico, pico2, pico_w, pico2_w. All run this firmware unchanged; the
-# radio variants simply leave their radio unused.
-BOARD=${BOARD:-pico2}
+# Board. Defaults to the Waveshare RP2350-Plus (4MB, USB-C). Any RP2040/RP2350
+# board works; pico-sdk 2.3.0 ships definitions for 12 Waveshare RP2350 boards
+# plus the stock pico/pico2. Run this to list them:
+#   ls "$PICO_SDK_PATH"/src/boards/include/boards/
+BOARD=${BOARD:-waveshare_rp2350_plus_4mb}

 export PICO_SDK_PATH=${PICO_SDK_PATH:-$HOME/git/pico-sdk}

@@ -15,6 +15,60 @@
 static CPicoLEDStrip s_Strip (LED_COUNT, WS2812_PIN);
 static CPianoLEDs s_PianoLEDs (s_Strip);

+// --------------------------------------------------------------------------
+// Status indicator
+// --------------------------------------------------------------------------
+//
+// Most Waveshare RP2350 boards have an onboard WS2812 rather than a plain LED,
+// and the board header names the pin. Where it exists, it is the only feedback
+// this headless board has: it answers "did the firmware boot" and "did the host
+// enumerate us" before the strip is even wired.
+
+#if STATUS_LED_ENABLED && defined(PICO_DEFAULT_WS2812_PIN)
+#define HAVE_STATUS_LED	1
+
+static CPicoLEDStrip s_Status (1, PICO_DEFAULT_WS2812_PIN);
+
+enum TStatus
+{
+	StatusFault,		// strip did not initialise
+	StatusWaiting,		// no USB host yet
+	StatusReady,		// enumerated, running normally
+	StatusCalibrating	// a calibration pattern is active
+};
+
+static TStatus s_LastStatus = StatusFault;
+static bool s_bStatusValid = false;
+
+static void ShowStatus (TStatus Status)
+{
+	if (   s_bStatusValid
+	    && Status == s_LastStatus)
+	{
+		return;			// only touch the LED when it changes
+	}
+
+	const uint8_t B = STATUS_LED_BRIGHTNESS;
+
+	switch (Status)
+	{
+	case StatusFault:	s_Status.SetLED (0, B, 0, 0); break;
+	case StatusWaiting:	s_Status.SetLED (0, B, B / 2, 0); break;
+	case StatusReady:	s_Status.SetLED (0, 0, B, 0); break;
+	case StatusCalibrating:	s_Status.SetLED (0, 0, 0, B); break;
+	}
+
+	s_Status.Update ();
+
+	s_LastStatus = Status;
+	s_bStatusValid = true;
+}
+#else
+#define HAVE_STATUS_LED	0
+static void ShowStatus (int) {}
+enum { StatusFault, StatusWaiting, StatusReady, StatusCalibrating };
+#endif
+
 // Number of MIDI bytes carried by a USB MIDI event packet, indexed by its
 // Code Index Number (USB MIDI 1.0, table 4-1). 0 means "not a message we
 // forward".
@@ -53,6 +107,13 @@ extern "C" void tud_resume_cb (void)

 // --------------------------------------------------------------------------

+// sleep_ms takes uint32_t, which is unsigned long on this target; the portable
+// header deliberately knows nothing about platform types.
+static void DelayMs (unsigned nMilliSeconds)
+{
+	sleep_ms (nMilliSeconds);
+}
+
 static void PollMIDI (void)
 {
 	uint8_t Packet[4];
@@ -80,18 +141,26 @@ int main (void)
 {
 	stdio_init_all ();

+#if HAVE_STATUS_LED
+	s_Status.Initialize ();
+#endif
+
 	if (!s_Strip.Initialize ())
 	{
-		// Nothing sensible left to do; make the failure visible rather
-		// than sitting dark and looking like a power problem.
+		// Sit here showing red. A dark board would look like a power
+		// problem; this says the firmware ran and the strip did not.
 		while (true)
 		{
+			ShowStatus (StatusFault);
 			tight_loop_contents ();
 		}
 	}

 	s_PianoLEDs.Initialize ();

+	// Before USB, so the sweep runs on power alone and needs no PC.
+	s_PianoLEDs.RunSelfTest (DelayMs);
+
 	tusb_init ();

 	while (true)
@@ -104,6 +173,21 @@ int main (void)
 		// rather than in the USB callback. Returns immediately when
 		// nothing has changed.
 		s_PianoLEDs.Update ();
+
+		s_PianoLEDs.SetHostConnected (tud_mounted ());
+
+		if (!tud_mounted ())
+		{
+			ShowStatus (StatusWaiting);
+		}
+		else if (s_PianoLEDs.GetCalibrationPattern () != CALIB_PATTERN_OFF)
+		{
+			ShowStatus (StatusCalibrating);
+		}
+		else
+		{
+			ShowStatus (StatusReady);
+		}
 	}

 	return 0;
@@ -190,6 +190,59 @@
 #define CALIB_PATTERN_WALK	4	// one pixel, chosen by CC21/CC22
 #define CALIB_PATTERN_ALL	5	// every pixel, for voltage droop testing

+// --------------------------------------------------------------------------
+// Bring-up feedback
+// --------------------------------------------------------------------------
+//
+// A bare board with no display gives no sign of life. These two cost nothing
+// and answer most first-power-up questions without a PC attached.
+
+// Sweep one pixel from index 0 to the far end once at startup, then clear.
+// Watching it answers, in one glance: the firmware runs, PIO drives the line,
+// the strip is the length LED_COUNT claims, the far end still has voltage,
+// and - because you see which end it starts from - whether STRIP_REVERSED is
+// the right way round.
+#ifndef BOOT_SELF_TEST
+#define BOOT_SELF_TEST		1
+#endif
+
+// Milliseconds per pixel during the sweep. 176 pixels at 6ms is about a
+// second, which is long enough to watch and short enough not to be a delay.
+#ifndef BOOT_SELF_TEST_MS
+#define BOOT_SELF_TEST_MS	6
+#endif
+
+// While no USB host is connected, hold one dim pixel lit at the strip's start
+// as a heartbeat. This distinguishes "powered and running, waiting for the PC"
+// from "no power" and from "crashed".
+#ifndef IDLE_INDICATOR
+#define IDLE_INDICATOR		1
+#endif
+
+// --------------------------------------------------------------------------
+// Status indicator
+// --------------------------------------------------------------------------
+//
+// Most Waveshare RP2350 boards carry an onboard WS2812 RGB LED instead of a
+// plain one, and the SDK's board header names the pin. Where that exists it is
+// used as a status light, which is the only feedback a headless board has:
+//
+//	red     strip failed to initialise
+//	amber   running, USB host has not enumerated us
+//	green   USB connected, normal operation
+//	blue    a calibration pattern is active
+//
+// Set to 0 to leave the onboard LED alone.
+#ifndef STATUS_LED_ENABLED
+#define STATUS_LED_ENABLED	1
+#endif
+
+// Deliberately dim. This is an indicator, not illumination, and on some boards
+// the onboard pixel is uncomfortably bright at full scale.
+#ifndef STATUS_LED_BRIGHTNESS
+#define STATUS_LED_BRIGHTNESS	12
+#endif
+
 // --------------------------------------------------------------------------
 // Hardware wiring - platform specific
 // --------------------------------------------------------------------------
@@ -17,6 +17,7 @@
 CPianoLEDs::CPianoLEDs (ILEDStrip &Strip)
 :	m_Strip (Strip),
 	m_bDirty (true),
+	m_bHostConnected (false),
 	m_nCalibPattern (CALIB_PATTERN_OFF),
 	m_nCalibIndex (0),
 	m_nCalibIndexHi (0)
@@ -184,6 +185,55 @@ void CPianoLEDs::SetKey (uint8_t ucNote, uint8_t ucVelocity, bool bHint)
 	m_bDirty = true;
 }

+void CPianoLEDs::SetHostConnected (bool bConnected)
+{
+	if (m_bHostConnected != bConnected)
+	{
+		m_bHostConnected = bConnected;
+		m_bDirty = true;
+	}
+}
+
+void CPianoLEDs::RunSelfTest (TDelayMs *pDelay)
+{
+#if BOOT_SELF_TEST
+	assert (pDelay != nullptr);
+
+	// Sweep in strip order, not key order, so what you watch is the strip's
+	// own geometry: it starts at pixel 0 wherever that physically is.
+	for (unsigned i = 0; i < LED_COUNT; i++)
+	{
+		if (i > 0)
+		{
+			m_Strip.SetLED (i - 1, 0, 0, 0);
+		}
+
+		m_Strip.SetLED (i, GLOBAL_BRIGHTNESS, GLOBAL_BRIGHTNESS,
+				   GLOBAL_BRIGHTNESS);
+		m_Strip.Update ();
+
+		pDelay (BOOT_SELF_TEST_MS);
+	}
+
+	m_Strip.Blackout ();
+
+	// The sweep left the strip in a state the renderer does not know about.
+	m_bDirty = true;
+#else
+	(void) pDelay;
+#endif
+}
+
+void CPianoLEDs::RenderIdle (void)
+{
+#if IDLE_INDICATOR
+	// One dim pixel at the strip's start: powered and running, no host yet.
+	const uint8_t B = GLOBAL_BRIGHTNESS / 8 ? GLOBAL_BRIGHTNESS / 8 : 1;
+
+	PaintPixel (0, B, B, B);
+#endif
+}
+
 void CPianoLEDs::AllOff (void)
 {
 	memset ((void *) m_KeyVelocity, 0, sizeof m_KeyVelocity);
@@ -388,13 +438,19 @@ void CPianoLEDs::Update (void)
 		m_Strip.SetLED (i, 0, 0, 0);
 	}

-	if (m_nCalibPattern == CALIB_PATTERN_OFF)
+	if (m_nCalibPattern != CALIB_PATTERN_OFF)
 	{
-		RenderNotes ();
+		RenderCalibration ();
+	}
+	else if (!m_bHostConnected)
+	{
+		// No host means no notes can arrive, so show a heartbeat rather
+		// than a strip that looks identical to an unpowered one.
+		RenderIdle ();
 	}
 	else
 	{
-		RenderCalibration ();
+		RenderNotes ();
 	}

 	m_Strip.Update ();
@@ -35,6 +35,16 @@ public:
 	// (re)connection, so notes held at disconnect do not stay lit.
 	void AllOff (void);

+	// Tell the visualizer whether a USB host is present, so it can show the
+	// idle indicator while there is nothing to display.
+	void SetHostConnected (bool bConnected);
+
+	// One-shot startup sweep. Blocking: it paces itself with the delay
+	// callback, so call it from the main loop before entering the loop
+	// proper. Does nothing when BOOT_SELF_TEST is 0.
+	typedef void TDelayMs (unsigned nMilliSeconds);
+	void RunSelfTest (TDelayMs *pDelay);
+
 	// Currently active calibration pattern, CALIB_PATTERN_OFF when running
 	// normally. Exposed for tests.
 	unsigned GetCalibrationPattern (void) const	{ return m_nCalibPattern; }
@@ -50,6 +60,7 @@ private:
 	void BuildKeyMap (void);

 	void RenderNotes (void);
+	void RenderIdle (void);
 	void RenderCalibration (void);

 	// Light one key's span, clamping to the strip.
@@ -72,6 +83,8 @@ private:

 	volatile bool m_bDirty;

+	volatile bool m_bHostConnected;
+
 	// Calibration overlay, set from the MIDI callback.
 	volatile unsigned m_nCalibPattern;
 	volatile unsigned m_nCalibIndex;
@@ -23,6 +23,9 @@ for CFG in \
 	"positive offset:-DLED_OFFSET=3" \
 	"negative offset:-DLED_OFFSET=-2" \
 	"offset reversed:-DLED_OFFSET=4 -DSTRIP_REVERSED=1" \
+	"no self test:-DBOOT_SELF_TEST=0" \
+	"no idle indicator:-DIDLE_INDICATOR=0" \
+	"minimal bring-up:-DBOOT_SELF_TEST=0 -DIDLE_INDICATOR=0" \
 ; do
 	NAME=${CFG%%:*}
 	FLAGS=${CFG#*:}
@@ -69,12 +69,16 @@ static void AllOffAndClear (void)
 }
 #endif

+static unsigned g_nDelayCalls = 0;
+static void CountingDelay (unsigned) { g_nDelayCalls++; }
+
 int main (void)
 {
 	printf ("STRIP_REVERSED=%d LED_COUNT=%d MAX_LIT_KEYS=%d GLOBAL_BRIGHTNESS=%d\n\n",
 		STRIP_REVERSED, LED_COUNT, MAX_LIT_KEYS, GLOBAL_BRIGHTNESS);

 	LEDs.Initialize ();
+	LEDs.SetHostConnected (true);	// tests exercise the connected path

 	// --- lowest key, A0 = note 21 -------------------------------------
 	Inject (0x90, 21, 127);
@@ -324,6 +328,45 @@ int main (void)
 	Inject (0x80, 60, 0);
 	LEDs.Update ();

+	// --- idle indicator ---------------------------------------------------
+	LEDs.AllOff ();
+	LEDs.SetHostConnected (false);
+	LEDs.Update ();
+#if IDLE_INDICATOR
+	Check ("with no host, one dim pixel marks the strip alive",
+	       CountLit () == 1 && !Dark (0));
+#else
+	Check ("with no host and no indicator, the strip is dark", CountLit () == 0);
+#endif
+
+	// notes cannot arrive without a host, and must not display if they do
+	Inject (0x90, 60, 127);
+	LEDs.Update ();
+	Check ("notes do not display while no host is connected",
+	       CountLit () <= 1);
+
+	LEDs.SetHostConnected (true);
+	LEDs.Update ();
+	Check ("reconnecting restores note display", !Dark (LedFor (60)));
+	LEDs.AllOff ();
+	LEDs.Update ();
+
+	// --- self test --------------------------------------------------------
+	g_nDelayCalls = 0;
+	LEDs.RunSelfTest (CountingDelay);
+#if BOOT_SELF_TEST
+	Check ("self test steps once per pixel", g_nDelayCalls == LED_COUNT);
+	Check ("self test leaves the strip dark", CountLit () == 0);
+	// and the renderer must not think the strip still holds what it drew
+	Inject (0x90, 60, 127);
+	LEDs.Update ();
+	Check ("display works after the self test", !Dark (LedFor (60)));
+	LEDs.AllOff ();
+	LEDs.Update ();
+#else
+	Check ("self test is compiled out", g_nDelayCalls == 0);
+#endif
+
 	printf ("\n%s\n", g_nFail ? "FAILURES" : "all tests passed");
 	return g_nFail != 0;
 }