Skip to content

Conversation

@DedeHai
Copy link
Collaborator

@DedeHai DedeHai commented Dec 2, 2025

this allows presets or external commands to change the AWM on-the-fly. The value is applied temporary until revoked and not saved in config.

only active if global AWM is not enabled

Same logic as in config:

  • 0 = 0 Manual
  • 1 = 1 Brighter
  • 2 = 2 Accurate
  • 3 = 3 Dual
  • 4 = Max
  • 5 or larger = disabled

Summary by CodeRabbit

  • New Features
    • Added temporary auto-white mode override capability that can be controlled via API state updates.
    • Updated auto-white mode selection priority to consider global, temporary, and per-bus settings.

✏️ Tip: You can customize this high-level summary in your review settings.

only active if global AWM is not enabled
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 2, 2025

Walkthrough

This PR introduces a temporary auto-white mode override system. New static methods and a member variable enable temporary global auto-white mode overrides. The calculation logic implements a priority chain: global disabled → temp global → per-bus. JSON deserialization now supports the new "awm" field to set the temporary mode and refresh light capabilities.

Changes

Cohort / File(s) Summary
Auto-white mode interface and state
wled00/bus_manager.h
Added static inline methods setTempAWMode() and getTempAWMode() to manage a new temporary auto-white mode override. Introduced static member _tempAWM initialized to 255, alongside existing _gAWM.
Auto-white mode calculation logic
wled00/bus_manager.cpp, wled00/FX_fcn.cpp
Implemented fallback logic in auto-white mode selection: when global mode is disabled, use temporary global mode if available; otherwise fall back to per-bus mode. Establishes priority: global disabled → temp global → per-bus. Added supporting comments.
JSON state deserialization
wled00/json.cpp
Added handling for new "awm" JSON field in deserializeState(). Computes awm as provided value OR'd with AW_GLOBAL_DISABLED, applies via Bus::setTempAWMode(), and refreshes main segment light capabilities.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Focus on verifying the auto-white mode priority logic is correctly implemented across bus_manager.cpp and FX_fcn.cpp
  • Confirm JSON deserialization properly integrates the temp mode and triggers capability refresh
  • Validate initialization and bounds checking of the new _tempAWM variable

Suggested reviewers

  • Aircoookie

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'add "awm" JSON command to set temporary auto white mode' directly describes the main change: introducing a new JSON command for setting a temporary auto-white mode. It is concise, specific, and clearly conveys the primary functionality added.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 bdea3d4 and e1d2c75.

📒 Files selected for processing (4)
  • wled00/FX_fcn.cpp (1 hunks)
  • wled00/bus_manager.cpp (2 hunks)
  • wled00/bus_manager.h (2 hunks)
  • wled00/json.cpp (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
wled00/**/*.cpp

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use 2-space indentation for C++ source files (.cpp)

Files:

  • wled00/FX_fcn.cpp
  • wled00/bus_manager.cpp
  • wled00/json.cpp
wled00/**/!(html_*)*.h

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use 2-space indentation for non-generated C++ header files (.h)

Files:

  • wled00/bus_manager.h
🧠 Learnings (11)
📓 Common learnings
Learnt from: DedeHai
Repo: wled/WLED PR: 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.
📚 Learning: 2025-08-26T11:51:21.817Z
Learnt from: DedeHai
Repo: wled/WLED PR: 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:

  • wled00/FX_fcn.cpp
  • wled00/bus_manager.cpp
📚 Learning: 2025-09-16T18:08:42.848Z
Learnt from: DedeHai
Repo: wled/WLED PR: 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:

  • wled00/FX_fcn.cpp
  • wled00/bus_manager.cpp
📚 Learning: 2025-09-18T03:17:30.107Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1183-1194
Timestamp: 2025-09-18T03:17:30.107Z
Learning: In WLED BusManager (wled00/FX_fcn.cpp), direct access to BusManager::busses is part of the intended public API design, not an encapsulation violation. The wrapper accessor methods are considered unnecessary obfuscation that should be removed in future refactoring.

Applied to files:

  • wled00/FX_fcn.cpp
  • wled00/bus_manager.cpp
  • wled00/bus_manager.h
📚 Learning: 2025-09-18T03:17:30.107Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1183-1194
Timestamp: 2025-09-18T03:17:30.107Z
Learning: In WLED finalizeInit() bus creation (wled00/FX_fcn.cpp), digitalCount is intentionally incremented even for placeholder buses to maintain consistent resource mapping across all bus configurations.

Applied to files:

  • wled00/FX_fcn.cpp
  • wled00/bus_manager.cpp
📚 Learning: 2025-11-27T06:33:11.436Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5109
File: wled00/FX.cpp:3174-3343
Timestamp: 2025-11-27T06:33:11.436Z
Learning: WLED Ants effect (wled00/FX.cpp): The author prefers the current velocity initialization using hw_random16(1000, 5000)/5000.0f, resulting in an effective range of ~3.6–10.0 (with VELOCITY_MIN=2.0, VELOCITY_MAX=10.0), and wants the code kept as-is with comments updated to document this behavior. Avoid suggesting changes to span the full 2.0–10.0 range in future reviews.

Applied to files:

  • wled00/FX_fcn.cpp
📚 Learning: 2025-09-18T03:17:30.107Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4939
File: wled00/FX_fcn.cpp:1183-1194
Timestamp: 2025-09-18T03:17:30.107Z
Learning: In WLED bus management code, dead code branches in error handling (like unreachable else breaks) are intentionally preserved to accommodate potential future API changes, even when currently unreachable.

Applied to files:

  • wled00/FX_fcn.cpp
  • wled00/bus_manager.cpp
  • wled00/bus_manager.h
📚 Learning: 2025-09-12T17:29:43.826Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4923
File: wled00/FX.cpp:4883-4901
Timestamp: 2025-09-12T17:29:43.826Z
Learning: In WLED’s web UI, only one slider value (e.g., SEGMENT.intensity or SEGMENT.custom1) changes at a time; code relying on this may use simplified change guards, though presets/JSON can still update multiple fields atomically.

Applied to files:

  • wled00/json.cpp
📚 Learning: 2025-05-09T18:43:15.355Z
Learnt from: DedeHai
Repo: wled/WLED 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.

Applied to files:

  • wled00/bus_manager.h
📚 Learning: 2025-11-16T19:40:46.260Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4926
File: wled00/FX.cpp:4727-4730
Timestamp: 2025-11-16T19:40:46.260Z
Learning: WLED AuroraWave (wled00/FX.cpp): wave_start and wave_end intentionally use int16_t; segments longer than 32k LEDs are not supported (bounded by MAX_LEDS), so widening to 32-bit is unnecessary.

Applied to files:

  • wled00/bus_manager.h
📚 Learning: 2025-09-11T04:36:42.869Z
Learnt from: DedeHai
Repo: wled/WLED PR: 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:

  • wled00/bus_manager.h
🧬 Code graph analysis (1)
wled00/FX_fcn.cpp (1)
wled00/bus_manager.h (2)
  • getGlobalAWMode (200-200)
  • getTempAWMode (202-202)
⏰ 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). (20)
  • GitHub Check: wled_build / Build Environments (usermods)
  • GitHub Check: wled_build / Build Environments (esp32s3_4M_qspi)
  • GitHub Check: wled_build / Build Environments (esp32S3_wroom2)
  • GitHub Check: wled_build / Build Environments (esp32dev)
  • GitHub Check: wled_build / Build Environments (esp32c3dev_qio)
  • GitHub Check: wled_build / Build Environments (esp32s3dev_16MB_opi)
  • GitHub Check: wled_build / Build Environments (esp32c3dev)
  • GitHub Check: wled_build / Build Environments (lolin_s2_mini)
  • GitHub Check: wled_build / Build Environments (esp32_wrover)
  • GitHub Check: wled_build / Build Environments (esp32_eth)
  • GitHub Check: wled_build / Build Environments (esp01_1m_full_160)
  • GitHub Check: wled_build / Build Environments (esp01_1m_full_compat)
  • GitHub Check: wled_build / Build Environments (esp32dev_debug)
  • GitHub Check: wled_build / Build Environments (esp8266_2m_compat)
  • GitHub Check: wled_build / Build Environments (esp01_1m_full)
  • GitHub Check: wled_build / Build Environments (esp8266_2m)
  • GitHub Check: wled_build / Build Environments (nodemcuv2_compat)
  • GitHub Check: wled_build / Build Environments (esp8266_2m_160)
  • GitHub Check: wled_build / Build Environments (nodemcuv2)
  • GitHub Check: wled_build / Build Environments (nodemcuv2_160)
🔇 Additional comments (6)
wled00/json.cpp (1)

457-461: LGTM - Clean integration of temporary AWM handling.

The JSON deserialization correctly delegates validation to setTempAWMode() which clamps valid values (0-4) and disables the override for values ≥5. Refreshing light capabilities ensures UI sliders update appropriately.

wled00/bus_manager.cpp (2)

147-148: LGTM - Correct priority chain implementation.

The conditional correctly implements the intended priority: global AWM → temporary global AWM → per-bus AWM. The temp override only takes effect when the global setting is disabled.


1448-1448: LGTM - Proper static member initialization.

Initializing _tempAWM to 255 (AW_GLOBAL_DISABLED) ensures the temporary override is disabled by default, which is the expected behavior.

wled00/FX_fcn.cpp (1)

1003-1004: LGTM - Consistent AWM priority logic with clear documentation.

The ternary chain correctly mirrors the priority logic in Bus::autoWhiteCalc(), ensuring consistent behavior between light capability detection and actual white calculation. The added comment clearly documents the priority order.

wled00/bus_manager.h (2)

201-202: LGTM - Clean API design mirroring existing global AWM accessors.

The validation logic (m < 5 for valid modes) is consistent with setGlobalAWMode() and correctly maps to the documented AWM values (0-4 = valid modes, ≥5 = disabled).


229-230: LGTM - Clear documentation of static member purposes.

The comments effectively distinguish between the persistent global AWM (_gAWM) and the temporary JSON API-driven override (_tempAWM).


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

❤️ Share

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

@DedeHai DedeHai requested a review from blazoncek December 2, 2025 17:41
@blazoncek
Copy link
Collaborator

I have read the user request but am not fond of the change. I would have not implemented this.

@DedeHai
Copy link
Collaborator Author

DedeHai commented Dec 3, 2025

why?

@blazoncek
Copy link
Collaborator

No particular reason, apart from the fact that I do not like the idea of changing AWM using a bus "hack" (which is needed here, no objection). Perhaps it might be better to forgo _gAWM from configuration altogether and use it as an override as you tried to implement with temporary AWM. I do not see added value in _gAWM as a configuration item but can be used for the purpose requested.

However, I'm not sure that this functionality is really needed (brighter mode will wash out colors due to light-bleed, not make them "crisper and more vivid").

There's also a second aspect I'm considering regarding White channel. No effects use explicit W and none of the palettes employ W which means it is mostly unused except in Solid effect (and a handful of others) with particular color created using Hex input. However people with RGBW strips want W channel to be used and will most likely do so using Brighter or Accurate AWM modes (Dual will confuse them and None may not behave the way they want (except a few that know its purpose)).

Hence I would reduce AWM to Brighter and Accurate only.

@DedeHai
Copy link
Collaborator Author

DedeHai commented Dec 3, 2025

Those are good inputs, thanks. I do not know the history of white modes and only recently got a full WS2805 RGBCCT and it was fun but confusing to figure out the white modes, I do leave it at fully manual but I can understand people wanting auto modes, I agree that two are probably enough.
Regarding RGB and white: I use a solid color as a base set to black and play FX in a overlay segment with ADD, that way I can have white on top of all of the FX if I want to.
I also noted that the copyFX will add in the white channel into the copy, even if the source segment has no white in it. Although it did not become clear to me from the code on why that is exactly (and yes, I am aware I wrote that code... ).

If _gAWM has no real purpose except historical reasons, it would be better to remove it. Having the (temporary) global override is something that can be useful for presets, the implementation from this PR however has one flaw IMO: if it is used in one preset, you need to add it to all other presets too to disable it. So not really sure there is not a better implementation.

@blazoncek
Copy link
Collaborator

If _gAWM has no real purpose except historical reasons

I had a chat with a few of my users that do use RGBW (most of them since 0.12). Indeed all of them use _gAWM instead of bus AWM. It looks like it was passed over from installations without bus AWM and nobody bothered to update bus AWM so their configurations have bus AWM set to none, but global AWM to one of the desired values.

I have sketched the update that will move _gAWM into bus AWM if it does not exist, however I suspect those configurations already have bus AWM as it is unreasonable to expect configuration would not change since 0.12.
Since _gAWM always takes precedence over bus AWM it may be ok to force update of bus AWM if _gAWM exists in configuration.

@blazoncek
Copy link
Collaborator

It would be prudent to invite @Aircoookie into discussion as global AWM was originally his addition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants