New custom palettes editor #5010
Conversation
separating iro.js adds about 400bytes to the bin file, this is due to reduced gzip compression when using it as an external file (no dictionary reuse in index.x)
link properly minifies, @import is skipped completely.
many changes to cpal UI, fixed bugs, added buttons (WIP), added cached loading
- load iro.js sequentially - no parallel requests in cpal.htm - update UI buttons - fix showing sequential loading of palettes (using opacity) - better UX for mobile (larger markers, larger editor) - various fixes
WalkthroughAdds iro.js as a generated and served asset, replaces the inline palette editor with an iro-based deferred color-picker UI, makes palette discovery tolerant of gaps via a new gap limit macro, and changes palette removal to index-based deletion via JSON and usermod 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
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: 3
🧹 Nitpick comments (7)
wled00/data/cpal/cpal.htm (3)
138-143: Freeze segments during preview to avoid effects overriding the gradientWhen enabling preview, freeze selected segments; unfreeze on disable/unload.
- gId('chkPreview').addEventListener('change', (e) => { - prvEn = e.target.checked; - if (prvEn) applyLED(); - else requestJson({seg:{frz:false}}); - }); + gId('chkPreview').addEventListener('change', async (e) => { + prvEn = e.target.checked; + if (prvEn) { + await requestJson({seg:{frz:true}}); + await applyLED(); + } else { + await requestJson({seg:{frz:false}}); + } + });
85-106: Cap and back off loader retries to avoid tight retry loops on low heap or 404Current 100ms infinite retries can spam the device. Use capped exponential backoff per resource.
+ let _cssRetry=0, _iroRetry=0, _comRetry=0; @@ - l1.onerror = () => setTimeout(() => document.head.appendChild(l1), 100); + l1.onerror = () => { + const d = Math.min(100 * (2 ** (_cssRetry++)), 4000); + if (_cssRetry <= 6) setTimeout(() => document.head.appendChild(l1), d); + }; @@ - l2.onerror = () => setTimeout(() => document.head.appendChild(l2), 100); + l2.onerror = () => { + const d = Math.min(100 * (2 ** (_iroRetry++)), 4000); + if (_iroRetry <= 6) setTimeout(() => document.head.appendChild(l2), d); + }; @@ - l3.onerror = () => setTimeout(() => document.head.appendChild(l3), 100); + l3.onerror = () => { + const d = Math.min(100 * (2 ** (_comRetry++)), 4000); + if (_comRetry <= 6) setTimeout(() => document.head.appendChild(l3), d); + };
76-81: Remove debug fetch overrideOverriding window.fetch is noisy and can surprise consumers. Recommend removing or gating behind a debug flag.
-const origFetch = window.fetch; -window.fetch = function(...args) { - console.log('FETCH:', args[0]); - return origFetch.apply(this, args); -}; +// Debug-only: consider enabling request logging behind a query flag if needed.usermods/audioreactive/audio_reactive.cpp (1)
1735-1739: rmcpal gating should not rely on customPalettes.size() (gaps now allowed).With gap-based discovery, rmcpal can target indices beyond loaded count. Gate on rmcpal presence (and local palettes>0), not vector size.
Apply:
- if (root.containsKey(F("rmcpal")) && root[F("rmcpal")].as<byte>() < customPalettes.size()) { + if (palettes > 0 && root.containsKey(F("rmcpal"))) { // handle removal of custom palettes from JSON call so we don't break things removeAudioPalettes(); }wled00/colors.cpp (1)
252-254: Right idea to reuse a single JsonDocument; consider sizing.1536 bytes may be higher than needed for current formats. If RAM constrained, measure and drop to the smallest safe capacity (e.g., 512–1024) or add a static_assert/comment tying capacity to max entries.
wled00/data/index.js (1)
47-64: Make iro.js loader more robust.Avoid tight retry loops and duplicate handlers; add backoff, cap retries, and mark listeners once.
Apply:
+(function(){ + let _iroRetry = 0; + function loadIro() { + const l = d.createElement('script'); + l.src = 'iro.js'; + l.async = true; l.defer = true; + l.onload = () => { + _iroRetry = 0; + cpick = new iro.ColorPicker("#picker", { width: 260, wheelLightness: false, wheelAngle: 270, wheelDirection: "clockwise", layout: [{component: iro.ui.Wheel, options: {}}] }); + (d.readyState === 'complete') ? onLoad() : window.addEventListener('load', onLoad, { once: true }); + }; + l.onerror = () => { + const delay = Math.min(1000 * Math.pow(2, _iroRetry++), 10000); + setTimeout(loadIro, delay); + }; + document.head.appendChild(l); + } + loadIro(); +})(); - -(function loadIro() { - const l = d.createElement('script'); - l.src = 'iro.js'; - l.onload = () => { - cpick = new iro.ColorPicker("#picker", { width: 260, wheelLightness: false, wheelAngle: 270, wheelDirection: "clockwise", layout: [{component: iro.ui.Wheel, options: {}}] }); - d.readyState === 'complete' ? onLoad() : window.addEventListener('load', onLoad); - }; - l.onerror = () => setTimeout(loadIro, 100); - document.head.appendChild(l); -})();wled00/data/index.htm (1)
363-371: Remember to rebuild embedded assets for web changes.After editing files under wled00/data, run “npm run build” to regenerate embedded headers (e.g., js_iro.h) before firmware build. As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.gitignore(1 hunks)tools/cdata.js(1 hunks)usermods/audioreactive/audio_reactive.cpp(1 hunks)wled00/colors.cpp(2 hunks)wled00/const.h(1 hunks)wled00/data/cpal/cpal.htm(1 hunks)wled00/data/index.htm(3 hunks)wled00/data/index.js(3 hunks)wled00/json.cpp(1 hunks)wled00/wled_server.cpp(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/colors.cppwled00/wled_server.cppwled00/json.cpp
tools/cdata.js
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use tools/cdata.js to convert web assets to embedded C++ headers; do not reimplement this logic elsewhere
Files:
tools/cdata.js
wled00/**/!(html_*)*.h
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for non-generated C++ header files (.h)
Files:
wled00/const.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/index.jswled00/data/index.htmwled00/data/cpal/cpal.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/index.jswled00/data/index.htmwled00/data/cpal/cpal.htm
wled00/data/index.htm
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Main web interface entry file is index.htm; ensure it remains present and functional
Files:
wled00/data/index.htm
🧠 Learnings (5)
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
PR: wled/WLED#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:
.gitignorewled00/wled_server.cpp
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
PR: wled/WLED#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:
.gitignore
📚 Learning: 2025-10-05T15:24:05.545Z
Learnt from: CR
PR: wled/WLED#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-05T15:24:05.545Z
Learning: Applies to wled00/html_*.h : Always commit generated html_*.h files along with related source changes
Applied to files:
.gitignore
📚 Learning: 2025-08-29T00:26:15.808Z
Learnt from: ksedgwic
PR: wled/WLED#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:
.gitignore
📚 Learning: 2025-04-26T19:19:07.600Z
Learnt from: blazoncek
PR: wled/WLED#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
🧬 Code graph analysis (1)
wled00/json.cpp (1)
wled00/colors.cpp (2)
loadCustomPalettes(248-296)loadCustomPalettes(248-248)
🔇 Additional comments (7)
.gitignore (1)
28-28: LGTM: ignore generated JS asset headersThe new /wled00/js_.h ignore matches the generated js_iro.h asset headers. Keep committing html_.h alongside UI source edits. As per coding guidelines.
wled00/wled_server.cpp (1)
13-13: iro.js static serving integration looks correctIncludes js_iro.h and serves /iro.js via handleStaticContent with FS override and ETag, consistent with /common.js. No issues spotted.
Also applies to: 31-31, 271-273
wled00/const.h (1)
18-18: LGTM: explicit scan-gap cap for custom palette discoveryThe guard is clear and documented. Ensure colors.cpp uses this define as intended.
usermods/audioreactive/audio_reactive.cpp (1)
1742-1746: Re-add condition looks good.Using WLED_MAX_CUSTOM_PALETTES avoids hardcoding. LGTM.
wled00/colors.cpp (1)
292-294: Gap threshold semantics.If “max consecutive missing files” is the intent, consider breaking on >= WLED_MAX_CUSTOM_PALETTE_GAP to match the name precisely. Verify desired behavior.
wled00/data/index.js (1)
1694-1703: Palette editor toggle hook is correct.Showing #editPal only when palette UI is active matches UX. LGTM.
wled00/data/index.htm (1)
132-133: Single “Custom palette editor” entry replaces add/remove — good consolidation.UI wiring aligns with index.js (#editPal). LGTM.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wled00/data/cpal/cpal.htm (1)
648-648: Consider extracting magic numbers to named constants.The HTTP queue limit (
5), throttle delay (120ms), and preview delays (50/500ms) are currently inline magic numbers. While the code works correctly, extracting these to named constants would improve maintainability.For example:
+const HTTP_QUEUE_LIMIT = 5; +const HTTP_THROTTLE_DELAY = 120; +const PREVIEW_DELAY_WS = 50; +const PREVIEW_DELAY_HTTP = 500; + // ... later in code ... -if (_httpQueue.length >= 5) { +if (_httpQueue.length >= HTTP_QUEUE_LIMIT) { return Promise.resolve(-1); } // ... and ... -await new Promise(r => setTimeout(r, 120)); +await new Promise(r => setTimeout(r, HTTP_THROTTLE_DELAY));Also applies to: 666-666, 288-288
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
usermods/audioreactive/audio_reactive.cpp(1 hunks)wled00/data/cpal/cpal.htm(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- usermods/audioreactive/audio_reactive.cpp
🧰 Additional context used
📓 Path-based instructions (2)
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/cpal/cpal.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/cpal/cpal.htm
🔇 Additional comments (1)
wled00/data/cpal/cpal.htm (1)
195-196: No critical issues found—maxCol=50 value is well-documented and justified.The TODO comment reflects healthy caution, but verification confirms the limit is sound: line 73 documents that ESP8266 is limited to ~50 colors per chunk due to 500-byte websocket constraints, which aligns with the constraint enforced in
index.js(line 1737). The chunking logic inapplyLED()(line 695) correctly uses this value. While end-to-end testing on real hardware would be ideal, the implementation is justified by documented constraints and requires no changes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
wled00/data/cpal/cpal.htm (4)
195-196: Verify ESP8266 chunk size and consider defining constants.The TODO comment indicates the ESP8266 maxCol value of 50 has not been tested. An incorrect value could cause crashes (too large) or degraded performance (too small).
Based on learnings, consider defining these as named constants rather than magic numbers, e.g.:
const MAX_COLORS_ESP8266 = 50; // TODO: verify with hardware testing const MAX_COLORS_ESP32 = 128;Would you like me to generate a shell script to check if there are any related constants defined elsewhere in the codebase, or any documentation about these limits?
303-303: Consider defining max marker count as a named constant.Based on coding guidelines, the hardcoded value 16 would be clearer as a named constant:
const MAX_PALETTE_MARKERS = 16; function canAdd() { return gr.querySelectorAll('.mk').length < MAX_PALETTE_MARKERS; }This improves maintainability and makes the constraint more discoverable.
363-394: Well-designed harmonic palette generation.The implementation creates visually pleasing palettes by:
- Using harmonic color relationships (triadic/tetradic) for 67% of generated palettes
- Proper hue offset calculation:
(base + 360 * (i % hcount) / hcount) % 360- Different lightness ranges for random vs. harmonic modes
Optional: The commented-out
console.logstatements on lines 373 and 381 could be removed if no longer needed for debugging.
505-505: Consider defining the custom palette warning threshold as a constant.Based on coding guidelines, the hardcoded value 10 would be clearer as a named constant:
const CUSTOM_PALETTE_WARNING_THRESHOLD = 10;Then use:
gId('memWarn').style.display = (cpc > CUSTOM_PALETTE_WARNING_THRESHOLD) ? 'block' : 'none';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wled00/data/cpal/cpal.htm(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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/cpal/cpal.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/cpal/cpal.htm
🧠 Learnings (7)
📓 Common learnings
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
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.
📚 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/cpal/cpal.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/cpal/cpal.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/cpal/cpal.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/cpal/cpal.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: Manually validate the web UI after changes (load index, navigation, color controls, effects, settings) and check browser console for JS errors
Applied to files:
wled00/data/cpal/cpal.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/cpal/cpal.htm
🔇 Additional comments (7)
wled00/data/cpal/cpal.htm (7)
75-97: Resource loading with infinite retry is acceptable for embedded context.The sequential loading with indefinite retry logic is well-suited for WLED's embedded environment where resources are guaranteed to exist but heap pressure may cause temporary 503 errors. The 100ms retry delay prevents overwhelming the server.
138-182: Excellent touch and keyboard support for marker manipulation.The pointer event handling properly:
- Distinguishes clicks from drags using the
isDraggingflag- Prevents locked endpoints from being moved
- Clamps positions to valid ranges (minT/maxT logic)
- Includes accessible keyboard nudging with Shift modifier for larger steps
- Properly cleans up event listeners
261-281: Robust palette cache implementation with proper interpolation.The cache building correctly:
- Handles all edge cases (empty, before first, after last stop)
- Protects against division by zero when stops coincide
- Uses linear RGB interpolation for smooth gradients
- Generates the 256-entry hex cache (without
#) needed for LED previews and JSON serialization
635-672: Well-designed request queuing for resource-constrained devices.The implementation properly:
- Prefers WebSocket when available for lower latency
- Limits HTTP queue to 5 requests to prevent memory issues
- Uses 120ms delays between HTTP requests to avoid overwhelming the ESP
- Employs an IIFE-based mutex pattern (
_httpRun) to serialize requestsThis is appropriate for WLED's embedded environment.
674-701: Robust LED preview with proper chunking and segment handling.The implementation correctly:
- Fetches current state and identifies selected/main segments
- Maps 2D segment dimensions to 1D for gradient visualization
- Chunks color data based on
maxColto prevent buffer overflow (ESP8266: 50, ESP32: 128)- Awaits each chunk to ensure sequential delivery without overwhelming the device
- Wraps everything in try-catch for resilience
799-810: Excellent touch-friendly responsive design.The
@media (pointer: coarse)query appropriately:
- Increases gradient height to 60px for easier interaction
- Widens marker handles to 20px for better touch targeting
- Adjusts sticky positioning accordingly
This aligns with the PR objectives for improved touch-device experience.
756-783: Sticky positioning enables seamless multi-palette workflow.The sticky positioning with proper z-index hierarchy (editor: 5, empty slot: 4) allows users to:
- Keep the gradient editor visible while browsing palettes
- Always see the first empty slot for quick saves
This aligns with the PR objective "allows adding multiple custom palettes without returning to the main UI."
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wled00/colors.cpp (1)
253-253: Consider using explicit integer type for embedded systems.While
unsignedis correct, usinguint8_torunsigned intwould be more explicit and conventional for embedded systems code, especially since the maximum value (WLED_MAX_CUSTOM_PALETTE_GAP = 20) easily fits in a byte.🔎 Suggested refinement
- unsigned emptyPaletteGap = 0; // count gaps in palette files to stop looking for more (each exists() call takes ~5ms) + uint8_t emptyPaletteGap = 0; // count gaps in palette files to stop looking for more (each exists() call takes ~5ms)
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
tools/cdata.jsusermods/audioreactive/audio_reactive.cppwled00/colors.cppwled00/const.hwled00/data/index.htmwled00/data/index.jswled00/json.cppwled00/wled_server.cpp
🚧 Files skipped from review as they are similar to previous changes (5)
- wled00/data/index.js
- usermods/audioreactive/audio_reactive.cpp
- wled00/json.cpp
- wled00/data/index.htm
- tools/cdata.js
🧰 Additional context used
📓 Path-based instructions (2)
wled00/**/*.cpp
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use 2-space indentation for C++ source files (.cpp)
Files:
wled00/wled_server.cppwled00/colors.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.h
🧠 Learnings (25)
📓 Common learnings
Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR #4798, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.
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.
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-15T20:25:25.815Z
Learning: The CSS palette preview in wled00/data/index.js genPalPrevCss() function uses raw RGB values in CSS linear-gradient() without applying WLED's gamma correction, while actual LED output goes through NeoGammaWLEDMethod gamma correction. This causes inherent discrepancies between the web UI palette preview and actual LED colors, especially noticeable with different gamma settings.
📚 Learning: 2025-09-14T18:43:59.338Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4932
File: wled00/data/cpal/cpal.htm:14-14
Timestamp: 2025-09-14T18:43:59.338Z
Learning: In WLED, static web pages like cpal.htm are served directly at the root URL level (e.g., "/cpal.htm") via handleStaticContent() from embedded page data, regardless of their physical source folder location in the repository. Relative script references like "common.js" resolve correctly from the served URL path, not the source file path.
Applied to files:
wled00/wled_server.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/html_*.h : DO NOT edit generated embedded web header files (wled00/html_*.h)
Applied to files:
wled00/wled_server.cppwled00/colors.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/wled_server.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: In WLED projects, ArduinoJson.h is not directly included via #include <ArduinoJson.h> - the ArduinoJson symbols are made available through the WLED build system and wled.h transitive includes, so explicitly adding #include <ArduinoJson.h> is not necessary and may not work.
Applied to files:
wled00/wled_server.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/html_*.h : Always commit generated html_*.h files along with related source changes
Applied to files:
wled00/wled_server.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/wled_server.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/colors.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/const.hwled00/colors.cpp
📚 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.hwled00/colors.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/const.hwled00/colors.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/const.hwled00/colors.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/const.hwled00/colors.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/const.hwled00/colors.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/const.hwled00/colors.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, file operations (especially file.open()) should be checked to ensure they respect LittleFS filename limitations. The default LittleFS filename limit is 255 bytes (LFS_NAME_MAX). Reviews should assume default WLED configuration defines and not extreme edge-case values (e.g., WLED_MAX_SEGNAME_LEN = 512 would not be standard). File paths should be validated to stay within the 255-byte limit.
Applied to files:
wled00/const.h
📚 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/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-02-19T12:43:34.200Z
Learnt from: blazoncek
Repo: wled/WLED PR: 4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.200Z
Learning: In WLED, maximum segment name length varies by platform:
- ESP8266: 32 characters (WLED_MAX_SEGNAME_LEN = 32)
- ESP32: 64 characters (WLED_MAX_SEGNAME_LEN = 64)
This platform difference can cause truncation when syncing longer names from ESP32 to ESP8266. Additionally, the WLED UI has limitations regarding modified maximum segment name lengths.
Applied to files:
wled00/const.h
📚 Learning: 2025-05-09T18:43:15.355Z
Learnt from: DedeHai
Repo: wled/WLED PR: 4682
File: wled00/FX.cpp:8997-9005
Timestamp: 2025-05-09T18:43:15.355Z
Learning: In the WLED codebase, SEGMENT.custom3 is always constrained to the range 0-31 and will not exceed this range.
Applied to files:
wled00/const.h
📚 Learning: 2025-06-15T09:59:52.720Z
Learnt from: netmindz
Repo: wled/WLED PR: 4728
File: wled00/FX.h:378-378
Timestamp: 2025-06-15T09:59:52.720Z
Learning: In WLED's FX.h, MODE_COUNT represents the highest FX_MODE_ ID + 1, not the total count of FX_MODE_ definitions. This is because some effects may have been removed but their IDs are not reused, creating gaps in the numbering sequence. MODE_COUNT is used for array bounds and range checking.
Applied to files:
wled00/const.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/colors.cpp
📚 Learning: 2025-12-15T20:25:25.815Z
Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-12-15T20:25:25.815Z
Learning: The CSS palette preview in wled00/data/index.js genPalPrevCss() function uses raw RGB values in CSS linear-gradient() without applying WLED's gamma correction, while actual LED output goes through NeoGammaWLEDMethod gamma correction. This causes inherent discrepancies between the web UI palette preview and actual LED colors, especially noticeable with different gamma settings.
Applied to files:
wled00/colors.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/colors.cpp
📚 Learning: 2025-11-22T12:12:53.486Z
Learnt from: DedeHai
Repo: wled/WLED PR: 5105
File: wled00/FX.h:968-968
Timestamp: 2025-11-22T12:12:53.486Z
Learning: In WLED's WS2812FX class (wled00/FX.h), the _pixels array stores LED colors indexed by unmapped (logical) LED positions, not physical positions. The getPixelColor(n) function intentionally checks if the logical LED at index n maps to a valid physical position via getMappedPixelIndex(n), then accesses _pixels[n] with the unmapped index. If the mapping is invalid (e.g., -1 for gaps in LED maps, or out of bounds), it returns 0 (black). This allows peek preview to correctly show gaps in LED mappings as black pixels. The mapping from logical to physical positions is only applied when writing to the hardware bus, not when reading from the pixel buffer.
<!-- [add_learning]
Applied to files:
wled00/colors.cpp
🔇 Additional comments (4)
wled00/wled_server.cpp (1)
9-9: LGTM! Clean implementation following established patterns.The addition of iro.js serving support follows the exact same pattern as the existing common.js asset. The include, path constant, and HTTP handler are all correctly implemented and consistent with WLED's conventions for serving static JavaScript resources.
Also applies to: 40-40, 355-357
wled00/const.h (1)
18-18: LGTM! Well-documented performance threshold.The new constant is appropriately placed alongside related palette definitions and includes a helpful performance note. The value of 20 provides a reasonable balance between allowing gaps in custom palettes and limiting filesystem check overhead.
wled00/colors.cpp (2)
258-258: LGTM! Correct gap counter reset logic.Resetting the gap counter when a palette file exists is the correct approach, allowing the discovery process to tolerate non-consecutive palette indices with gaps up to the defined threshold.
292-293: No action needed—boundary condition is correct.The
>comparison is intentional. The const.h comment "max number of empty palette files in a row before stopping to look for more (20 takes 100ms)" indicates that the code should allow checking up to 20 consecutive missing files. The current logic does exactly that: the gap counter increments, and breaks when it exceeds 20 (on the 21st missing file). This is the correct implementation.
|
@softhack007 @willmmiles if you have some time to spare I would love to get some high-level feedback on this, you do not have to dive into the details, I did thorough testing so it should all be good. |
1 participant
I made a new and improved custom palette editor.
Features:
All these features come at a cost of 2k of flash
Also fixed a bug in AR usermod regarding new "unlimited" custom palettes.
New added categories with 800+ palettes to choose from:
Editor and empty slot are "sticky":
On touch screens editor has larger handlers and larger editor gradient:
Summary by CodeRabbit
New Features
UI/UX Improvements
Behavior Changes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.