Skip to content

fix: warn when effect registration exceeds the list limit - #5837

Open
keeltrace wants to merge 1 commit into
wled:mainfrom
keeltrace:keeltrace/issue-5827-effect-overflow-warning
Open

fix: warn when effect registration exceeds the list limit#5837
keeltrace wants to merge 1 commit into
wled:mainfrom
keeltrace:keeltrace/issue-5827-effect-overflow-warning

Conversation

@keeltrace

@keeltrace keeltrace commented Sep 7, 2026

Copy link
Copy Markdown

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

  • Bug Fixes
    • Added diagnostic messages when the effect list reaches its maximum capacity, including a warning and the affected effect name.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 4c100c7f-f5d8-4b96-887e-7f6665026470

📥 Commits

Reviewing files that changed from the base of the PR and between 92064ae and b4567b7.

📒 Files selected for processing (1)
  • wled00/FX.cpp

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 before returning 255.

Changes

Effect List Diagnostics

Layer / File(s) Summary
Log rejected effects
wled00/FX.cpp
The full-list failure path logs that the effect list is full and prints the effect name before returning 255.

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

Merge Risk: ⚪ Minimal · up to b4567

When the effect list is full, rejected effects now produce a debug warning with their metadata while retaining existing registration behavior. No current merge-blocking risk remains.

Suggested reviewers: dedehai

🚥 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 added warning for effect registration attempts beyond the list limit.
Linked Issues check ✅ Passed The change satisfies issue #5827 by adding debug output when the effect list is full, including the dropped effect name, while preserving the existing effect-count limit.
Out of Scope Changes check ✅ Passed The changes are limited to diagnostic output in the full-list failure path and do not introduce unrelated behavior or scope.
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 1 functions across 1 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

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

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.

addEffect() fails silently when the effect list is full

1 participant