Skip to content

feat: configurable multi-LED effects engine#16

Open
Curious-r wants to merge 5 commits into
TheMaxMur:mainfrom
Curious-r:ws2812-num-leds
Open

feat: configurable multi-LED effects engine#16
Curious-r wants to merge 5 commits into
TheMaxMur:mainfrom
Curious-r:ws2812-num-leds

Conversation

@Curious-r

Copy link
Copy Markdown

What

Add support for boards with multiple daisy-chained WS2812 addressable LEDs. Previously NUM_LEDS was hardcoded to 1, so only the first LED ever received data.

Since I wasn't sure if this improvement fits into your long-term plans for the project, I decided to open it as a draft PR. Please let me know if you have any concerns regarding the scope, design, or implementation; I'm more than happy to adjust it based on your feedback. If you'd rather implement this feature yourself, you can absolutely use this draft as a reference, and I'd be glad to help iron out the details. That said, if this doesn't align with the project's goals, no worries at all if you want to close it. Thanks for all your hard work on this project!

The main additions:

  • Build flag LED_NUM (build.rs) — sets the number of addressable LEDs at compile time (default 1, range 1..=256). Boards with e.g. 4 LEDs build with LED_NUM=4.
  • Effect engine (led.rs) — replaces the single-colour broadcast with a dispatch-based system. Five effects are defined (LEGACY, VAPOR, BOUNCE, FLOW, SPARKLE), each a pure function of (status, tick). Per-status STATUS_EFFECT and STATUS_SPEED atomics control the active effect.
  • EF_LED_CONF extended — from 9 bytes ([steady, (color, brightness) × 4]) to 13 bytes ([steady, (effect, color, brightness) × 4]). load_block handles all three wire formats (13/9/2 bytes) for seamless upgrade.
  • SET LED extended (vendor.rs) — accepts 1–2 optional data bytes for effect and speed. No data = unchanged behaviour (backward compatible).
  • rsk led updated (tools/rsk/led.py) — parses the new 13-byte block; new --effect and --speed flags.
  • Documentation (docs/guides/led.md, docs/hardware.md) — LED_NUM build knob, effects table with default per-status mapping, customisation examples.

How it was tested

  • nix develop -c ./scripts/check.sh passes locally
  • On hardware: Waveshare RP2350-One clone with 4x WS2812 on a single GPIO; built with LED_NUM=4 LED_ORDER=grb — all four LEDs light, each status shows its default effect (idle = breathing, processing = warm flow, touch = smooth bounce, boot = random sparkle). rsk led --get reports correct effect IDs.
  • Host-only change (CLI / docs / CI) — no device behavior touched

Checklist

  • Device behavior / image changed → config.device_release (bcdDevice) bumped by one (hex)
    in firmware/src/main.rsneeds bump (not done in this PR)
  • Fixes something a downgrade would reopen → flag whether the next release should advance the
    rollback epoch (docs/production.md, stage 3) — a seal-time --rollback call, not a code bump
  • New or changed unsafe → justified in docs/unsafe.md
  • User-visible behavior → the matching guide under docs/ updated
  • New files carry the SPDX header (AGPL-3.0-only)
  • Commit messages use the zone prefix style (fido:, piv:, rsk:, docs:, …)

@Curious-r Curious-r changed the title Draft:: configurable multi-LED effects engine Draft: configurable multi-LED effects engine Jun 22, 2026
@Curious-r Curious-r marked this pull request as draft June 22, 2026 20:38
@Curious-r Curious-r marked this pull request as ready for review June 22, 2026 20:38
@TheMaxMur

Copy link
Copy Markdown
Owner

Thanks for this — it's a genuinely nice feature and the PR is well put together (clean
commit separation, SPDX headers, backward-compatible wire formats, clippy-clean, and
tested on real multi-LED hardware). A few things to sort before it can land:

Must address

  1. Keep the American color spelling — please revert the colour rename. The codebase
    uses color consistently in code/identifiers/CLI (STATUS_COLOR, color_rgb, COLORS,
    the --color flag, the color= line in rsk led --get, and the TUI's "cycle idle
    color" action); British "colour" only ever appears in prose/comments. Renaming the
    tool's identifiers + --get output desyncs led.py from the firmware's STATUS_COLOR,
    changes scriptable --get output, and is unrelated to the multi-LED feature. Keep
    COLORS / COLOR_NAMES / --color / color= as-is.

  2. Make the LED count runtime via the phy record, not a compile-time const. All LED
    wiring (pin/driver/order) is runtime-configurable through the phy record
    (crates/rsk-rescue/src/phy.rs, applied at boot in firmware/src/main.rs, set via
    rsk hw, PicoForge-compatible); the build flags are only boot defaults. The LED count
    is the same kind of hardware-wiring property, so it should follow led_order: a new phy
    TLV tag next to 0x0D (parse + serialize + the Kani proof), applied at boot, exposed as
    rsk hw --led-num, with LED_NUM kept only as the boot default. Implementation note:
    NUM_LEDS is a const generic on PioWs2812 and the frame arrays, so you'll need a
    compile-time MAX_LEDS ceiling for the buffer plus a runtime count ≤ MAX.

  3. Surface the new controls in rsk-tui. tools/tui has a LED section ("read state" +
    "cycle idle color") that's kept in parity with rsk led; please add effect (and the
    runtime count) there too.

  4. --speed is currently a no-op. set_status_effect stores it into STATUS_SPEED,
    but config_block() never persists it (no speed byte in the 13-byte block, so it's lost
    on reboot) and no effect function reads it (the periods are hardcoded). Either wire it
    through end-to-end (add a speed byte to the block + have the effects read STATUS_SPEED)
    or drop the flag for now.

  5. Bump bcdDevice (config.device_release in firmware/src/main.rs) — you flagged it
    in the checklist; the image/behavior changes, so it's required. I'll reconcile the exact
    value against the in-flight changes on develop.

  6. Docs aren't fully updated:

    • docs/build.md — add LED_NUM to the build-knob table (your led.md links here for
      the knobs).
    • docs/protocol.md — the third-party/PicoForge host-protocol reference still documents
      SET LED as P2-only and a 9-byte GET LED block; update it for the optional effect/speed
      data bytes and the 13-byte [steady, (effect, color, brightness) × 4] layout (and the
      new phy tag once the count moves there).
    • docs/guides/led.md now says "Cycle idle colour" but the TUI label is "cycle idle
      color".

