Skip to content

Separate effect text parameter from the segment name - #5955

Draft
netmindz wants to merge 3 commits into
wled:mainfrom
netmindz:effect-text-param
Draft

netmindz wants to merge 3 commits into
wled:mainfrom
netmindz:effect-text-param

Conversation

@netmindz

@netmindz netmindz commented Nov 3, 2026

Copy link
Copy Markdown
Member

Summary

Several effects use the segment name as a hidden string parameter. This PR gives segments a separate effect text parameter ("txt" in the JSON API). Effects declare it in their metadata, and the UI shows a text box for it only for effects that use it. Old presets and API calls keep working: they are migrated as they are loaded, so presets.json is never rewritten.

Why

These all use seg.name as effect input today:

User What the name is used for
Image GIF filename
Scrolling Text Text to scroll, including #TIME-style tokens
Morse (user_fx) Text to encode
pov_display usermod .bmp filename

Because the name means two things, a user label and effect data:

  • A segment playing fire.gif can't also be called "Hallway".
  • Switching effects reinterprets the field. Scroll text becomes a filename that fails to load, and "fire.gif" scrolls across the matrix.
  • Nothing in the UI says the name field is where the file or text goes. The effect metadata doesn't mention it either.
  • Presets rename segments. Every preset saves and restores "n", which is how GIF presets work today, but it also means a "Solid red" preset resets your segment labels.
  • Syncing segment names (being discussed in (POC) Sync protocol v2 prototype (soft-state replication) #5954) would implicitly also sync filenames and scroll text.

What changed

Segment (FX.h, FX_fcn.cpp)

  • New char *fxText. Like name, it is heap-allocated only when set, and limited to WLED_MAX_FXTEXT_LEN (defaults to WLED_MAX_SEGNAME_LEN, so 32 bytes on ESP8266 and 64 on ESP32).
  • It is handled in the copy/move constructors, the copy/move assignments, the destructor and getSize().
  • setFxText() does nothing when the text is unchanged. It starts the Scrolling Text transition, which setName() used to do.
  • The transition blend checks in WS2812FX::service() and blendSegment() now compare fxText instead of name.

Effect metadata

  • Entry 9 of the slider group (after the 5 sliders and 3 checkboxes) is the text field's label, for example "Image@!,Blur,,,,,,,File;;;12;...".
  • effectHasFxText(mode) in util.cpp checks for it.
  • The current UI parser already ignores extra entries, so older UIs and clients just don't show the field.

JSON / presets (json.cpp)

  • "txt" is serialised next to "n". Presets always contain it ("" when unset), so loading a preset never keeps a stale text.
  • Migration: if "txt" is absent, "n" is present, the effect declares a text parameter, and the call is a preset or also sets "fx", the name is copied into the text.
    • Old presets, playlists and old scripts that send {"fx":53,"n":"fire.gif"} keep working.
    • A plain rename from the UI ({"n":"Hallway"} without "fx") only changes the label.
  • rpt already keeps every key except "n", so repeated segments now get the text as well.
  • A text change sets stateChanged.

Effects and usermods

  • Image, Scrolling Text, Morse (user_fx) and pov_display read fxText and declare the field ("File" or "Text").
  • Their buffers are sized with WLED_MAX_FXTEXT_LEN.
  • The pov_display README is updated.
  • udp_name_sync is unchanged: it still syncs the main segment's name.

UI (index.htm, index.js, index.css)

  • A text box below the effect checkboxes, labelled from the metadata. It sends {"seg":{"txt":...}} to the selected segments.
  • It is not overwritten by state updates while it has focus.
  • The checkbox row is now hidden when an effect declares none of the 3 checkboxes, even if its metadata has more than 5 entries.

User-visible changes

  • Image, Scrolling Text, Morse and POV now show a File or Text box under the effect sliders. The segment name is just a label again.
  • Typing a filename or text into the segment name no longer changes the effect. Users need the new box. Existing presets migrate automatically, and on a running device the first preset load (or any API call that sets fx) copies the name across.
  • External tools that set the GIF or scroll text through "n" without "fx" must send "txt" instead.
  • Presets get one more key per segment ("txt":"", about 9 bytes).

Not in this PR (follow-ups to discuss)

  • Name as a layout setting: saving "n" only with "save segment bounds" (sb), so effect presets stop renaming segments. This is a separate user-visible decision.
  • A file picker with an extension filter for "File" fields.
  • Sync: legacy UDP sync sends neither names nor this text. In sync v2 ((POC) Sync protocol v2 prototype (soft-state replication) #5954) this would be its own optional block.
  • Documentation of the metadata entry in WLED-Docs.

Also fixed

  • Copy assignment Segment::operator=(const Segment&) left name and pixels pointing at the source's buffers when it returned early for an inactive source (!stop), which could cause a double free. It now clears those pointers right after the memcpy, as the copy constructor already does.

Testing

  • npm run build, then npm test: 16/16 pass.
  • Builds pass for nodemcuv2, esp32dev and esp32s3dev_8MB_opi, plus esp32dev with custom_usermods = user_fx pov_display and nodemcuv2 with user_fx (local override, not committed).
  • Not yet tested on hardware. To check: GIF playback from a new and an old preset, Scrolling Text with tokens, the text transition, the rename-only API call, and rpt.

This PR is AI-assisted (Claude Code). The new code is marked with // AI: comments; the remaining one-line pointer handling changes in FX_fcn.cpp follow the existing name handling. Everything was reviewed against the source and build-tested as above.

🤖 Generated with Claude Code

Image, Scrolling Text, Morse (user_fx) and the pov_display usermod used the
segment name as their file name / text. Segments now have a separate effect
text ("txt" in the JSON API), declared as the 9th entry of the effect
metadata slider group and shown as a text box in the UI.

Presets and API calls without "txt" are migrated on load: the name is copied
when the effect declares a text parameter and the call is a preset or also
sets "fx", so plain renames only change the label.

Also clears name/pixels after memcpy in the Segment copy assignment, which
left them aliased to the source on the early return for inactive segments.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

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