Adding center bin selection to 2D GEQ #4764

DedeHai · 2025-07-10T07:15:03Z

setting custom3 to 0 gives the "old" behaviour, this is the default for new presets. Existing presets will have the custom3 slider at the center, changing presets that do not use the full width.

Summary by CodeRabbit

New Features
- Added a new control to the 2D graphic equalizer effect, allowing users to center the frequency bands around a user-defined bin.
- Updated effect settings to include a "Bin" parameter for enhanced customization of the equalizer display.

setting custom3 to 0 gives the "old" behaviour, this is the default. existing presets will have the custom3 slider at the center, changing presets that do not use the full width.

coderabbitai · 2025-07-10T07:15:14Z

Walkthrough

The mode_2DGEQ function in wled00/FX.cpp was updated to introduce a new CENTER_BIN parameter, allowing the 2D graphic equalizer effect to center its frequency band mapping around a user-defined bin. The effect's metadata string was also updated to reflect this new parameter.

Changes

File(s)	Change Summary
wled00/FX.cpp	Added `CENTER_BIN` parameter to `mode_2DGEQ`, revised band mapping logic, and updated effect metadata string to include "Bin" parameter.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c0875b3 and d25fc2e.

📒 Files selected for processing (1)

wled00/FX.cpp (3 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.

wled00/FX.cpp (7)

undefined

<retrieved_learning>
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.
</retrieved_learning>

<retrieved_learning>
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.
</retrieved_learning>

<retrieved_learning>
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.
</retrieved_learning>

<retrieved_learning>
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.
</retrieved_learning>

<retrieved_learning>
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.
</retrieved_learning>

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)

GitHub Check: wled_build / Build Environments (esp32_wrover)
GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
GitHub Check: wled_build / Build Environments (esp32dev_V4)
GitHub Check: wled_build / Build Environments (lolin_s2_mini)
GitHub Check: wled_build / Build Environments (esp32dev)
GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
GitHub Check: wled_build / Build Environments (esp32s3dev_8MB_opi)
GitHub Check: wled_build / Build Environments (usermods)
GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
GitHub Check: wled_build / Build Environments (esp32_eth)
GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
GitHub Check: wled_build / Build Environments (esp32c3dev)
GitHub Check: wled_build / Build Environments (nodemcuv2_compat)
GitHub Check: wled_build / Build Environments (esp8266_2m_160)
GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
GitHub Check: wled_build / Build Environments (nodemcuv2_160)
GitHub Check: wled_build / Build Environments (nodemcuv2)
GitHub Check: wled_build / Build Environments (esp01_1m_full)
GitHub Check: wled_build / Build Environments (esp8266_2m)

🔇 Additional comments (3)

wled00/FX.cpp (3)

7238-7238: LGTM: Proper parameter mapping for CENTER_BIN.

The mapping from SEGMENT.custom3 (0-31) to CENTER_BIN (0-15) is correct and aligns with the 16 available frequency bins. The retrieved learnings confirm that SEGMENT.custom3 is always constrained to 0-31 range, so no additional bounds checking is needed.

7260-7267: LGTM: Well-implemented centering logic with proper edge case handling.

The band centering implementation is sound:

Correct constraint calculation: constrain(CENTER_BIN - NUM_BANDS/2, 0, 15 - NUM_BANDS + 1) properly ensures the band range fits within [0, 15]

Edge case handling: The special case for NUM_BANDS <= 1 prevents issues with the map() function by directly using CENTER_BIN

Backward compatibility: When custom3=0 (default), CENTER_BIN=0 effectively restores the previous behavior

The integer division in NUM_BANDS/2 is intentional and appropriate for centering calculations.

7289-7289: LGTM: Proper metadata string update with appropriate default.

The metadata string correctly adds the "Bin" parameter with c3=0 as the default value, which maintains backward compatibility by centering around bin 0 (effectively restoring the previous behavior for existing presets).

✨ Finishing Touches

📝 Generate Docstrings

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

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

DedeHai · 2025-07-10T07:15:59Z

this is the proper implementation of #4760 and replaces that PR.

netmindz · 2025-07-10T17:24:24Z

wled00/FX.cpp

@@ -7235,6 +7235,7 @@ uint16_t mode_2DGEQ(void) { // By Will Tatam. Code reduction by Ewoud Wijma.
  if (!strip.isMatrix || !SEGMENT.is2D()) return mode_static(); // not a 2D set-up

  const int NUM_BANDS = map(SEGMENT.custom1, 0, 255, 1, 16);
+  const int CENTER_BIN = map(SEGMENT.custom3, 0, 31, 0, 15);


Can we use the constant rather than a magic number or is AC missing that constant?

I never came across such a constant if it exists. It could be defined at the top of FX.cpp.

It is in the usermod. I have moved AR effects into usremod in my fork which would make such use of constant easier.

DedeHai added 3 commits July 10, 2025 09:09

adding center bin selection to 2D GEQ

0bd8399

setting custom3 to 0 gives the "old" behaviour, this is the default. existing presets will have the custom3 slider at the center, changing presets that do not use the full width.

revert blank removal

832bae3

change comment, remove old code

d25fc2e

DedeHai mentioned this pull request Jul 10, 2025

Add new effect for a single EQ bar with peak marker and decay #4760

Closed

netmindz reviewed Jul 10, 2025

View reviewed changes

netmindz added the effect label Jul 27, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Adding center bin selection to 2D GEQ #4764

Adding center bin selection to 2D GEQ #4764

Uh oh!

DedeHai commented Jul 10, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jul 10, 2025 •

edited

Loading

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

DedeHai commented Jul 10, 2025

Uh oh!

netmindz Jul 10, 2025

Uh oh!

DedeHai Jul 10, 2025

Uh oh!

blazoncek Jul 10, 2025

Uh oh!

Uh oh!

Uh oh!

Adding center bin selection to 2D GEQ #4764

Are you sure you want to change the base?

Adding center bin selection to 2D GEQ #4764

Uh oh!

Conversation

DedeHai commented Jul 10, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

DedeHai commented Jul 10, 2025

Uh oh!

netmindz Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

DedeHai Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

blazoncek Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DedeHai commented Jul 10, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jul 10, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)