From 59bd3b4cea952ea9b4c733517c01de095dbb804d Mon Sep 17 00:00:00 2001 From: prosolis <5590409+prosolis@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:22:56 -0700 Subject: [PATCH] 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 --- README.md | 36 ++++++++++++++-- firmware/kernel.cpp | 10 +++++ pico/CMakeLists.txt | 5 +-- pico/build.sh | 8 ++-- pico/main.cpp | 88 +++++++++++++++++++++++++++++++++++++++- src/config.h | 53 ++++++++++++++++++++++++ src/pianoleds.cpp | 62 ++++++++++++++++++++++++++-- src/pianoleds.h | 13 ++++++ tests/run.sh | 3 ++ tests/test_pianoleds.cpp | 43 ++++++++++++++++++++ 10 files changed, 307 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 30a2cde..bf1ab55 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/firmware/kernel.cpp b/firmware/kernel.cpp index d4f6510..ce19f2d 100644 --- a/firmware/kernel.cpp +++ b/firmware/kernel.cpp @@ -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 (); diff --git a/pico/CMakeLists.txt b/pico/CMakeLists.txt index 62e42bc..6cf45bc 100644 --- a/pico/CMakeLists.txt +++ b/pico/CMakeLists.txt @@ -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) diff --git a/pico/build.sh b/pico/build.sh index c84216b..f5028d1 100755 --- a/pico/build.sh +++ b/pico/build.sh @@ -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} diff --git a/pico/main.cpp b/pico/main.cpp index 7308e57..6676651 100644 --- a/pico/main.cpp +++ b/pico/main.cpp @@ -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; diff --git a/src/config.h b/src/config.h index e54df1a..24cc42f 100644 --- a/src/config.h +++ b/src/config.h @@ -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 // -------------------------------------------------------------------------- diff --git a/src/pianoleds.cpp b/src/pianoleds.cpp index 6ca125d..789125a 100644 --- a/src/pianoleds.cpp +++ b/src/pianoleds.cpp @@ -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 (); diff --git a/src/pianoleds.h b/src/pianoleds.h index 3a5422b..19f0d3e 100644 --- a/src/pianoleds.h +++ b/src/pianoleds.h @@ -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; diff --git a/tests/run.sh b/tests/run.sh index 8b1ec5e..559f5a9 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -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#*:} diff --git a/tests/test_pianoleds.cpp b/tests/test_pianoleds.cpp index afe207f..e1d8de7 100644 --- a/tests/test_pianoleds.cpp +++ b/tests/test_pianoleds.cpp @@ -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; }