FEATURE: WLED Power Consumption in JSON API #5261
Conversation
WalkthroughAdds LED supply voltage and power‑monitoring mode; exposes per‑bus voltage and global watts, persists settings, updates UI controls, and extends ABL/current‑estimation to run when either ABL or power monitoring is enabled, affecting brightness/current limiting logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (4)wled00/**/*.cpp📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
wled00/data/**/*.{htm,html,css,js}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
wled00/data/**📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
wled00/data/settings*.htm📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (17)📓 Common learnings📚 Learning: 2025-09-18T03:17:30.107ZApplied to files:
📚 Learning: 2025-08-26T11:51:21.817ZApplied to files:
📚 Learning: 2025-09-18T03:17:30.107ZApplied to files:
📚 Learning: 2025-09-16T18:08:42.848ZApplied to files:
📚 Learning: 2025-12-28T14:06:48.772ZApplied to files:
📚 Learning: 2025-04-28T20:51:29.773ZApplied to files:
📚 Learning: 2025-10-20T09:38:51.997ZApplied to files:
📚 Learning: 2025-10-05T15:24:05.545ZApplied to files:
📚 Learning: 2025-09-12T17:29:43.826ZApplied to files:
📚 Learning: 2025-10-05T15:24:05.545ZApplied to files:
📚 Learning: 2025-12-01T07:01:16.949ZApplied to files:
📚 Learning: 2025-09-18T03:17:30.107ZApplied to files:
📚 Learning: 2025-11-14T13:37:30.955ZApplied to files:
📚 Learning: 2025-11-14T13:37:11.994ZApplied to files:
📚 Learning: 2025-10-10T18:34:06.550ZApplied to files:
📚 Learning: 2025-11-14T13:37:30.955ZApplied to files:
🧬 Code graph analysis (1)wled00/bus_manager.cpp (1)
🔇 Additional comments (10)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
wled00/set.cpp (1)
150-151: Consider adding voltage range validation.The voltage parameter lacks upper bound validation. While the
> 0check prevents zero values, users could potentially enter unrealistic values (e.g., 255) via the API. Common LED strip voltages are 5V, 12V, 24V, or 48V.🔎 Optional validation enhancement
uint8_t ledVoltage = request->arg(F("LV")).toInt(); -if (ledVoltage > 0) BusManager::setVoltage(ledVoltage); +if (ledVoltage > 0 && ledVoltage <= 48) BusManager::setVoltage(ledVoltage);Or more permissive:
uint8_t ledVoltage = request->arg(F("LV")).toInt(); -if (ledVoltage > 0) BusManager::setVoltage(ledVoltage); +// Common values: 5V, 12V, 24V, 48V +if (ledVoltage > 0 && ledVoltage <= 100) BusManager::setVoltage(ledVoltage);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
wled00/bus_manager.cppwled00/bus_manager.hwled00/cfg.cppwled00/const.hwled00/data/settings_leds.htmwled00/json.cppwled00/set.cppwled00/xml.cpp
🧰 Additional context used
📓 Path-based instructions (5)
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/cfg.cppwled00/bus_manager.cppwled00/xml.cppwled00/json.cppwled00/set.cpp
wled00/**/!(html_*)*.h
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for non-generated C++ header files (.h)
Files:
wled00/const.hwled00/bus_manager.h
wled00/data/**/*.{htm,html,css,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use tabs for indentation in web files (.htm/.html/.css/.js) under wled00/data
Files:
wled00/data/settings_leds.htm
wled00/data/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
wled00/data/**: When modifying web UI files, runnpm run buildto regenerate embedded headers before any firmware build
For web UI changes, edit files only under wled00/data (not firmware or generated files)
Files:
wled00/data/settings_leds.htm
wled00/data/settings*.htm
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Name settings pages as settings*.htm within the web UI
Files:
wled00/data/settings_leds.htm
🧠 Learnings (21)
📓 Common learnings
Learnt from: netmindz
Repo: wled/WLED PR: 5093
File: wled00/util.cpp:1159-1182
Timestamp: 2025-11-20T00:04:04.829Z
Learning: In WLED PR #5093, the deviceId feature is designed for opt-in usage reporting that tracks only version/upgrade information (non-behavioral data), not user activity patterns. The deterministic salt approach (MAC + "WLED" + chip model/revision) is acceptable for this limited use case, as correlating MAC addresses to version history represents minimal privacy risk compared to behavioral tracking.
📚 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/cfg.cppwled00/bus_manager.cppwled00/bus_manager.hwled00/data/settings_leds.htmwled00/xml.cppwled00/json.cppwled00/set.cpp
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
Repo: wled/WLED PR: 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:
wled00/cfg.cpp
📚 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/cfg.cppwled00/data/settings_leds.htm
📚 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/cfg.cppwled00/bus_manager.cppwled00/bus_manager.hwled00/json.cppwled00/set.cpp
📚 Learning: 2025-09-01T10:26:17.959Z
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/wled_eeprom.cpp:0-0
Timestamp: 2025-09-01T10:26:17.959Z
Learning: In WLED PR #4876, the DMXStartLED EEPROM backward compatibility issue was partially addressed by keeping it at address 2550 and reading it as a 16-bit value, with DMXChannelsValue array moved to addresses 2552-2566. This maintains compatibility with pre-0.11 EEPROM layouts for DMXStartLED, though legacy "Set to 255" (code 6) configurations may still need migration logic.
Applied to files:
wled00/cfg.cppwled00/data/settings_leds.htm
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: WLED uses a vendored ArduinoJson library (version 6) located at "src/dependencies/json/ArduinoJson-v6.h" which is included through wled.h. Usermods should not directly include ArduinoJson headers but instead rely on wled.h for ArduinoJson symbols. The standard pattern is to include wled.h and use JsonObject, JsonArray, DynamicJsonDocument, etc. without additional includes.
Applied to files:
wled00/cfg.cppwled00/json.cpp
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/const.hwled00/data/settings_leds.htm
📚 Learning: 2025-04-26T19:19:07.600Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4658
File: wled00/const.h:140-141
Timestamp: 2025-04-26T19:19:07.600Z
Learning: In WLED, the WLED_MAX_PANELS macro is intentionally defined as a fixed constant value (18) with no redefinition mechanism, making it "unoverridable" - there's no need for a static assertion to check its maximum value.
Applied to files:
wled00/const.h
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/const.hwled00/data/settings_leds.htm
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)
Applied to files:
wled00/const.hwled00/data/settings_leds.htm
📚 Learning: 2025-12-01T07:01:16.949Z
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Applied to files:
wled00/const.h
📚 Learning: 2025-11-30T15:29:00.726Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4456
File: usermods/deep_sleep/deep_sleep.cpp:224-230
Timestamp: 2025-11-30T15:29:00.726Z
Learning: In WLED, `touchThreshold` is a global variable defined in wled00/wled.h and is accessible to all usermods. It stores the touch sensitivity threshold for touch-capable GPIO pins on ESP32 variants and should not be flagged as undefined when used in usermods.
Applied to files:
wled00/const.h
📚 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/const.h
📚 Learning: 2025-10-20T09:38:51.997Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4995
File: wled00/FX.cpp:5223-5226
Timestamp: 2025-10-20T09:38:51.997Z
Learning: WLED matrices: each dimension (SEG_W, SEG_H) is limited to ≤255; 256 or larger per side is not supported/feasible on ESP32, so effects should assume per-dimension max 255.
Applied to files:
wled00/const.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/bus_manager.cppwled00/bus_manager.h
📚 Learning: 2025-12-28T14:06:48.764Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.764Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Applied to files:
wled00/bus_manager.cppwled00/data/settings_leds.htm
📚 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/bus_manager.cppwled00/data/settings_leds.htmwled00/xml.cppwled00/set.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/bus_manager.cpp
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/** : For web UI changes, edit files only under wled00/data (not firmware or generated files)
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/settings*.htm : Name settings pages as settings*.htm within the web UI
Applied to files:
wled00/data/settings_leds.htm
🧬 Code graph analysis (5)
wled00/cfg.cpp (3)
usermods/Battery/UMBattery.h (1)
setVoltage(105-111)wled00/bus_manager.cpp (2)
getVoltage(364-366)getVoltage(364-364)wled00/bus_manager.h (1)
getVoltage(262-263)
wled00/bus_manager.cpp (1)
wled00/bus_manager.h (1)
getVoltage(262-263)
wled00/bus_manager.h (1)
wled00/bus_manager.cpp (4)
getVoltage(364-366)getVoltage(364-364)currentWatts(1452-1454)currentWatts(1452-1452)
wled00/xml.cpp (3)
wled00/util.cpp (4)
printSetFormValue(114-116)printSetFormValue(114-114)printSetFormValue(121-123)printSetFormValue(121-121)wled00/bus_manager.cpp (2)
getVoltage(364-366)getVoltage(364-364)wled00/bus_manager.h (1)
getVoltage(262-263)
wled00/set.cpp (1)
usermods/Battery/UMBattery.h (1)
setVoltage(105-111)
🔇 Additional comments (14)
wled00/json.cpp (1)
707-711: LGTM! Clean API extension.The new voltage and watt-based power fields integrate seamlessly into the existing JSON info payload. The implementation correctly leverages the BusManager interface to expose voltage configuration and power calculations.
wled00/cfg.cpp (2)
161-162: LGTM!Voltage deserialization follows the established configuration pattern with appropriate default fallback.
926-926: LGTM!Voltage serialization correctly persists the configured voltage back to the hardware configuration.
wled00/xml.cpp (1)
377-377: LGTM!Voltage field correctly exposed in the LED settings UI via the established form value pattern.
wled00/const.h (1)
539-541: LGTM!The default voltage constant is well-placed and appropriately defaults to 5V for common WS281x LED strips. The
#ifndefguard properly allows build-time overrides.wled00/data/settings_leds.htm (2)
471-482: LGTM!The UI changes for power monitoring configuration are well-structured. The updated label clarifies the purpose, and the min/max constraints are appropriate.
872-874: LGTM!The voltage input field is well-designed with sensible bounds and helpful guidance text for users.
wled00/bus_manager.h (2)
143-143: LGTM!The virtual
getVoltage()method provides a clean abstraction with sensible default behavior.
502-506: LGTM!The voltage accessors and power calculation functions are well-designed. The inline
ablWattsMax()correctly uses floating-point division for precision.wled00/bus_manager.cpp (5)
296-305: LGTM!The refactored power tracking logic correctly decouples current estimation from ABL. The condition
_milliAmpsPerLed > 0ensures power monitoring works whenever LED current is configured, regardless of ABL state.
364-367: LGTM!Clean delegation pattern. The implementation allows for future per-bus voltage support if needed.
1400-1450: LGTM!The refactored
applyABL()correctly separates current estimation (always performed) from brightness limiting (only when ABL enabled). The logic properly handles all three scenarios: ABL disabled, global ABL, and per-bus ABL.
1452-1454: LGTM!The power calculation is straightforward and correct. The floating-point division ensures appropriate precision for watt values.
1471-1471: LGTM!Consistent default voltage initialization matching the UI and const.h definition.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wled00/data/settings_leds.htmwled00/set.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- wled00/set.cpp
🧰 Additional context used
📓 Path-based instructions (3)
wled00/data/**/*.{htm,html,css,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use tabs for indentation in web files (.htm/.html/.css/.js) under wled00/data
Files:
wled00/data/settings_leds.htm
wled00/data/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
wled00/data/**: When modifying web UI files, runnpm run buildto regenerate embedded headers before any firmware build
For web UI changes, edit files only under wled00/data (not firmware or generated files)
Files:
wled00/data/settings_leds.htm
wled00/data/settings*.htm
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Name settings pages as settings*.htm within the web UI
Files:
wled00/data/settings_leds.htm
🧠 Learnings (16)
📚 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/data/settings_leds.htm
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/** : For web UI changes, edit files only under wled00/data (not firmware or generated files)
Applied to files:
wled00/data/settings_leds.htm
📚 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/data/settings_leds.htm
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-09-01T10:26:17.959Z
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/wled_eeprom.cpp:0-0
Timestamp: 2025-09-01T10:26:17.959Z
Learning: In WLED PR #4876, the DMXStartLED EEPROM backward compatibility issue was partially addressed by keeping it at address 2550 and reading it as a 16-bit value, with DMXChannelsValue array moved to addresses 2552-2566. This maintains compatibility with pre-0.11 EEPROM layouts for DMXStartLED, though legacy "Set to 255" (code 6) configurations may still need migration logic.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-12-28T14:06:48.764Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.764Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Applied to files:
wled00/data/settings_leds.htm
📚 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/data/settings_leds.htm
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/settings*.htm : Name settings pages as settings*.htm within the web UI
Applied to files:
wled00/data/settings_leds.htm
📚 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/data/settings_leds.htm
📚 Learning: 2025-12-01T07:01:16.949Z
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Applied to files:
wled00/data/settings_leds.htm
📚 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/data/settings_leds.htm
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
Repo: wled/WLED PR: 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:
wled00/data/settings_leds.htm
📚 Learning: 2025-10-10T18:34:06.550Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.
Applied to files:
wled00/data/settings_leds.htm
🔇 Additional comments (4)
wled00/data/settings_leds.htm (4)
178-179: LGTM! Power monitoring validation logic is correct.The minimum value logic correctly requires non-zero mA values (min=1) for digital LEDs to enable power consumption calculations, while allowing 0 for non-digital types that don't participate in power monitoring.
289-294: LGTM! LED type change handling is correct.The UI updates properly show/hide the mA configuration fields and set appropriate minimum validation values based on whether the LED type is digital, ensuring power monitoring requirements are met.
471-482: HTML template is correct with minor observation.The template properly structures the mA/LED configuration with clear labeling for power monitoring. The hardcoded
min="1"at line 480 serves as a default and gets dynamically updated based on LED type (lines 178, 293), so this is functionally correct.
697-697: LGTM! Config loading is correct.The code correctly loads the global voltage value from configuration. The previous review concern about per-bus voltage access has been addressed—this implementation only uses the global
LVfield as intended.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wled00/data/settings_leds.htm (1)
871-873: LGTM! Voltage input field properly implemented.The field now uses
min="1"(addressing the previous review concern), has a sensible default (5V), and includes clear documentation of typical values. The placement outside the ABL section correctly makes it always available for power monitoring.Optional: Consider supporting fractional voltages.
If users need to specify voltages like 3.3V, consider changing
step="1"tostep="0.1":🔎 Suggested enhancement
- LED Strip Voltage: <input name="LV" type="number" class="m" step="1" min="1" max="50" value="5" oninput="UI()"> V<br> + LED Strip Voltage: <input name="LV" type="number" class="m" step="0.1" min="1" max="50" value="5" oninput="UI()"> V<br>
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wled00/data/settings_leds.htm
🧰 Additional context used
📓 Path-based instructions (3)
wled00/data/**/*.{htm,html,css,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use tabs for indentation in web files (.htm/.html/.css/.js) under wled00/data
Files:
wled00/data/settings_leds.htm
wled00/data/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
wled00/data/**: When modifying web UI files, runnpm run buildto regenerate embedded headers before any firmware build
For web UI changes, edit files only under wled00/data (not firmware or generated files)
Files:
wled00/data/settings_leds.htm
wled00/data/settings*.htm
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Name settings pages as settings*.htm within the web UI
Files:
wled00/data/settings_leds.htm
🧠 Learnings (15)
📓 Common learnings
Learnt from: netmindz
Repo: wled/WLED PR: 5093
File: wled00/util.cpp:1159-1182
Timestamp: 2025-11-20T00:04:04.829Z
Learning: In WLED PR #5093, the deviceId feature is designed for opt-in usage reporting that tracks only version/upgrade information (non-behavioral data), not user activity patterns. The deterministic salt approach (MAC + "WLED" + chip model/revision) is acceptable for this limited use case, as correlating MAC addresses to version history represents minimal privacy risk compared to behavioral tracking.
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.
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/** : For web UI changes, edit files only under wled00/data (not firmware or generated files)
Applied to files:
wled00/data/settings_leds.htm
📚 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/data/settings_leds.htm
📚 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/data/settings_leds.htm
📚 Learning: 2025-12-28T14:06:48.764Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.764Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Applied to files:
wled00/data/settings_leds.htm
📚 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/data/settings_leds.htm
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-09-01T10:26:17.959Z
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/wled_eeprom.cpp:0-0
Timestamp: 2025-09-01T10:26:17.959Z
Learning: In WLED PR #4876, the DMXStartLED EEPROM backward compatibility issue was partially addressed by keeping it at address 2550 and reading it as a 16-bit value, with DMXChannelsValue array moved to addresses 2552-2566. This maintains compatibility with pre-0.11 EEPROM layouts for DMXStartLED, though legacy "Set to 255" (code 6) configurations may still need migration logic.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-12-01T07:01:16.949Z
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Applied to files:
wled00/data/settings_leds.htm
📚 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/data/settings_leds.htm
📚 Learning: 2025-10-10T18:34:06.550Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.
Applied to files:
wled00/data/settings_leds.htm
📚 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/data/settings_leds.htm
🔇 Additional comments (5)
wled00/data/settings_leds.htm (5)
178-178: LGTM! Power monitoring validation is correct.Setting minimum mA to 1 for digital LEDs ensures valid power consumption calculations regardless of ABL state, which aligns with the PR objective.
289-289: LGTM! Field visibility now supports power monitoring independent of ABL.Displaying the mA/LED field for all digital LEDs (not just when ABL is enabled) correctly implements the PR objective of power monitoring regardless of ABL state.
293-294: LGTM! Validation minimums correctly updated on type change.The minimum values are appropriately set: LA (per-LED mA) requires min=1 for digital LEDs for power monitoring, while MA (PSU mA) requires min=250 for standard PSU limits.
471-482: LGTM! HTML template correctly implements power monitoring fields.The template includes appropriate labels ("for power monitoring"), consistent IDs for visibility control, and correct validation attributes (min="1" for custom mA).
697-697: LGTM! Config loading correctly restored.The code now properly loads only the global voltage field (
LV) with appropriate existence checking. The previous per-bus voltage access issue has been resolved.
|
thanks for contributing. |
Ok, I agree. What do you think if I make these calculations behind a flag (smth like:
No, not yet. I plan to implement it by adding power value to the WLED entity (but based on W, not mA). Actually, my first idea was to use pwr and just multiply it by a known voltage in HA and call it a day, but:
Yes, they wont, but this rough approximation:
|
|
Since HA allows to calculate power from current I do not see the point in doing it in WLED, it just bloats the code and makes setup more complicated for the normal use-case just for the convenience of not having to set it up in HA. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wled00/data/settings_leds.htm (1)
884-884: Warning message could be more specific (optional refinement).The current warning is appropriate and correctly informs users about CPU usage. However, it could be more precise about what happens:
Current: "calculates current every frame and uses CPU resources"
Could be: "accumulates color data and calculates power consumption every frame, which uses CPU resources"
This is an optional refinement—the existing warning adequately discourages unnecessary enabling of power monitoring on slower devices.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
wled00/bus_manager.cppwled00/bus_manager.hwled00/cfg.cppwled00/data/settings_leds.htmwled00/set.cppwled00/xml.cpp
🚧 Files skipped from review as they are similar to previous changes (3)
- wled00/xml.cpp
- wled00/bus_manager.h
- wled00/cfg.cpp
🧰 Additional context used
📓 Path-based instructions (4)
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/set.cppwled00/bus_manager.cpp
wled00/data/**/*.{htm,html,css,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use tabs for indentation in web files (.htm/.html/.css/.js) under wled00/data
Files:
wled00/data/settings_leds.htm
wled00/data/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
wled00/data/**: When modifying web UI files, runnpm run buildto regenerate embedded headers before any firmware build
For web UI changes, edit files only under wled00/data (not firmware or generated files)
Files:
wled00/data/settings_leds.htm
wled00/data/settings*.htm
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Name settings pages as settings*.htm within the web UI
Files:
wled00/data/settings_leds.htm
🧠 Learnings (19)
📓 Common learnings
Learnt from: netmindz
Repo: wled/WLED PR: 5093
File: wled00/util.cpp:1159-1182
Timestamp: 2025-11-20T00:04:04.829Z
Learning: In WLED PR #5093, the deviceId feature is designed for opt-in usage reporting that tracks only version/upgrade information (non-behavioral data), not user activity patterns. The deterministic salt approach (MAC + "WLED" + chip model/revision) is acceptable for this limited use case, as correlating MAC addresses to version history represents minimal privacy risk compared to behavioral tracking.
📚 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/set.cppwled00/data/settings_leds.htmwled00/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/set.cppwled00/data/settings_leds.htmwled00/bus_manager.cpp
📚 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/set.cppwled00/data/settings_leds.htmwled00/bus_manager.cpp
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/set.cppwled00/data/settings_leds.htmwled00/bus_manager.cpp
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/set.cppwled00/data/settings_leds.htmwled00/bus_manager.cpp
📚 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/set.cppwled00/bus_manager.cpp
📚 Learning: 2025-09-01T10:26:17.959Z
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/wled_eeprom.cpp:0-0
Timestamp: 2025-09-01T10:26:17.959Z
Learning: In WLED PR #4876, the DMXStartLED EEPROM backward compatibility issue was partially addressed by keeping it at address 2550 and reading it as a 16-bit value, with DMXChannelsValue array moved to addresses 2552-2566. This maintains compatibility with pre-0.11 EEPROM layouts for DMXStartLED, though legacy "Set to 255" (code 6) configurations may still need migration logic.
Applied to files:
wled00/set.cpp
📚 Learning: 2025-08-31T03:38:14.114Z
Learnt from: BobLoeffler68
Repo: wled/WLED PR: 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:
wled00/set.cppwled00/bus_manager.cpp
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/** : For web UI changes, edit files only under wled00/data (not firmware or generated files)
Applied to files:
wled00/data/settings_leds.htm
📚 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/data/settings_leds.htm
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-12-01T07:01:16.949Z
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Applied to files:
wled00/data/settings_leds.htm
📚 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/data/settings_leds.htmwled00/bus_manager.cpp
📚 Learning: 2025-10-10T18:34:06.550Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-10-20T09:38:51.997Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4995
File: wled00/FX.cpp:5223-5226
Timestamp: 2025-10-20T09:38:51.997Z
Learning: WLED matrices: each dimension (SEG_W, SEG_H) is limited to ≤255; 256 or larger per side is not supported/feasible on ESP32, so effects should assume per-dimension max 255.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-12-28T14:06:48.772Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.772Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Applied to files:
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 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/bus_manager.cpp
🧬 Code graph analysis (2)
wled00/set.cpp (1)
usermods/Battery/UMBattery.h (1)
setVoltage(105-111)
wled00/bus_manager.cpp (1)
wled00/bus_manager.h (1)
getVoltage(262-263)
🔇 Additional comments (7)
wled00/set.cpp (1)
150-154: LGTM! Voltage and power monitoring settings are handled correctly.The voltage validation (1-50V) is consistent with the UI constraints and appropriate for common LED strip voltages (5V, 12V, 24V). The power monitoring flag is correctly set based on the checkbox state.
wled00/data/settings_leds.htm (2)
136-141: LGTM! Power monitoring UI integration is well-structured.The enPM() function correctly manages visibility of the power monitoring section and warning message. The integration with setABL() ensures proper UI updates when ABL state changes.
Also applies to: 212-212
186-186: Good refactor: LED mA minimum now based on LED type rather than ABL state.This change correctly decouples LED current configuration from ABL, allowing power monitoring to work independently. Digital LEDs require at least 1mA to be configured for power calculations, which makes sense.
wled00/bus_manager.cpp (4)
296-305: Good change: Enables power monitoring independently of ABL.The updated logic accumulates color sums whenever LED current is configured (
_milliAmpsPerLed > 0), not just when ABL is enabled. This allows power monitoring to function without ABL, which aligns with the PR objectives. The accumulation itself is inexpensive (simple addition per pixel), while expensive calculations remain gated behind feature flags inapplyABL().
364-366: LGTM! Simple voltage getter implementation.The method appropriately delegates to
BusManager::getVoltage(), providing per-bus access to the global voltage setting.
1400-1446: Well-structured separation of ABL and power monitoring concerns.The rewritten
applyABL()correctly optimizes for the case where neither feature is enabled (early return at line 1402-1405). When power monitoring is enabled without ABL:
- Current is estimated via
estimateCurrent()(needed for watt calculation)newBriremains 255 (no brightness limiting)applyBriLimit(255)resets_colorSumwithout expensive per-pixel operationsThis design allows power monitoring to function independently while sharing the current estimation logic with ABL.
1448-1450: Power calculation is correct; constant is properly defined.The
currentWatts()calculation correctly implements W = V × (mA / 1000). Static initializations are appropriate:
_gVoltage = LED_VOLTAGE_DEFAULTdefaults to 5V (defined in const.h for WS281x)_usePowerMonitoring = falsecorrectly makes power monitoring opt-in
|
@DedeHai, I've addressed the performance concern by making power monitoring user-configurable and disabled by default: Changes:
core vs usermod:The main issue with a usermod approach is that power calculations in
Both approaches are more complex and less maintainable than a simple flag in core. This feature enables seamless support for power monitoring without:
Setup complexity:
For users who don't need this: nothing changes. Defaults remain the same, no new required fields. For users who do need it: one checkbox + voltage value (defaults to 5V). |
wled00/bus_manager.cpp
Outdated
|
|
||
| if (BusManager::_useABL) { | ||
| // if using ABL, sum all color channels to estimate current and limit brightness in show() | ||
| // Always sum color channels for power monitoring (used for ABL and power consumption tracking) |
| } | ||
|
|
||
| void BusManager::applyABL() { | ||
| if (_useABL) { |
There was a problem hiding this comment.
please revert unnecessary changes, it makes it hard to review and spot issues.
There was a problem hiding this comment.
Reverted changes, added "if", but now there is code duplication and no early return use (ok, I guess)
wled00/bus_manager.h
Outdated
| inline bool getPowerMonitoring() { return _usePowerMonitoring; } | ||
| inline void setPowerMonitoring(bool pm) { _usePowerMonitoring = pm; } | ||
| float currentWatts(); // calculate total power consumption in Watts | ||
| inline float ablWattsMax() { return (_gMilliAmpsMax * _gVoltage) / 1000.0; } |
wled00/bus_manager.h
Outdated
| inline uint8_t getVoltage() { return _gVoltage; } | ||
| inline void setVoltage(uint8_t v) { _gVoltage = v; } | ||
| inline bool getPowerMonitoring() { return _usePowerMonitoring; } | ||
| inline void setPowerMonitoring(bool pm) { _usePowerMonitoring = pm; } |
wled00/json.cpp
Outdated
| JsonObject leds = root.createNestedObject(F("leds")); | ||
| leds[F("count")] = strip.getLengthTotal(); | ||
| leds[F("pwr")] = BusManager::currentMilliamps(); | ||
| leds[F("pwr_w")] = BusManager::currentWatts(); |
There was a problem hiding this comment.
name it "watts" to avoid confustion
wled00/set.cpp
Outdated
| BusManager::setMilliampsMax(ablMilliampsMax); | ||
|
|
||
| uint8_t ledVoltage = request->arg(F("LV")).toInt(); | ||
| if (ledVoltage > 0 && ledVoltage <= 50) BusManager::setVoltage(ledVoltage); |
There was a problem hiding this comment.
I could only find 48v leds (UCS2903). So 50V seemed like an appropriate choice.
There was a problem hiding this comment.
if there is no technical reason, leave it to the user. there may be 120V strips one day, who knows.
There was a problem hiding this comment.
removed this check. In html bumped up to 255.
wled00/data/settings_leds.htm
Outdated
| <i>Make sure you enter correct value for each LED output.<br> | ||
| If using multiple outputs with only one PSU, distribute its power proportionally amongst outputs.</i><br> | ||
| </div> | ||
| <i class="warn">⚠ Automatic brightness limiter uses CPU resources. This can lower effect FPS on slower devices.</i><br> |
There was a problem hiding this comment.
keep text as brief as possible, it uses flash.
wled00/data/settings_leds.htm
Outdated
| </select><br> | ||
| <div id="LAdis${s}" style="display: none;">max. mA/LED: <input name="LA${s}" type="number" min="1" max="255" oninput="UI()"> mA<br></div> | ||
| <div id="PSU${s}">PSU: <input name="MA${s}" type="number" class="xl" min="250" max="65000" oninput="UI()" value="250"> mA<br></div> | ||
| <div id="LAdis${s}" style="display: none;">Custom mA/LED: <input name="LA${s}" type="number" min="1" max="255" oninput="UI()"> mA<br></div> |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wled00/data/settings_leds.htm (1)
706-706: Verify element existence for robustness (optional improvement).The code assumes
getElementsByName("LV")[0]exists. While the LV field should always be present in the form (line 882), consider adding a null check for defensive coding:if (l.voltage && d.getElementsByName("LV").length > 0) d.getElementsByName("LV")[0].value = l.voltage;However, this pattern is consistent with other config loading in the file, and the element should always exist during normal operation.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
wled00/bus_manager.cppwled00/bus_manager.hwled00/data/settings_leds.htmwled00/json.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- wled00/json.cpp
🧰 Additional context used
📓 Path-based instructions (5)
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
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/bus_manager.cpp
wled00/data/**/*.{htm,html,css,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use tabs for indentation in web files (.htm/.html/.css/.js) under wled00/data
Files:
wled00/data/settings_leds.htm
wled00/data/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
wled00/data/**: When modifying web UI files, runnpm run buildto regenerate embedded headers before any firmware build
For web UI changes, edit files only under wled00/data (not firmware or generated files)
Files:
wled00/data/settings_leds.htm
wled00/data/settings*.htm
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Name settings pages as settings*.htm within the web UI
Files:
wled00/data/settings_leds.htm
🧠 Learnings (15)
📓 Common learnings
Learnt from: netmindz
Repo: wled/WLED PR: 5093
File: wled00/util.cpp:1159-1182
Timestamp: 2025-11-20T00:04:04.829Z
Learning: In WLED PR #5093, the deviceId feature is designed for opt-in usage reporting that tracks only version/upgrade information (non-behavioral data), not user activity patterns. The deterministic salt approach (MAC + "WLED" + chip model/revision) is acceptable for this limited use case, as correlating MAC addresses to version history represents minimal privacy risk compared to behavioral tracking.
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-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/bus_manager.hwled00/bus_manager.cppwled00/data/settings_leds.htm
📚 Learning: 2025-09-02T01:45:58.047Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.
Applied to files:
wled00/bus_manager.h
📚 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/bus_manager.cppwled00/data/settings_leds.htm
📚 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/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/bus_manager.cppwled00/data/settings_leds.htm
📚 Learning: 2025-12-28T14:06:48.772Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.772Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Applied to files:
wled00/bus_manager.cpp
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/** : For web UI changes, edit files only under wled00/data (not firmware or generated files)
Applied to files:
wled00/data/settings_leds.htm
📚 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/data/settings_leds.htm
📚 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/data/settings_leds.htm
📚 Learning: 2025-12-01T07:01:16.949Z
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-10-10T18:34:06.550Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.
Applied to files:
wled00/data/settings_leds.htm
🧬 Code graph analysis (2)
wled00/bus_manager.h (1)
wled00/bus_manager.cpp (4)
getVoltage(363-365)getVoltage(363-363)currentWatts(1455-1457)currentWatts(1455-1455)
wled00/bus_manager.cpp (1)
wled00/bus_manager.h (1)
getVoltage(262-263)
🔇 Additional comments (14)
wled00/data/settings_leds.htm (6)
136-141: LGTM: Power monitoring UI control logic is correct.The function properly shows/hides the power monitoring container and warning message. The conditional logic for
pmwarn(visible only when power monitoring is enabled AND ABL is disabled) makes sense since ABL already performs the CPU-intensive calculations.
147-148: LGTM: ABL state change properly updates power monitoring warning.The logic correctly hides the warning when ABL is enabled (since calculations are already running) and shows it when power monitoring is on but ABL is off.
186-186: LGTM: Minimum validation now based on LED type.Setting minimum to 1 for digital LEDs ensures valid mA/LED values for power monitoring calculations, while allowing 0 for non-digital types where it's not applicable.
880-885: LGTM: Power monitoring UI properly implemented.The UI structure is well-designed with:
- Clear opt-in checkbox (disabled by default) addressing performance concerns
- Sensible voltage defaults (5V for WS281x)
- Helpful description and CPU usage warning
- Conditional warning visibility based on ABL state
The
min="1"validation prevents invalid 0V values as addressed in previous reviews.
298-298: LGTM: Per-LED mA controls properly gated by LED type.Showing mA/LED controls only for digital LEDs is correct, as these values are needed for power monitoring calculations on digital strips.
480-491: LGTM: LED output HTML structure is well-organized.The per-LED mA controls and PSU settings are properly structured with appropriate IDs, names, and visibility control. The markup follows WLED conventions and uses tab indentation as required.
wled00/bus_manager.h (3)
143-143: LGTM: Virtual voltage accessor with sensible default.The virtual
getVoltage()method with default implementation returningLED_VOLTAGE_DEFAULTallows derived classes to override while providing a reasonable fallback. The implicit conversion from uint8_t to float is acceptable for this use case.
262-262: LGTM: BusDigital voltage accessor override.The override declaration is properly specified with const qualifier. Implementation in bus_manager.cpp appropriately delegates to BusManager::getVoltage().
485-487: LGTM: BusManager voltage and power monitoring additions.The new members and accessors are well-designed:
_gVoltage(uint8_t) provides sufficient range for typical LED supply voltages (1-50V)_usePowerMonitoringflag enables opt-in power calculation- Inline accessors are appropriately simple
currentWatts()calculation delegated to .cpp implementationThis design allows power monitoring to be disabled by default, addressing the CPU performance concerns raised in PR discussion.
Also applies to: 503-507
wled00/bus_manager.cpp (5)
296-304: LGTM: Color sum accumulation now supports both ABL and power monitoring.The conditional accumulation correctly gates the CPU-intensive color sum calculation behind either the ABL or power monitoring flags. Since power monitoring is disabled by default (as seen in the UI), this addresses the performance concerns raised in the PR discussion. The calculation only runs when explicitly enabled by the user.
363-365: LGTM: Clean voltage accessor delegation.The implementation properly delegates to
BusManager::getVoltage()with appropriate type conversion from uint8_t to float.
1438-1450: LGTM: Power monitoring mode correctly estimates current without limiting brightness.The power monitoring branch appropriately:
- Calls
estimateCurrent()to calculate current based on accumulated color sums- Uses
applyBriLimit(255)to reset state without applying brightness limits- Accumulates total current for consumption tracking
This provides the desired functionality for Home Assistant integration while maintaining the opt-in nature to avoid unwanted CPU usage.
1455-1457: LGTM: Power calculation formula is correct.The watts calculation properly applies the formula P = V × I:
currentMilliamps()returns total current (LEDs + ESP overhead)- Multiplication by
_gVoltageand division by 1000.0 converts mA and V to watts- Float return type allows fractional watt precision
1474-1476: LGTM: Static member initialization with sensible defaults.Both members are initialized appropriately:
_gVoltageusesLED_VOLTAGE_DEFAULT(likely 5V for typical LED strips)_usePowerMonitoringdefaults tofalse(disabled), ensuring the feature is opt-in
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
wled00/bus_manager.h (2)
143-143: Type inconsistency between Bus and BusManager voltage APIs.
Bus::getVoltage()returnsfloatwhileBusManager::getVoltage()(line 503) returnsuint8_t. TheBusDigital::getVoltage()implementation inbus_manager.cppreturns afloatfrom an implicit conversion of theuint8_tvalue. This works but is inconsistent.Consider aligning the return types — either both
float(if fractional voltages may be needed in future) or bothuint8_t(since only 5/12/24V are supported).
503-507: Consider consistent getter/setter naming for power monitoring.The voltage accessors use a symmetric pattern (
getVoltage/setVoltage), but the power monitoring accessors are asymmetric (powerMonitoring()vsenablePowerMonitoring()). For consistency, consider either:
isPowerMonitoringEnabled()/enablePowerMonitoring(), orgetPowerMonitoring()/setPowerMonitoring()This is a minor naming nit — the current implementation works correctly.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
wled00/bus_manager.hwled00/cfg.cppwled00/set.cppwled00/xml.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- wled00/set.cpp
- wled00/xml.cpp
🧰 Additional context used
📓 Path-based instructions (2)
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
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/cfg.cpp
🧠 Learnings (11)
📓 Common learnings
Learnt from: netmindz
Repo: wled/WLED PR: 5093
File: wled00/util.cpp:1159-1182
Timestamp: 2025-11-20T00:04:04.829Z
Learning: In WLED PR #5093, the deviceId feature is designed for opt-in usage reporting that tracks only version/upgrade information (non-behavioral data), not user activity patterns. The deterministic salt approach (MAC + "WLED" + chip model/revision) is acceptable for this limited use case, as correlating MAC addresses to version history represents minimal privacy risk compared to behavioral tracking.
📚 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/bus_manager.hwled00/cfg.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 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/bus_manager.h
📚 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/bus_manager.hwled00/cfg.cpp
📚 Learning: 2025-09-02T01:45:58.047Z
Learnt from: willmmiles
Repo: wled/WLED PR: 4890
File: lib/NeoESP32RmtHI/include/NeoEsp32RmtHIMethod.h:31-36
Timestamp: 2025-09-02T01:45:58.047Z
Learning: For the NeoEsp32RmtHI driver, RISC-V ESP32-C3 support is currently disabled via bus_wrapper.h rather than compile-time guards, as the maintainer willmmiles is working on resolving underlying nested interrupt issues and prefers to centralize the workaround in one location.
Applied to files:
wled00/bus_manager.h
📚 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/cfg.cpp
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/cfg.cpp
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/cfg.cpp
📚 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/cfg.cpp
📚 Learning: 2025-09-01T10:26:17.959Z
Learnt from: mval-sg
Repo: wled/WLED PR: 4876
File: wled00/wled_eeprom.cpp:0-0
Timestamp: 2025-09-01T10:26:17.959Z
Learning: In WLED PR #4876, the DMXStartLED EEPROM backward compatibility issue was partially addressed by keeping it at address 2550 and reading it as a 16-bit value, with DMXChannelsValue array moved to addresses 2552-2566. This maintains compatibility with pre-0.11 EEPROM layouts for DMXStartLED, though legacy "Set to 255" (code 6) configurations may still need migration logic.
Applied to files:
wled00/cfg.cpp
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
Repo: wled/WLED PR: 4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.808Z
Learning: WLED uses a vendored ArduinoJson library (version 6) located at "src/dependencies/json/ArduinoJson-v6.h" which is included through wled.h. Usermods should not directly include ArduinoJson headers but instead rely on wled.h for ArduinoJson symbols. The standard pattern is to include wled.h and use JsonObject, JsonArray, DynamicJsonDocument, etc. without additional includes.
Applied to files:
wled00/cfg.cpp
🧬 Code graph analysis (2)
wled00/bus_manager.h (1)
wled00/bus_manager.cpp (4)
getVoltage(363-365)getVoltage(363-363)currentWatts(1455-1457)currentWatts(1455-1455)
wled00/cfg.cpp (3)
usermods/Battery/UMBattery.h (1)
setVoltage(105-111)wled00/bus_manager.h (1)
getVoltage(262-263)wled00/bus_manager.cpp (2)
getVoltage(363-365)getVoltage(363-363)
🔇 Additional comments (4)
wled00/cfg.cpp (2)
161-164: LGTM — voltage and power monitoring config loading follows established patterns.The implementation correctly reads
voltageandpwrmonfrom the JSON config with appropriate defaults (LED_VOLTAGE_DEFAULTandfalserespectively), matching the pattern used for other LED settings likemaxpwrabove.
928-929: LGTM — config serialization is consistent with deserialization.wled00/bus_manager.h (2)
485-487: LGTM — global state declarations follow existing patterns.The new
_gVoltageand_usePowerMonitoringdeclarations are consistent with the existing_gMilliAmpsMaxand_useABLdeclarations in this namespace.
262-262: LGTM — override declaration is correct.The override properly delegates to
BusManager::getVoltage()as shown in the implementation.
wled00/bus_manager.cpp
Outdated
| } | ||
| _gMilliAmpsUsed = milliAmpsSum; | ||
| } | ||
| else if (_usePowerMonitoring) { |
There was a problem hiding this comment.
there is no need for code duplication, just adjust the logic above:
if (_useABL || _usePowerMonitoring)
and
if (_gMilliAmpsMax > 0 && _useABL)
There was a problem hiding this comment.
changed, but still needed to do this:
} else if (_usePowerMonitoring) {
for (auto &bus : busses) {
if (bus->isDigital() && bus->isOk()) {
BusDigital &busd = static_cast<BusDigital&>(*bus);
busd.applyBriLimit(255);
}
}
}
because we need to reset _colorSum independently from abl process (it will accumulate over time making wrong calculations)
wled00/data/settings_leds.htm
Outdated
| gId('LAdis'+n).style.display = s.selectedIndex==5 ? "inline" : "none"; // show/hide custom mA field | ||
| if (s.value!=="0") d.Sf["LA"+n].value = s.value; // set value from select object | ||
| d.Sf["LA"+n].min = (!isDig(t) || !abl) ? 0 : 1; // set minimum value for validation | ||
| d.Sf["LA"+n].min = (!isDig(t)) ? 0 : 1; // set minimum value for validation (required for power monitoring) |
There was a problem hiding this comment.
there is a reason the field is hidden if its not used - to not confuse unexperienced users (which there are a lot of)
There was a problem hiding this comment.
made it appear only when the abl/power fields are selected
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
wled00/data/settings_leds.htm (1)
182-189: Enforce minimum mA/LED value when power monitoring is enabled.The minimum value for the
LA(mA/LED) field is currently set to 1 only when ABL is enabled. However, when power monitoring is enabled without ABL, users should still enter valid mA/LED values (≥1) for accurate power consumption calculations. With the current code, a value of 0 is allowed, which would produce meaningless power readings.🔎 Proposed fix
function enLA(s,n) { const abl = d.Sf.ABL.checked; + const pm = d.Sf.PM.checked; const t = parseInt(d.Sf["LT"+n].value); // LED type SELECT gId('LAdis'+n).style.display = s.selectedIndex==5 ? "inline" : "none"; // show/hide custom mA field if (s.value!=="0") d.Sf["LA"+n].value = s.value; // set value from select object - d.Sf["LA"+n].min = (!isDig(t) || !abl) ? 0 : 1; // set minimum value for validation + d.Sf["LA"+n].min = (isDig(t) && (abl || pm)) ? 1 : 0; // set minimum value for validation }wled00/bus_manager.cpp (1)
1400-1448: Critical: Fix per-bus ABL and power monitoring logic conflicts.The
applyABL()function has two critical logic bugs that cause incorrect brightness limiting behavior:Bug 1 (lines 1407-1408): When power monitoring is enabled without ABL (
_usePowerMonitoring=true,_useABL=false), the code unconditionally callsapplyBriLimit(0)for per-bus limiting. This incorrectly applies brightness limits even though only power monitoring is active. Power monitoring should calculate current without affecting brightness.Bug 2 (line 1435): When per-bus ABL is active (
_useABL=true,_gMilliAmpsMax=0), the code callsapplyBriLimit(0)in the first loop (line 1408), then the else-if at line 1435 callsapplyBriLimit(255), which resets the brightness limits and effectively undoes the ABL. The else-if should only execute when power monitoring is enabled WITHOUT ABL.🔎 Proposed fixes
Fix 1: Add _useABL check before applying per-bus limit:
busd.estimateCurrent(); // sets _milliAmpsTotal, current is estimated for all buses even if they have the limit set to 0 - if (_gMilliAmpsMax == 0) + if (_gMilliAmpsMax == 0 && _useABL) busd.applyBriLimit(0); // apply per bus ABL limit, updates _milliAmpsTotal if limit reached milliAmpsSum += busd.getUsedCurrent();Fix 2: Add !_useABL check to power monitoring reset:
} } - } else if (_usePowerMonitoring) { + } else if (_usePowerMonitoring && !_useABL) { // reset _colorSum for power monitoring without applying brightness limits for (auto &bus : busses) {
♻️ Duplicate comments (1)
wled00/data/settings_leds.htm (1)
301-306: Enforce minimum mA/LED value for power monitoring (duplicate issue).This has the same issue as line 188: the minimum value should be 1 when either ABL or power monitoring is enabled for digital LEDs. The current code only enforces
min=1when ABL is enabled.🔎 Proposed fix
if (change) { // did we change LED type? gId("rf"+n).checked = (gId("rf"+n).checked || t == 31); // LEDs require data in off state (mandatory for TM1814) if (isAna(t)) d.Sf["LC"+n].value = 1; // for sanity change analog count just to 1 LED - d.Sf["LA"+n].min = (!isDig(t) || !abl) ? 0 : 1; // set minimum value for LED mA + d.Sf["LA"+n].min = (isDig(t) && (abl || d.Sf.PM.checked)) ? 1 : 0; // set minimum value for LED mA d.Sf["MA"+n].min = (!isDig(t)) ? 0 : 250; // set minimum value for PSU mA }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
wled00/bus_manager.cppwled00/data/settings_leds.htmwled00/set.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- wled00/set.cpp
🧰 Additional context used
📓 Path-based instructions (4)
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/bus_manager.cpp
wled00/data/**/*.{htm,html,css,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use tabs for indentation in web files (.htm/.html/.css/.js) under wled00/data
Files:
wled00/data/settings_leds.htm
wled00/data/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
wled00/data/**: When modifying web UI files, runnpm run buildto regenerate embedded headers before any firmware build
For web UI changes, edit files only under wled00/data (not firmware or generated files)
Files:
wled00/data/settings_leds.htm
wled00/data/settings*.htm
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Name settings pages as settings*.htm within the web UI
Files:
wled00/data/settings_leds.htm
🧠 Learnings (17)
📓 Common learnings
Learnt from: netmindz
Repo: wled/WLED PR: 5093
File: wled00/util.cpp:1159-1182
Timestamp: 2025-11-20T00:04:04.829Z
Learning: In WLED PR #5093, the deviceId feature is designed for opt-in usage reporting that tracks only version/upgrade information (non-behavioral data), not user activity patterns. The deterministic salt approach (MAC + "WLED" + chip model/revision) is acceptable for this limited use case, as correlating MAC addresses to version history represents minimal privacy risk compared to behavioral tracking.
📚 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/bus_manager.cppwled00/data/settings_leds.htm
📚 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/bus_manager.cppwled00/data/settings_leds.htm
📚 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/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/bus_manager.cppwled00/data/settings_leds.htm
📚 Learning: 2025-12-28T14:06:48.772Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-28T14:06:48.772Z
Learning: In WLED post-commit ee9ac947, the rendering pipeline uses per-segment buffers and per-pixel bus updates. Unmapped (0xFFFF) mapping entries are now skipped in WS2812FX::show() (no “clear to black”), which can leave physical gap LEDs with stale/random colors unless they are explicitly cleared. This is a behavior change from pre-0.16 where a full physical buffer was effectively refreshed each frame.
Applied to files:
wled00/bus_manager.cpp
📚 Learning: 2025-04-28T20:51:29.773Z
Learnt from: KrX3D
Repo: wled/WLED 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.
Applied to files:
wled00/bus_manager.cpp
📚 Learning: 2025-10-20T09:38:51.997Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4995
File: wled00/FX.cpp:5223-5226
Timestamp: 2025-10-20T09:38:51.997Z
Learning: WLED matrices: each dimension (SEG_W, SEG_H) is limited to ≤255; 256 or larger per side is not supported/feasible on ESP32, so effects should assume per-dimension max 255.
Applied to files:
wled00/bus_manager.cpp
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/** : For web UI changes, edit files only under wled00/data (not firmware or generated files)
Applied to files:
wled00/data/settings_leds.htm
📚 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/data/settings_leds.htm
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/data/settings*.htm : Name settings pages as settings*.htm within the web UI
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-12-01T07:01:16.949Z
Learnt from: blazoncek
Repo: wled/WLED PR: 5140
File: wled00/data/settings_time.htm:66-76
Timestamp: 2025-12-01T07:01:16.949Z
Learning: In WLED PR #5134, the fix for macros being initialized with the enable bit set only handles new configurations, not existing ones. If there is a bug in timer/macro handling code (e.g., in settings_time.htm), it must be fixed to work correctly for existing configurations as well.
Applied to files:
wled00/data/settings_leds.htm
📚 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/data/settings_leds.htm
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-11-14T13:37:11.994Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-10-10T18:34:06.550Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4997
File: wled00/e131.cpp:33-44
Timestamp: 2025-10-10T18:34:06.550Z
Learning: In WLED's DDP packet handling (ws.cpp and e131.cpp), only prevent out-of-bounds memory access rather than enforcing DDP spec compliance. Don't check the 1440-byte spec limit—accept out-of-spec packets assuming correct encoding. The bounds check `maxDataIndex = c + numLeds * ddpChannelsPerLed; if (maxDataIndex > dataLen) reject` is sufficient and already covers the timecode flag case (when c=4) without needing separate validation.
Applied to files:
wled00/data/settings_leds.htm
📚 Learning: 2025-11-14T13:37:30.955Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, verify that file operations (especially file.open()) respect LittleFS filename limitations. Assume default WLED configuration with LittleFS default filename limit of 255 bytes. Do not assume extreme configuration values like WLED_MAX_SEGNAME_LEN = 512 which would not be standard configurations.
Applied to files:
wled00/data/settings_leds.htm
🧬 Code graph analysis (1)
wled00/bus_manager.cpp (1)
wled00/bus_manager.h (1)
getVoltage(262-263)
🔇 Additional comments (10)
wled00/data/settings_leds.htm (6)
136-142: LGTM!The
enPM()function correctly manages power monitoring UI visibility and warning states, following the existing pattern used byenABL().
148-149: LGTM!The modification correctly updates the power monitoring warning visibility when ABL state changes, ensuring the warning only appears when power monitoring runs without ABL.
300-300: LGTM!The condition correctly shows the mA/LED field when either ABL or power monitoring is enabled, allowing users to configure LED current for both features.
190-217: LGTM!The call to
enPM()at line 214 correctly initializes the power monitoring UI state during setup, ensuring the interface reflects the current configuration.
482-492: LGTM!The HTML structure for the per-LED mA display is correctly organized and consistent with other conditional UI elements in the form.
882-887: LGTM!The power monitoring UI elements are well-structured:
- Clear enable/disable toggle
- Appropriate voltage input range (1-255V)
- Helpful description and CPU usage warning
- Warning correctly hidden when ABL is already active
wled00/bus_manager.cpp (4)
296-304: LGTM!The modification correctly enables color channel summing for both ABL and power monitoring modes, ensuring accurate current estimation regardless of which feature is active.
363-365: LGTM!The
getVoltage()wrapper correctly exposes the BusManager voltage setting, following the established pattern for accessing manager-level configuration.
1450-1452: LGTM!The
currentWatts()calculation correctly converts milliamps to watts using the formula P = I × V with proper unit scaling (mA → A → W).
1469-1471: LGTM!The static member initialization is correct:
_gVoltageuses the appropriate constant default_usePowerMonitoringdefaults tofalse, matching the PR's design goal of opt-in power monitoring
2 participants
Adds approximate Watt-based power reporting to /json/info endpoint for easier integration with Home Assistant.
New API Fields
UI: Voltage Configuration
Added voltage configuration in LED settings to support different strip voltages (5V/12V/24V):
UPD:
Bug (?) Fix
Power monitoring now works when ABL disabled. Previously, it only reported ESP consumption.
Existing pwr and maxpwr mA fields - unchanged
Why
I think it's cool that there will soon be an option to add WLED consumption to Home Assistant to track these values over time.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.