Worth considering (not blockers)

  1. On a stock single-LED board the feature is dormant (dispatch forces NUM_LEDS == 1 → legacy, and the count is compile-time). vapor (breathing) and sparkle are meaningful
    on one LED too — consider allowing them rather than hard-forcing legacy. This ties into
    making the count runtime (docs: reflect the v0.1.0 release in project status and security policy #2).
  2. flow / sparkle ignore the configured --color (flow hardcodes yellow→red; the
    default-mapping table even calls processing "green→red"). Fine as an artistic choice, but
    make the table match what the effect actually renders.
  3. Add unit tests for load_block's three wire formats and dispatch — you already factored
    dispatch out "so it can be unit-tested."
  4. Minor: the title still says "Draft:" though the PR isn't a draft; and the clippy/docs
    fixup commits could be squashed.

Happy to help iron out any of these — especially the phy-tag plumbing for #2, which mirrors
the existing led_order path exactly.

@TheMaxMur

Copy link
Copy Markdown
Owner

And

image

@Curious-r Curious-r marked this pull request as draft June 22, 2026 21:32
@Curious-r Curious-r force-pushed the ws2812-num-leds branch 4 times, most recently from f32892d to c4e335f Compare June 22, 2026 22:35
@Curious-r

Copy link
Copy Markdown
Author

I have made the improvements based on your feedback and resolved some issues with the committing. The on-device testing has also been re-run successfully. It's now ready for the next round of review!

@Curious-r Curious-r marked this pull request as ready for review June 22, 2026 22:58
@Curious-r Curious-r changed the title Draft: configurable multi-LED effects engine feat: configurable multi-LED effects engine Jun 22, 2026
The PIO state machine and frame buffers are sized to this compile-time ceiling (default 8, range 1-64). The actual connected LED count is configurable at runtime via the phy record (rsk hw --led-num) and must be <= MAX_LEDS.
Replace the single-colour broadcast in ws2812_task with a dispatch-based effect system. Five effects (LEGACY, VAPOR, BOUNCE, FLOW, SPARKLE) are selectable per status via STATUS_EFFECT/STATUS_SPEED atomics. MAX_LEDS is the compile-time buffer ceiling; RUNTIME_LEDS is the runtime count.

Effect functions are pure fn(status, tick) -> [RGB8; MAX_LEDS], dispatched from ws2812_task each tick. The Blinker is kept for gpio/pimoroni backends (unchanged).

EF_LED_CONF extended from 9 to 17 bytes:
  [steady, (effect, color, brightness, speed) x 4]
load_block handles four wire formats (17/13/9/2 bytes).
New TLV tag TAG_LED_NUM (0x0E) stores the number of physically-connected addressable LEDs. At boot, main.rs reads it and calls led::set_runtime_leds(n). Zero/unset = use the build's MAX_LEDS default.

Also adds the led_num field to PhyData with parse/serialize support, updates PHY_MAX_SIZE, and extends the Kani proof and roundtrip test.
vendor: INS_SET_LED accepts optional data bytes for effect (data[0]) and speed (data[1]), backward compatible.

rsk led: new --effect and --speed flags; parses 17-byte GET LED block with adaptive stride. Color spelling kept as 'color' (American English) to match firmware identifiers.

rsk hw: new --led-num flag writes TAG_LED_NUM (0x0E) to the phy record.

rsk-tui: led_get and led_cycle_idle parse 17/13/9-byte block formats with correct stride; led_get displays effect per status.
guides/led.md: rewrite with build-time knobs (MAX_LEDS, LED_KIND, LED_PIN, LED_ORDER), effects table with default mapping, customisation examples with --effect/--speed, updated wire-format reference, troubleshooting entries.
build.md: add MAX_LEDS build knob, update boot-defaults sentence.
hardware.md: add MAX_LEDS knob, update three-LED-knobs wording to four.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants