Update user_fx usermod README.md #4754
Conversation
WalkthroughUpdated Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
usermods/user_fx/README.md (2)
3-6: Correct intro typos & tighten wordingGrammar and spelling mistakes diminish the authority of the doc (e.g. “depracated”, informal “mess with”).
Proposed fix:-This Usermod is a common place to put various user's WLED Effects. It gives you a way to load in your own custom WLED effects, or to load in depracated WLED effects that you want to bring back--all without having to mess with the core WLED source code. +This usermod is a common place to host custom WLED effects. It lets you load your own effects—or revive deprecated ones—without modifying the WLED core.Also consider lower-casing “Effects” here and on line 5 for consistency with the rest of the doc.
470-478: Minor grammar corrections in “Combining Multiple Effects” section-So now let's say that you wanted add the effects "Diffusion Fire" and "Sinelon" through this same Usermod file: +Suppose you want to add both “Diffusion Fire” and “Sinelon” through the same usermod file:Also replace “different than” (l 474) → “different from”.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
usermods/user_fx/README.md (4)
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: KrX3D
PR: wled/WLED#4237
File: usermods/INA219_v2/INA219_v2.cpp:265-276
Timestamp: 2025-04-18T22:27:58.634Z
Learning: When implementing MQTT message handling in WLED usermods, use `strstr(topic, "/specific/path")` instead of `strcmp_P(topic, PSTR("/specific/path"))` to properly match topics that include the device prefix. The full MQTT topic typically follows the pattern `<mqttDeviceTopic>/specific/path`.
🪛 LanguageTool
usermods/user_fx/README.md
[style] ~20-~20: Consider removing “of” to be more concise
Context: ...area is where you place the FX code for all of the custom effects you want to use. This m...
(ALL_OF_THE)
[style] ~22-~22: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...mods have to be registered so that they are able to be compiled into your binary. We will ...
(BE_ABLE_TO)
[style] ~22-~22: Avoid the passive voice after ‘to be able to’.
Context: ... be registered so that they are able to be compiled into your binary. We will go into grea...
(ABLE_TO_PASSIVE)
[uncategorized] ~33-~33: You might be missing the article “the” here.
Context: ...elColor()` 4. The function is called at current frame rate. Below are some helpful var...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~43-~43: You might be missing the article “a” here.
Context: ...be changed at any time if the user sets new segment size(s). | | SEGCOLOR(x) ...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~44-~44: You might be missing the article “the” here.
Context: ...??? | Gets user-selected colors from UI, where x is an integer 1, 2, or 3 fr pr...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~47-~47: Possible missing comma found.
Context: ... | Easy way to specify a Palette. This function which should be favoured over `ColorFro...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~57-~57: It appears that a comma is missing.
Context: ... Understanding 2D WLED Effects In this section we give some advice to those who are ne...
(DURING_THAT_TIME_COMMA)
[uncategorized] ~61-~61: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...le. This file handles all other imports and it has all the global variable declarat...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~77-~77: Possible missing comma found.
Context: ...lock contains several constant variable definitions which essentially serve to extract the ...
(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~98-~98: Nonstandard phrase. Did you mean: “from 0 to 255”?
Context: ...MENT.speed (user-controllable parameter from 0–255) to a value between 20 and 80 Hz. * T...
(FROM_X_Y)
[uncategorized] ~100-~100: Possible missing comma found.
Context: ...gher speed = more frames per second). * Next we convert refresh rate from Hz to mill...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~100-~100: You might be missing the article “the” here.
Context: ...e frames per second). * Next we convert refresh rate from Hz to milliseconds. (It’s eas...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~104-~104: Possible missing comma found.
Context: ...re diffusion = smoother flame spread. * Next we assign SEGMENT.intensity (user inp...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~126-~126: You might be missing the article “the” here.
Context: ...nd the effect can’t proceed. * It calls previously defined mode_static() fallback effect...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~155-~155: The preposition “of” seems more likely in this position.
Context: ...conditional statement in the first line fo code ensures the effect updates on a fi...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
[uncategorized] ~173-~173: Possible missing preposition found.
Context: ...whether we even attempt to spawn sparks this frame. * hw_random8() gives a rando...
(AI_HYDRA_LEO_MISSING_IN)
[uncategorized] ~204-~204: Possible missing article found.
Context: ...Starting with the current heat value of pixel (x, y) assigned v: * if there’s a p...
(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~229-~229: Possible missing comma found.
Context: ... color-rendering for each pixel row.) * Next we update the main segment data with th...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~231-~231: Use the plural noun “” with the number ‘255’.
Context: ...olor` from the current palette with 255 brightness, and no wrapping in palette lookup. *...
(IN_CD_PUNCTUATION)
[typographical] ~233-~233: Consider adding a comma after ‘Finally’ for more clarity.
Context: ... flames (e.g., yellow → red → black). * Finally we set the rendered color for the pixel...
(RB_LY_COMMA)
[grammar] ~235-~235: The word “fallback” is a noun. The verb is spelled with a space.
Context: ...h row. * If palette use is disabled, we fallback to fading a base color. * SEGCOLOR(0)...
(NOUN_VERB_CONFUSION)
[grammar] ~256-~256: Did you mean the adverb/preposition “within”?
Context: ...ines how the effect is to be interacted with in the UI: ```cpp static const char data...
(WITH_IN)
[typographical] ~283-~283: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...s is enabled, the effect uses a palette ColorFromPalette(), otherwise it fades from SEGCOLOR(0). The first...
(THUS_SENTENCE)
[style] ~285-~285: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...s to be selected by the user. But used in conjunction with the checkbox argument, palette use can ...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[uncategorized] ~301-~301: You might be missing the article “a” here.
Context: ... line of code defines sinelon base as static helper function. This is how all effec...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~333-~333: Loose punctuation mark.
Context: ...2_t color2 = SEGCOLOR(2); ``` * color1: main moving dot color, chosen from pale...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~333-~333: You might be missing the article “a” here.
Context: ...r1`: main moving dot color, chosen from palette using the current position as index. * ...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~334-~334: Loose punctuation mark.
Context: ...he current position as index. * color2: secondary color from user-configured co...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~336-~336: This word is normally spelled with a hyphen.
Context: ... account the optional argument for if a Rainbow colored palette is in use: ```cpp if (rainbow...
(RAINBOW_COLORED)
[style] ~350-~350: ‘takes into account’ might be wordy. Consider a shorter alternative.
Context: ...with the selected color. The next line takes into account another one of the optional arguments f...
(EN_WORDINESS_PREMIUM_TAKES_INTO_ACCOUNT)
[uncategorized] ~350-~350: Possible missing comma found.
Context: ...ect to potentially handle dual mirrored dots which create the animation: ```cpp if...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~360-~360: You might be missing the article “the” here.
Context: ...If color2 is not set (0), fallback to same palette color as color1. * In `rain...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~380-~380: You might be missing the article “the” here.
Context: ...pos; } ``` * The first line checks if current position has changed since last frame. ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~380-~380: The verb form ‘has’ does not seem to match the subject ‘checks’.
Context: ...e first line checks if current position has changed since last frame. (Prevents ho...
(SUBJECT_VERB_AGREEMENT_PLURAL)
[uncategorized] ~380-~380: You might be missing the article “the” here.
Context: ...s if current position has changed since last frame. (Prevents holes if the dot move...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~381-~381: You might be missing the article “a” here.
Context: ....aux0) and new position (pos) to ensure smooth continuous trail. * Works in both dir...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~449-~449: Possible missing comma found.
Context: ...important one that registers the custom effect so WLED knows about it. * 255: Tempor...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~452-~452: Loose punctuation mark.
Context: ...ction. * _data_FX_MODE_DIFFUSIONFIRE: Metadata string stored in PROGMEM, desc...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~472-~472: The verb “add” needs to be in the to-infinitive form.
Context: ...this Usermod So now let's say that you wanted add the effects "Diffusion Fire" and "Sine...
(MISSING_TO_BEFORE_A_VERB)
[style] ~474-~474: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...ell--and to name it something different than what's already inside the core WLED cod...
(DIFFERENT_THAN)
[duplication] ~478-~478: Possible typo: you repeated a word.
Context: ... Usermod class. * Compile the code! ## Compiling Compiling WLED yourself is beyond the scope of th...
(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
39-39: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
264-264: Hard tabs
Column: 12
(MD010, no-hard-tabs)
266-266: Hard tabs
Column: 5
(MD010, no-hard-tabs)
267-267: Hard tabs
Column: 5
(MD010, no-hard-tabs)
268-268: Hard tabs
Column: 5
(MD010, no-hard-tabs)
269-269: Hard tabs
Column: 5
(MD010, no-hard-tabs)
270-270: Hard tabs
Column: 5
(MD010, no-hard-tabs)
271-271: Hard tabs
Column: 12
(MD010, no-hard-tabs)
272-272: Hard tabs
Column: 5
(MD010, no-hard-tabs)
273-273: Hard tabs
Column: 5
(MD010, no-hard-tabs)
🔇 Additional comments (1)
usermods/user_fx/README.md (1)
258-288: Fix hard-tabs & pipe style to satisfy markdownlint
MD010(no-hard-tabs) andMD055(table-pipe-style) are triggered here. Use spaces instead of\tand close the table with a trailing pipe:-| "Diffusion Fire@! | Name. (The @ symbol marks the end of the Effect Name, and the begining of the Parameter String elements.) | -| !, | Use default UI entry; for the first space, this will automatically create a slider for Speed | +| "Diffusion Fire@!" | Name. (The `@` marks the end of the effect name and the start of the parameter list.) | +| `!,` | Use default UI entry; for the first slot this automatically creates a speed slider. |Apply the same spacing & trailing-pipe fix to every row.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
usermods/user_fx/README.md (1)
37-48: “???” placeholders still present in Size columnThe earlier review asked to replace these placeholders with actual units (e.g.,
uint8_t,bytes,milliseconds) or drop the column. They remain unchanged.Please resolve to avoid shipping unfinished docs.
(Original feedback already given; still applies.)
🧹 Nitpick comments (6)
usermods/user_fx/README.md (6)
3-6: Fix spelling + remove stray link line
- “depracated” → deprecated
- Line 6 is an orphan URL that renders as plain text – it should be deleted (the bullets below already provide working TOC links).
-This Usermod is a common place to put various user's WLED Effects. It gives you a way to load in your own custom WLED effects, or to load in depracated WLED effects that you want to bring back--all without having to mess with the core WLED source code. +This usermod is a common place to put various users’ WLED effects. It lets you load your own custom effects, or bring back deprecated ones, without touching the core WLED source code. - -./README.md#how-the-usermod-works
24-25: Typo: “abour” → “about”-We will go into greater detail on how custom effects work in the usermod and how to go abour creating your own in the section below. +We will go into greater detail on how custom effects work in the usermod and how to go about creating your own in the section below.
37-48: Table formatting fails markdown-lint (MD055)Each row needs a trailing pipe (
|) for consistent pipe style:-| Syntax Element | Size | Description | +| Syntax Element | Size | Description |…and so on for every row in this table.
Also consider aligning the header/rows for readability once the “???” values are fixed.
264-274: Hard tabs inside table – breaks renderingTabs are used between the columns, triggering
markdownlintMD010. Replace with single spaces:-| Parameter | Default tooltip label | -| sx | Effect speed | +| Parameter | Default tooltip label | +| sx | Effect speed |Repeat for all rows in this block.
295-296: Typo: “tep by step”-we will break this effect down tep by step. +We will break this effect down step by step.
459-463: Spelling: “instatiation” → “instantiation”-The final part of this file handles instatiation and initialization: +The final part of this file handles instantiation and initialization:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
usermods/user_fx/README.md (4)
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: KrX3D
PR: wled/WLED#4237
File: usermods/INA219_v2/INA219_v2.cpp:265-276
Timestamp: 2025-04-18T22:27:58.634Z
Learning: When implementing MQTT message handling in WLED usermods, use `strstr(topic, "/specific/path")` instead of `strcmp_P(topic, PSTR("/specific/path"))` to properly match topics that include the device prefix. The full MQTT topic typically follows the pattern `<mqttDeviceTopic>/specific/path`.
🪛 LanguageTool
usermods/user_fx/README.md
[style] ~20-~20: Consider removing “of” to be more concise
Context: ...area is where you place the FX code for all of the custom effects you want to use. This m...
(ALL_OF_THE)
[style] ~22-~22: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...mods have to be registered so that they are able to be compiled into your binary. We will ...
(BE_ABLE_TO)
[style] ~22-~22: Avoid the passive voice after ‘to be able to’.
Context: ... be registered so that they are able to be compiled into your binary. We will go into grea...
(ABLE_TO_PASSIVE)
[uncategorized] ~33-~33: You might be missing the article “the” here.
Context: ...elColor()` 4. The function is called at current frame rate. Below are some helpful var...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~43-~43: You might be missing the article “a” here.
Context: ...be changed at any time if the user sets new segment size(s). | | SEGCOLOR(x) ...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~44-~44: You might be missing the article “the” here.
Context: ...??? | Gets user-selected colors from UI, where x is an integer 1, 2, or 3 fr pr...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~47-~47: Possible missing comma found.
Context: ... | Easy way to specify a Palette. This function which should be favoured over `ColorFro...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~57-~57: It appears that a comma is missing.
Context: ... Understanding 2D WLED Effects In this section we give some advice to those who are ne...
(DURING_THAT_TIME_COMMA)
[uncategorized] ~61-~61: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...le. This file handles all other imports and it has all the global variable declarat...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~77-~77: Possible missing comma found.
Context: ...lock contains several constant variable definitions which essentially serve to extract the ...
(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~98-~98: Nonstandard phrase. Did you mean: “from 0 to 255”?
Context: ...MENT.speed (user-controllable parameter from 0–255) to a value between 20 and 80 Hz. * T...
(FROM_X_Y)
[uncategorized] ~100-~100: Possible missing comma found.
Context: ...gher speed = more frames per second). * Next we convert refresh rate from Hz to mill...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~100-~100: You might be missing the article “the” here.
Context: ...e frames per second). * Next we convert refresh rate from Hz to milliseconds. (It’s eas...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~104-~104: Possible missing comma found.
Context: ...re diffusion = smoother flame spread. * Next we assign SEGMENT.intensity (user inp...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~126-~126: You might be missing the article “the” here.
Context: ...nd the effect can’t proceed. * It calls previously defined mode_static() fallback effect...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~155-~155: The preposition “of” seems more likely in this position.
Context: ...conditional statement in the first line fo code ensures the effect updates on a fi...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
[uncategorized] ~173-~173: Possible missing preposition found.
Context: ...whether we even attempt to spawn sparks this frame. * hw_random8() gives a rando...
(AI_HYDRA_LEO_MISSING_IN)
[uncategorized] ~204-~204: Possible missing article found.
Context: ...Starting with the current heat value of pixel (x, y) assigned v: * if there’s a p...
(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~229-~229: Possible missing comma found.
Context: ... color-rendering for each pixel row.) * Next we update the main segment data with th...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~231-~231: Use the plural noun “” with the number ‘255’.
Context: ...olor` from the current palette with 255 brightness, and no wrapping in palette lookup. *...
(IN_CD_PUNCTUATION)
[typographical] ~233-~233: Consider adding a comma after ‘Finally’ for more clarity.
Context: ... flames (e.g., yellow → red → black). * Finally we set the rendered color for the pixel...
(RB_LY_COMMA)
[grammar] ~235-~235: The word “fallback” is a noun. The verb is spelled with a space.
Context: ...h row. * If palette use is disabled, we fallback to fading a base color. * SEGCOLOR(0)...
(NOUN_VERB_CONFUSION)
[grammar] ~256-~256: Did you mean the adverb/preposition “within”?
Context: ...ines how the effect is to be interacted with in the UI: ```cpp static const char data...
(WITH_IN)
[typographical] ~283-~283: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...s is enabled, the effect uses a palette ColorFromPalette(), otherwise it fades from SEGCOLOR(0). The first...
(THUS_SENTENCE)
[style] ~285-~285: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...s to be selected by the user. But used in conjunction with the checkbox argument, palette use can ...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[uncategorized] ~301-~301: You might be missing the article “a” here.
Context: ... line of code defines sinelon base as static helper function. This is how all effec...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~333-~333: Loose punctuation mark.
Context: ...2_t color2 = SEGCOLOR(2); ``` * color1: main moving dot color, chosen from pale...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~333-~333: You might be missing the article “a” here.
Context: ...r1`: main moving dot color, chosen from palette using the current position as index. * ...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~334-~334: Loose punctuation mark.
Context: ...he current position as index. * color2: secondary color from user-configured co...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~336-~336: This word is normally spelled with a hyphen.
Context: ... account the optional argument for if a Rainbow colored palette is in use: ```cpp if (rainbow...
(RAINBOW_COLORED)
[style] ~350-~350: ‘takes into account’ might be wordy. Consider a shorter alternative.
Context: ...with the selected color. The next line takes into account another one of the optional arguments f...
(EN_WORDINESS_PREMIUM_TAKES_INTO_ACCOUNT)
[uncategorized] ~350-~350: Possible missing comma found.
Context: ...ect to potentially handle dual mirrored dots which create the animation: ```cpp if...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~360-~360: You might be missing the article “the” here.
Context: ...If color2 is not set (0), fallback to same palette color as color1. * In `rain...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~380-~380: You might be missing the article “the” here.
Context: ...pos; } ``` * The first line checks if current position has changed since last frame. ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~380-~380: The verb form ‘has’ does not seem to match the subject ‘checks’.
Context: ...e first line checks if current position has changed since last frame. (Prevents ho...
(SUBJECT_VERB_AGREEMENT_PLURAL)
[uncategorized] ~380-~380: You might be missing the article “the” here.
Context: ...s if current position has changed since last frame. (Prevents holes if the dot move...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~381-~381: You might be missing the article “a” here.
Context: ....aux0) and new position (pos) to ensure smooth continuous trail. * Works in both dir...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~449-~449: Possible missing comma found.
Context: ...important one that registers the custom effect so WLED knows about it. * 255: Tempor...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~452-~452: Loose punctuation mark.
Context: ...ction. * _data_FX_MODE_DIFFUSIONFIRE: Metadata string stored in PROGMEM, desc...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~472-~472: The verb “add” needs to be in the to-infinitive form.
Context: ...this Usermod So now let's say that you wanted add the effects "Diffusion Fire" and "Sine...
(MISSING_TO_BEFORE_A_VERB)
[style] ~474-~474: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...ell--and to name it something different than what's already inside the core WLED cod...
(DIFFERENT_THAN)
[duplication] ~478-~478: Possible typo: you repeated a word.
Context: ... Usermod class. * Compile the code! ## Compiling Compiling WLED yourself is beyond the scope of th...
(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
39-39: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
264-264: Hard tabs
Column: 12
(MD010, no-hard-tabs)
266-266: Hard tabs
Column: 5
(MD010, no-hard-tabs)
267-267: Hard tabs
Column: 5
(MD010, no-hard-tabs)
268-268: Hard tabs
Column: 5
(MD010, no-hard-tabs)
269-269: Hard tabs
Column: 5
(MD010, no-hard-tabs)
270-270: Hard tabs
Column: 5
(MD010, no-hard-tabs)
271-271: Hard tabs
Column: 12
(MD010, no-hard-tabs)
272-272: Hard tabs
Column: 5
(MD010, no-hard-tabs)
273-273: Hard tabs
Column: 5
(MD010, no-hard-tabs)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
usermods/user_fx/README.md (1)
37-55: Table still contains placeholders and minor typosThe “Size” column is still filled with
???, and there are a few spelling issues inside the descriptions (vriables,Fuction,RBG).
This was already flagged in a previous review but hasn’t been addressed.-| `SEGMENT.setPixelColor / setPixelColorXY` | --- | Fuction that paints a single pixel to your specified color. `setPixelColor` assumes 1D array and requires one positional argument, while `setPixelColorXY` takes two positional arguments (x and y), and then the RBG color value. | +| `SEGMENT.setPixelColor / setPixelColorXY` | --- | Function that paints a single pixel to your specified color. `setPixelColor` assumes a 1-D array and requires one positional argument, while `setPixelColorXY` takes two positional arguments (x and y) and then the RGB color value. | -| `SEGMENT.color_from_palette()` | ??? | Gets a single color from the currently selected palette for a segment. (This function which should be favoured over `ColorFromPalette()`.) <br />Defined in ... +| `SEGMENT.color_from_palette()` | uint32 | Returns one RGB color from the active palette for the segment (preferred over `ColorFromPalette()`). <br />Defined in ... -| `fade` | ??? | ... +| `fade*` | helper | ... -| `move()` | ??? | ... +| `move()` | helper | ... -| `blur / blur2d` | ??? | ... +| `blur / blur2d` | helper | ...Either supply the real sizes (e.g.
uint32,helper, or remove the column entirely) and fix the misspellings, or the table will continue to look unfinished.
Also consider adding the trailing|in each row to satisfy MD055.
🧹 Nitpick comments (7)
usermods/user_fx/README.md (7)
3-6: Correct obvious spelling mistake in first paragraph
depracated→deprecatedThis is the very first thing readers will see—leaving a typo here undermines credibility.
-...load in depracated WLED effects... +...load in deprecated WLED effects...
24-25: Fix typo “abour” → “about”-...go abour creating your own in the section below. +...go about creating your own in the section below.
160-162: Minor typo inside inline comment
fo code→of code-* The conditional statement in the first line fo code ensures the effect updates on a fixed interval... +* The conditional statement in the first line of code ensures the effect updates on a fixed interval...
300-304: Typo and casing (“tep by step”, “Sinelon base”)-we will break this effect down tep by step. +We will break this effect down step by step. -static uint16_t sinelon_base(bool dual, bool rainbow=false) { +static uint16_t sinelon_base(bool dual, bool rainbow = false) {Consistent casing and correct spelling improve readability.
416-422: Spelling: “speicfy” → “specify”-And then the last part defines the metadata strings for each effect to speicfy how it will be portrayed in the UI: +And then the last part defines the metadata strings for each effect to specify how it will be portrayed in the UI:
464-468: Spelling: “instatiation” → “instantiation”-The final part of this file handles instatiation and initialization: +The final part of this file handles instantiation and initialization:
477-481: Grammar: missing “to” before verb-So now let's say that you wanted add the effects +So now let's say that you wanted to add the effects
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
usermods/user_fx/README.md (4)
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: KrX3D
PR: wled/WLED#4237
File: usermods/INA219_v2/INA219_v2.cpp:265-276
Timestamp: 2025-04-18T22:27:58.634Z
Learning: When implementing MQTT message handling in WLED usermods, use `strstr(topic, "/specific/path")` instead of `strcmp_P(topic, PSTR("/specific/path"))` to properly match topics that include the device prefix. The full MQTT topic typically follows the pattern `<mqttDeviceTopic>/specific/path`.
🪛 LanguageTool
usermods/user_fx/README.md
[style] ~20-~20: Consider removing “of” to be more concise
Context: ...area is where you place the FX code for all of the custom effects you want to use. This m...
(ALL_OF_THE)
[style] ~22-~22: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...mods have to be registered so that they are able to be compiled into your binary. We will ...
(BE_ABLE_TO)
[style] ~22-~22: Avoid the passive voice after ‘to be able to’.
Context: ... be registered so that they are able to be compiled into your binary. We will go into grea...
(ABLE_TO_PASSIVE)
[grammar] ~40-~40: Possible subject-verb agreement error detected.
Context: ...16-bit | These are state variables that persists between function calls, and they are fr...
(PLURAL_THAT_AGREEMENT)
[uncategorized] ~46-~46: Possible missing article found.
Context: ...--- | Gets user-selected colors from UI, where x is an integer 1, 2, or 3 for p...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~47-~47: Possible missing article found.
Context: ...ecified color. setPixelColor assumes 1D array and requires one positional argum...
(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~49-~49: Possible missing comma found.
Context: ...y selected palette for a segment. (This function which should be favoured over `ColorFro...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~62-~62: It appears that a comma is missing.
Context: ... Understanding 2D WLED Effects In this section we give some advice to those who are ne...
(DURING_THAT_TIME_COMMA)
[uncategorized] ~66-~66: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...le. This file handles all other imports and it has all the global variable declarat...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~82-~82: Possible missing comma found.
Context: ...lock contains several constant variable definitions which essentially serve to extract the ...
(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~103-~103: Nonstandard phrase. Did you mean: “from 0 to 255”?
Context: ...MENT.speed (user-controllable parameter from 0–255) to a value between 20 and 80 Hz. * T...
(FROM_X_Y)
[uncategorized] ~105-~105: Possible missing comma found.
Context: ...gher speed = more frames per second). * Next we convert refresh rate from Hz to mill...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~109-~109: Possible missing comma found.
Context: ...re diffusion = smoother flame spread. * Next we assign SEGMENT.intensity (user inp...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~178-~178: Possible missing preposition found.
Context: ...whether we even attempt to spawn sparks this frame. * hw_random8() gives a rando...
(AI_HYDRA_LEO_MISSING_IN)
[uncategorized] ~209-~209: Possible missing article found.
Context: ...Starting with the current heat value of pixel (x, y) assigned v: * if there’s a p...
(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~234-~234: Possible missing comma found.
Context: ... color-rendering for each pixel row.) * Next we update the main segment data with th...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~236-~236: Use the plural noun “” with the number ‘255’.
Context: ...olor` from the current palette with 255 brightness, and no wrapping in palette lookup. *...
(IN_CD_PUNCTUATION)
[typographical] ~238-~238: Consider adding a comma after ‘Finally’ for more clarity.
Context: ... flames (e.g., yellow → red → black). * Finally we set the rendered color for the pixel...
(RB_LY_COMMA)
[grammar] ~240-~240: The word “fallback” is a noun. The verb is spelled with a space.
Context: ...h row. * If palette use is disabled, we fallback to fading a base color. * SEGCOLOR(0)...
(NOUN_VERB_CONFUSION)
[grammar] ~261-~261: Did you mean the adverb/preposition “within”?
Context: ...ines how the effect is to be interacted with in the UI: ```cpp static const char data...
(WITH_IN)
[typographical] ~288-~288: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...s is enabled, the effect uses a palette ColorFromPalette(), otherwise it fades from SEGCOLOR(0). The first...
(THUS_SENTENCE)
[style] ~290-~290: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...s to be selected by the user. But used in conjunction with the checkbox argument, palette use can ...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[uncategorized] ~338-~338: Loose punctuation mark.
Context: ...2_t color2 = SEGCOLOR(2); ``` * color1: main moving dot color, chosen from pale...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~339-~339: Loose punctuation mark.
Context: ...he current position as index. * color2: secondary color from user-configured co...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~341-~341: This word is normally spelled with a hyphen.
Context: ... account the optional argument for if a Rainbow colored palette is in use: ```cpp if (rainbow...
(RAINBOW_COLORED)
[style] ~355-~355: ‘takes into account’ might be wordy. Consider a shorter alternative.
Context: ...with the selected color. The next line takes into account another one of the optional arguments f...
(EN_WORDINESS_PREMIUM_TAKES_INTO_ACCOUNT)
[uncategorized] ~355-~355: Possible missing comma found.
Context: ...ect to potentially handle dual mirrored dots which create the animation: ```cpp if...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~385-~385: The verb form ‘has’ does not seem to match the subject ‘checks’.
Context: ...e first line checks if current position has changed since last frame. (Prevents ho...
(SUBJECT_VERB_AGREEMENT_PLURAL)
[uncategorized] ~385-~385: Possible missing article found.
Context: ...s if current position has changed since last frame. (Prevents holes if the dot move...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~454-~454: Possible missing comma found.
Context: ...important one that registers the custom effect so WLED knows about it. * 255: Tempor...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~457-~457: Loose punctuation mark.
Context: ...ction. * _data_FX_MODE_DIFFUSIONFIRE: Metadata string stored in PROGMEM, desc...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~477-~477: The verb “add” needs to be in the to-infinitive form.
Context: ...this Usermod So now let's say that you wanted add the effects "Diffusion Fire" and "Sine...
(MISSING_TO_BEFORE_A_VERB)
[style] ~479-~479: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...ell--and to name it something different than what's already inside the core WLED cod...
(DIFFERENT_THAN)
[duplication] ~483-~483: Possible typo: you repeated a word.
Context: ... Usermod class. * Compile the code! ## Compiling Compiling WLED yourself is beyond the scope of th...
(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
39-39: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
269-269: Hard tabs
Column: 12
(MD010, no-hard-tabs)
271-271: Hard tabs
Column: 5
(MD010, no-hard-tabs)
272-272: Hard tabs
Column: 5
(MD010, no-hard-tabs)
273-273: Hard tabs
Column: 5
(MD010, no-hard-tabs)
274-274: Hard tabs
Column: 5
(MD010, no-hard-tabs)
275-275: Hard tabs
Column: 5
(MD010, no-hard-tabs)
276-276: Hard tabs
Column: 12
(MD010, no-hard-tabs)
277-277: Hard tabs
Column: 5
(MD010, no-hard-tabs)
278-278: Hard tabs
Column: 5
(MD010, no-hard-tabs)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
usermods/user_fx/README.md (4)
3-6: Fix multiple typos in opening paragraph
depracated→deprecated,user's→users', plus minor grammar tweaks.-This Usermod is a common place to put various user's WLED Effects. It gives you a way to load in your own custom WLED effects, or to load in depracated WLED effects that you want to bring back--all without having to mess with the core WLED source code. +This usermod is a common place to put various users' WLED effects. It lets you load your own custom effects—or resurrect deprecated ones—without modifying the core WLED source code.
18-25: Minor spelling + wording cleanup
abour→about; tighten phrasing for clarity.-We will go into greater detail on how custom effects work in the usermod and how to go abour creating your own in the section below. +We’ll dive into how custom effects work in this usermod and how to create your own in the sections below.
45-46: Remove unresolved placeholder question
The parenthetical “(and how it relates to meta string??)” is a note to the author, not the reader.-`SEGCOLOR(x)` | — | Gets user-selected colors from UI, where x is an integer 1, 2, or 3 for primary, secondary, and tertiary colors, respectively. (and how it relates to meta string??) | +`SEGCOLOR(x)` | — | Retrieves the user-selected color `x` (1 = primary, 2 = secondary, 3 = tertiary). |
266-276: Replace hard tabs with spaces to satisfy MD010
The metadata-parameter table uses literal tab characters, triggering markdown-lint. Convert to two or four spaces.-| Parameter | Default tooltip label | +| Parameter | Default tooltip label |(repeat for every row)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
usermods/user_fx/README.md (5)
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-137
Timestamp: 2025-04-27T10:06:22.545Z
Learning: In the Seven Segment Display Reloaded usermod, the dimension mismatch between the default array (umSSDRNumbers[11][7]) and the override (umSSDRNumbers[11][10] = umSSDR_NUMBERS) is intentional by design, allowing for different use cases with varying numbers of segments per digit.
Learnt from: KrX3D
PR: wled/WLED#4237
File: usermods/INA219_v2/INA219_v2.cpp:265-276
Timestamp: 2025-04-18T22:27:58.634Z
Learning: When implementing MQTT message handling in WLED usermods, use `strstr(topic, "/specific/path")` instead of `strcmp_P(topic, PSTR("/specific/path"))` to properly match topics that include the device prefix. The full MQTT topic typically follows the pattern `<mqttDeviceTopic>/specific/path`.
🪛 LanguageTool
usermods/user_fx/README.md
[style] ~20-~20: Consider removing “of” to be more concise
Context: ...area is where you place the FX code for all of the custom effects you want to use. This m...
(ALL_OF_THE)
[style] ~22-~22: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...mods have to be registered so that they are able to be compiled into your binary. We will ...
(BE_ABLE_TO)
[style] ~22-~22: Avoid the passive voice after ‘to be able to’.
Context: ... be registered so that they are able to be compiled into your binary. We will go into grea...
(ABLE_TO_PASSIVE)
[grammar] ~40-~40: Possible subject-verb agreement error detected.
Context: ...16-bit | These are state variables that persists between function calls, and they are fr...
(PLURAL_THAT_AGREEMENT)
[uncategorized] ~46-~46: Possible missing article found.
Context: ...ecified color. setPixelColor assumes 1D array and requires one positional argum...
(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~48-~48: Possible missing comma found.
Context: ...y selected palette for a segment. (This function which should be favoured over `ColorFro...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~53-~53: Consider removing “of” to be more concise
Context: ..._fcn.cpp). | Importing wled.h brings all of the variables, files, and functions listed ...
(ALL_OF_THE)
[typographical] ~59-~59: It appears that a comma is missing.
Context: ... Understanding 2D WLED Effects In this section we give some advice to those who are ne...
(DURING_THAT_TIME_COMMA)
[uncategorized] ~63-~63: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...le. This file handles all other imports and it has all the global variable declarat...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~79-~79: Possible missing comma found.
Context: ...lock contains several constant variable definitions which essentially serve to extract the ...
(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~100-~100: Nonstandard phrase. Did you mean: “from 0 to 255”?
Context: ...MENT.speed (user-controllable parameter from 0–255) to a value between 20 and 80 Hz. * T...
(FROM_X_Y)
[uncategorized] ~102-~102: Possible missing comma found.
Context: ...gher speed = more frames per second). * Next we convert refresh rate from Hz to mill...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~106-~106: Possible missing comma found.
Context: ...re diffusion = smoother flame spread. * Next we assign SEGMENT.intensity (user inp...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~175-~175: Possible missing preposition found.
Context: ...whether we even attempt to spawn sparks this frame. * hw_random8() gives a rando...
(AI_HYDRA_LEO_MISSING_IN)
[uncategorized] ~206-~206: Possible missing article found.
Context: ...Starting with the current heat value of pixel (x, y) assigned v: * if there’s a p...
(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~231-~231: Possible missing comma found.
Context: ... color-rendering for each pixel row.) * Next we update the main segment data with th...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~233-~233: Use the plural noun “” with the number ‘255’.
Context: ...olor` from the current palette with 255 brightness, and no wrapping in palette lookup. *...
(IN_CD_PUNCTUATION)
[typographical] ~235-~235: Consider adding a comma after ‘Finally’ for more clarity.
Context: ... flames (e.g., yellow → red → black). * Finally we set the rendered color for the pixel...
(RB_LY_COMMA)
[grammar] ~237-~237: The word “fallback” is a noun. The verb is spelled with a space.
Context: ...h row. * If palette use is disabled, we fallback to fading a base color. * SEGCOLOR(0)...
(NOUN_VERB_CONFUSION)
[grammar] ~258-~258: Did you mean the adverb/preposition “within”?
Context: ...ines how the effect is to be interacted with in the UI: ```cpp static const char data...
(WITH_IN)
[typographical] ~285-~285: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...s is enabled, the effect uses a palette ColorFromPalette(), otherwise it fades from SEGCOLOR(0). The first...
(THUS_SENTENCE)
[style] ~287-~287: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...s to be selected by the user. But used in conjunction with the checkbox argument, palette use can ...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[uncategorized] ~335-~335: Loose punctuation mark.
Context: ...2_t color2 = SEGCOLOR(2); ``` * color1: main moving dot color, chosen from pale...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~336-~336: Loose punctuation mark.
Context: ...he current position as index. * color2: secondary color from user-configured co...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~338-~338: This word is normally spelled with a hyphen.
Context: ... account the optional argument for if a Rainbow colored palette is in use: ```cpp if (rainbow...
(RAINBOW_COLORED)
[style] ~352-~352: ‘takes into account’ might be wordy. Consider a shorter alternative.
Context: ...with the selected color. The next line takes into account another one of the optional arguments f...
(EN_WORDINESS_PREMIUM_TAKES_INTO_ACCOUNT)
[uncategorized] ~352-~352: Possible missing comma found.
Context: ...ect to potentially handle dual mirrored dots which create the animation: ```cpp if...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~382-~382: The verb form ‘has’ does not seem to match the subject ‘checks’.
Context: ...e first line checks if current position has changed since last frame. (Prevents ho...
(SUBJECT_VERB_AGREEMENT_PLURAL)
[uncategorized] ~382-~382: Possible missing article found.
Context: ...s if current position has changed since last frame. (Prevents holes if the dot move...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~451-~451: Possible missing comma found.
Context: ...important one that registers the custom effect so WLED knows about it. * 255: Tempor...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~454-~454: Loose punctuation mark.
Context: ...ction. * _data_FX_MODE_DIFFUSIONFIRE: Metadata string stored in PROGMEM, desc...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~474-~474: The verb “add” needs to be in the to-infinitive form.
Context: ...this Usermod So now let's say that you wanted add the effects "Diffusion Fire" and "Sine...
(MISSING_TO_BEFORE_A_VERB)
[style] ~476-~476: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...ell--and to name it something different than what's already inside the core WLED cod...
(DIFFERENT_THAN)
[duplication] ~480-~480: Possible typo: you repeated a word.
Context: ... Usermod class. * Compile the code! ## Compiling Compiling WLED yourself is beyond the scope of th...
(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
39-39: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
266-266: Hard tabs
Column: 12
(MD010, no-hard-tabs)
268-268: Hard tabs
Column: 5
(MD010, no-hard-tabs)
269-269: Hard tabs
Column: 5
(MD010, no-hard-tabs)
270-270: Hard tabs
Column: 5
(MD010, no-hard-tabs)
271-271: Hard tabs
Column: 5
(MD010, no-hard-tabs)
272-272: Hard tabs
Column: 5
(MD010, no-hard-tabs)
273-273: Hard tabs
Column: 12
(MD010, no-hard-tabs)
274-274: Hard tabs
Column: 5
(MD010, no-hard-tabs)
275-275: Hard tabs
Column: 5
(MD010, no-hard-tabs)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
usermods/user_fx/README.md (1)
37-52: Table still breaks MD055 & contains typos – add trailing pipes and clean wordingPrevious feedback on this exact table hasn’t been applied; rendering is still broken and several typos remain. Example issues:
• no trailing|→ GitHub table collapses
•vriables→variables
•Fuction→Function
•RBG→RGB
• “persists” → “persist”-| Syntax Element | Size | Description | -| :---------------------------------------------- | :----- | :---------- | -| `SEGMENT.intensity / speed / custom1 etc.` | 8-bit | These read-only variables help you control aspects of your custom effect using the UI sliders. You can edit these vriables through the UI sliders when WLED is running your effect. (These variables can be controlled by the API as well.) -| `SEGENV.aux0 / aux1` | 16-bit | These are state variables that persists between function calls, and they are free to be overwritten by the user for any use case. | -| `SEGENV.call` | 32-bit | A counter for how many times this effect function has been invoked since it started. | -| `strip.now` | 32-bit | Current timestamp in milliseconds. (Equivalent to `millis()`, but use `strip.now()` instead.) | +| Syntax element | Size | Description | +| :-----------------------------------------------| :----:| :---------- | +| `SEGMENT.intensity / speed / custom1 …` | 8-bit | Read-only variables mapped to the UI sliders (and API). | +| `SEGENV.aux0 / aux1` | 16-bit | User-owned state variables that persist between calls. | +| `SEGENV.call` | 32-bit | Invocation counter since the effect started. | +| `strip.now` | 32-bit | Current timestamp in milliseconds (`millis()` alias). |…and continue the same way for the remaining rows, ensuring each ends with a pipe.
🧹 Nitpick comments (3)
usermods/user_fx/README.md (3)
3-6: Fix spelling + minor grammar in the introduction
depracated→deprecated,Multiple Effects can be specified …→ add comma after “specified”, and consider trimming the double-space before “as we will illustrate”.-This Usermod is a common place to put various user's WLED Effects. It gives you a way to load in your own custom WLED effects, or to load in depracated WLED effects that you want to bring back--all without having to mess with the core WLED source code. -Multiple Effects can be specified inside this single usermod, as we will illustrate below. You will be able to define them with custom names, sliders, etc. as with any other Effect. +This usermod is a common place to host various WLED effects. It lets you load your own custom effects or revive deprecated ones—without touching core WLED source code. +Multiple effects can be specified inside this single usermod, as we illustrate below. You can give each effect custom names, sliders, etc., just like any other effect.
59-66: Typos in section header paragraph
tep by step→step by step; missing comma after “imports”.-In this section we give some advice to those who are new to WLED Effect creation. We will illustrate how to load in multiple Effects using this single usermod, and we will do a deep dive into the anatomy of a 1D Effect as well as a 2D Effect. +In this section, we give advice to newcomers to WLED effect creation. We illustrate how to load multiple effects with this single usermod and dive into the anatomy of both 1D and 2D effects.
266-276: Replace hard tabs with spaces in the metadata-parameter tableThe table uses literal tab characters, violating MD010 and rendering inconsistently on some editors. Convert tabs to two or three spaces per column.
No diff shown because the change is mechanical across all rows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
usermods/user_fx/README.md (13)
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-137
Timestamp: 2025-04-27T10:06:22.545Z
Learning: In the Seven Segment Display Reloaded usermod, the dimension mismatch between the default array (umSSDRNumbers[11][7]) and the override (umSSDRNumbers[11][10] = umSSDR_NUMBERS) is intentional by design, allowing for different use cases with varying numbers of segments per digit.
Learnt from: KrX3D
PR: wled/WLED#4237
File: usermods/INA219_v2/INA219_v2.cpp:265-276
Timestamp: 2025-04-18T22:27:58.634Z
Learning: When implementing MQTT message handling in WLED usermods, use `strstr(topic, "/specific/path")` instead of `strcmp_P(topic, PSTR("/specific/path"))` to properly match topics that include the device prefix. The full MQTT topic typically follows the pattern `<mqttDeviceTopic>/specific/path`.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Learnt from: willmmiles
PR: wled/WLED#4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs by falling back to the main segment ID, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
🪛 LanguageTool
usermods/user_fx/README.md
[style] ~20-~20: Consider removing “of” to be more concise
Context: ...area is where you place the FX code for all of the custom effects you want to use. This m...
(ALL_OF_THE)
[style] ~22-~22: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...mods have to be registered so that they are able to be compiled into your binary. We will ...
(BE_ABLE_TO)
[style] ~22-~22: Avoid the passive voice after ‘to be able to’.
Context: ... be registered so that they are able to be compiled into your binary. We will go into grea...
(ABLE_TO_PASSIVE)
[grammar] ~40-~40: Possible subject-verb agreement error detected.
Context: ...16-bit | These are state variables that persists between function calls, and they are fr...
(PLURAL_THAT_AGREEMENT)
[uncategorized] ~46-~46: Possible missing article found.
Context: ...ecified color. setPixelColor assumes 1D array and requires one positional argum...
(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~48-~48: Possible missing comma found.
Context: ...y selected palette for a segment. (This function which should be favoured over `ColorFro...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~53-~53: Consider removing “of” to be more concise
Context: ..._fcn.cpp). | Importing wled.h brings all of the variables, files, and functions listed ...
(ALL_OF_THE)
[typographical] ~59-~59: It appears that a comma is missing.
Context: ... Understanding 2D WLED Effects In this section we give some advice to those who are ne...
(DURING_THAT_TIME_COMMA)
[uncategorized] ~63-~63: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...le. This file handles all other imports and it has all the global variable declarat...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~79-~79: Possible missing comma found.
Context: ...lock contains several constant variable definitions which essentially serve to extract the ...
(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~100-~100: Nonstandard phrase. Did you mean: “from 0 to 255”?
Context: ...MENT.speed (user-controllable parameter from 0–255) to a value between 20 and 80 Hz. * T...
(FROM_X_Y)
[uncategorized] ~102-~102: Possible missing comma found.
Context: ...gher speed = more frames per second). * Next we convert refresh rate from Hz to mill...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~106-~106: Possible missing comma found.
Context: ...re diffusion = smoother flame spread. * Next we assign SEGMENT.intensity (user inp...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~175-~175: Possible missing preposition found.
Context: ...whether we even attempt to spawn sparks this frame. * hw_random8() gives a rando...
(AI_HYDRA_LEO_MISSING_IN)
[uncategorized] ~206-~206: Possible missing article found.
Context: ...Starting with the current heat value of pixel (x, y) assigned v: * if there’s a p...
(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~231-~231: Possible missing comma found.
Context: ... color-rendering for each pixel row.) * Next we update the main segment data with th...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~233-~233: Use the plural noun “” with the number ‘255’.
Context: ...olor` from the current palette with 255 brightness, and no wrapping in palette lookup. *...
(IN_CD_PUNCTUATION)
[typographical] ~235-~235: Consider adding a comma after ‘Finally’ for more clarity.
Context: ... flames (e.g., yellow → red → black). * Finally we set the rendered color for the pixel...
(RB_LY_COMMA)
[grammar] ~237-~237: The word “fallback” is a noun. The verb is spelled with a space.
Context: ...h row. * If palette use is disabled, we fallback to fading a base color. * SEGCOLOR(0)...
(NOUN_VERB_CONFUSION)
[grammar] ~258-~258: Did you mean the adverb/preposition “within”?
Context: ...ines how the effect is to be interacted with in the UI: ```cpp static const char data...
(WITH_IN)
[typographical] ~285-~285: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...s is enabled, the effect uses a palette ColorFromPalette(), otherwise it fades from SEGCOLOR(0). The first...
(THUS_SENTENCE)
[style] ~287-~287: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...s to be selected by the user. But used in conjunction with the checkbox argument, palette use can ...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[uncategorized] ~335-~335: Loose punctuation mark.
Context: ...2_t color2 = SEGCOLOR(2); ``` * color1: main moving dot color, chosen from pale...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~336-~336: Loose punctuation mark.
Context: ...he current position as index. * color2: secondary color from user-configured co...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~338-~338: This word is normally spelled with a hyphen.
Context: ... account the optional argument for if a Rainbow colored palette is in use: ```cpp if (rainbow...
(RAINBOW_COLORED)
[style] ~352-~352: ‘takes into account’ might be wordy. Consider a shorter alternative.
Context: ...with the selected color. The next line takes into account another one of the optional arguments f...
(EN_WORDINESS_PREMIUM_TAKES_INTO_ACCOUNT)
[uncategorized] ~352-~352: Possible missing comma found.
Context: ...ect to potentially handle dual mirrored dots which create the animation: ```cpp if...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~382-~382: The verb form ‘has’ does not seem to match the subject ‘checks’.
Context: ...e first line checks if current position has changed since last frame. (Prevents ho...
(SUBJECT_VERB_AGREEMENT_PLURAL)
[uncategorized] ~382-~382: Possible missing article found.
Context: ...s if current position has changed since last frame. (Prevents holes if the dot move...
(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~451-~451: Possible missing comma found.
Context: ...important one that registers the custom effect so WLED knows about it. * 255: Tempor...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~454-~454: Loose punctuation mark.
Context: ...ction. * _data_FX_MODE_DIFFUSIONFIRE: Metadata string stored in PROGMEM, desc...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~474-~474: The verb “add” needs to be in the to-infinitive form.
Context: ...this Usermod So now let's say that you wanted add the effects "Diffusion Fire" and "Sine...
(MISSING_TO_BEFORE_A_VERB)
[style] ~476-~476: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...ell--and to name it something different than what's already inside the core WLED cod...
(DIFFERENT_THAN)
[duplication] ~480-~480: Possible typo: you repeated a word.
Context: ... Usermod class. * Compile the code! ## Compiling Compiling WLED yourself is beyond the scope of th...
(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
39-39: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
266-266: Hard tabs
Column: 12
(MD010, no-hard-tabs)
268-268: Hard tabs
Column: 5
(MD010, no-hard-tabs)
269-269: Hard tabs
Column: 5
(MD010, no-hard-tabs)
270-270: Hard tabs
Column: 5
(MD010, no-hard-tabs)
271-271: Hard tabs
Column: 5
(MD010, no-hard-tabs)
272-272: Hard tabs
Column: 5
(MD010, no-hard-tabs)
273-273: Hard tabs
Column: 12
(MD010, no-hard-tabs)
274-274: Hard tabs
Column: 5
(MD010, no-hard-tabs)
275-275: Hard tabs
Column: 5
(MD010, no-hard-tabs)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
usermods/user_fx/README.md (3)
3-6: Fix spelling & tone in the opening paragraph
Minor spelling errors (“depracated”) and conversational phrasing reduce polish.-This Usermod is a common place to put various user's WLED Effects. It gives you a way to load in your own custom WLED effects, or to load in depracated WLED effects that you want to bring back--all without having to mess with the core WLED source code. +This usermod serves as a central place for custom or formerly-deprecated WLED effects, letting you add them without touching the core WLED source code.
24-25: Typo: “abour” → “about”-We will go into greater detail on how custom effects work in the usermod and how to go abour creating your own in the section below. +We will go into greater detail on how custom effects work in the usermod and how to go about creating your own in the section below.
266-276: Remove hard tabs and quote parameter names in the metadata tableHard tabs violate MD010 and render inconsistently. Also quoting the parameter names improves readability.
-| Parameter | Default tooltip label | -| :-------- | :-------------------- | -| sx | Effect speed | -| ix | Effect intensity | -| c1 | Custom 1 | -| c2 | Custom 2 | -| c3 | Custom 3 | -| o1 | Option 1 | -| o2 | Option 2 | -| o3 | Option 3 | +| Parameter | Default tooltip label | +| :-------- | :-------------------- | +| `sx` | Effect speed | +| `ix` | Effect intensity | +| `c1` | Custom 1 | +| `c2` | Custom 2 | +| `c3` | Custom 3 | +| `o1` | Option 1 | +| `o2` | Option 2 | +| `o3` | Option 3 |
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
usermods/user_fx/README.md (13)
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-137
Timestamp: 2025-04-27T10:06:22.545Z
Learning: In the Seven Segment Display Reloaded usermod, the dimension mismatch between the default array (umSSDRNumbers[11][7]) and the override (umSSDRNumbers[11][10] = umSSDR_NUMBERS) is intentional by design, allowing for different use cases with varying numbers of segments per digit.
Learnt from: KrX3D
PR: wled/WLED#4237
File: usermods/INA219_v2/INA219_v2.cpp:265-276
Timestamp: 2025-04-18T22:27:58.634Z
Learning: When implementing MQTT message handling in WLED usermods, use `strstr(topic, "/specific/path")` instead of `strcmp_P(topic, PSTR("/specific/path"))` to properly match topics that include the device prefix. The full MQTT topic typically follows the pattern `<mqttDeviceTopic>/specific/path`.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Learnt from: willmmiles
PR: wled/WLED#4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs by falling back to the main segment ID, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
🪛 LanguageTool
usermods/user_fx/README.md
[style] ~20-~20: Consider removing “of” to be more concise
Context: ...area is where you place the FX code for all of the custom effects you want to use. This m...
(ALL_OF_THE)
[style] ~22-~22: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...mods have to be registered so that they are able to be compiled into your binary. We will ...
(BE_ABLE_TO)
[style] ~22-~22: Avoid the passive voice after ‘to be able to’.
Context: ... be registered so that they are able to be compiled into your binary. We will go into grea...
(ABLE_TO_PASSIVE)
[uncategorized] ~33-~33: You might be missing the article “the” here.
Context: ...elColor()` 4. The function is called at current frame rate. Below are some helpful var...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~40-~40: Possible subject-verb agreement error detected.
Context: ...16-bit | These are state variables that persists between function calls, and they are fr...
(PLURAL_THAT_AGREEMENT)
[uncategorized] ~41-~41: A punctuation mark might be missing here.
Context: ...itten by the user for any use case. | | [SEGENV.call](https://github.com/wled/W...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
[uncategorized] ~46-~46: You might be missing the article “the” here.
Context: ...cro that gets user-selected colors from UI, where x is an integer 1, 2, or 3 for p...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~47-~47: You might be missing the article “a” here.
Context: ...ecified color. setPixelColor assumes 1D array and requires one positional argum...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~48-~48: Possible missing comma found.
Context: ...y selected palette for a segment. (This function which should be favoured over `ColorFro...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~53-~53: Consider removing “of” to be more concise
Context: ...d segment. | Importing wled.h brings all of the variables, files, and functions listed ...
(ALL_OF_THE)
[typographical] ~59-~59: It appears that a comma is missing.
Context: ... Understanding 2D WLED Effects In this section we give some advice to those who are ne...
(DURING_THAT_TIME_COMMA)
[uncategorized] ~63-~63: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...le. This file handles all other imports and it has all the global variable declarat...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~79-~79: Possible missing comma found.
Context: ...lock contains several constant variable definitions which essentially serve to extract the ...
(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~100-~100: Nonstandard phrase. Did you mean: “from 0 to 255”?
Context: ...MENT.speed (user-controllable parameter from 0–255) to a value between 20 and 80 Hz. * T...
(FROM_X_Y)
[uncategorized] ~102-~102: Possible missing comma found.
Context: ...gher speed = more frames per second). * Next we convert refresh rate from Hz to mill...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~102-~102: You might be missing the article “the” here.
Context: ...e frames per second). * Next we convert refresh rate from Hz to milliseconds. (It’s eas...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~106-~106: Possible missing comma found.
Context: ...re diffusion = smoother flame spread. * Next we assign SEGMENT.intensity (user inp...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~128-~128: You might be missing the article “the” here.
Context: ...nd the effect can’t proceed. * It calls previously defined mode_static() fallback effect...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~157-~157: The preposition “of” seems more likely in this position.
Context: ...conditional statement in the first line fo code ensures the effect updates on a fi...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
[uncategorized] ~175-~175: Possible missing preposition found.
Context: ...whether we even attempt to spawn sparks this frame. * hw_random8() gives a rando...
(AI_HYDRA_LEO_MISSING_IN)
[uncategorized] ~206-~206: Possible missing article found.
Context: ...Starting with the current heat value of pixel (x, y) assigned v: * if there’s a p...
(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~231-~231: Possible missing comma found.
Context: ... color-rendering for each pixel row.) * Next we update the main segment data with th...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~233-~233: Use the plural noun “” with the number ‘255’.
Context: ...olor` from the current palette with 255 brightness, and no wrapping in palette lookup. *...
(IN_CD_PUNCTUATION)
[typographical] ~235-~235: Consider adding a comma after ‘Finally’ for more clarity.
Context: ... flames (e.g., yellow → red → black). * Finally we set the rendered color for the pixel...
(RB_LY_COMMA)
[grammar] ~237-~237: The word “fallback” is a noun. The verb is spelled with a space.
Context: ...h row. * If palette use is disabled, we fallback to fading a base color. * SEGCOLOR(0)...
(NOUN_VERB_CONFUSION)
[grammar] ~258-~258: Did you mean the adverb/preposition “within”?
Context: ...ines how the effect is to be interacted with in the UI: ```cpp static const char data...
(WITH_IN)
[typographical] ~285-~285: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...s is enabled, the effect uses a palette ColorFromPalette(), otherwise it fades from SEGCOLOR(0). The first...
(THUS_SENTENCE)
[style] ~287-~287: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...s to be selected by the user. But used in conjunction with the checkbox argument, palette use can ...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[uncategorized] ~303-~303: You might be missing the article “a” here.
Context: ... line of code defines sinelon base as static helper function. This is how all effec...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~335-~335: Loose punctuation mark.
Context: ...2_t color2 = SEGCOLOR(2); ``` * color1: main moving dot color, chosen from pale...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~335-~335: You might be missing the article “a” here.
Context: ...r1`: main moving dot color, chosen from palette using the current position as index. * ...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~336-~336: Loose punctuation mark.
Context: ...he current position as index. * color2: secondary color from user-configured co...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~338-~338: This word is normally spelled with a hyphen.
Context: ... account the optional argument for if a Rainbow colored palette is in use: ```cpp if (rainbow...
(RAINBOW_COLORED)
[style] ~352-~352: ‘takes into account’ might be wordy. Consider a shorter alternative.
Context: ...with the selected color. The next line takes into account another one of the optional arguments f...
(EN_WORDINESS_PREMIUM_TAKES_INTO_ACCOUNT)
[uncategorized] ~352-~352: Possible missing comma found.
Context: ...ect to potentially handle dual mirrored dots which create the animation: ```cpp if...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~362-~362: You might be missing the article “the” here.
Context: ...If color2 is not set (0), fallback to same palette color as color1. * In `rain...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~382-~382: You might be missing the article “the” here.
Context: ...pos; } ``` * The first line checks if current position has changed since last frame. ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~382-~382: The verb form ‘has’ does not seem to match the subject ‘checks’.
Context: ...e first line checks if current position has changed since last frame. (Prevents ho...
(SUBJECT_VERB_AGREEMENT_PLURAL)
[uncategorized] ~382-~382: You might be missing the article “the” here.
Context: ...s if current position has changed since last frame. (Prevents holes if the dot move...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~383-~383: You might be missing the article “a” here.
Context: ....aux0) and new position (pos) to ensure smooth continuous trail. * Works in both dir...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~451-~451: Possible missing comma found.
Context: ...important one that registers the custom effect so WLED knows about it. * 255: Tempor...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~454-~454: Loose punctuation mark.
Context: ...ction. * _data_FX_MODE_DIFFUSIONFIRE: Metadata string stored in PROGMEM, desc...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~474-~474: The verb “add” needs to be in the to-infinitive form.
Context: ...this Usermod So now let's say that you wanted add the effects "Diffusion Fire" and "Sine...
(MISSING_TO_BEFORE_A_VERB)
[style] ~476-~476: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...ell--and to name it something different than what's already inside the core WLED cod...
(DIFFERENT_THAN)
[duplication] ~480-~480: Possible typo: you repeated a word.
Context: ... Usermod class. * Compile the code! ## Compiling Compiling WLED yourself is beyond the scope of th...
(ENGLISH_WORD_REPEAT_RULE)
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
39-39: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
266-266: Hard tabs
Column: 12
(MD010, no-hard-tabs)
268-268: Hard tabs
Column: 5
(MD010, no-hard-tabs)
269-269: Hard tabs
Column: 5
(MD010, no-hard-tabs)
270-270: Hard tabs
Column: 5
(MD010, no-hard-tabs)
271-271: Hard tabs
Column: 5
(MD010, no-hard-tabs)
272-272: Hard tabs
Column: 5
(MD010, no-hard-tabs)
273-273: Hard tabs
Column: 12
(MD010, no-hard-tabs)
274-274: Hard tabs
Column: 5
(MD010, no-hard-tabs)
275-275: Hard tabs
Column: 5
(MD010, no-hard-tabs)
netmindz
added
the
documentation
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
usermods/user_fx/README.md (1)
37-51: Table still breaks MD055 & contains stale links / typosThe earlier feedback was only partially applied:
- Missing trailing pipes (
|) on every data row → renders inconsistently.- Rows 44 & 47 still point to your personal fork (
danewhero/WLED/blob/user_fx) rather thanwled/WLED/main.- Spelling issues:
vriables,Fuction,RBG.Example fix (apply pattern to all rows):
-| [`SEGPALETTE`](https://github.com/danewhero/WLED/blob/user_fx/wled00/FX.h) | --- | Macro that gets the currently selected palette for the currently processing segment. | +| [`SEGPALETTE`](https://github.com/wled/WLED/blob/main/wled00/FX.h) | — | Retrieves the palette selected for the current segment. | -| ...setPixelColorXY`](https://github.com/danewhero/WLED/blob/user_fx/wled00/FX_2Dfcn.cpp) | --- | Fuction that paints a single pixel to your specified color. `setPixelColor` assumes 1D array and requires one positional argument, while `setPixelColorXY` takes two positional arguments (x and y), and then the RBG color value. | +| ...setPixelColorXY`](https://github.com/wled/WLED/blob/main/wled00/FX_2Dfcn.cpp) | — | Function that paints a single pixel. `setPixelColor` is 1-D; `setPixelColorXY` takes `(x, y)` then the RGB value. |Don’t forget to add a closing
|at the end of every row.
🧹 Nitpick comments (6)
usermods/user_fx/README.md (6)
3-6: Fix intro typos and wording for professionalism
depracated→deprecated, and consider tightening phrasing (“custom WLED effects or deprecated effects you want to revive”). Small polish, big first-impression gains.-...load in depracated WLED effects that you want to bring back--all without... +...load in deprecated WLED effects that you want to bring back—all without...
24-25: Correct typo “abour” → “about”Minor spelling slip.
-We will go into greater detail on how custom effects work in the usermod and how to go abour creating your own in the section below. +We will go into greater detail on how custom effects work in the usermod and how to go about creating your own in the section below.
297-299: Spell-check heading narrative
tep by step→step by step.-we will break this effect down tep by step. +We will break this effect down step by step.
338-345: Minor wording & grammar
The next part taked into account→The next part takes into account.-The next part taked into account the optional argument for if a Rainbow colored palette is in use: +The next part takes into account the optional argument when a rainbow palette is in use:
461-465: Typo: “instatiation” → “instantiation”-The final part of this file handles instatiation and initialization: +The final part of this file handles instantiation and initialization:
480-482: Typo in “official”-...on the offical WLED documentation website. +...on the official WLED documentation website.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
usermods/user_fx/README.md (16)
Learnt from: netmindz
PR: #4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: netmindz
PR: #4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Learnt from: willmmiles
PR: #4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Learnt from: KrX3D
PR: #4237
File: usermods/INA219_v2/INA219_v2.cpp:265-276
Timestamp: 2025-04-18T22:27:58.634Z
Learning: When implementing MQTT message handling in WLED usermods, use strstr(topic, "/specific/path") instead of strcmp_P(topic, PSTR("/specific/path")) to properly match topics that include the device prefix. The full MQTT topic typically follows the pattern <mqttDeviceTopic>/specific/path.
Learnt from: blazoncek
PR: #4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
Learnt from: blazoncek
PR: #4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Learnt from: blazoncek
PR: #4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize)) is unnecessary and could be problematic, as it bypasses proper size verification.
Learnt from: willmmiles
PR: #4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Learnt from: KrX3D
PR: #4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
Learnt from: blazoncek
PR: #4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the getSegment() function guards against out-of-bounds segment IDs, and getFirstSelectedSegId() falls back to getMainSegmentId() if no segments are selected, ensuring no crashes when used through the setValuesFromFirstSelectedSeg() macro.
Learnt from: blazoncek
PR: #4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the getSegment() function guards against out-of-bounds segment IDs, and getFirstSelectedSegId() falls back to getMainSegmentId() if no segments are selected, ensuring no crashes when used through the setValuesFromFirstSelectedSeg() macro.
Learnt from: blazoncek
PR: #4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the getSegment() function guards against out-of-bounds segment IDs by falling back to the main segment ID, and getFirstSelectedSegId() falls back to getMainSegmentId() if no segments are selected, ensuring no crashes when used through the setValuesFromFirstSelectedSeg() macro.
Learnt from: DedeHai
PR: #4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Learnt from: KrX3D
PR: #4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-137
Timestamp: 2025-04-27T10:06:22.545Z
Learning: In the Seven Segment Display Reloaded usermod, the dimension mismatch between the default array (umSSDRNumbers[11][7]) and the override (umSSDRNumbers[11][10] = umSSDR_NUMBERS) is intentional by design, allowing for different use cases with varying numbers of segments per digit.
Learnt from: KrX3D
PR: #4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:430-435
Timestamp: 2025-04-28T20:51:29.773Z
Learning: In WLED, bri is a global variable used for brightness control.
Learnt from: DedeHai
PR: #4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dxdx + dydy to be a practical concern in particle system effects.
🪛 LanguageTool
usermods/user_fx/README.md
[style] ~20-~20: Consider removing “of” to be more concise
Context: ...area is where you place the FX code for all of the custom effects you want to use. This m...
(ALL_OF_THE)
[style] ~22-~22: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...mods have to be registered so that they are able to be compiled into your binary. We will ...
(BE_ABLE_TO)
[style] ~22-~22: Avoid the passive voice after ‘to be able to’.
Context: ... be registered so that they are able to be compiled into your binary. We will go into grea...
(ABLE_TO_PASSIVE)
[style] ~53-~53: Consider removing “of” to be more concise
Context: ...d segment. | Importing wled.h brings all of the variables, files, and functions listed ...
(ALL_OF_THE)
[style] ~287-~287: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...s to be selected by the user. But used in conjunction with the checkbox argument, palette use can ...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[style] ~352-~352: ‘takes into account’ might be wordy. Consider a shorter alternative.
Context: ...with the selected color. The next line takes into account another one of the optional arguments f...
(EN_WORDINESS_PREMIUM_TAKES_INTO_ACCOUNT)
[style] ~476-~476: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...ell--and to name it something different than what's already inside the core WLED cod...
(DIFFERENT_THAN)
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
39-39: Table pipe style
Expected: leading_and_trailing; Actual: leading_only; Missing trailing pipe
(MD055, table-pipe-style)
266-266: Hard tabs
Column: 12
(MD010, no-hard-tabs)
268-268: Hard tabs
Column: 5
(MD010, no-hard-tabs)
269-269: Hard tabs
Column: 5
(MD010, no-hard-tabs)
270-270: Hard tabs
Column: 5
(MD010, no-hard-tabs)
271-271: Hard tabs
Column: 5
(MD010, no-hard-tabs)
272-272: Hard tabs
Column: 5
(MD010, no-hard-tabs)
273-273: Hard tabs
Column: 12
(MD010, no-hard-tabs)
274-274: Hard tabs
Column: 5
(MD010, no-hard-tabs)
275-275: Hard tabs
Column: 5
(MD010, no-hard-tabs)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
usermods/user_fx/README.md (1)
37-55: Correct API names, link targets, and sizes in the reference table
- Use main repo links (avoid personal fork).
SEGMENT.option1..3naming is inaccurate; WLED exposesSEGMENT.check1/2/3.SEGPALETTEis not a 32-bit value; treat as palette object/selector (no “Size”).- Fix “Fuction” → “Function”, “RBG” → “RGB”, clarify WRGB return.
-| [`SEGLEN / SEG_W / SEG_H`](https://github.com/wled/WLED/blob/main/wled00/FX_fcn.cpp) | 16-bit | These variables are macros that help define the length and width of your LED strip/matrix segment. | -| [`SEGPALETTE`](https://github.com/danewhero/WLED/blob/user_fx/wled00/FX.h) | 32-bit | Macro that gets the currently selected palette for the currently processing segment. | +| [`SEGLEN / SEG_W / SEG_H`](https://github.com/WLED/WLED/blob/main/wled00/FX_fcn.cpp) | 16-bit | Macros for segment length/width/height. | +| `SEGPALETTE` | — | Currently selected palette for the segment (palette object/selector). | @@ -| [`SEGMENT.setPixelColor`](https://github.com/danewhero/WLED/blob/user_fx/wled00/FX_fcn.cpp) / [`setPixelColorXY`](https://github.com/danewhero/WLED/blob/user_fx/wled00/FX_2Dfcn.cpp) | 32-bit | Fuction that paints a single pixel to your specified color. `setPixelColor` assumes 1D array and requires one positional argument, while `setPixelColorXY` takes two positional arguments (x and y), and then the RBG color value. | +| [`SEGMENT.setPixelColor`](https://github.com/WLED/WLED/blob/main/wled00/FX_fcn.cpp) / [`setPixelColorXY`](https://github.com/WLED/WLED/blob/main/wled00/FX_2Dfcn.cpp) | — | Function that paints a pixel. `setPixelColor` is 1-D; `setPixelColorXY` expects `(x, y)` and a 32‑bit WRGB color. |Also adjust Line 39 text:
-... `SEGMENT.custom3` is actually 5-bit. The other three bits are used by the boolean parameters `SEGMENT.option1` through `SEGMENT.option3` ... +... `SEGMENT.custom3` is actually 5‑bit. The other three bits back `SEGMENT.check1`, `check2`, and `check3` (bit‑packed).
🧹 Nitpick comments (13)
usermods/user_fx/README.md (13)
3-6: Fix typos and tighten phrasing in intro
- “depracated” → “deprecated”
- Prefer concise phrasing and single spaces after periods.
-This Usermod is a common place to put various user's WLED Effects. It gives you a way to load in your own custom WLED effects, or to load in depracated WLED effects that you want to bring back--all without having to mess with the core WLED source code. +This usermod is a common place to put user WLED effects. It lets you load custom effects or revive deprecated ones without modifying core WLED.
20-25: Typo: “abour” → “about”; minor style fixes-We will go into greater detail on how custom effects work in the usermod and how to go abour creating your own in the section below. +We will go into greater detail on how custom effects work in the usermod and how to go about creating your own in the section below.
68-75: Clarify “fallback static effect” wordingSlightly more direct, avoid implying pixels are “off” unless that’s the intent.
-This is usually left as `SEGMENT.fill(SEGCOLOR(0));` to leave all pixels off if the effect fails to load, +This is usually `SEGMENT.fill(SEGCOLOR(0));` to render a known fallback if the effect fails to load,
97-106: Minor math clarityRefresh math is fine; consider explicit cast to avoid unintended integer truncation in examples.
-const unsigned refresh_ms = 1000 / refresh_hz; +const unsigned refresh_ms = 1000u / refresh_hz; // integer division by Hz (20–80)
246-256: “Always return FRAMETIME” is too absoluteMost effects should return
FRAMETIME, but non‑FRAMETIME returns are supported. Rephrase to “should” and note deviations must be deliberate.-**Also note that an Effect should ALWAYS return FRAMETIME. Not doing so can cause glitches.** +Effects should generally return FRAMETIME; if you return a different interval, do it deliberately and test for timing side effects.
263-293: Metadata string: verify fields and explain “!” semantics; fix minor typos
- Confirm “!” usage and field count matches current JSON API docs.
- Typo: “begining” → “beginning”.
- Tighten descriptions.
-| "Diffusion Fire@! | Name. (The @ symbol marks the end of the Effect Name, and the begining of the Parameter String elements.) | +| "Diffusion Fire@! | Name. (`@` ends the name, begins the parameter list.) |Also, convert tabs to spaces in Lines 269–278 to satisfy MD010.
-| Parameter | Default tooltip label | +| Parameter | Default tooltip label |
300-307: Typos and naming
- “tep” → “step”
- Backticks around function name should match identifier:
sinelon_base.-we will break this effect down tep by step. +We will break this effect down step by step. @@ -* The first line of code defines `sinelon base` as static helper function. +* The first line defines `sinelon_base` as a static helper function.
416-421: Typo: “speicfy” → “specify”; quick wording cleanup-And then the last part defines the metadata strings for each effect to speicfy how it will be portrayed in the UI: +Finally, define the metadata strings for each effect to specify how it appears in the UI:
429-447: Tiny clarity on ID 255 noteMake the “temporary ID” detail explicit, one line.
-// use id=255 for all custom user FX (the final id is assigned when adding the effect) +// Use id=255 for all custom user FX; WLED assigns the final ID at registration.
464-471: Typo: “instatiation” → “instantiation”-The final part of this file handles instatiation and initialization: +The final part of this file handles instantiation and initialization:
478-482: Link hygiene and phrasing
- The commit‑pinned link is fine; if you want the latest on main, drop the hash.
- Style: “different from” instead of “different than”.
-...name it something different than what's already inside the core WLED code. +...name it something different from what’s already in core WLED.
484-485: Typo: “offical” → “official”-...on the offical WLED documentation website. +...on the official WLED documentation website.
47-47: Add WRGB note once near color-returning APIsMany WLED color helpers return 32‑bit WRGB. One parenthetical avoids repeating.
-...and then the RGB color value. +...and then a 32‑bit WRGB color value.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
PR: wled/WLED#4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
PR: wled/WLED#4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:43:15.355Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-06-15T09:59:52.720Z
Learnt from: netmindz
PR: wled/WLED#4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Applied to files:
usermods/user_fx/README.md
🪛 LanguageTool
usermods/user_fx/README.md
[grammar] ~3-~3: Ensure spelling is correct
Context: ... own custom WLED effects, or to load in depracated WLED effects that you want to bring bac...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~7-~7: There might be a mistake here.
Context: ... other Effect. * How The Usermod Works * [Basic Syntax for WLED Effect Creation](....
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...* Basic Syntax for WLED Effect Creation * [Understanding 2D WLED Effects](./README....
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...eation) * Understanding 2D WLED Effects * [The Metadata String](./README.md#the-met...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...-2d-wled-effects) * The Metadata String * [Understanding 1D WLED Effects](./README....
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...string) * Understanding 1D WLED Effects * [Combining Multiple Effects in this Userm...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...mbining Multiple Effects in this Usermod](./README.md#combining-multiple-effects-in-this-usermod) * Compiling * [Cha...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...le-effects-in-this-usermod) * Compiling * Change Log ## ...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...can be broken down into four main parts: * static effect definition - This is a s...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...played if an effect fails to initialize. * User FX function definition(s) - This ...
(QB_NEW_EN)
[style] ~20-~20: Consider removing “of” to be more concise
Context: ...area is where you place the FX code for all of the custom effects you want to use. This m...
(ALL_OF_THE)
[grammar] ~20-~20: There might be a mistake here.
Context: ...e/interfaces/json-api/#effect-metadata). * Usermod Class definition(s) - The clas...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...y usermod, for that matter) are created. * Usermod registration - All usermods ha...
(QB_NEW_EN)
[style] ~22-~22: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...mods have to be registered so that they are able to be compiled into your binary. We will ...
(BE_ABLE_TO)
[style] ~22-~22: Avoid the passive voice after ‘to be able to’.
Context: ... be registered so that they are able to be compiled into your binary. We will go into grea...
(ABLE_TO_PASSIVE)
[grammar] ~24-~24: Ensure spelling is correct
Context: ...fects work in the usermod and how to go abour creating your own in the section below....
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~29-~29: There might be a mistake here.
Context: ...a certain procedure for their operation: 1. Determine dimension of segment 2. Calcul...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...their operation: 1. Determine dimension of segment 2. Calculate new state if neede...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...PixelColor()` 4. The function is called at current frame rate. Below are some hel...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...cked to conserve data size and memory. | | [SEGENV.aux0 / aux1](https://github....
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ...ry, and tertiary colors, respectively. | | [SEGMENT.setPixelColor](https://gith...
(QB_NEW_EN)
[grammar] ~47-~47: Ensure spelling is correct
Context: ...x/wled00/FX_2Dfcn.cpp) | 32-bit | Fuction that paints a single pixel to your spec...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~47-~47: There might be a mistake here.
Context: ... your specified color. setPixelColor assumes 1D array and requires one positional ar...
(QB_NEW_EN)
[grammar] ~47-~47: Ensure spelling is correct
Context: ...ional arguments (x and y), and then the RBG color value. | | [`SEGMENT.color_wheel(...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~56-~56: Consider removing “of” to be more concise
Context: ... pixel(s). | Importing wled.h brings all of the variables, files, and functions listed ...
(ALL_OF_THE)
[grammar] ~62-~62: There might be a mistake here.
Context: ...y of a 1D Effect as well as a 2D Effect. (Special thanks to @mryndzionek for offe...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ...pixels off. ### User Effect Definitions Pre-loaded in this template is an exampl...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...ice, as defined in the metadata string.) The effect starts off by checking to see...
(QB_NEW_EN)
[grammar] ~89-~89: There might be a mistake here.
Context: ..., used to traverse the matrix correctly. * Next, we assign the number of rows (heig...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...or order (left to right, top to bottom). * This lambda helps with mapping a local 1...
(QB_NEW_EN)
[grammar] ~108-~108: There might be a mistake here.
Context: ... more diffusion = smoother flame spread. * Next we assign SEGMENT.intensity (user...
(QB_NEW_EN)
[grammar] ~121-~121: There might be a mistake here.
Context: ...segment (i.e., cols * rows in a matrix). * This fire effect models heat values per ...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ...lse, and the effect can’t proceed. * It calls previously defined mode_static() fall...
(QB_NEW_EN)
[grammar] ~159-~159: There might be a mistake here.
Context: ... update time (set during initialization or previous frame). refresh_ms is how l...
(QB_NEW_EN)
[grammar] ~159-~159: There might be a mistake here.
Context: ...computed earlier based on SEGMENT.speed. * The conditional statement in the first l...
(QB_NEW_EN)
[grammar] ~160-~160: There might be a mistake here.
Context: ... interval — e.g., every 20 ms for 50 Hz. * The second line of code declares a tempo...
(QB_NEW_EN)
[grammar] ~180-~180: There might be a mistake here.
Context: ...arameter (SEGMENT.custom2, set earlier). * Higher turbulence means this block is le...
(QB_NEW_EN)
[grammar] ~182-~182: There might be a mistake here.
Context: ...lating natural flicker and chaotic fire. * The next line loops over all columns in ...
(QB_NEW_EN)
[grammar] ~186-~186: There might be a mistake here.
Context: ... comes from SEGMENT.intensity (0–255). * High intensity means more frequent ignit...
(QB_NEW_EN)
[grammar] ~188-~188: There might be a mistake here.
Context: ...ion index in the bottom row at column x. * The final line here sets the heat at thi...
(QB_NEW_EN)
[grammar] ~208-~208: There might be a mistake here.
Context: ...s across each column in the current row. * Starting with the current heat value of ...
(QB_NEW_EN)
[grammar] ~212-~212: There might be a mistake here.
Context: ...omplish is: v = center + left + right. * The final line of code applies diffusion...
(QB_NEW_EN)
[grammar] ~216-~216: There might be a mistake here.
Context: ... ...)` clamps the result to 8-bit range. * This entire line of code stores the smoo...
(QB_NEW_EN)
[grammar] ~234-~234: There might be a mistake here.
Context: ... with the smoothed value for this pixel. * The if statement creates a conditional r...
(QB_NEW_EN)
[grammar] ~235-~235: There might be a mistake here.
Context: ...se a color palette to display the flame. * The next line converts the heat value (`...
(QB_NEW_EN)
[grammar] ~236-~236: There might be a mistake here.
Context: ...ness, and no wrapping in palette lookup. * This creates rich gradient flames (e.g.,...
(QB_NEW_EN)
[grammar] ~238-~238: There might be a mistake here.
Context: ...the rendered color for the pixel (x, y). * This repeats for each pixel in each row...
(QB_NEW_EN)
[grammar] ~239-~239: There might be a mistake here.
Context: ...his repeats for each pixel in each row. * If palette use is disabled, we fallback ...
(QB_NEW_EN)
[grammar] ~241-~241: There might be a mistake here.
Context: ...rst user-selected color for the segment. * The final line of code fades that base c...
(QB_NEW_EN)
[grammar] ~242-~242: There might be a mistake here.
Context: ...color according to the heat value (acts as brightness multiplier). The final piec...
(QB_NEW_EN)
[grammar] ~253-~253: There might be a mistake here.
Context: ...sponding to ~60 FPS (frames per second). * Even though the effect logic itself cont...
(QB_NEW_EN)
[grammar] ~255-~255: There might be a mistake here.
Context: ...ETIME. Not doing so can cause glitches.** * The final bracket closes the `mode_diffu...
(QB_NEW_EN)
[grammar] ~269-~269: There might be a mistake here.
Context: ...vention in the table below: | Parameter | Default tooltip label | | :-------- | ...
(QB_NEW_EN)
[grammar] ~269-~269: There might be a mistake here.
Context: ...: | Parameter | Default tooltip label | | :-------- | :-------------------- | | ...
(QB_NEW_EN)
[grammar] ~270-~270: There might be a mistake here.
Context: ... | | :-------- | :-------------------- | | sx | Effect speed | | ix ...
(QB_NEW_EN)
[grammar] ~271-~271: There might be a mistake here.
Context: ...--------- | | sx | Effect speed | | ix | Effect intensity | | c1 ...
(QB_NEW_EN)
[grammar] ~272-~272: There might be a mistake here.
Context: ...speed | | ix | Effect intensity | | c1 | Custom 1 | | c2 | C...
(QB_NEW_EN)
[grammar] ~273-~273: There might be a mistake here.
Context: ...ect intensity | | c1 | Custom 1 | | c2 | Custom 2 | | c3 | C...
(QB_NEW_EN)
[grammar] ~274-~274: There might be a mistake here.
Context: ... | Custom 1 | | c2 | Custom 2 | | c3 | Custom 3 | | o1 | O...
(QB_NEW_EN)
[grammar] ~275-~275: There might be a mistake here.
Context: ... | Custom 2 | | c3 | Custom 3 | | o1 | Option 1 | | o2 | O...
(QB_NEW_EN)
[grammar] ~276-~276: There might be a mistake here.
Context: ... | Custom 3 | | o1 | Option 1 | | o2 | Option 2 | | o3 | O...
(QB_NEW_EN)
[grammar] ~277-~277: There might be a mistake here.
Context: ... | Option 1 | | o2 | Option 2 | | o3 | Option 3 | Using this inf...
(QB_NEW_EN)
[grammar] ~284-~284: Ensure spelling is correct
Context: ...rks the end of the Effect Name, and the begining of the Parameter String elements.) | | ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~286-~286: There might be a mistake here.
Context: ...writes the default value of Intensity. | | (blank), | unus...
(QB_NEW_EN)
[grammar] ~288-~288: There might be a mistake here.
Context: ...e beginning of the Colors parameter. | | Color; | Custom color...
(QB_NEW_EN)
[grammar] ~289-~289: There might be a mistake here.
Context: ... | Custom color field (SEGCOLOR(0)) | | (blank); | Empty mean...
(QB_NEW_EN)
[style] ~290-~290: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...s to be selected by the user. But used in conjunction with the checkbox argument, palette use can ...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[grammar] ~291-~291: There might be a mistake here.
Context: ... the effect requires a 2D matrix setup | | pal=35" | Default Pal...
(QB_NEW_EN)
[grammar] ~300-~300: Ensure spelling is correct
Context: ... the UI. we will break this effect down tep by step. (This effect was originally on...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~300-~300: There might be a mistake here.
Context: ...will break this effect down tep by step. (This effect was originally one of the F...
(QB_NEW_EN)
[grammar] ~306-~306: There might be a mistake here.
Context: ...rst line of code defines sinelon base as static helper function. This is how al...
(QB_NEW_EN)
[grammar] ~306-~306: There might be a mistake here.
Context: ...s how all effects are initially defined. * Notice that it has some optional flags; ...
(QB_NEW_EN)
[grammar] ~318-~318: There might be a mistake here.
Context: ...LEDs each frame using SEGMENT.intensity as fade amount. * Creates the trailing "co...
(QB_NEW_EN)
[grammar] ~318-~318: There might be a mistake here.
Context: ... using SEGMENT.intensity as fade amount. * Creates the trailing "comet" effect by l...
(QB_NEW_EN)
[grammar] ~331-~331: There might be a mistake here.
Context: ... temporary state variable to keep track of last position.) The next lines of code...
(QB_NEW_EN)
[style] ~355-~355: ‘takes into account’ might be wordy. Consider a shorter alternative.
Context: ...with the selected color. The next line takes into account another one of the optional arguments f...
(EN_WORDINESS_PREMIUM_TAKES_INTO_ACCOUNT)
[grammar] ~365-~365: There might be a mistake here.
Context: ...lback to same palette color as color1. * In rainbow mode, force both dots to sh...
(QB_NEW_EN)
[grammar] ~385-~385: There might be a mistake here.
Context: ... = pos; } ``` * The first line checks if current position has changed since last...
(QB_NEW_EN)
[grammar] ~385-~385: There might be a mistake here.
Context: ... checks if current position has changed since last frame. (Prevents holes if the dot...
(QB_NEW_EN)
[grammar] ~385-~385: There might be a mistake here.
Context: ... logic to update the rest of the pixels. * Fills in all pixels between previous pos...
(QB_NEW_EN)
[grammar] ~386-~386: There might be a mistake here.
Context: ...(pos) to ensure smooth continuous trail. * Works in both directions: Forward (if ne...
(QB_NEW_EN)
[grammar] ~387-~387: There might be a mistake here.
Context: ...s), and Backward (if new pos < old pos). * Updates SEGENV.aux0 to current positio...
(QB_NEW_EN)
[grammar] ~388-~388: There might be a mistake here.
Context: ...pos < old pos). * Updates SEGENV.aux0 to current position at the end. Finally, ...
(QB_NEW_EN)
[grammar] ~395-~395: There might be a mistake here.
Context: ...} ``` * Returns FRAMETIME constant to set effect update rate (usually ~16 ms). T...
(QB_NEW_EN)
[grammar] ~397-~397: There might be a mistake here.
Context: ...r functions for different Sinelon modes. Notice that there are three different mo...
(QB_NEW_EN)
[grammar] ~416-~416: Ensure spelling is correct
Context: ...the metadata strings for each effect to speicfy how it will be portrayed in the UI: ```...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~453-~453: There might be a mistake here.
Context: ...tches the Usermod base class definition. * The strip.addEffect line is an importa...
(QB_NEW_EN)
[grammar] ~455-~455: There might be a mistake here.
Context: ...e all custom effects with the 255 ID.**) * &mode_diffusionfire: Pointer to the effect function. * `_...
(QB_NEW_EN)
[grammar] ~456-~456: There might be a mistake here.
Context: ...onfire: Pointer to the effect function. * _data_FX_MODE_DIFFUSIONFIRE`: Metadata string stored in PROGMEM, des...
(QB_NEW_EN)
[grammar] ~464-~464: Ensure spelling is correct
Context: ...y. The final part of this file handles instatiation and initialization: ```cpp static UserF...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~470-~470: There might be a mistake here.
Context: ...valid usermod — load it during startup.” * WLED adds it to the list of active userm...
(QB_NEW_EN)
[grammar] ~475-~475: Ensure spelling is correct
Context: ... ## Combining Multiple Effects in this Usermod So now let's say that you wanted add the...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~478-~478: There might be a mistake here.
Context: ...9fbbe82aa23a9b97b8/wled00/FX.cpp#L3110). * Copy this code, and place it below the m...
(QB_NEW_EN)
[style] ~479-~479: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...ell--and to name it something different than what's already inside the core WLED cod...
(DIFFERENT_THAN)
[grammar] ~479-~479: There might be a mistake here.
Context: ...ing section above for more information.) * Register the effect using the `addEffect...
(QB_NEW_EN)
[grammar] ~483-~483: There might be a mistake here.
Context: ...class. * Compile the code! ## Compiling Compiling WLED yourself is beyond the scope of th...
(QB_NEW_EN)
[grammar] ~484-~484: Ensure spelling is correct
Context: ...ed.ge/advanced/compiling-wled/), on the offical WLED documentation website. ## Change...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
269-269: Hard tabs
Column: 12
(MD010, no-hard-tabs)
271-271: Hard tabs
Column: 5
(MD010, no-hard-tabs)
272-272: Hard tabs
Column: 5
(MD010, no-hard-tabs)
273-273: Hard tabs
Column: 5
(MD010, no-hard-tabs)
274-274: Hard tabs
Column: 5
(MD010, no-hard-tabs)
275-275: Hard tabs
Column: 5
(MD010, no-hard-tabs)
276-276: Hard tabs
Column: 12
(MD010, no-hard-tabs)
277-277: Hard tabs
Column: 5
(MD010, no-hard-tabs)
278-278: Hard tabs
Column: 5
(MD010, no-hard-tabs)
🔇 Additional comments (2)
usermods/user_fx/README.md (2)
323-329: Function name likelybeatsin16, notbeatsin16_tPlease confirm the exact helper available in WLED; examples typically use
beatsin16.-unsigned pos = beatsin16_t(SEGMENT.speed/10, 0, SEGLEN-1); +unsigned pos = beatsin16(SEGMENT.speed / 10, 0, SEGLEN - 1);
223-229: Fix palette call and constant
LINEARBLEND_NOWRAPis not a validColorFromPaletteblend type. Either useSEGMENT.color_from_palette(...)(preferred) orLINEARBLEND.-uint32_t color = ColorFromPalette(SEGPALETTE, tmp_row[x], 255, LINEARBLEND_NOWRAP); +uint32_t color = SEGMENT.color_from_palette(tmp_row[x], true /*wrap*/, false /*blend*/, 0);Or:
-uint32_t color = ColorFromPalette(SEGPALETTE, tmp_row[x], 255, LINEARBLEND_NOWRAP); +uint32_t color = ColorFromPalette(SEGPALETTE, tmp_row[x], 255, LINEARBLEND);Likely an incorrect or invalid review comment.
usermods/user_fx/README.md
Outdated
| uint8_t tmp_row[cols]; | ||
| SEGENV.step = strip.now; | ||
| // scroll up |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid variable-length array (non-standard C++)
uint8_t tmp_row[cols]; is a VLA and not valid in standard C++. Use a small heap buffer or reuse SEGENV.data/a scratch buffer.
-uint8_t tmp_row[cols];
+std::unique_ptr<uint8_t[]> tmp_row(new uint8_t[cols]);Or in WLED style without STL:
-uint8_t tmp_row[cols];
+uint8_t* tmp_row = static_cast<uint8_t*>(malloc(cols));
+if (!tmp_row) return mode_static();
+// ... use tmp_row ...
+free(tmp_row);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In usermods/user_fx/README.md around lines 149 to 151, the code uses a
variable-length array uint8_t tmp_row[cols]; which is not valid in standard C++;
replace the VLA with either a small heap allocation (e.g., allocate/free a
uint8_t* with new[]/delete[] or malloc/free) or reuse an existing fixed-size
scratch buffer such as SEGENV.data (or a static/extern fixed-size array) to hold
the row, and update usages to refer to the new buffer; ensure proper bounds
checks and cleanup if using heap allocation.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
DedeHai
left a comment
DedeHai
left a comment
There was a problem hiding this comment.
Very nice rundown of everything important, a few minor changes as suggested, then its good to merge.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
usermods/user_fx/README.md (2)
275-284: Replace hard tabs in table (MD010) and align columnsTabs break rendering and linting. Use spaces.
-| Parameter | Default tooltip label | +| Parameter | Default tooltip label | @@ -| sx | Effect speed | -| ix | Effect intensity | -| c1 | Custom 1 | -| c2 | Custom 2 | -| c3 | Custom 3 | -| o1 | Option 1 | -| o2 | Option 2 | -| o3 | Option 3 | +| sx | Effect speed | +| ix | Effect intensity | +| c1 | Custom 1 | +| c2 | Custom 2 | +| c3 | Custom 3 | +| o1 | Option 1 | +| o2 | Option 2 | +| o3 | Option 3 |
120-125: Allocate 2D state for the virtual area, not physical lengthUsing SEGMENT.length() can over/under-allocate when grouping/mirroring is active. Use cols*rows (or SEGLEN) and prefer size_t.
-unsigned dataSize = SEGMENT.length(); +size_t dataSize = cols * rows; // or: SEGLEN
🧹 Nitpick comments (7)
usermods/user_fx/README.md (7)
165-167: Prose mismatch with code: use SEGENV.data, not SEGMENT.dataThe text still says “update SEGMENT.data” while the code correctly uses SEGENV.data.
-You'll see later that it writes results here before updating `SEGMENT.data`. +You'll see later that it writes results here before updating `SEGENV.data`.
168-168: Fix typos and units in the VLA cautionTighten wording; use KiB and “LEDs”.
-| IMPORTANT NOTE: Creating variable length arrays is non C++ standard, but this practic is used frequently throughout WLED as it works just fine. But be aware that creating variable length arrays puts them in stack memory, which is limited in size. If for example the array is built from the length of the segment (in 1D), that can overflow the stack and make it crash. The limit that can be safely used is ~1kb; an array with 4000 leds would be 4k and will likely crash. It gets worse if using `uint16_t`. So anything larger than 1k should definitely go into `SEGENV.data` which has a much higher limit. | +| IMPORTANT NOTE: Creating variable‑length arrays is non‑standard C++, but this practice is used in WLED and works. Be aware VLAs live on the stack, which is limited. If the array scales with segment length (1D), it may overflow the stack and crash. Keep VLAs ≤ ~1 KiB; an array for 4000 LEDs (~4 KiB as uint8_t) will likely crash. It’s worse with uint16_t. Anything larger than ~1 KiB should go into `SEGENV.data`, which has a much higher limit. |
49-51: Use permalinks for function references to avoid line driftA few links still point to main with line numbers. Switch to the same commit hash you already use elsewhere (e.g., 75f6de9…) to keep links stable.
-| [`SEGMENT.setPixelColor`](https://github.com/WLED/WLED/blob/main/wled00/FX_fcn.cpp#L848) / [`setPixelColorXY`](https://github.com/WLED/WLED/blob/main/wled00/FX_2Dfcn.cpp#L86) | 32-bit | Function that paints one pixel. `setPixelColor` is 1‑D; `setPixelColorXY` expects `(x, y)` and an RGBW color value. | +| [`SEGMENT.setPixelColor`](https://github.com/WLED/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX_fcn.cpp) / [`setPixelColorXY`](https://github.com/WLED/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX_2Dfcn.cpp) | 32-bit | Function that paints one pixel. `setPixelColor` is 1‑D; `setPixelColorXY` expects `(x, y)` and an RGBW color value. |Repeat similarly for lines 50–51.
294-294: Name the palette helper consistentlyThe code uses SEGMENT.color_from_palette; the prose says ColorFromPalette. Align the text.
-... the effect uses a palette `ColorFromPalette()`, otherwise it fades ... +... the effect uses a palette via `SEGMENT.color_from_palette(...)`, otherwise it fades ...
305-307: Minor grammar/capitalization-we will break this effect down step by step. +We will break this effect down step by step.
312-314: Typo: function name in prose-* The first line of code defines `sinelon base` as static helper function. This is how all effects are initially defined. +* The first line defines `sinelon_base` as a static helper function. This is how the effect is initially defined.
261-261: Soften the absolute “ALWAYS return FRAMETIME”Some effects intentionally vary the returned delay. Recommend “should” rather than “always”.
-**Also note that an Effect should ALWAYS return FRAMETIME. Not doing so can cause glitches.** +**In most cases an effect should return FRAMETIME; deviating can cause timing glitches unless you know the implications.**
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
PR: wled/WLED#4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
PR: wled/WLED#4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:43:15.355Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-30T05:41:03.633Z
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-09-11T04:36:42.851Z
Learnt from: DedeHai
PR: wled/WLED#4754
File: usermods/user_fx/README.md:169-179
Timestamp: 2025-09-11T04:36:42.851Z
Learning: In WLED, SEGENV.data is already typed as uint8_t* and can be accessed directly without casting when storing per-pixel state data.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
PR: wled/WLED#4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-27T09:37:28.415Z
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-08T17:31:47.330Z
Learnt from: netmindz
PR: wled/WLED#4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-06-07T15:58:42.579Z
Learnt from: willmmiles
PR: wled/WLED#4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-21T00:00:17.196Z
Learnt from: willmmiles
PR: wled/WLED#4859
File: usermods/udp_name_sync/udp_name_sync.cpp:62-70
Timestamp: 2025-08-21T00:00:17.196Z
Learning: In WLED codebase, UDP packet processing code must never rely on null termination being present on the wire and must always validate packet size first. The codebase already has some vulnerabilities where malformed data can crash the software, so avoiding introduction of new ones is a priority.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-02-19T12:43:34.200Z
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:48:21.296Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dx*dx + dy*dy to be a practical concern in particle system effects.
Applied to files:
usermods/user_fx/README.md
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
275-275: Hard tabs
Column: 12
(MD010, no-hard-tabs)
277-277: Hard tabs
Column: 5
(MD010, no-hard-tabs)
278-278: Hard tabs
Column: 5
(MD010, no-hard-tabs)
279-279: Hard tabs
Column: 5
(MD010, no-hard-tabs)
280-280: Hard tabs
Column: 5
(MD010, no-hard-tabs)
281-281: Hard tabs
Column: 5
(MD010, no-hard-tabs)
282-282: Hard tabs
Column: 12
(MD010, no-hard-tabs)
283-283: Hard tabs
Column: 5
(MD010, no-hard-tabs)
284-284: Hard tabs
Column: 5
(MD010, no-hard-tabs)
🔇 Additional comments (2)
usermods/user_fx/README.md (2)
16-25: Solid high-level structure — clear and approachableSection cleanly explains the usermod’s parts and flow. Nice work.
329-336: No change —beatsin16_tis correct and present in the repo. Declared in wled00/fcn_declare.h and defined in wled00/util.cpp; the README snippet is accurate.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
usermods/user_fx/README.md (6)
119-125: Allocate state for the rendered 2D subset (cols*rows or SEGLEN), not SEGMENT.length()For 2D segments, prefer the virtual area to size per‑pixel state. This stays correct with grouping/mirroring and aligns with WLED internals.
Apply:
-unsigned dataSize = SEGMENT.length(); +unsigned dataSize = cols * rows; // equivalently: SEGLENAlso adjust the explanatory bullet below to reference cols*rows (virtual dimensions) rather than “total number of LEDs”.
49-51: Stabilize GitHub links — avoid “main#L…” anchors that will rotLinks with line numbers against the moving main branch will drift. Use permalinks (commit SHA) or drop the “#L…” anchors.
Example:
-| [`SEGMENT.setPixelColor`](https://github.com/WLED/WLED/blob/main/wled00/FX_fcn.cpp#L848) / [`setPixelColorXY`](https://github.com/WLED/WLED/blob/main/wled00/FX_2Dfcn.cpp#L86) | 32-bit | Function that paints one pixel. `setPixelColor` is 1‑D; `setPixelColorXY` expects `(x, y)` and an RGBW color value. | +| [`SEGMENT.setPixelColor`](https://github.com/WLED/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX_fcn.cpp) / [`setPixelColorXY`](https://github.com/WLED/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX_2Dfcn.cpp) | 32-bit | Function that paints one pixel. `setPixelColor` is 1‑D; `setPixelColorXY` expects `(x, y)` and an RGBW color value. |Repeat for color_wheel, color_from_palette, move, blur entries.
Also applies to: 55-56
44-47: “SEGPALETTE” size column is misleadingSEGPALETTE is a macro that resolves to the active palette, not a 32‑bit value. Mark size as “—” to avoid implying width.
-| [`SEGPALETTE`](https://github.com/wled/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX.h#L115) | 32-bit | Macro that gets the currently selected palette for the currently processing segment. | +| [`SEGPALETTE`](https://github.com/wled/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX.h#L115) | — | Currently selected palette for the segment. |
163-168: Fix minor typos and units in the VLA noteSmall copyedits improve clarity.
-* The conditional statement in the first line fo code ensures the effect updates on a fixed interval — e.g., every 20 ms for 50 Hz. +* The conditional statement in the first line of code ensures the effect updates on a fixed interval — e.g., every 20 ms for 50 Hz. @@ -> **_IMPORTANT NOTE:_** Creating variable length arrays is non C++ standard, but this practic is used frequently throughout WLED as it works just fine. But be aware that creating variable length arrays puts them in stack memory, which is limited in size. If for example the array is built from the length of the segment (in 1D), that can overflow the stack and make it crash. The limit that can be safely used is ~1kb; an array with 4000 leds would be 4k and will likely crash. It gets worse if using `uint16_t`. So anything larger than 1k should definitely go into `SEGENV.data` which has a much higher limit. +> **_IMPORTANT NOTE:_** Creating variable‑length arrays is non‑standard C++, but this practice is used throughout WLED and works in practice. Be aware VLAs live on the stack, which is limited. If the array scales with segment length (1D), it can overflow the stack and crash. Keep VLAs ≲ ~1 KiB; an array with 4000 LEDs is ~4 KiB and will likely crash. It’s worse with `uint16_t`. Anything larger than ~1 KiB should go into `SEGENV.data`, which has a higher limit.
275-285: Replace hard tabs in table (MD010) and align columnsTabs break markdownlint and can render inconsistently.
-| Parameter | Default tooltip label | -| :-------- | :-------------------- | -| sx | Effect speed | -| ix | Effect intensity | -| c1 | Custom 1 | -| c2 | Custom 2 | -| c3 | Custom 3 | -| o1 | Option 1 | -| o2 | Option 2 | -| o3 | Option 3 | +| Parameter | Default tooltip label | +| :-------- | :-------------------- | +| sx | Effect speed | +| ix | Effect intensity | +| c1 | Custom 1 | +| c2 | Custom 2 | +| c3 | Custom 3 | +| o1 | Option 1 | +| o2 | Option 2 | +| o3 | Option 3 |
300-300: Optional: link to metadata docs once, then remove repeated “More information…” lineAvoid redundancy; you already linked earlier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
PR: wled/WLED#4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
PR: wled/WLED#4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:43:15.355Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-30T05:41:03.633Z
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-09-11T04:36:42.851Z
Learnt from: DedeHai
PR: wled/WLED#4754
File: usermods/user_fx/README.md:169-179
Timestamp: 2025-09-11T04:36:42.851Z
Learning: In WLED, SEGENV.data is already typed as uint8_t* and can be accessed directly without casting when storing per-pixel state data.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
PR: wled/WLED#4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-08T17:31:47.330Z
Learnt from: netmindz
PR: wled/WLED#4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-27T09:37:28.415Z
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-06-07T15:58:42.579Z
Learnt from: willmmiles
PR: wled/WLED#4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-21T00:00:17.196Z
Learnt from: willmmiles
PR: wled/WLED#4859
File: usermods/udp_name_sync/udp_name_sync.cpp:62-70
Timestamp: 2025-08-21T00:00:17.196Z
Learning: In WLED codebase, UDP packet processing code must never rely on null termination being present on the wire and must always validate packet size first. The codebase already has some vulnerabilities where malformed data can crash the software, so avoiding introduction of new ones is a priority.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-02-19T12:43:34.200Z
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:48:21.296Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dx*dx + dy*dy to be a practical concern in particle system effects.
Applied to files:
usermods/user_fx/README.md
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
275-275: Hard tabs
Column: 12
(MD010, no-hard-tabs)
277-277: Hard tabs
Column: 5
(MD010, no-hard-tabs)
278-278: Hard tabs
Column: 5
(MD010, no-hard-tabs)
279-279: Hard tabs
Column: 5
(MD010, no-hard-tabs)
280-280: Hard tabs
Column: 5
(MD010, no-hard-tabs)
281-281: Hard tabs
Column: 5
(MD010, no-hard-tabs)
282-282: Hard tabs
Column: 12
(MD010, no-hard-tabs)
283-283: Hard tabs
Column: 5
(MD010, no-hard-tabs)
284-284: Hard tabs
Column: 5
(MD010, no-hard-tabs)
🔇 Additional comments (1)
usermods/user_fx/README.md (1)
227-236: Good fix: use SEGENV.data and SEGMENT.color_from_palette correctlyPer-effect state now writes to SEGENV.data (uint8_t*) and rendering uses SEGMENT.color_from_palette; this matches WLED conventions and avoids aliasing issues. Nicely done.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
usermods/user_fx/user_fx.cpp (2)
62-72: Diffusion reads from SEGENV.data are consistent; consider minor numeric safetyReads are consistent. Small nit: v can reach 3255=765; v100 fits in 32-bit, but ensure v is at least 16-bit/32-bit to avoid accidental narrowing. If you want to be explicit:
- declare v as uint16_t or uint32_t.
- unsigned v = SEGENV.data[XY(x, y)]; + uint16_t v = SEGENV.data[XY(x, y)];
40-41: VLA on stack is fine here; document or guard for larger matricesuint8_t tmp_row[cols] is a VLA and commonly used in WLED, but stack‑limited. Since this effect targets ≤16×16 it’s safe; for bigger matrices consider reusing SEGENV.data or heap to avoid stack overflow.
- uint8_t tmp_row[cols]; + // Keep for ≤~1 KiB; otherwise consider heap or reuse SEGENV.data as scratch. + uint8_t tmp_row[cols];usermods/user_fx/README.md (4)
168-168: Fix typo: “Bbut” → “But”Minor doc typo in the IMPORTANT NOTE paragraph.
-> **_IMPORTANT NOTE:_** Creating variable‑length arrays (VLAs) is non‑standard C++, but this practice is used throughout WLED and works in practice. Bbut be aware that VLAs live on the stack, which is limited. +> **_IMPORTANT NOTE:_** Creating variable‑length arrays (VLAs) is non‑standard C++, but this practice is used throughout WLED and works in practice. But be aware that VLAs live on the stack, which is limited.
275-285: Replace hard tabs with spaces (MD010) and normalize spacing in the params tableHard tabs break markdownlint and inconsistent alignment on GitHub.
-| Parameter | Default tooltip label | -| :-------- | :-------------------- | -| sx | Effect speed | -| ix | Effect intensity | -| c1 | Custom 1 | -| c2 | Custom 2 | -| c3 | Custom 3 | -| o1 | Option 1 | -| o2 | Option 2 | -| o3 | Option 3 | +| Parameter | Default tooltip label | +| :-------- | :-------------------- | +| sx | Effect speed | +| ix | Effect intensity | +| c1 | Custom 1 | +| c2 | Custom 2 | +| c3 | Custom 3 | +| o1 | Option 1 | +| o2 | Option 2 | +| o3 | Option 3 |
119-125: Clarify SEGLEN parenthetical“(or SEGLEN)” might confuse new readers; explicitly state SEGLEN is the 2D virtual length (vWidth*vHeight).
-unsigned dataSize = cols * rows; // or: SEGLEN for virtual length +unsigned dataSize = cols * rows; // SEGLEN (virtual length) is equivalent to vWidth()*vHeight() for 2D
227-236: Palette path wording: align with implementationText says “ColorFromPalette()” but code uses SEGMENT.color_from_palette(...). Update description to match the implementation and note RGBW + wrap behavior here (you already mention it earlier).
-... the effect uses a palette `ColorFromPalette()`, otherwise it fades ... +... the effect uses `SEGMENT.color_from_palette(...)` (RGBW-aware, respects wrap), otherwise it fades ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
usermods/user_fx/README.md(1 hunks)usermods/user_fx/user_fx.cpp(3 hunks)
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: 2025-09-11T04:36:42.851Z
Learnt from: DedeHai
PR: wled/WLED#4754
File: usermods/user_fx/README.md:169-179
Timestamp: 2025-09-11T04:36:42.851Z
Learning: In WLED, SEGENV.data is already typed as uint8_t* and can be accessed directly without casting when storing per-pixel state data.
Applied to files:
usermods/user_fx/user_fx.cppusermods/user_fx/README.md
📚 Learning: 2025-06-07T15:58:42.579Z
Learnt from: willmmiles
PR: wled/WLED#4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Applied to files:
usermods/user_fx/user_fx.cppusermods/user_fx/README.md
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
PR: wled/WLED#4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
PR: wled/WLED#4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:43:15.355Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-30T05:41:03.633Z
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
PR: wled/WLED#4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-08T17:31:47.330Z
Learnt from: netmindz
PR: wled/WLED#4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-27T09:37:28.415Z
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-21T00:00:17.196Z
Learnt from: willmmiles
PR: wled/WLED#4859
File: usermods/udp_name_sync/udp_name_sync.cpp:62-70
Timestamp: 2025-08-21T00:00:17.196Z
Learning: In WLED codebase, UDP packet processing code must never rely on null termination being present on the wire and must always validate packet size first. The codebase already has some vulnerabilities where malformed data can crash the software, so avoiding introduction of new ones is a priority.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-02-19T12:43:34.200Z
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:48:21.296Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dx*dx + dy*dy to be a practical concern in particle system effects.
Applied to files:
usermods/user_fx/README.md
🧬 Code graph analysis (1)
usermods/user_fx/user_fx.cpp (1)
usermods/usermod_v2_animartrix/usermod_v2_animartrix.cpp (1)
SEGENV(64-76)
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
275-275: Hard tabs
Column: 12
(MD010, no-hard-tabs)
277-277: Hard tabs
Column: 5
(MD010, no-hard-tabs)
278-278: Hard tabs
Column: 5
(MD010, no-hard-tabs)
279-279: Hard tabs
Column: 5
(MD010, no-hard-tabs)
280-280: Hard tabs
Column: 5
(MD010, no-hard-tabs)
281-281: Hard tabs
Column: 5
(MD010, no-hard-tabs)
282-282: Hard tabs
Column: 12
(MD010, no-hard-tabs)
283-283: Hard tabs
Column: 5
(MD010, no-hard-tabs)
284-284: Hard tabs
Column: 5
(MD010, no-hard-tabs)
🔇 Additional comments (4)
usermods/user_fx/user_fx.cpp (4)
30-33: Good: allocate per‑pixel state for the 2D virtual areaUsing cols*rows (virtual area) with SEGENV.allocateData() is correct and avoids over/under‑allocating when mirroring/grouping are enabled.
39-49: Scroll-up copy now uses SEGENV.data correctlySwitching the state buffer from SEGMENT.data to SEGENV.data for the in-place upward shift is the right fix for per‑effect state.
50-58: Bottom-row spark writes use SEGENV.data — correctDirect writes to SEGENV.data[dst] = 255 align with WLED’s effect-local storage. No cast needed since SEGENV.data is uint8_t*.
74-83: Palette path and option toggle look good; tiny API sanity check
- Using SEGMENT.option1 and SEGMENT.color_from_palette(...) is the preferred path; base-color fade fallback is fine.
- If you haven’t already, double‑check color_from_palette’s current parameter order in main to ensure the booleans match intended mapping/wrap behavior.
made CodeRabbit Changes to reflect what's going in the README.
made some final nitpick changes suggested by coderabbit
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
usermods/user_fx/README.md (1)
275-285: Replace hard tabs with spaces (MD010) and align tableTabs break GitHub rendering and fail markdownlint. Convert to spaces and keep pipe alignment.
-| Parameter | Default tooltip label | +| Parameter | Default tooltip label | | :-------- | :-------------------- | -| sx | Effect speed | -| ix | Effect intensity | -| c1 | Custom 1 | -| c2 | Custom 2 | -| c3 | Custom 3 | -| o1 | Option 1 | -| o2 | Option 2 | -| o3 | Option 3 | +| sx | Effect speed | +| ix | Effect intensity | +| c1 | Custom 1 | +| c2 | Custom 2 | +| c3 | Custom 3 | +| o1 | Option 1 | +| o2 | Option 2 | +| o3 | Option 3 |
🧹 Nitpick comments (7)
usermods/user_fx/README.md (7)
165-167: Fix stale reference: SEGENV.data, not SEGMENT.dataText says it updates SEGMENT.data, but code uses SEGENV.data. Align wording.
-* You'll see later that it writes results here before updating `SEGMENT.data`. +* You'll see later that it writes results here before updating `SEGENV.data`.
123-124: Tighten wording (“or (or SEGLEN)” typo)Minor grammar nit.
-* `cols * rows` or `(or SEGLEN)` returns the total number of LEDs in the current segment. +* `cols * rows` (i.e., `SEGLEN`) returns the total number of LEDs in the current segment.
227-235: Use option1 consistently (docs and code)Earlier you document option1/2/3; here you use check1. Prefer option1 for reader consistency.
- if (SEGMENT.check1) { + if (SEGMENT.option1) {-* The if statement creates a conditional rendering path — the user can toggle this. If `check1` is enabled in the effect metadata, we use a color palette to display the flame. +* The if statement creates a conditional rendering path — the user can toggle this. If `option1` is enabled in the effect metadata, we use a color palette to display the flame.Also applies to: 241-247
44-51: Normalize link strategy (permalinks vs main)Mixed use of commit-pinned permalinks and “main” URLs can drift. Pick one strategy (prefer permalinks for code line anchors; “main” for stable files) and apply consistently across the table.
Do you want me to generate a commit replacing all line-anchored links with permalinks to a single commit hash?
Also applies to: 49-56
288-299: Repair metadata breakdown table cells and quotingLeft column entries should be self-contained tokens; add closing quotes/backticks and tidy wording.
-| "Diffusion Fire@! | Name. (The @ symbol marks the end of the Effect Name, and the beginning of the Parameter String elements.) | -| !, | Use default UI entry; for the first space, this will automatically create a slider for Speed | -| Spark rate, Diffusion Speed, Turbulence, | UI sliders for Spark Rate, Diffusion Speed, and Turbulence. Defining slider 2 as "Spark Rate" overwrites the default value of Intensity. | -| (blank), | unused (empty field with not even a space) | -| Use palette; | This occupies the spot for the 6th effect parameter, which automatically makes this a checkbox argument `o1` called Use palette in the UI. When this is enabled, the effect uses `SEGMENT.color_from_palette(...)` (RGBW-aware, respects wrap), otherwise it fades from `SEGCOLOR(0)`. The first semicolon marks the end of the Effect Parameters and the beginning of the `Colors` parameter. | -| Color; | Custom color field `(SEGCOLOR(0))` | -| (blank); | Empty means the effect does not allow Palettes to be selected by the user. But used in conjunction with the checkbox argument, palette use can be turned on/off by the user. | -| 2; | Flag specifying that the effect requires a 2D matrix setup | -| pal=35" | Default Palette ID. this is the setting that the effect starts up with. | +| "Diffusion Fire@!" | Name. The "@" marks the end of the effect name and the start of the parameter string. | +| `!,` | Use default UI entry; for the first slot this auto-creates the Speed slider. | +| `Spark rate, Diffusion Speed, Turbulence,` | UI sliders for Spark Rate, Diffusion Speed, and Turbulence. Naming slider 2 “Spark rate” overrides the default Intensity label. | +| `(blank),` | Unused (empty field; not even a space). | +| `Use palette;` | Occupies the 6th parameter slot, creating checkbox `o1` (“Use palette”). When enabled, uses `SEGMENT.color_from_palette(...)` (RGBW-aware, respects wrap); otherwise fades from `SEGCOLOR(0)`. The semicolon ends Parameters and begins Colors. | +| `Color;` | Custom color field (`SEGCOLOR(0)`). | +| `(blank);` | No palette selector shown in UI; combined with `o1`, palette use can be toggled on/off. | +| `2;` | Flag: effect requires a 2D matrix setup. | +| `pal=35` | Default palette ID the effect starts with. |
52-55: Fix spelling: “dependant” → “dependent”-| [`fade_out()`](... ) | --- | fade out function, higher rate = quicker fade. fading is highly dependant on frame rate (higher frame rates, faster fading). each frame will fade at max 9% or as little as 0.8%. | +| [`fade_out()`](... ) | --- | Fade-out function, higher rate = quicker fade. Fading is highly dependent on frame rate (higher frame rates, faster fading). Each frame will fade at up to ~9% or as little as ~0.8%. |
493-495: Either populate “Change Log” or remove the empty sectionEmpty headers read as unfinished.
Want me to convert this into a simple “Notable changes” list summarizing the tutorial additions?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
PR: wled/WLED#4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
PR: wled/WLED#4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:43:15.355Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-30T05:41:03.633Z
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-09-11T04:36:42.851Z
Learnt from: DedeHai
PR: wled/WLED#4754
File: usermods/user_fx/README.md:169-179
Timestamp: 2025-09-11T04:36:42.851Z
Learning: In WLED, SEGENV.data is already typed as uint8_t* and can be accessed directly without casting when storing per-pixel state data.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
PR: wled/WLED#4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-08T17:31:47.330Z
Learnt from: netmindz
PR: wled/WLED#4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-27T09:37:28.415Z
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-06-07T15:58:42.579Z
Learnt from: willmmiles
PR: wled/WLED#4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-21T00:00:17.196Z
Learnt from: willmmiles
PR: wled/WLED#4859
File: usermods/udp_name_sync/udp_name_sync.cpp:62-70
Timestamp: 2025-08-21T00:00:17.196Z
Learning: In WLED codebase, UDP packet processing code must never rely on null termination being present on the wire and must always validate packet size first. The codebase already has some vulnerabilities where malformed data can crash the software, so avoiding introduction of new ones is a priority.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-02-19T12:43:34.200Z
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:48:21.296Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dx*dx + dy*dy to be a practical concern in particle system effects.
Applied to files:
usermods/user_fx/README.md
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
275-275: Hard tabs
Column: 12
(MD010, no-hard-tabs)
277-277: Hard tabs
Column: 5
(MD010, no-hard-tabs)
278-278: Hard tabs
Column: 5
(MD010, no-hard-tabs)
279-279: Hard tabs
Column: 5
(MD010, no-hard-tabs)
280-280: Hard tabs
Column: 5
(MD010, no-hard-tabs)
281-281: Hard tabs
Column: 5
(MD010, no-hard-tabs)
282-282: Hard tabs
Column: 12
(MD010, no-hard-tabs)
283-283: Hard tabs
Column: 5
(MD010, no-hard-tabs)
284-284: Hard tabs
Column: 5
(MD010, no-hard-tabs)
🔇 Additional comments (1)
usermods/user_fx/README.md (1)
151-160: Good: SEGENV.data usage, timing gate, and VLA caveatState buffer is correctly moved to SEGENV.data, updates are time-gated with strip.now/SEGENV.step, and the VLA size warning is spot on.
Also applies to: 168-170, 171-183, 198-212, 225-237
changed uint16_t back to unsigned on line 64.
updated line 64, grammer fixes
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
usermods/user_fx/README.md (1)
275-285: Replace hard tabs with spaces in the parameter table (MD010)Tabs break rendering and fail markdownlint. Convert to spaces.
-| Parameter | Default tooltip label | +| Parameter | Default tooltip label | @@ -| sx | Effect speed | +| sx | Effect speed | @@ -| ix | Effect intensity | +| ix | Effect intensity | @@ -| c1 | Custom 1 | +| c1 | Custom 1 | @@ -| c2 | Custom 2 | +| c2 | Custom 2 | @@ -| c3 | Custom 3 | +| c3 | Custom 3 | @@ -| o1 | Option 1 | +| o1 | Option 1 | @@ -| o2 | Option 2 | +| o2 | Option 2 | @@ -| o3 | Option 3 | +| o3 | Option 3 |
🧹 Nitpick comments (6)
usermods/user_fx/README.md (6)
121-124: Tighten wording; fix small typo (“or (or SEGLEN)”)Clarify SEGLEN as virtual length and remove duplicated “or”.
- * `cols * rows` or `(or SEGLEN)` returns the total number of LEDs in the current segment. - * This fire effect models heat values per pixel (not just colors), so we need persistent storage — one uint8_t per pixel — for the entire effect. + * `cols * rows` (or `SEGLEN`) gives the virtual pixel count of the current segment (for 2D: `vWidth()*vHeight()`). + * This fire effect models heat values per pixel (not just colors), so we need persistent storage — one `uint8_t` per pixel — for the entire effect.
94-96: Note that XY here is for state only, not physical mappingAvoid confusion with segment’s serpentine/rotation mapping handled by setPixelColorXY.
-* This lambda helps with mapping a local 1D array to a 2D one. +* This lambda maps a local state buffer only. Physical LED mapping (rotation/serpentine) is handled by `SEGMENT.setPixelColorXY(...)`.
241-246: Align terminology: map o1 (metadata) to check1 (code)Readers may not know o1 corresponds to
SEGMENT.check1.-* The if statement creates a conditional rendering path — the user can toggle this. If `check1` is enabled in the effect metadata, we use a color palette to display the flame. +* The if statement creates a conditional rendering path — the user can toggle this. If `o1` (metadata) / `SEGMENT.check1` is enabled, we use a color palette to display the flame. @@ -| Use palette; | This occupies the spot for the 6th effect parameter, which automatically makes this a checkbox argument `o1` called Use palette in the UI. When this is enabled, the effect uses `SEGMENT.color_from_palette(...)` (RGBW-aware, respects wrap), otherwise it fades from `SEGCOLOR(0)`. The first semicolon marks the end of the Effect Parameters and the beginning of the `Colors` parameter. | +| Use palette; | This creates checkbox `o1` (exposed as `SEGMENT.check1`). When enabled, the effect uses `SEGMENT.color_from_palette(...)` (RGBW-aware, respects wrap); otherwise it fades from `SEGCOLOR(0)`. The first semicolon ends the Effect Parameters and begins `Colors`. |Also applies to: 294-296
259-263: Soften “ALWAYS return FRAMETIME” — it’s conventional, not absoluteSome effects intentionally throttle via the return value. Your gating on
strip.nowis fine; recommend “generally return FRAMETIME”.-* ⚠️ Important: Because the actual frame logic is gated by strip.now - SEGENV.step, returning FRAMETIME here doesn’t cause excessive updates — it just keeps the engine responsive. **Also note that an Effect should ALWAYS return FRAMETIME. Not doing so can cause glitches.** +* ⚠️ Important: Because the frame logic is gated by `strip.now - SEGENV.step`, returning `FRAMETIME` keeps the engine responsive without over-updating. **In general, effects should return `FRAMETIME`; return a custom delay only if you intentionally throttle via the return value.**
50-51: Tighten wording for palette helpers; avoid “which should be favoured”Concise, active voice; keep the technical distinction.
-| [`SEGMENT.color_wheel()`](https://github.com/wled/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX_fcn.cpp#L1092) | 32-bit | Input 0–255 to get a color. Transitions r→g→b→r. In HSV terms, `pos` is H. Note: only returns palette color unless the Default palette is selected. | +| [`SEGMENT.color_wheel()`](https://github.com/wled/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX_fcn.cpp#L1092) | 32-bit | Input 0–255 → r→g→b→r (HSV hue). Returns wheel color only if the Default palette is selected; otherwise returns a palette-derived color. | @@ -| [`SEGMENT.color_from_palette()`](https://github.com/wled/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX_fcn.cpp#L1093) | 32-bit | Gets a single color from the currently selected palette for a segment. (This function which should be favoured over `ColorFromPalette()` because this function returns an RGBW color with white from the `SEGCOLOR` passed, while also respecting the setting for palette wrapping. On the other hand, `ColorFromPalette()` simply gets the RGB palette color.) | +| [`SEGMENT.color_from_palette()`](https://github.com/wled/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX_fcn.cpp#L1093) | 32-bit | Preferred over `ColorFromPalette()`: returns RGBW (adds W from `SEGCOLOR`), and respects palette-wrapping settings. `ColorFromPalette()` returns RGB only. |
306-307: Capitalize sentence startTiny polish.
-we will break this effect down step by step. +We will break this effect down step by step.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
PR: wled/WLED#4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
PR: wled/WLED#4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:43:15.355Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-30T05:41:03.633Z
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-09-11T04:36:42.851Z
Learnt from: DedeHai
PR: wled/WLED#4754
File: usermods/user_fx/README.md:169-179
Timestamp: 2025-09-11T04:36:42.851Z
Learning: In WLED, SEGENV.data is already typed as uint8_t* and can be accessed directly without casting when storing per-pixel state data.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
PR: wled/WLED#4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-08T17:31:47.330Z
Learnt from: netmindz
PR: wled/WLED#4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-27T09:37:28.415Z
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-06-07T15:58:42.579Z
Learnt from: willmmiles
PR: wled/WLED#4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-21T00:00:17.196Z
Learnt from: willmmiles
PR: wled/WLED#4859
File: usermods/udp_name_sync/udp_name_sync.cpp:62-70
Timestamp: 2025-08-21T00:00:17.196Z
Learning: In WLED codebase, UDP packet processing code must never rely on null termination being present on the wire and must always validate packet size first. The codebase already has some vulnerabilities where malformed data can crash the software, so avoiding introduction of new ones is a priority.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-02-19T12:43:34.200Z
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:48:21.296Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dx*dx + dy*dy to be a practical concern in particle system effects.
Applied to files:
usermods/user_fx/README.md
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
275-275: Hard tabs
Column: 12
(MD010, no-hard-tabs)
277-277: Hard tabs
Column: 5
(MD010, no-hard-tabs)
278-278: Hard tabs
Column: 5
(MD010, no-hard-tabs)
279-279: Hard tabs
Column: 5
(MD010, no-hard-tabs)
280-280: Hard tabs
Column: 5
(MD010, no-hard-tabs)
281-281: Hard tabs
Column: 5
(MD010, no-hard-tabs)
282-282: Hard tabs
Column: 12
(MD010, no-hard-tabs)
283-283: Hard tabs
Column: 5
(MD010, no-hard-tabs)
284-284: Hard tabs
Column: 5
(MD010, no-hard-tabs)
🔇 Additional comments (1)
usermods/user_fx/README.md (1)
341-351: Confirm helper signatures used in README examplesVerify SEGMENT.color_from_palette(...) parameter order/types and whether the helper name is beatsin16_t or beatsin16; update the examples in usermods/user_fx/README.md (lines 227–233 and 341–351) if the API changed.
| Next we will look at some lines of code that handle memory allocation and effect initialization: | ||
|
|
||
| ```cpp | ||
| unsigned dataSize = cols * rows; // SEGLEN (virtual length) is equivalent to vWidth()*vHeight() for 2D |
There was a problem hiding this comment.
a quick not what virtual length means would be helpful. it is for example:
1D mapped to 2D -> virtual length depends on selected mode
grouping / spacing: virtual length shrinks
There was a problem hiding this comment.
to be honest, I think I need more clarification here as to what you mean. (which probably suggests that this explanation of virtual length would be helpful to many other readers too.)
@DedeHai Can you provide more detail on this one?
There was a problem hiding this comment.
not sure what is unclear... if you have a strip of say length = 60 and you enable grouping=2, virtual length will be 30, i.e. the FX renders 30 pixels, not 60.
edit: same goes for mirroring, it halves the size, or adding gaps, or rendering a 1D strip to 2D, there may be other cases, this is just from the top of my head.
There was a problem hiding this comment.
ok, take a look at what I wrote and see if it's conveying the info correctly:
"NOTE: Virtual lengths vWidth() and vHeight() will be evaluated differently based on your own custom effect, and based on what other settings are active. For example: If you have an LED strip of length = 60 and you enable grouping = 2, then the virtual length will be 30, so the FX will render 30 pixels instead of 60. This is also true for mirroring or adding gaps--it halves the size. For a 1D strip mapped to 2D, the virtual length depends on selected mode. Keep these things in mind during your custom effect's creation."
|
I added a few minor clarifications, after you update those, I think this is finally ready to merge. |
adding email contact info
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
usermods/user_fx/README.md (1)
277-287: Replace hard tabs in table (MD010).Tabs still present; switch to spaces for consistent rendering.
-| Parameter | Default tooltip label | +| Parameter | Default tooltip label | @@ -| sx | Effect Speed | -| ix | Effect Intensity | -| c1 | Custom 1 | -| c2 | Custom 2 | -| c3 | Custom 3 | -| o1 | Checkbox 1 | -| o2 | Checkbox 2 | -| o3 | Checkbox 3 | +| sx | Effect Speed | +| ix | Effect Intensity | +| c1 | Custom 1 | +| c2 | Custom 2 | +| c3 | Custom 3 | +| o1 | Checkbox 1 | +| o2 | Checkbox 2 | +| o3 | Checkbox 3 |
🧹 Nitpick comments (3)
usermods/user_fx/README.md (3)
41-42: Clarify custom3 range (0–31).Document the effective range to avoid surprises when users map UI values.
-| [`SEGMENT.custom3`](https://github.com/wled/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX.h#L454) | 5-bit | Another optional UI slider for custom effect control. While `SEGMENT.speed` through `SEGMENT.custom2` are 8-bit variables, `SEGMENT.custom3` is actually 5-bit. | +| [`SEGMENT.custom3`](https://github.com/wled/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX.h#L454) | 5-bit | Optional UI slider. Unlike the others, this is 5‑bit (range 0–31). |
88-98: Add a note about XY mapping vs physical layout.Your local XY lambda is fine for the state grid, but rendering handles serpentine/rotation. Add a one‑liner to prevent confusion.
* The third line declares a lambda function named `XY` to map (x, y) matrix coordinates into a 1D index in the LED array. This assumes row-major order (left to right, top to bottom). * This lambda helps with mapping a local 1D array to a 2D one. + * Note: this is for your effect’s internal state only. Physical mapping (serpentine, rotation) is handled by `SEGMENT.setPixelColorXY()`.
229-238: Consider naming thecolor_from_paletteparams in prose.Small doc tweak to explain
(index, mapping=true, wrap=false, mcol=0)for readers scanning the example.- uint32_t color = SEGMENT.color_from_palette(tmp_row[x], true, false, 0); + uint32_t color = SEGMENT.color_from_palette(tmp_row[x], /*mapping=*/true, /*wrap=*/false, /*mcol=*/0);And add a one‑liner below the snippet clarifying the parameter meanings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
PR: wled/WLED#4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
PR: wled/WLED#4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:43:15.355Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-30T05:41:03.633Z
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-09-11T04:36:42.869Z
Learnt from: DedeHai
PR: wled/WLED#4754
File: usermods/user_fx/README.md:169-179
Timestamp: 2025-09-11T04:36:42.869Z
Learning: In WLED, SEGENV.data is already typed as uint8_t* and can be accessed directly without casting when storing per-pixel state data.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
PR: wled/WLED#4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-08T17:31:47.330Z
Learnt from: netmindz
PR: wled/WLED#4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-27T09:37:28.415Z
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-06-07T15:58:42.579Z
Learnt from: willmmiles
PR: wled/WLED#4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-21T00:00:17.196Z
Learnt from: willmmiles
PR: wled/WLED#4859
File: usermods/udp_name_sync/udp_name_sync.cpp:62-70
Timestamp: 2025-08-21T00:00:17.196Z
Learning: In WLED codebase, UDP packet processing code must never rely on null termination being present on the wire and must always validate packet size first. The codebase already has some vulnerabilities where malformed data can crash the software, so avoiding introduction of new ones is a priority.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-09-16T18:08:42.848Z
Learnt from: DedeHai
PR: wled/WLED#4939
File: wled00/FX_fcn.cpp:1176-1187
Timestamp: 2025-09-16T18:08:42.848Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), intentionally allowing memory overruns when bus configurations exceed MAX_LED_MEMORY is a deliberate design choice. The trade-off prioritizes creating buses with reduced LED counts over completely failing to create buses, which would cause no LED output and UI failures. This approach forces users to update configurations after migrating to version 0.16 while maintaining basic functionality.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:48:21.296Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dx*dx + dy*dy to be a practical concern in particle system effects.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-02-19T12:43:34.200Z
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Applied to files:
usermods/user_fx/README.md
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
277-277: Hard tabs
Column: 12
(MD010, no-hard-tabs)
279-279: Hard tabs
Column: 5
(MD010, no-hard-tabs)
280-280: Hard tabs
Column: 5
(MD010, no-hard-tabs)
281-281: Hard tabs
Column: 5
(MD010, no-hard-tabs)
282-282: Hard tabs
Column: 5
(MD010, no-hard-tabs)
283-283: Hard tabs
Column: 5
(MD010, no-hard-tabs)
284-284: Hard tabs
Column: 12
(MD010, no-hard-tabs)
285-285: Hard tabs
Column: 5
(MD010, no-hard-tabs)
286-286: Hard tabs
Column: 5
(MD010, no-hard-tabs)
🔇 Additional comments (9)
usermods/user_fx/README.md (9)
7-15: TOC links look correct (relative + resilient).Good switch to relative anchors; these won’t 404 after merge.
43-46: Grammar: plural agreement (“persist”).“variables that persists” → “variables that persist”.
[raise_nitpick_refactor]-| [`SEGENV.aux0 / aux1`](https://github.com/wled/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX.h#L467) | 16-bit | These are state variables that persists between function calls, and they are free to be overwritten by the user for any use case. | +| [`SEGENV.aux0 / aux1`](https://github.com/wled/WLED/blob/75f6de9dc29fc7da5f301fc1388ada228dcb3b6e/wled00/FX.h#L467) | 16-bit | These are state variables that persist between function calls, and they are free to be overwritten by the user for any use case. |
53-53: Tighten wording for palette helper.Minor phrasing fix; keeps guidance intact.
[raise_nitpick_refactor]-| [`SEGMENT.color_from_palette()`](.../FX_fcn.cpp#L1093) | 32-bit | Gets a single color from the currently selected palette for a segment. (This function which should be favoured over `ColorFromPalette()` because this function returns an RGBW color with white from the `SEGCOLOR` passed, while also respecting the setting for palette wrapping. On the other hand, `ColorFromPalette()` simply gets the RGB palette color.) | +| [`SEGMENT.color_from_palette()`](.../FX_fcn.cpp#L1093) | 32-bit | Preferred over `ColorFromPalette()`: returns RGBW (adds W from `SEGCOLOR`) and respects palette wrapping; `ColorFromPalette()` returns RGB only. |
121-127: Clarify “virtual length” note; fix typo.Remove duplicated “or” and be explicit about SEGLEN in 1‑D vs 2‑D.
[raise_nitpick_refactor]-unsigned dataSize = cols * rows; // SEGLEN (virtual length) is equivalent to vWidth()*vHeight() for 2D +unsigned dataSize = cols * rows; // In 2‑D, SEGLEN (= vWidth()*vHeight()); in 1‑D, SEGLEN = vLength() @@ -* `cols * rows` or `(or SEGLEN)` returns the total number of pixels in the current segment. +* `cols * rows` (i.e., SEGLEN in 2‑D) is the total number of virtual pixels in the current segment.
154-171: Good VLA caution; matches WLED practice.Clear guidance on the ~1 KiB stack limit and when to switch to SEGENV.data.
If you want, I can add a short “rule of thumb” box summarizing: “≤1 KiB: VLA OK; >1 KiB or uint16_t rows: use SEGENV.data.”
175-183: State writes correctly use SEGENV.data.This fixes the earlier SEGMENT.data/SEGENV mismatch and avoids aliasing/casting. LGTM.
307-310: Capitalize sentence start.Minor polish.
[raise_nitpick_refactor]-we will break this effect down step by step. +We will break this effect down step by step.
335-337: Confirmbeatsin16_tdescription.If “beatsin16_t” is timebase‑aware variant, add a brief note or link; otherwise revert to “beatsin16”.
Would you like me to scan the codebase to confirm the exact semantics and add a permalinked reference?
500-504: Publishing contact email — confirm intent.PR summary suggested uncertainty about sharing contact info. Confirm you want this email public before merge.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
usermods/user_fx/README.md (7)
278-287: Fix MD010: replace hard tabs in tableTabs break rendering and violate markdownlint; switch to spaces.
-| Parameter | Default tooltip label | -| :-------- | :-------------------- | -| sx | Effect Speed | -| ix | Effect Intensity | -| c1 | Custom 1 | -| c2 | Custom 2 | -| c3 | Custom 3 | -| o1 | Checkbox 1 | -| o2 | Checkbox 2 | -| o3 | Checkbox 3 | +| Parameter | Default tooltip label | +| :-------- | :-------------------- | +| sx | Effect Speed | +| ix | Effect Intensity | +| c1 | Custom 1 | +| c2 | Custom 2 | +| c3 | Custom 3 | +| o1 | Checkbox 1 | +| o2 | Checkbox 2 | +| o3 | Checkbox 3 |
125-125: Typo: remove duplicated “or”Minor editorial polish.
- * `cols * rows` or `(or SEGLEN)` returns the total number of pixels in the current segment. + * `cols * rows` (or `SEGLEN`) returns the total number of pixels in the current segment.
44-44: Clarify SEGENV.step semantics (stores timestamp, not “elapsed time”)It’s the last-update timestamp set to strip.now each frame.
-| [`SEGENV.step`](https://github.../FX.h#L465) | 32-bit | This is a timestamp variable that contains the last update time. It is initially set during effect initialization to 0, and then it updates with the elapsed time after each frame runs. | +| [`SEGENV.step`](https://github.../FX.h#L465) | 32-bit | Timestamp of the last update (ms). Initialized on first run; then set to `strip.now` at each frame boundary. |
264-264: Soften “ALWAYS return FRAMETIME”Some effects intentionally return other intervals. Recommend “generally”.
-* ⚠️ Important: Because the actual frame logic is gated by strip.now - SEGENV.step, returning FRAMETIME here doesn’t cause excessive updates — it just keeps the engine responsive. **Also note that an Effect should ALWAYS return FRAMETIME. Not doing so can cause glitches.** +* ⚠️ Important: Because the actual frame logic is gated by `strip.now - SEGENV.step`, returning `FRAMETIME` keeps the engine responsive without over-updating. **In most cases, effects should return `FRAMETIME`; deviate only if you deliberately need a different cadence.**
53-53: Tighten wording and styleMinor grammar/style improvement; project docs elsewhere use “preferred” over “favoured”.
-| [`SEGMENT.color_from_palette()`](...) | 32-bit | Gets a single color from the currently selected palette for a segment. (This function which should be favoured over `ColorFromPalette()` because this function returns an RGBW color with white from the `SEGCOLOR` passed, while also respecting the setting for palette wrapping. On the other hand, `ColorFromPalette()` simply gets the RGB palette color.) | +| [`SEGMENT.color_from_palette()`](...) | 32-bit | Returns a color from the current palette (RGBW-aware; uses white from `SEGCOLOR` and respects palette wrap). Prefer this over `ColorFromPalette()`, which returns only RGB. |
5-5: Consistency nit: sentence‑case “effects”Keep capitalization consistent with the rest of the doc.
-Multiple Effects can be specified inside this single usermod, as we will illustrate below. You will be able to define them with custom names, sliders, etc. as with any other Effect. +Multiple effects can be specified inside this single usermod, as we will illustrate below. You can define them with custom names, sliders, etc., like any other effect.
332-336: Confirm function name: use beatsin16_t (not beatsin16) and add a link to its declaration.
Declaration found:uint16_t beatsin16_t(...)in wled00/util.cpp (declared in wled00/fcn_declare.h); update usermods/user_fx/README.md (lines 332–336) to reference beatsin16_t and link to the declaration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
usermods/user_fx/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
PR: wled/WLED#4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
PR: wled/WLED#4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.630Z
Learning: The WLED codebase has opportunities for refactoring hardcoded array bounds (like the "15" used for DMX channels) to use sizeof(array)/sizeof(array[0]) for more maintainable code, but such changes should be done consistently across the entire codebase in a dedicated refactoring effort.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:43:15.355Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-30T05:41:03.633Z
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-09-11T04:36:42.869Z
Learnt from: DedeHai
PR: wled/WLED#4754
File: usermods/user_fx/README.md:169-179
Timestamp: 2025-09-11T04:36:42.869Z
Learning: In WLED, SEGENV.data is already typed as uint8_t* and can be accessed directly without casting when storing per-pixel state data.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
PR: wled/WLED#4891
File: wled00/FX.cpp:3333-3349
Timestamp: 2025-08-31T03:38:14.114Z
Learning: WLED PacMan effect (wled00/FX.cpp): Keep pacmancharacters_t position fields as signed int (not int16_t). Maintainer preference (blazoncek) prioritizes avoiding potential overhead/regressions over minor RAM savings. Avoid type shrinking here unless memory pressure is demonstrated.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-08T17:31:47.330Z
Learnt from: netmindz
PR: wled/WLED#4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-04-27T09:37:28.415Z
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-06-07T15:58:42.579Z
Learnt from: willmmiles
PR: wled/WLED#4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-08-21T00:00:17.196Z
Learnt from: willmmiles
PR: wled/WLED#4859
File: usermods/udp_name_sync/udp_name_sync.cpp:62-70
Timestamp: 2025-08-21T00:00:17.196Z
Learning: In WLED codebase, UDP packet processing code must never rely on null termination being present on the wire and must always validate packet size first. The codebase already has some vulnerabilities where malformed data can crash the software, so avoiding introduction of new ones is a priority.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-09-16T18:08:42.848Z
Learnt from: DedeHai
PR: wled/WLED#4939
File: wled00/FX_fcn.cpp:1176-1187
Timestamp: 2025-09-16T18:08:42.848Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), intentionally allowing memory overruns when bus configurations exceed MAX_LED_MEMORY is a deliberate design choice. The trade-off prioritizes creating buses with reduced LED counts over completely failing to create buses, which would cause no LED output and UI failures. This approach forces users to update configurations after migrating to version 0.16 while maintaining basic functionality.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-05-09T18:48:21.296Z
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dx*dx + dy*dy to be a practical concern in particle system effects.
Applied to files:
usermods/user_fx/README.md
📚 Learning: 2025-02-19T12:43:34.200Z
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Applied to files:
usermods/user_fx/README.md
🪛 markdownlint-cli2 (0.17.2)
usermods/user_fx/README.md
278-278: Hard tabs
Column: 12
(MD010, no-hard-tabs)
280-280: Hard tabs
Column: 5
(MD010, no-hard-tabs)
281-281: Hard tabs
Column: 5
(MD010, no-hard-tabs)
282-282: Hard tabs
Column: 5
(MD010, no-hard-tabs)
283-283: Hard tabs
Column: 5
(MD010, no-hard-tabs)
284-284: Hard tabs
Column: 5
(MD010, no-hard-tabs)
285-285: Hard tabs
Column: 12
(MD010, no-hard-tabs)
286-286: Hard tabs
Column: 5
(MD010, no-hard-tabs)
287-287: Hard tabs
Column: 5
(MD010, no-hard-tabs)
| uint32_t color = SEGMENT.color_from_palette(tmp_row[x], true, false, 0); | ||
| SEGMENT.setPixelColorXY(x, y, color); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Double-check color_from_palette signature/flags
Verify param order and boolean meanings to avoid misleading readers (mapping/wrap/offset).
🏁 Script executed:
#!/bin/bash
# Locate definition and overloads
rg -nP -C3 '\bcolor_from_palette\s*\(' wled00
# Also show Segment class declaration
rg -nP -C5 'class\s+Segment\b|struct\s+Segment\b' wled00Length of output: 57575
Rename/fix color_from_palette third parameter (mapping vs wrap)
- Confirmed signature: Segment::color_from_palette(uint16_t i, bool mapping, bool moving, uint8_t mcol, uint8_t pbri = 255) (wled00/FX.h / wled00/FX_fcn.cpp).
- Semantics: 2nd arg = mapping (maps virtual length); 3rd arg is actually used as a palette "wrap/cutoff" flag in the implementation and many call sites pass PALETTE_* constants (not simple booleans) — the name/type "moving" is misleading and can lose information.
- Action: change the 3rd parameter to a descriptive name and appropriate type (e.g. uint8_t or an enum like PALETTE_* / bool wrap if only wrap/no-wrap is intended), update FX.h/FX_fcn.cpp and all call sites, and update the README example (usermods/user_fx/README.md lines ~233-234) to match.
Hey @DedeHai, just following up here. I believe the ball is in your court whenever you have time to check out the current state of the README. 👍 |
|
thanks for the reminder, I forgot to look at it, all good now. |
4 participants
Updating README for upcoming user_fx usermod.
(I had another PR open but in trying to resolve merge conflicts, I accidently closed it. That previous PR can be found here for reference.)
From my perspective, the last thing(s) to add here are:
Summary by CodeRabbit
Documentation
Improvements