Missing define examples and new define options #4992
Conversation
WalkthroughIntroduces macro-driven default configuration values in wled00/wled.h, replacing hard-coded initializers for WiFi, MQTT, NTP, colors, sync, UDP ports, and other flags. Adds extensive commented guidance and sample -D options to platformio_override.sample.ini. No public API or functional logic changes; only default initialization and documentation updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (1)
wled00/wled.h (1)
755-778: Consider documenting the bit manipulation logic.The macros
RECEIVE_DEFAULT_OPTIONS(line 774) andNOTIFY_DEFAULT_OPTIONS(line 778) use complex bit manipulation with hardcoded bitmasks (0x67and0x0F). While functionally correct, the logic could benefit from inline comments explaining:
- What each bit position represents
- Why these specific bitmask values were chosen
- How the conditional bit setting works
Example documentation to add above line 774:
+// RECEIVE_DEFAULT_OPTIONS builds a bitfield from 0x67 (0b01100111): +// Bits 0-6: Brightness(1), Color(1), Effects(1), SegmentOptions(0), SegmentBounds(0), Direct(from RECEIVE_DEFAULT_DIRECT), Palette(1) #define RECEIVE_DEFAULT_OPTIONS ( (0x67 & ~(1<<5)) | ( (RECEIVE_DEFAULT_DIRECT) ? (1<<5) : 0 ) )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
platformio_override.sample.ini(5 hunks)wled00/wled.h(17 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/!(html_*)*.h
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for non-generated C++ header files (.h)
Files:
wled00/wled.h
🧠 Learnings (2)
📚 Learning: 2025-05-26T16:09:34.325Z
Learnt from: blazoncek
PR: wled/WLED#4700
File: wled00/wled_server.cpp:409-414
Timestamp: 2025-05-26T16:09:34.325Z
Learning: In wled00/wled_server.cpp, the OTA update access control logic intentionally allows OTA updates from different subnets when otaSameSubnet is true AND a PIN is set. This was a conscious design decision by blazoncek to provide flexibility for remote OTA updates with PIN protection, though they acknowledged it may not be the optimal security model.
Applied to files:
wled00/wled.h
📚 Learning: 2025-04-28T20:51:29.773Z
Learnt from: KrX3D
PR: wled/WLED#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/wled.h
🔇 Additional comments (13)
wled00/wled.h (13)
338-341: LGTM!The NTP server default is properly defined with a fallback to a WLED-specific NTP pool address. The
#ifndefguard allows customization via build flags.
348-379: LGTM!The platform-specific WiFi sleep defaults are appropriate—ESP32 defaults to
true(no sleep) for better stability, while ESP8266 defaults tofalseto conserve power. The macro is correctly applied to both the compact (WLED_SAVE_RAM) and standard struct variants.
416-424: LGTM!The LED boot behavior defaults are sensible: turning LEDs on at boot by default (
true) and not applying a preset (0). Both macros properly use#ifndefguards for customization.
446-474: LGTM!The default color values are reasonable:
- Primary color defaults to orange (255, 160, 0, 0) which is a warm, visible color
- Secondary color defaults to black/off (0, 0, 0, 0)
All eight macros (four per color) have proper
#ifndefguards for customization.
493-500: LGTM!Both node list and broadcast features default to enabled (
true), which allows WLED instances to discover and communicate with each other out of the box. This is appropriate for most use cases.
563-603: LGTM!The extensive MQTT default macros are well-organized. Default values are appropriate:
- MQTT disabled by default (
false)- Standard MQTT port (1883)
- Empty strings for server/credentials requiring user configuration
- Group topic defaults to "wled/all"
- Retain defaults to
falseAll macros properly use
#ifndefguards for customization.
643-647: LGTM!The NTP AM/PM format defaults to
false(24-hour format), which is the international standard and appropriate for most use cases.
683-686: LGTM!The OTA same-subnet restriction defaults to
true, which is a secure default that prevents OTA updates from external networks unless a PIN is set (as noted in learnings).Based on learnings
730-733: LGTM!The default brightness of 128 (50% of 255) is a sensible middle-ground value that's visible but not overwhelming.
741-746: LGTM!MQTT button publish defaults to
false, which is appropriate as most users won't want button presses to trigger MQTT messages unless explicitly configured.
763-770: LGTM!Sync send and receive groups both default to
0x01(group 1), enabling basic synchronization out of the box while allowing users to customize via build flags.
861-906: LGTM!UDP port defaults are correctly set:
- Main port: 21234 (standard WLED notifier port)
- Second port: 65506 (supplemental port)
- RGB port: 19446 (Hyperion port)
Both compact (WLED_SAVE_RAM) and standard struct variants use the same default values via the new macros.
953-966: LGTM!Realtime defaults are sensible:
- Main segment only defaults to
false(apply to all segments)- Respect LED maps defaults to
true(honor user-defined mappings)These defaults provide flexible behavior while respecting user configurations.
1 participant
Hi,
i added some missing define examples to platformio_override.sample.ini
The defines where present but no info was there how to use them, i also added the location of the option on the webpage like:
Settings -> Wifi Setup -> Network name (SSID, empty to not connect)
for a better understanding what this define is for.
And together with that i added a couple more things that can be defined. Mostly the one i currently use, since i got an device i bought that keeps resetting to the compiled defaults... , but i would like to add more in future when i got time (not in this PR)
The defines can also be added to the my_config file like i did, here is an list of the new and already present once added with the PR.
Summary by CodeRabbit
Documentation
Chores