warn when effect registration exceeds the list limit

[keeltrace]

AI-generated contribution. I am the human operator responsible for follow-up with maintainers. Please feel free to close this PR or request any changes.

Fixes #5827.

When WS2812FX::addEffect() reaches the 254-effect capacity, it currently returns 255 silently. Most usermods discard that return value, so dropped effects are difficult to diagnose.

This adds a debug warning only on the full-list failure path and includes the effect metadata string that could not be registered. It intentionally does not change the 8-bit effect ID limit or overlap with the separate larger-effect-ID work.

Validation:

• git diff --check
• reviewed against current main

I could not run a firmware build locally because PlatformIO is not installed in this environment.

Summary by CodeRabbit

• Diagnostics

  • Added diagnostic messages when the effect list reaches its maximum capacity, including a warning and the affected effect name. This makes failed effect registration easier to identify during troubleshooting.

• Documentation

  • Clarified that effect names supplied when registering effects must be stored in program memory, helping developers provide effect metadata in the required format.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: fd9439c9-1551-4cf8-aabe-f162d280c689

📥 Commits

Reviewing files that changed from the base of the PR and between c09cccb and 696da0e.

📒 Files selected for processing (1)

• wled00/FX.h

🚧 Files skipped from review as they are similar to previous changes (1)

• wled00/FX.h

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

Walkthrough

WS2812FX::addEffect now logs a warning and the rejected effect name when the effect list is full. Its declaration comment now states that mode_name must point to PROGMEM data.

Changes

Effect List Diagnostics

Layer / File(s)	Summary
Log rejected effects wled00/FX.cpp, wled00/FX.h	The full-list failure path logs that the effect list is full and prints the effect name before returning 255. The declaration documents the PROGMEM requirement for mode_name.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 696da

The change improves diagnostics for rejected effects without changing the effect limit or registration behavior. No actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: adding a warning when effect registration exceeds the list limit.
Linked Issues check	✅ Passed	The changes satisfy issue #5827 by logging a warning and the dropped effect name when addEffect() reaches the full-list failure path. The existing effect-count limit remains unchanged, and the documen…
Out of Scope Changes check	✅ Passed	The changes are limited to the requested warning and related addEffect() documentation. No unrelated code or behavior changes are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[softhack007]

I could not run a firmware build locally because PlatformIO is not installed in this environment.

@keeltrace you are an AI ? I ;-)

[keeltrace]

I could not run a firmware build locally because PlatformIO is not installed in this environment.

@keeltrace you are an AI ? I ;-)

Lol. Keeltrace is indeed my coding agent. And he wasn't able to test that as stated.

He's my pr bot. Just trying to help out the community. Feel free to close unmerged or tell me what changes you want.

• Keeltraces handler

[softhack007]

I could not run a firmware build locally because PlatformIO is not installed in this environment.

@keeltrace you are an AI ? I ;-)

Lol. Keeltrace is indeed my coding agent. And he wasn't able to test that as stated.

He's my pr bot. Just trying to help out the community. Feel free to close unmerged or tell me what changes you want.

• Keeltraces handler

@keeltrace fair. For future PR's, please make sure you add a statement in the description saying that you used a coding agent. Some maintainers here are very allergic to undeclared AI source code, especially when we have to find out ourselves by looking for typical "AI smells".

[keeltrace]

I could not run a firmware build locally because PlatformIO is not installed in this environment.

@keeltrace you are an AI ? I ;-)

Lol. Keeltrace is indeed my coding agent. And he wasn't able to test that as stated.

He's my pr bot. Just trying to help out the community. Feel free to close unmerged or tell me what changes you want.

• Keeltraces handler

@keeltrace fair. For future PR's, please make sure you add a statement in the description saying that you used a coding agent. Some maintainers here are very allergic to undeclared AI source code, especially when we have to find out ourselves by looking for typical "AI smells".

Super valid. My goal with keeltrace is to be helpful. It found this repo related to light shows which is a hobby of mine.

I will add that disclaimer going forward. Sorry if I upset anyone

[softhack007]

I will add that disclaimer going forward. Sorry if I upset anyone

Thanks, no need to worry - this is a small PR, and knowing it was AI generated helps to set reviewer's expectations.
I can fully understand that hobby developers may need a coding agent, not everybody has spent a lifetime developing software like myself ;-). Generally we are willing to reviews and accept AI generated source code - if it is properly declared, well-designed and well-written.

Edit "managed by a human" is another of these AI smells 😉 . Sometimes we understand that like "I have no idea of the code, just did some testing and it worked for me".

[keeltrace]

On further thinking this is my bad. -keeltrace handler

[softhack007]

@keeltrace fun fact: while professional developers are good at coding, many of us are bad at creative things like "building an awesome light show". So we definitely need creative hobby devs, who bring in their ideas for new light effects.

[keeltrace]

I checked the call sites directly. All in-tree WS2812FX::addEffect() callers pass PROGMEM-backed metadata (_data_FX_MODE_*, UsermodTemperature::_data_fx, and the POV usermod string), and the stored _modeData pointers are already consumed elsewhere with strncpy_P, strncmp_P, or FPSTR. So this debug line does not create a new flash-only requirement; it follows the existing mode_name contract. I would keep the API as const char * and keep FPSTR(mode_name) here rather than widen this small PR into an interface change. A third-party SRAM string was already incompatible with the existing _modeData consumers.

[softhack007]

@keeltrace yes that was also my conclusion, especially after looking at #5837 (comment). My question was if we are introducing a new (implied) constraint on mode_name. I think that's not the case. Just wondering if a small comment should be added to the declaration of WS2812FX::addEffect() in fx.h.

[keeltrace]

@keeltrace yes that was also my conclusion, especially after looking at #5837 (comment). My question was if we are introducing a new (implied) constraint on mode_name. I think that's not the case. Just wondering if a small comment should be added to the declaration of WS2812FX::addEffect() in fx.h.

Agreed, seems like a good fix. Comment so it's clear for others in the future.

Regarding the bug/issue existing before this PR, would you be open to an issue from keeltrace. My preference would be identify bug, suggest a few fixes, present issue with info that allows you to understand and make an informed decision. Then I step out and let human coders do the fix? Unless you'd prefer to cherry pick from the fix keeltrace (you can open the pr and own it) would be welcomed.

And if the issue isn't wanted or whatever. Feel free to close it, no hard feelings

[keeltrace]

Added the requested documentation note in wled00/FX.h: the addEffect() declaration now explicitly states that mode_name must point to PROGMEM data. No runtime behavior or API shape changed; this only documents the contract we confirmed was already in use. git diff --check is clean.

AI-assisted change; I am responsible for the update and any maintainer follow-up.

[keeltrace]

@keeltrace yes that was also my conclusion, especially after looking at #5837 (comment). My question was if we are introducing a new (implied) constraint on mode_name. I think that's not the case. Just wondering if a small comment should be added to the declaration of WS2812FX::addEffect() in fx.h.

Sorry for the triple comment. Looks like an issue already exists, here's keeltrace analysis. I'm gonna call this good for now. Feel free to ping me with any further change requests

#5827 (comment)

-keels handler