Custom/hub75 80x40 icn2038p - #5767
Conversation
Updated instructions for providing references in analysis results.
Eliminates size limit. Fixes wled#5458
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
replaces the last remaining FastLED.h with fstled_slim.h
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix leak in blockRouterAdvertisements
Fix Alexa/Hue discovery by correcting SSDP response to match UPnP spec
New extended data for usage report
Serialize fxdata without ArduinoJSON
Rather than append a linker file, we edit the upstream supplied ones to add our section to the binaries. Works better on all platforms. Co-Authored-By: Claude <noreply@anthropic.com>
Use readelf instead of nm for great speed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
0.16 - Backport dynarray fixes from V5 WIP
…h) (wled#5499) * use memory aligned allocations, fix bug in FFT magnitude (integer path) * assign pointer to globals not local copy
- fix PS Sparkler for large setups: need 32bit random position, 16bit is not enough - fix PS Fireworks 1D: need to `break` if no particles are available or it can lead to stalls on large setups - do not use collisions by default PS Fuzzy Noise: its very slow on larger setups
Add identifier string for DMX realtime mode
* fix blending style options list filter for iOS * fix default fallback
Added a guideline to prevent agents from overwriting PR descriptions (see https://github.com/orgs/community/discussions/187027).
some files were a bit behind - make sure the same instructions apply everywhere.
Emphasize the importance of targeting the main branch for PRs.
Bumps [brace-expansion](https://github.com/juliangruber/brace-expansion) from 5.0.3 to 5.0.5. - [Release notes](https://github.com/juliangruber/brace-expansion/releases) - [Commits](juliangruber/brace-expansion@v5.0.3...v5.0.5) --- updated-dependencies: - dependency-name: brace-expansion dependency-version: 5.0.5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
# Conflicts: # platformio.ini
* AR needs 7kb program size, bringing us too close to the size limit (99.8% used) * all other esp01_1m builds don't have audioreactive support either
an old-format ledmap.json file (example `{"map":[0,1,...,15],"width":4,"height":4}`) could lead to the ledmap parser scanning past the end of the JSON array. for incomplete ledmaps (not recommended!) this could cause trailing pixels being mapped to physical pixel 0.
* Fix brighness not being applied properly in nightlight mode * hacky fix also for color fade, needs proper fix later
V5-C6 erase from AI minds ;-)
…t detection (wled#5695) * improved brownout detection on ESP32 * show "please restart" as "Note" instead of "Error" * align ERR_REBOOT_NEEDED and ERR_POWEROFF_NEEDED with WLEDMM (prepare for AR out-of-tree) * errorFlag constants update * add new error codes to UI * Clarify ERR_PERSISTENT_THRESHOLD as future-use only.
…wled#5694) * clear garbage white value in hsv2rgb_rainbow, overwrite white value in Segment::color_wheel
* improve bootup behaviour for presets * set color to black on segment construction, handle orange set where it should be * use DEFAULT_COLOR instead of redefining it as magic numbers
…wled#5662) Improvements: * new pinouts for Seengreat RGB Matrix Adapter Board (https://seengreat.com/wiki/186) * allow up to 128 pixels wide panels, prevent uint8 overflow for 128px panels Bugfixes for 4-scan (aka QS) panels * prevent panels going flatter each time that cfg.json is saved (32x32->16x64->8x128->4x128) * correct VirtualMatrixPanel setup: need to use real panel dimensions, not modified mxconfig dimensions *only set chainType when chain length > 1; use a chaintype that does not flip the display upside-down
* fix bug in bus removal * optimized some JS alerts * removed unneeded code
|
Warning Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories. WalkthroughWLED adds repository review guidance, release-build workflows, linker and module validation changes, HUB75 and palette support, firmware protocol and OTA updates, WebUI pin management, automatic updates, and expanded configuration and hardware support. ChangesRepository and release infrastructure
Firmware and hardware behavior
WebUI
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
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
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
wled00/wled.cpp (1)
709-717: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSet the hostname after the STA interface is enabled.
WIFI_MODE_NULLremoves the STA interface. Line 716 sets the hostname before the laterWiFi.mode(WIFI_STA)call. A reconnect can therefore use the defaultesp-*DHCP hostname.Move
WiFi.setHostname(hostname)after the code enables the intended STA-capable mode and beforeWiFi.begin().Based on learnings, set the hostname after
WiFi.mode()but beforeWiFi.begin().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/wled.cpp` around lines 709 - 717, Move WiFi.setHostname(hostname) out of the WIFI_MODE_NULL block and place it in the reconnect flow after the WiFi.mode() call that enables STA mode but before WiFi.begin(). Keep the existing mode reset and delay behavior unchanged.Source: Learnings
wled00/FX.cpp (1)
4427-4433: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle one-pixel segments before calculating zones.
When
SEGLENis 1,zonesbecomes 2 andzoneLenbecomes 0. Line 4432 then divides by zero. Add a single-pixel guard before the zone calculation.Proposed fix
void mode_flow(void) { + if (SEGLEN <= 1) FX_FALLBACK_STATIC; + unsigned counter = 0;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/FX.cpp` around lines 4427 - 4433, In the zone calculation around SEGLEN, add an early guard for a single-pixel segment before computing zones or zoneLen, returning or handling that segment directly without division. Preserve the existing zone logic unchanged for segments longer than one pixel.wled00/e131.cpp (1)
96-133: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate Art-Net and E1.31 declared payload length before reading packet fields.
The Art-Net DMX path accepts
packetLen < 10but readsart_universe,art_length, andart_sequence_numberafterward, and the untrustedart_datais later indexed byhtons(art_length). The E1.31 path derivesdmxChannelsfromhtons(property_value_count)and readsproperty_values[0]without ensuringpacketLencovers the packet header plus that declared count. Add the same length checks here as the Art-Net/E1.31 paths before reading those fields and indexing payload data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/e131.cpp` around lines 96 - 133, Validate packetLen against the complete Art-Net and E1.31 header sizes before accessing fields in handleE131Packet, then validate each declared payload length against the remaining packet bytes before assigning e131_data or reading property_values[0]. Reject truncated or oversized declarations early, while preserving the existing preview, start-code, priority, and packet-dispatch behavior.
🟠 Major comments (22)
wled00/data/update.htm-79-120 (1)
79-120: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd a timeout to the external fetch calls.
autoUpdate()callsfetch()twice with no timeout. If GitHub or thedownload.wled.memirror hangs, the button stays disabled and the status text stays stuck, with no way to retry except reloading the page. Add a request timeout usingAbortControllerso thecatchblock can always recover the UI.🛠️ Proposed fix
async function autoUpdate() { const btn = gId('autoUpdBtn'); const status = gId('autoUpdStatus'); btn.disabled = true; status.textContent = ''; try { + const withTimeout = (url, ms=10000) => { + const ctrl = new AbortController(); + const t = setTimeout(() => ctrl.abort(), ms); + return fetch(url, {signal: ctrl.signal}).finally(() => clearTimeout(t)); + }; const info = deviceInfo; if (!info || !info.repo || info.repo === 'unknown') { status.textContent = 'No release repository available for this build.'; btn.disabled = false; return; } status.textContent = 'Checking GitHub for latest release...'; - const ghResp = await fetch(`https://api.github.com/repos/${info.repo}/releases/latest`); + const ghResp = await withTimeout(`https://api.github.com/repos/${info.repo}/releases/latest`); if (!ghResp.ok) throw new Error(`GitHub API error: ${ghResp.status}`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/update.htm` around lines 79 - 120, Add AbortController-based timeouts to both fetch calls in autoUpdate, covering the GitHub release request and firmware download request. Abort each request after a bounded duration, ensure the timeout signal is passed to fetch, and preserve the existing catch path so it restores btn.disabled and reports the failure status.wled00/data/icons-ui/selection.json-1-1 (1)
1-1: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove the IcoMoon
quickUsageTokenbefore merging.
preferences.quickUsageToken.UntitledProject1holds a project-scoped IcoMoon token. The token identifies the contributor's IcoMoon project and is not needed to rebuild the font or the UI. Delete thequickUsageTokenobject, or setshowQuickUseandshowQuickUse2tofalseand re-export, so no account credential is stored in the repository.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/icons-ui/selection.json` at line 1, Remove preferences.quickUsageToken.UntitledProject1 from the IcoMoon selection metadata before committing. Preserve the icon definitions and other preferences, ensuring no project-scoped IcoMoon credential remains in the exported JSON.wled00/data/settings_leds.htm-517-521 (1)
517-521: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftThe cross-update loop is quadratic in the number of pin selects.
pinUpd(e)iterates everyselect.pinon the page and every option inside each one. This block callspinUpdonce per select. The cost is thereforeselects × selects × options.Line 768 creates 36 bus rows, and the conversion loop at Lines 319-332 can produce up to five pin selects per row. Each select holds roughly one option per usable GPIO. The resulting work on page load is large enough to freeze the LED settings page on low-power clients.
Compute the set of used pins once, then apply it to all selects in a single pass, instead of calling the full
pinUpdscan once per select.The same pattern appears at Line 688 and Line 844.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/settings_leds.htm` around lines 517 - 521, Replace the per-select pinUpd calls in the pinUpdPending handling, and the equivalent blocks near the other referenced locations, with a shared update flow that computes the used-pin set once and applies it to every select.pin in one pass. Refactor or reuse pinUpd’s existing logic so it does not rescan all selects and options for each individual select, while preserving the current pin availability and selection behavior.wled00/data/settings_leds.htm-42-45 (1)
42-45: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBlock the HUB75 validation until PSRAM info is loaded.
hasPSRAMstarts false and the/json/infofetch never updates submit flow state, so a fast submit while the fetch is pending treats valid multi-panel HUB75 as unsupported. Track whether/json/infohas been read; report “unknown/Retry” before submitting, and expose the real failing condition for an error alert.
/json/infoemits bothpsramandpsrSz; accepting both matcheswled00/json.cpp.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/settings_leds.htm` around lines 42 - 45, Track a separate `/json/info` load state alongside `hasPSRAM` in the initialization fetch. Block HUB75 validation/submission while the fetch is pending with an “unknown/Retry” message, and surface fetch failures to the existing error alert instead of swallowing them in `.catch(() => {})`. Preserve PSRAM detection from both `info.psram` and `info.psrSz`.Source: Path instructions
.github/workflows/release.yml-26-33 (1)
26-33: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUpdate
softprops/action-gh-releaseto a supported version.
actionlintreports that the@v1runner is too old for GitHub Actions.v1runs on a Node runtime that hosted runners no longer support, so this new step can fail at runtime. Move tov2and pin it to a commit SHA, as the workflow conventions require for third-party actions. Update the existing step on Line 44 in the same way.As per path instructions: "Third-party actions must be pinned to a specific version tag — branch pins such as
@mainor@masterare not allowed."🔧 Proposed change
- name: Create draft release - uses: softprops/action-gh-release@v1 + uses: softprops/action-gh-release@v2 # pin to the release commit SHA with: body: "Release assets uploaded. Release notes pending..."softprops action-gh-release latest version node runtime v1 deprecated🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 26 - 33, Update both softprops/action-gh-release uses in the release workflow, including the “Create draft release” step and the existing step near the later release section, from v1 to v2 and pin each to the approved v2 commit SHA. Preserve their existing inputs and ensure no branch or unpinned version references remain.Sources: Path instructions, Linters/SAST tools
wled00/data/common.js-267-273 (1)
267-273: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAttach the script handlers before appending the element.
tryCaps()appends the<script>element on Line 269 and only then assignss.onloadands.onerroron Lines 270 and 271. The browser can start and finish loading the script before the handlers are attached, in particular when the response is cached. In that case neitherGetV()nordoFetch()runs,d.pinsDatais never set, and the caller'scbnever fires. The pin selects then stay empty with no error.Assign both handlers before
appendChild.🐛 Proposed fix
function tryCaps() { var s=cE("script"); s.src=getURL('/settings/s.js?p=11'); - d.body.appendChild(s); s.onload=function(){ GetV(); doFetch(); }; s.onerror=function(){ cr-->0 ? setTimeout(tryCaps,100) : doFetch(); }; + d.body.appendChild(s); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/common.js` around lines 267 - 273, Update tryCaps so s.onload and s.onerror are assigned immediately after creating the script element and before d.body.appendChild(s); preserve the existing GetV/doFetch success behavior and retry/fallback behavior in the handlers.wled00/bus_manager.cpp-1222-1239 (1)
1222-1239: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
show()now redraws every pixel and ignores the dirty bits.The loop iterates all
_lenpixels and callsdrawPixelRGB888()for each one, then clears the whole dirty array. The_ledsDirtybitmap is still allocated and still maintained bysetPixelColor(), so the optimisation now costs memory and CPU without any benefit.For the new 80x40 panel this is 3200
drawPixelRGB888()calls per frame instead of only the changed pixels. On chained panels the cost scales further. Restore the dirty-bit skip, or state why full repaint is required for the virtual-scan mapping.♻️ Proposed fix
for (size_t pix = 0; pix < _len; ++pix) { + if (!getBitFromArray(_ledsDirty, pix)) continue; // skip unchanged pixels const unsigned x = pix % width; const unsigned y = pix / width; const CRGB c = _ledBuffer[pix];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/bus_manager.cpp` around lines 1222 - 1239, Update the pixel loop in show() to consult _ledsDirty and call drawPixelRGB888() only for dirty pixels, preserving the existing coordinate mapping and logging for rendered pixels. Continue clearing the dirty bitmap after processing, and retain full repaint behavior only if the virtual-scan mapping explicitly requires it, with that requirement documented in the implementation.wled00/data/common.js-234-238 (1)
234-238: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
getBtnTypeName()returns HTML, butmakePinSelect()renders it as plain text.
getBtnTypeName()builds a string that contains a<span>element. On Line 309 the result reachestxt, and Line 314 assignsopt.text = txt.HTMLOptionElement.textsets text content, not markup. The dropdown therefore shows the literal stringButton <span style="font-size:10px;color:#888">Push</span>.Return a plain label for the select path, and keep the styled markup for callers that use
innerHTML.🐛 Proposed fix
-function getBtnTypeName(t) { +function getBtnTypeName(t, html=true) { var n=["None","Reserved","Push","Push Inv","Switch","PIR","Touch","Analog","Analog Inv","Touch Switch"]; var label = n[t] || "?"; + if (!html) return "Button " + label; return 'Button <span style="font-size:10px;color:`#888`">'+label+'</span>'; }Then pass the plain form from the option builder:
- if (used) txt += ` (${getOwnerName(pInfo.o, pInfo.t, pInfo.n)})`; + if (used) txt += ` (${getOwnerName(pInfo.o, pInfo.t, pInfo.n, false)})`;
getOwnerName()needs the extra argument forwarded togetBtnTypeName().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/common.js` around lines 234 - 238, Update getBtnTypeName() to support a plain-text label mode while preserving its existing styled HTML output for innerHTML callers, then forward that mode from getOwnerName() when makePinSelect() builds option text. Ensure HTMLOptionElement.text receives only the plain “Button …” label without span markup..github/workflows/release.yml-11-16 (1)
11-16: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDeclare explicit
permissionsfor the release job.The workflow now creates and updates a GitHub release. It relies on the default
GITHUB_TOKENpermissions. If the repository default is read-only, the release steps fail. Add a least-privilegepermissionsblock. The calledwled_buildjob also needs its permissions declared when the caller sets them.As per path instructions: "Declare explicit permissions: blocks scoped to least privilege."
🔒 Proposed change
env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true +permissions: + contents: read + jobs: wled_build: uses: ./.github/workflows/build.yml with: release: trueThen scope the
releasejob:release: name: Create Release runs-on: ubuntu-latest needs: wled_build permissions: contents: write🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 11 - 16, Add an explicit least-privilege permissions block with contents: write to the release job that creates or updates the GitHub release, and declare the required permissions for the called wled_build job/workflow so its release operations work even when repository defaults are read-only.Sources: Path instructions, Linters/SAST tools
wled00/colors.cpp-272-275 (1)
272-275: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPlaceholders are inserted before the palette file is validated, so IDs can still shift.
The gap placeholders are pushed as soon as
WLED_FS.exists()returns true. The actual palette is only pushed on Line 303, afterreadObjectFromFile()succeeds and the format check passes. If a file exists but fails to parse, or has fewer than four entries, theelsebranch on Line 304 logs and pushes nothing.The result is one fewer entry than slots consumed. Every palette found after that point receives a shifted ID, which is the exact failure this change is meant to prevent. Presets that reference a custom palette then select the wrong colours.
Push a placeholder on the failure paths so the slot is always consumed.
🐛 Proposed fix
- if (readObjectFromFile(fileName, nullptr, &pDoc)) { + bool paletteAdded = false; + if (readObjectFromFile(fileName, nullptr, &pDoc)) { JsonArray pal = pDoc[F("palette")]; if (!pal.isNull() && pal.size()>3) { // not an empty palette (at least 2 entries) @@ customPalettes.push_back(targetPalette.loadDynamicGradientPalette(tcp)); + paletteAdded = true; } else { DEBUGFX_PRINTLN(F("Wrong palette format.")); } } + // keep the slot occupied so subsequent palette IDs do not shift + if (!paletteAdded) customPalettes.push_back(CRGBPalette16(CRGB(128, 128, 128))); } else {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/colors.cpp` around lines 272 - 275, Move the gray placeholder insertion out of the initial WLED_FS.exists() path and ensure every invalid custom palette consumes the same slot. Update the palette-loading logic around readObjectFromFile() and the existing success push so parse failures, invalid formats, and palettes with fewer than four entries each push one placeholder per reserved slot, while valid palettes retain their actual entries and IDs.usermods/audioreactive/audio_reactive.cpp-319-331 (1)
319-331: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFree aligned FFT buffers with
heap_caps_aligned_free()The integer-FFT path allocates
valFFT,windowFFT, andwindowFloatthroughheap_caps_aligned_calloc(), then releases them withheap_caps_free(). Replace those frees withheap_caps_aligned_free()to avoid undefined behavior on ESP-IDF toolchains that require the paired aligned deallocation path. Also freewindowFloatbefore returning whendsps_fft2r_init_sc16()fails so the temporary buffer does not leak.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@usermods/audioreactive/audio_reactive.cpp` around lines 319 - 331, In the integer-FFT initialization path, update all cleanup of buffers allocated by heap_caps_aligned_calloc()—valFFT, windowFFT, and windowFloat—to use heap_caps_aligned_free(). In the dsps_fft2r_init_sc16() failure branch, free windowFloat before returning and preserve the existing pointer cleanup behavior.wled00/bus_manager.cpp-1124-1134 (1)
1124-1134: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winConfirm
FOUR_SCAN_40_80PX_HFARCANexists in pinned Hub75 version.
platformio.inipinsESP32-HUB75-MatrixPanel-DMAto commitf17fb7fe9d487e9643f919eb5aeedea8d9d1f8d7; this commit does not defineFOUR_SCAN_40_80PX_HFARCAN, so thesetPhysicalPanelScanRate()call will fail to compile. Use an available scan-rate enum for the pinned version, or update the pin to a version that includes it.Also comment on chain-length limits for 80x40 panels:
mxconfig.chain_lengthis clamped to 4 and reset to 1 for PSRAM-less 64-pixel boards, so the existing wording should not imply unbounded chaining.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/bus_manager.cpp` around lines 1124 - 1134, Update the TYPE_HUB75MATRIX_20S branch around setPhysicalPanelScanRate to use a scan-rate enum defined by the pinned ESP32-HUB75-MatrixPanel-DMA revision, or update the dependency pin if FOUR_SCAN_40_80PX_HFARCAN is required. Revise the nearby chain-length comment to reflect the actual mxconfig.chain_length limits, including the clamp to 4 and the PSRAM-less 64-pixel-board reset to 1, rather than implying unbounded chaining.Source: Path instructions
wled00/const.h-322-322 (1)
322-322: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winConfirm that removing
TYPE_WS2812_2CH_X3is intentional.Commenting out this definition removes LED type 20 from firmware. Saved configs that use type 20 will not resolve to a valid bus type and may become a fallback after upgrade. Add a migration path, such as mapping type 20 to an existing driver, or document it as a user-visible breaking change.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/const.h` at line 322, Restore TYPE_WS2812_2CH_X3 or add an explicit migration for saved configurations using type 20, mapping it to the appropriate existing driver before bus-type resolution. If removal is intentional, document the user-visible breaking change and ensure type 20 receives deliberate fallback handling rather than becoming invalid.Source: Coding guidelines
wled00/bus_manager.cpp-860-882 (1)
860-882: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftApply the pixel-count depth branches only to panels shorter than 64 pixels before increasing depth.
The current thresholds make a classic 64x64 panel use 8-bit and a 32x32 panel use 5-bit instead of the old height-based rules, which can be more demanding on DMA-capable SRAM. Also add
WLED_HUB75_COLOR_DEPTHexamples toplatformio_override.sample.ini, since users customizing through that file only see it inplatformio.inicomments.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/bus_manager.cpp` around lines 860 - 882, Update the ESP32/ESP32S2 automatic depth selection around mxconfig.setPixelColorDepthBits so the totalPixels thresholds apply only when mxconfig.mx_height is below 64, while preserving the existing height-based handling for panels 64 pixels or taller and the WLED_HUB75_COLOR_DEPTH override. Add representative WLED_HUB75_COLOR_DEPTH configuration examples and its purpose to platformio_override.sample.ini.Source: Path instructions
usermods/Fix_unreachable_netservices_v2/usermod_Fix_unreachable_netservices.cpp-78-81 (1)
78-81: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep the update pending when configuration persistence fails.
serializeConfigToFS()returns early if it cannot acquire the JSON buffer lock. This code clearsm_updateConfigeven when no write occurred. The changed setting can then be lost after reboot.Make
serializeConfigToFS()report success, and clearm_updateConfigonly after a successful filesystem write.The implementation in
wled00/cfg.cpplines 822-840 provides the direct evidence for the early-return path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@usermods/Fix_unreachable_netservices_v2/usermod_Fix_unreachable_netservices.cpp` around lines 78 - 81, Change serializeConfigToFS() to return whether configuration persistence completed successfully, including false on the JSON buffer lock early-return path. In the m_updateConfig handling block, clear m_updateConfig only when serializeConfigToFS() reports success; otherwise leave the update pending for a later retry.custom_hub75_80x40_icn2038p.patch-1-1 (1)
1-1: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRegenerate or remove this patch file.
This file is not a valid unified diff because all records are on Line 1.
git applycannot parse it.The embedded environment also configures one 80x40 panel, but
platformio.iniconfigures two panels as an 80x80 matrix. Regenerate the patch from the final branch state, or remove it if the checked-in source files are the delivery mechanism.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@custom_hub75_80x40_icn2038p.patch` at line 1, Regenerate or remove custom_hub75_80x40_icn2038p.patch so it is a valid unified diff with proper line breaks and headers that git apply can parse. Ensure the final platformio environment env:esp32dev_hub75_80x40_icn2038p configures exactly one 80x40 panel, matching DATA_PINS and PIXEL_COUNTS, rather than an 80x80 two-panel matrix.pio-scripts/validate_modules.py-43-50 (1)
43-50: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle a nonzero
readelfexit status.A started
readelfprocess can fail without raising an exception. The current code then parses partial output and can reject every module. Apply the same conservative pass used for launch failures whenresult.returncode != 0.Proposed fix
result = subprocess.run( [readelf_path, "--debug-dump=info", "--dwarf-depth=1", str(elf_path)], capture_output=True, text=True, errors="ignore", timeout=120, ) + if result.returncode != 0: + secho("WARNING: readelf returned an error; skipping per-module validation", + fg="yellow", err=True) + return {Path(b.build_dir).name for b in module_lib_builders} output = result.stdoutThe provided build-script context specifies that
readelffailures must conservatively confirm all modules.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pio-scripts/validate_modules.py` around lines 43 - 50, Update the readelf execution flow around subprocess.run in the module validation function to check result.returncode after the process completes; when it is nonzero, emit the existing warning and return the same conservative set of all module build-directory names used for launch failures, instead of parsing result.stdout.platformio.ini-446-463 (1)
446-463: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDisable inherited audioreactive for the recovery build.
env:esp8266_2m_minextendsenv:esp8266_2m, which setscustom_usermods = audioreactiveat Line 422. The recovery image therefore still includes that usermod despite its minimal-feature purpose.Add an empty
custom_usermodsvalue in this environment.Proposed fix
[env:esp8266_2m_min] extends = env:esp8266_2m +custom_usermods = board = esp_wroom_02Based on learnings,
custom_usermods =disables inherited in-tree usermods.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@platformio.ini` around lines 446 - 463, Update the env:esp8266_2m_min configuration by adding an empty custom_usermods setting so it overrides the inherited audioreactive usermod from env:esp8266_2m while preserving the existing recovery build flags.Source: Learnings
wled00/FX.cpp-3502-3509 (1)
3502-3509: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReturn after the single-candle fallback.
Line 3503 calls
candle(false)after allocation failure, but the originalmulti == trueinvocation then continues. The fallback allocates only the timestamp. The outer call then accessescandleDatafor all candles beyond that allocation.Proposed fix
- if (!SEGENV.allocateData(dataSize)) candle(false); //allocation failed + if (!SEGENV.allocateData(dataSize)) { + candle(false); + return; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/FX.cpp` around lines 3502 - 3509, Update the allocation-failure branch in the multi-candle path around candle(false) to return immediately after invoking the single-candle fallback. This prevents the original multi-candle execution from continuing into candleData access after only timestamp-sized storage was allocated; preserve the existing fallback behavior for successful allocation.wled00/cfg.cpp-52-52 (1)
52-52: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix the "vid" fallback so legacy configs are still detected as legacy.
viddefaults to the currentVERSIONwhendoc[F("vid")]is missing. Acfg.jsonwritten by firmware from before "vid" existed (exactly the pre-16.0 population with the legacyhour=255sunrise/sunset encoding) has no "vid" key. For that file,vidbecomes the currentVERSION(>= 2605010), sovid < 2605010at line 702 is false, and the migration is skipped.After this first load, the config gets rewritten with the current
VERSIONas "vid" (per the comment on line 52), so the migration opportunity is permanently lost: any timer that was meant to becomeTH_SUNSETstays athour=255and is treated as sunrise from then on.Default to an old/low value (for example
0) instead ofVERSIONwhen "vid" is absent, so the absence of "vid" is itself treated as "pre-migration".🐛 Proposed fix for the vid fallback
- long vid = doc[F("vid")] | VERSION; // 2605010 note: "vid" can be used to detect an update from older versions but only on first call, it is written to the new VID after buses are initialized + long vid = doc[F("vid")] | 0; // note: "vid" can be used to detect an update from older versions but only on first call, it is written to the new VID after buses are initialized. Missing "vid" means the file predates this field, so treat it as the oldest possible version.Also applies to: 698-709
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/cfg.cpp` at line 52, Update the vid fallback in the configuration-loading logic to use an old/low value such as 0 when doc[F("vid")] is absent, rather than VERSION. Preserve the existing vid < 2605010 migration check so legacy configs without a vid key are converted before the configuration is rewritten with the current version.wled00/json.cpp-1255-1284 (1)
1255-1284: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAvoid returning 0 from
respondModeDatawhile mode data remains.
writeJSONStringElementreturns 0 whenlenis too small for, "<escaped>". If the callback’s first attempted entry fails,respondModeDatareturns 0 whilefx_index < strip.getModeCount(), which signals the chunked responder to stop; ESPAsyncWebServer treats a 0 callback return as end of response, leaving the JSON array unclosed. Use a “not ready yet” state that keeps the callback returning 0 without ending the transfer, or ensure enough reservedlenbefore starting an entry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/json.cpp` around lines 1255 - 1284, The chunk callback in respondModeData must not terminate the transfer when writeJSONStringElement returns 0 because the current buffer cannot fit the next mode entry. Reserve sufficient space before attempting each entry, or introduce the chunk API’s deferred/not-ready state so the callback retries while fx_index remains below strip.getModeCount(), ensuring the JSON array is eventually closed.wled00/ota_update.cpp-296-301 (1)
296-301: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDefeat the watchdog inside gzip decompression with no watchdog API available.
The largest gzipped validation work on ESP8266 is
METADATA_OFFSET + 512 = 0x101Cbytes, which means up to 66 4-byteESP.flashReadcalls fromuzlib_gzip_parse_headeranduzlib_uncompressin the response handler. Feeding the watchdog is not possible here becausewled.cpp/wled.honly use ESP32esp_task_wdt_reset(),esp_task_wdt_init(),esp_task_wdt_add(),esp_task_wdt_delete(), andESP.wdt*, while ESP8266 still has a system watchdog. Move gzipped validation out of the response handler and back-feed the decompressed block toUpdate.write()before completing the upload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/ota_update.cpp` around lines 296 - 301, Move the validateGzippedOTA flow out of the response-handler branch around gzipDetected and into the OTA upload processing path, where decompressed data can be fed incrementally to Update.write() before the upload completes. Ensure ESP8266 gzip validation no longer performs the full flash-read/decompression work synchronously in the response handler, while preserving context->errorMessage handling and the gzip completion state transition.
🟡 Minor comments (17)
wled00/data/settings_time.htm-222-227 (1)
222-227: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the compiled max segment count when building analog options
bAO()hardcodes segment options up to 32, butMAX_NUM_SEGMENTSis platform-dependent: 16 for ESP8266, 32 for ESP32-S2 without PSRAM, and 64 with PSRAM. Either expose/pass this value to this UI or build the<select>from the currently compiled max so higher segment IDs are reachable on supported builds. The missing-field behavior is handled byrequest->arg(...).toInt()for the omitted button macro fields.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/settings_time.htm` around lines 222 - 227, Update bAO() to derive the segment-option upper bound from the currently compiled MAX_NUM_SEGMENTS value instead of hardcoding 32, exposing or passing that value into the UI as needed. Preserve the existing analog-function options and include every valid segment ID through the platform’s compiled maximum.Source: Path instructions
wled00/data/settings.htm-12-12 (1)
12-12: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe comment does not match the behaviour of
@import.The comment states that
@import url("style.css")"inlines it for this page (only costs ~30bytes of flash)".@importdoes not inline the stylesheet. The browser issues a separate request forstyle.css, exactly as a<link>element does. The 30 bytes refer only to the@importstatement itself, not to the stylesheet.
@importinside a<style>block is discovered later than a<link>in<head>, so it usually delays the stylesheet and can make the reported white button flash worse. Correct the comment, and confirm the flash is actually reduced by this change.As per path instructions: "VERIFY comments match code behavior - AI frequently generates plausible but incorrect comments."
Also applies to: 22-22
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/settings.htm` at line 12, Update the comments near the style.css inclusion to accurately describe `@import` as a separate stylesheet request, not inlining; clarify that only the import statement adds roughly 30 bytes. Verify the actual rendering behavior and confirm the change reduces the white button flash before retaining any claim that it does.Source: Path instructions
wled00/data/index.js-1599-1599 (1)
1599-1599: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe "Note" toast is still rendered as an error.
The comment states that codes 100 and higher must appear as notes. The label switches to
'Note ', but the second argument toshowToaststaystrue, which marks the toast as an error. Pass the error flag conditionally.🐛 Proposed fix
- showToast(((s.error<100) ? 'Error ': 'Note ') + s.error + ": " + errstr, true); // show "please restart" as a note, all others as errors + showToast(((s.error<100) ? 'Error ': 'Note ') + s.error + ": " + errstr, s.error<100); // show "please restart" as a note, all others as errors🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/index.js` at line 1599, Update the showToast call in the error-display path so its error flag is conditional on s.error being below 100; codes 100 and higher must pass the non-error value while preserving the existing “Error”/“Note” label selection and message text.wled00/data/settings_leds.htm-125-127 (1)
125-127: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe select branch clears the pin without moving focus.
When the conflicting control is an
INPUT, the code clears the value and callsfocus(). When it is aSELECT, the code resets the value to-1but does not callfocus(). The user sees an alert, and a pin selection is silently cleared somewhere on the page with no indication of which field changed.🐛 Proposed fix
- if (nList[j].tagName==="SELECT") nList[j].value="-1"; else { nList[j].value=""; nList[j].focus(); } + if (nList[j].tagName==="SELECT") nList[j].value="-1"; else nList[j].value=""; + nList[j].focus();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/settings_leds.htm` around lines 125 - 127, Update the conflict-handling branch in the pin validation logic so the SELECT path that resets nList[j].value to "-1" also focuses nList[j], matching the existing INPUT behavior and identifying the cleared control to the user.wled00/data/settings_leds.htm-870-883 (1)
870-883: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
sel.dataset.valcan be stale, so a select can disable its own selected option.Line 874 skips the option that
selcurrently has selected, usingsel.dataset.val.dataset.valis only written at Line 854, which runs when that specific select is passed in ase. A select created bymakePinSelectand never passed topinUpdasetherefore has an absent or staledataset.val.When that happens, the skip fails and the select's own selected option is disabled at Line 881. The value still submits, but the option renders greyed out and the user cannot re-select it after switching away.
Compare against the live
sel.valueinstead.🐛 Proposed fix
Array.from(sel.options).forEach((i) => { - if (i.value == sel.dataset.val) return; // skip sel's own selected pin + if (i.value == sel.value) return; // skip sel's own selected pin🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/settings_leds.htm` around lines 870 - 883, Update the self-selected-option check in the pin select loop around `pinUpd` to compare each option’s value with the live `sel.value` instead of `sel.dataset.val`. Preserve the existing skip behavior so a select never disables or relabels its currently selected option, including selects created by `makePinSelect` that have not updated the dataset.wled00/data/settings_leds.htm-155-155 (1)
155-155: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe alert text does not state the PSRAM condition.
The guard now allows tall multi-panel HUB75 configurations when PSRAM is present. The message still says "only single panel allowed" without any reason. A user on a PSRAM-less board cannot tell that PSRAM removes the restriction.
🐛 Proposed fix
- if (h >= 64 && p > 1 && !hasPSRAM) {alert(`HUB75 error: height >= 64, only single panel allowed`); e.stopPropagation(); return false;} + if (h >= 64 && p > 1 && !hasPSRAM) {alert(`HUB75 error: height >= 64 with multiple panels requires PSRAM. Only a single panel is allowed on this device.`); e.stopPropagation(); return false;}This is the downstream site of the
hasPSRAMload race described in the comment on Lines 42-45.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/settings_leds.htm` at line 155, Update the alert in the HUB75 validation guard to explicitly state that the single-panel restriction applies only when PSRAM is unavailable, while preserving the existing condition and control flow.wled00/data/settings.htm-23-27 (1)
23-27: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winA fixed
height: 100pxonbodycan clip the settings buttons.
body { height: 100px; }sets a fixed height on the page body. The settings landing page renders a vertical list ofdisplay: blockbuttons whose font size scales up to2rem. The rendered content exceeds 100px on every viewport. Content overflows the body box, which can clip the lower buttons or break the centering created bymargin: 0 auto.If the intent is a minimum height for the layout, use
min-height.🐛 Proposed fix
body { - height: 100px; + min-height: 100px; margin: 0 auto; max-width: 520px; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/settings.htm` around lines 23 - 27, Replace the fixed height declaration in the body rule with min-height, preserving the existing 100px value and other layout properties so the vertically stacked settings buttons can expand beyond the minimum.wled00/data/settings_pininfo.htm-19-19 (1)
19-19: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winSlow down the pin-state polling and stop it when not needed.
settings_pininfo.htmpolls/json/pinsevery 250 ms with no visibility/unload handling. Pin allocation rarely changes, while responses may allocate the shared JSON buffer and read GPIO state. Use a longer interval, such as 1000 ms, and clearpinsTimeron page hide/close to avoid unnecessary server work.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/settings_pininfo.htm` at line 19, Update the pin polling setup around fetchPinInfo and pinsTimer to use a 1000 ms interval instead of 250 ms, and add page visibility/unload handling that clears the active pinsTimer when the page is hidden or closed. Preserve the initial loadPins call and avoid leaving any polling interval running after teardown.readme.md-74-74 (1)
74-74: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd alt text to the README images.
The two UI screenshots and the Discord widget image have no
altattribute. Add concise descriptions for screen-reader users and for cases where images fail to load.Proposed fix
-<img src="/images/macbook-pro-space-gray-on-the-wooden-table.jpg" width="50%"><img src="/images/walking-with-iphone-x.jpg" width="50%"> +<img src="/images/macbook-pro-space-gray-on-the-wooden-table.jpg" alt="WLED interface on a MacBook Pro" width="50%"><img src="/images/walking-with-iphone-x.jpg" alt="WLED interface on an iPhone" width="50%"> ... -<a href="https://discord.gg/QAh7wJHrRM"><img src="https://discordapp.com/api/guilds/473448917040758787/widget.png?style=banner2" width="25%"></a> +<a href="https://discord.gg/QAh7wJHrRM"><img src="https://discordapp.com/api/guilds/473448917040758787/widget.png?style=banner2" alt="Join the WLED Discord server" width="25%"></a>Also applies to: 88-88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@readme.md` at line 74, Update the README image tags at the referenced screenshot and Discord widget locations to include concise, descriptive alt attributes, ensuring each image remains understandable to screen-reader users and when loading fails.Source: Linters/SAST tools
wled00/bus_manager.cpp-1079-1081 (1)
1079-1081: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winCheck
_ledBufferallocation before use.
allocate_buffer()can returnnullptr, and this path only emits debug output when the buffer is null. Add anif (!_ledBuffer)failure path after the allocation instead of relying on the later debug-only signal.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/bus_manager.cpp` around lines 1079 - 1081, Update the allocation block in the bus manager constructor or initialization method after calling allocate_buffer for _ledBuffer to immediately check whether _ledBuffer is null. Add a failure path that handles the allocation failure reliably in production, rather than relying on later debug-only output, while preserving the existing successful allocation flow.wled00/data/common.js-346-359 (1)
346-359: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winResolve the
addOptionconflict with settings_um.htm.
wled00/data/settings_um.htmstill definesvar addOption = addO;, and its generated usermod config usesaddOption(dd, ...). That page-local legacy version will shadowcommon.js’s helper where it loads first, so usermod select options do not get auto-selected fromdata-val. Update/remove the settings-ui backwards-compat helper or use a namespaced helper for usermod output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/data/common.js` around lines 346 - 359, Resolve the duplicate addOption definition between common.js and settings_um.htm by removing or updating the page-local addOption alias so calls from generated usermod configuration use common.js’s data-val-aware helper. Preserve backward compatibility for other settings UI callers, or switch only usermod output to a distinct helper that retains auto-selection behavior..github/workflows/release.yml-43-47 (1)
43-47: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winFail the release job if changelog generation fails.
Update release descriptionmay evaluate${{ steps.changelog.outputs.changelog }}as empty if the previous step is skipped or fails, andsoftprops/action-gh-releasereplaces the release body with the provided value. Addif: failure()/ job failure handling or make the workflow fail before this release update so the draft body is not cleared.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 43 - 47, Ensure the release job stops before Update release description when the changelog step fails or is skipped, rather than passing an empty steps.changelog.outputs.changelog value to softprops/action-gh-release. Add appropriate failure handling around the changelog generation and release-update steps so the workflow fails without clearing the draft release body..github/workflows/build.yml-80-94 (1)
80-94: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winKeep firmware artifacts uniquely named per matrix environment.
Many release environments default to
Custom, andupload-artifact@v4fails when two matrix jobs upload the same artifact name. Add a descriptivename:to the upload step, but derive the artifact name directly frommatrix.environmentor add a unique per-environment suffix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build.yml around lines 80 - 94, Update the artifact naming flow around the artifact_name step and actions/upload-artifact@v4 so every matrix job produces a unique artifact name, including environments that share the Custom release name. Derive the uploaded artifact name directly from matrix.environment or append a unique per-environment suffix, and ensure the upload step uses that consistently.Source: Path instructions
platformio_override.sample.ini-492-493 (1)
492-493: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the unsupported
custom_usermodsinheritance.
platformio_override.sample.iniuses only PlatformIO-style overrides, but${env:esp01_1m_full.custom_usermods}relies on a customenv:key supported only byplatformio.ini/platformio_override.ini. With PlatformIO resolution, this leavesMY9291as the effectivecustom_usermodsvalue instead of inheriting the base usermods. Keep${env:esp01_1m_full.custom_usermods}inplatformio.iniand useMY9291here.Proposed fix
-custom_usermods = ${env:esp01_1m_full.custom_usermods} MY9291 +custom_usermods = MY9291🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@platformio_override.sample.ini` around lines 492 - 493, Update the custom_usermods setting for the MY9291 environment to use only MY9291, removing the unsupported ${env:esp01_1m_full.custom_usermods} inheritance; keep the inheritance expression in platformio.ini.wled00/udp.cpp-807-808 (1)
807-808: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not emit DDP sequence number zero.
The counter resets to zero after 15, and Line 808 then transmits zero. Wrap to 1 instead. Receivers can treat zero as unused, which weakens duplicate and out-of-order detection.
I can prepare the counter change and a packet-level test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/udp.cpp` around lines 807 - 808, Update the sequence-number emission in the DDP send path around sequenceNumber so wrapping after 15 produces 1 instead of transmitting 0. Preserve the existing 1–15 range and ensure the value written by ddpUdp.write never equals zero.wled00/json.cpp-1214-1238 (1)
1214-1238: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAvoid depending on ArduinoJson's internal
EscapeSequenceclass.
wled00/src/dependencies/json/ArduinoJson-v6.hexposesARDUINOJSON_NAMESPACE::EscapeSequence::escapeChar, but WLED includes its own copy of ArduinoJson, so future library updates are not controlled by the repository. Implement the small JSON string-escaping rules locally instead of depending on this library-internal symbol.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/json.cpp` around lines 1214 - 1238, Update writeJSONString to replace the ArduinoJson EscapeSequence::escapeChar dependency with local JSON escaping logic for supported escapes, including quotes, backslashes, and control characters. Preserve the existing buffer bounds checks, surrounding quotes, and return behavior while removing reliance on ArduinoJson internals.wled00/ota_update.h-37-47 (1)
37-47: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMove the documentation block so it attaches to the function.
The
@param/@returnblock sits directly aboveenum class OTAResultStatus. Doxygen attaches it to the enum, not togetOTAResult. Place the block immediately above the function declaration.📝 Proposed fix for the documentation placement
-/** - * Retrieve the OTA result. - * `@param` request Pointer to web request object - * `@return` OTAResultStatus indicating result state; string with error message if the update failed. - */ enum class OTAResultStatus { TryAgain, // caller must deferResponse() and retry - need additional resources (JSON lock) to complete validation Replied, // response already sent; no action needed Ready, // result available; send response based on error string }; + +/** + * Retrieve the OTA result. + * `@param` request Pointer to web request object + * `@return` OTAResultStatus indicating result state; string with error message if the update failed. + */ std::pair<OTAResultStatus, String> getOTAResult(AsyncWebServerRequest *request);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wled00/ota_update.h` around lines 37 - 47, Move the existing Doxygen block from above enum class OTAResultStatus to immediately above getOTAResult, so its `@param` and `@return` documentation attaches to the function declaration; leave the enum and function signatures unchanged.
| void BusHub75Matrix::show(void) { | ||
| if (!_valid) return; | ||
| if (_ledBuffer) { | ||
| // write out buffered LEDs | ||
| unsigned height = _isVirtual ? virtualDisp->height() : display->height(); | ||
| unsigned width = _panelWidth; | ||
|
|
||
| //while(!previousBufferFree) delay(1); // experimental - Wait before we allow any writing to the buffer. Stop flicker. | ||
| size_t pix = 0; // running pixel index | ||
| for (int y=0; y<height; y++) for (int x=0; x<width; x++) { | ||
| if (getBitFromArray(_ledsDirty, pix) == true) { // only repaint the "dirty" pixels | ||
| CRGB c = _ledBuffer[pix]; | ||
| //c.nscale8_video(_bri); // apply brightness | ||
| if (_isVirtual) virtualDisp->drawPixelRGB888(int16_t(x), int16_t(y), c.r, c.g, c.b); | ||
| else display->drawPixelRGB888(int16_t(x), int16_t(y), c.r, c.g, c.b); | ||
| } | ||
| pix++; | ||
| if (!_valid || !_ledBuffer) return; | ||
|
|
||
| const unsigned width = _panelWidth; | ||
| const unsigned height = _isVirtual ? virtualDisp->height() : display->height(); | ||
| const size_t expectedLen = size_t(width) * height; | ||
| if (width == 0 || expectedLen != _len) { | ||
| DEBUGBUS_PRINTF("HUB75 framebuffer geometry mismatch: %ux%u=%u, buffer=%u\n", width, height, expectedLen, _len); | ||
| Serial.printf("HUB75 framebuffer geometry mismatch: %ux%u=%u, buffer=%u\n", width, height, expectedLen, _len); | ||
| return; | ||
| } | ||
|
|
||
| const bool logFrame = _debugShowFrames < 3; | ||
| if (logFrame) { | ||
| DEBUGBUS_PRINTLN("show() called"); | ||
| Serial.println("show() called"); | ||
| DEBUGBUS_PRINTF("pixel count: %u\n", _len); | ||
| Serial.printf("pixel count: %u\n", _len); | ||
| DEBUGBUS_PRINTLN("first 20 pixel RGB values:"); | ||
| Serial.println("first 20 pixel RGB values:"); | ||
| } | ||
|
|
||
| size_t pixelsRendered = 0; | ||
| for (size_t pix = 0; pix < _len; ++pix) { | ||
| const unsigned x = pix % width; | ||
| const unsigned y = pix / width; | ||
| const CRGB c = _ledBuffer[pix]; | ||
|
|
||
| if (_isVirtual) virtualDisp->drawPixelRGB888(int16_t(x), int16_t(y), c.r, c.g, c.b); | ||
| else display->drawPixelRGB888(int16_t(x), int16_t(y), c.r, c.g, c.b); | ||
|
|
||
| pixelsRendered++; | ||
|
|
||
| if (logFrame && pix < 20) { | ||
| DEBUGBUS_PRINTF("pixel %u: x=%u y=%u rgb(%u,%u,%u)\n", pix, x, y, c.r, c.g, c.b); | ||
| Serial.printf("pixel %u: x=%u y=%u rgb(%u,%u,%u)\n", pix, x, y, c.r, c.g, c.b); | ||
| } | ||
| setBitArray(_ledsDirty, _len, false); // buffer shown - reset all dirty bits | ||
| } | ||
| } | ||
|
|
||
| setBitArray(_ledsDirty, _len, false); | ||
| if (logFrame) { | ||
| DEBUGBUS_PRINTF("pixels rendered: %u\n", pixelsRendered); | ||
| Serial.printf("pixels rendered: %u\n", pixelsRendered); | ||
| DEBUGBUS_PRINTLN("frame complete"); | ||
| Serial.println("frame complete"); | ||
| _debugShowFrames++; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Remove the raw Serial debug output from show().
Lines 1208, 1215, 1217, 1219, 1235, 1242 and 1244 call Serial.printf() and Serial.println() directly. These calls are not guarded by a debug macro, so they run in release firmware. Three consequences:
show()is on the LED output hot path. Blocking UART writes there stall the render loop.- The geometry-mismatch branch on Line 1208 prints on every frame while the mismatch persists.
- On builds that use the serial port for other protocols, this output corrupts the stream.
Use DEBUGBUS_PRINTF_P()/DEBUGBUS_PRINTLN() only, and remove the _debugShowFrames counter with the per-pixel dump.
As per coding guidelines: "use ... DEBUG_PRINTF()/DEBUG_PRINTLN() for debug output" and "Remove dead or unused code ... debug artifacts".
🐛 Proposed fix
if (width == 0 || expectedLen != _len) {
DEBUGBUS_PRINTF("HUB75 framebuffer geometry mismatch: %ux%u=%u, buffer=%u\n", width, height, expectedLen, _len);
- Serial.printf("HUB75 framebuffer geometry mismatch: %ux%u=%u, buffer=%u\n", width, height, expectedLen, _len);
return;
}
- const bool logFrame = _debugShowFrames < 3;
- if (logFrame) {
- DEBUGBUS_PRINTLN("show() called");
- Serial.println("show() called");
- DEBUGBUS_PRINTF("pixel count: %u\n", _len);
- Serial.printf("pixel count: %u\n", _len);
- DEBUGBUS_PRINTLN("first 20 pixel RGB values:");
- Serial.println("first 20 pixel RGB values:");
- }
-
- size_t pixelsRendered = 0;
for (size_t pix = 0; pix < _len; ++pix) {
const unsigned x = pix % width;
const unsigned y = pix / width;
const CRGB c = _ledBuffer[pix];
if (_isVirtual) virtualDisp->drawPixelRGB888(int16_t(x), int16_t(y), c.r, c.g, c.b);
else display->drawPixelRGB888(int16_t(x), int16_t(y), c.r, c.g, c.b);
-
- pixelsRendered++;
-
- if (logFrame && pix < 20) {
- DEBUGBUS_PRINTF("pixel %u: x=%u y=%u rgb(%u,%u,%u)\n", pix, x, y, c.r, c.g, c.b);
- Serial.printf("pixel %u: x=%u y=%u rgb(%u,%u,%u)\n", pix, x, y, c.r, c.g, c.b);
- }
}
setBitArray(_ledsDirty, _len, false);
- if (logFrame) {
- DEBUGBUS_PRINTF("pixels rendered: %u\n", pixelsRendered);
- Serial.printf("pixels rendered: %u\n", pixelsRendered);
- DEBUGBUS_PRINTLN("frame complete");
- Serial.println("frame complete");
- _debugShowFrames++;
- }
}Also remove the _debugShowFrames member declaration from wled00/bus_manager.h.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wled00/bus_manager.cpp` around lines 1200 - 1247, Update
BusHub75Matrix::show() to remove all direct Serial.printf/Serial.println calls,
using only the guarded DEBUGBUS_PRINTF_P()/DEBUGBUS_PRINTLN() debug macros for
diagnostics. Remove the _debugShowFrames counter and its per-frame/per-pixel
logging logic, including the member declaration in BusHub75Matrix’s header,
while preserving framebuffer validation and rendering behavior.
Source: Coding guidelines
| if (isM) { | ||
| // do we have 1D segment *after* the matrix? | ||
| if (start >= mw*mh) { | ||
| if (start >= mw*mh && s > 0) { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
The added s > 0 condition can cause a null dereference for segment 0.
populateSegments decides whether to render the Y inputs from isMSeg = isM && staX<mw*mh (Line 783). That expression has no segment-index condition. So for segment 0 with start >= mw*mh, seg0sY and seg0eY are not rendered.
updateLen now takes the else branch for that same segment because of && s > 0. The else branch reads parseInt(sY.value) at Line 1219 with no null guard. sY is null, so updateLen(0) throws a TypeError and the segment list stops updating.
Line 1259 still uses the original isM && start >= mw*mh condition, which confirms the two code paths now disagree.
Align the branch condition with isMSeg, or add the same s > 0 condition to isMSeg in populateSegments.
🐛 Proposed fix (align `updateLen` with `isMSeg`)
- if (start >= mw*mh && s > 0) {
+ if (start >= mw*mh) {If the intent is that segment 0 must always be a matrix segment, apply s > 0 to isMSeg in populateSegments and to Line 1259 as well.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (start >= mw*mh && s > 0) { | |
| if (start >= mw*mh) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wled00/data/index.js` at line 1206, Align the condition in updateLen with the
isMSeg calculation in populateSegments so segment 0 follows the same
matrix-segment branch when start >= mw*mh, preventing access to missing sY and
eY inputs. Update the condition around updateLen and keep the related logic at
the later start check consistent, or apply s > 0 consistently to isMSeg and that
later check if segment 0 is intentionally excluded.
| // E1.31 packet identifier (ACN_ID = "ASC-E1.17"), need at least 16 bytes to safely read acn_id (offset 4, length 12). | ||
| if (pktLen >= 16) { | ||
| if (!memcmp(sbuff->acn_id, ESPAsyncE131::ACN_ID, sizeof(sbuff->acn_id))) | ||
| protocol = P_E131; | ||
| } | ||
|
|
||
| if (protocol == P_ARTNET) { | ||
| if (memcmp(sbuff->art_id, ESPAsyncE131::ART_ID, sizeof(sbuff->art_id))) | ||
| error = true; //not "Art-Net" | ||
| if (sbuff->art_opcode != ARTNET_OPCODE_OPDMX && sbuff->art_opcode != ARTNET_OPCODE_OPPOLL) | ||
| error = true; //not a DMX or poll packet | ||
| } else { //E1.31 error handling | ||
| if (htonl(sbuff->root_vector) != ESPAsyncE131::VECTOR_ROOT) | ||
| error = true; | ||
| if (htonl(sbuff->frame_vector) != ESPAsyncE131::VECTOR_FRAME) | ||
| error = true; | ||
| if (sbuff->dmp_vector != ESPAsyncE131::VECTOR_DMP) | ||
| error = true; | ||
| if (sbuff->property_values[0] != 0) | ||
| error = true; | ||
| } | ||
|
|
||
| if (pktLen < 10) { | ||
| error = true; // Need at least Art-Net ID (8) + opcode (2) | ||
| } else { | ||
| if (memcmp(sbuff->art_id, ESPAsyncE131::ART_ID, sizeof(sbuff->art_id))) | ||
| error = true; //not "Art-Net" | ||
| if (sbuff->art_opcode != ARTNET_OPCODE_OPDMX && sbuff->art_opcode != ARTNET_OPCODE_OPPOLL) | ||
| error = true; //not a DMX or poll packet | ||
| } | ||
| } else { //E1.31 error handling | ||
| if (htonl(sbuff->root_vector) != ESPAsyncE131::VECTOR_ROOT) | ||
| error = true; | ||
| if (htonl(sbuff->frame_vector) != ESPAsyncE131::VECTOR_FRAME) | ||
| error = true; | ||
| if (sbuff->dmp_vector != ESPAsyncE131::VECTOR_DMP) | ||
| error = true; | ||
| if (sbuff->property_values[0] != 0) | ||
| error = true; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Add a length guard for the E1.31 branch. The current guard covers only acn_id.
The check at line 108 guarantees only 16 bytes. The E1.31 branch then reads far past that point in the packed e131_packet_t layout:
root_vectorat offset 18frame_vectorat offset 40dmp_vectorat offset 117property_values[0]at offset 125
A 16-byte UDP packet that starts with the ACN identifier selects the E1.31 branch and reads up to offset 125. That is an out-of-bounds read of the AsyncUDP receive buffer. Any host on the network can send such a packet.
Require the full E1.31 header before you classify the packet as E1.31.
🛡️ Proposed fix to guard the E1.31 header size
+// Minimum bytes needed to read every E1.31 field checked below,
+// through property_values[0] in the DMP layer.
+constexpr size_t E131_MIN_HEADER_LEN = offsetof(e131_packet_t, property_values) + 1;
+
void ESPAsyncE131::parsePacket(AsyncUDPPacket _packet) {
bool error = false;
uint8_t protocol = P_ARTNET;
const size_t pktLen = _packet.length();
e131_packet_t *sbuff = reinterpret_cast<e131_packet_t *>(_packet.data());
- // E1.31 packet identifier (ACN_ID = "ASC-E1.17"), need at least 16 bytes to safely read acn_id (offset 4, length 12).
- if (pktLen >= 16) {
+ // E1.31 packet identifier (ACN_ID = "ASC-E1.17"). Require the whole header,
+ // because the E1.31 branch below reads fields up to property_values[0].
+ if (pktLen >= E131_MIN_HEADER_LEN) {
if (!memcmp(sbuff->acn_id, ESPAsyncE131::ACN_ID, sizeof(sbuff->acn_id)))
protocol = P_E131;
}Note: offsetof on this anonymous packed struct member may need a different expression. If it does not compile, use an explicit constant with a comment that documents the layout.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // E1.31 packet identifier (ACN_ID = "ASC-E1.17"), need at least 16 bytes to safely read acn_id (offset 4, length 12). | |
| if (pktLen >= 16) { | |
| if (!memcmp(sbuff->acn_id, ESPAsyncE131::ACN_ID, sizeof(sbuff->acn_id))) | |
| protocol = P_E131; | |
| } | |
| if (protocol == P_ARTNET) { | |
| if (memcmp(sbuff->art_id, ESPAsyncE131::ART_ID, sizeof(sbuff->art_id))) | |
| error = true; //not "Art-Net" | |
| if (sbuff->art_opcode != ARTNET_OPCODE_OPDMX && sbuff->art_opcode != ARTNET_OPCODE_OPPOLL) | |
| error = true; //not a DMX or poll packet | |
| } else { //E1.31 error handling | |
| if (htonl(sbuff->root_vector) != ESPAsyncE131::VECTOR_ROOT) | |
| error = true; | |
| if (htonl(sbuff->frame_vector) != ESPAsyncE131::VECTOR_FRAME) | |
| error = true; | |
| if (sbuff->dmp_vector != ESPAsyncE131::VECTOR_DMP) | |
| error = true; | |
| if (sbuff->property_values[0] != 0) | |
| error = true; | |
| } | |
| if (pktLen < 10) { | |
| error = true; // Need at least Art-Net ID (8) + opcode (2) | |
| } else { | |
| if (memcmp(sbuff->art_id, ESPAsyncE131::ART_ID, sizeof(sbuff->art_id))) | |
| error = true; //not "Art-Net" | |
| if (sbuff->art_opcode != ARTNET_OPCODE_OPDMX && sbuff->art_opcode != ARTNET_OPCODE_OPPOLL) | |
| error = true; //not a DMX or poll packet | |
| } | |
| } else { //E1.31 error handling | |
| if (htonl(sbuff->root_vector) != ESPAsyncE131::VECTOR_ROOT) | |
| error = true; | |
| if (htonl(sbuff->frame_vector) != ESPAsyncE131::VECTOR_FRAME) | |
| error = true; | |
| if (sbuff->dmp_vector != ESPAsyncE131::VECTOR_DMP) | |
| error = true; | |
| if (sbuff->property_values[0] != 0) | |
| error = true; | |
| } | |
| constexpr size_t E131_MIN_HEADER_LEN = offsetof(e131_packet_t, property_values) + 1; | |
| // E1.31 packet identifier (ACN_ID = "ASC-E1.17"). Require the whole header, | |
| // because the E1.31 branch below reads fields up to property_values[0]. | |
| if (pktLen >= E131_MIN_HEADER_LEN) { | |
| if (!memcmp(sbuff->acn_id, ESPAsyncE131::ACN_ID, sizeof(sbuff->acn_id))) | |
| protocol = P_E131; | |
| } | |
| if (protocol == P_ARTNET) { | |
| if (pktLen < 10) { | |
| error = true; // Need at least Art-Net ID (8) + opcode (2) | |
| } else { | |
| if (memcmp(sbuff->art_id, ESPAsyncE131::ART_ID, sizeof(sbuff->art_id))) | |
| error = true; //not "Art-Net" | |
| if (sbuff->art_opcode != ARTNET_OPCODE_OPDMX && sbuff->art_opcode != ARTNET_OPCODE_OPPOLL) | |
| error = true; //not a DMX or poll packet | |
| } | |
| } else { //E1.31 error handling | |
| if (htonl(sbuff->root_vector) != ESPAsyncE131::VECTOR_ROOT) | |
| error = true; | |
| if (htonl(sbuff->frame_vector) != ESPAsyncE131::VECTOR_FRAME) | |
| error = true; | |
| if (sbuff->dmp_vector != ESPAsyncE131::VECTOR_DMP) | |
| error = true; | |
| if (sbuff->property_values[0] != 0) | |
| error = true; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wled00/src/dependencies/e131/ESPAsyncE131.cpp` around lines 107 - 131,
Require the complete E1.31 header length before assigning P_E131 in the ACN_ID
detection block, rather than only checking the 16-byte identifier. Use the
appropriate packed-layout size or offsetof expression covering
property_values[0] (with a documented constant fallback if needed), so the
subsequent root_vector, frame_vector, dmp_vector, and property_values accesses
in the E1.31 validation branch are safe.
| // AI: below section was generated by an AI | ||
| // WLED_HUB75_COLOR_DEPTH: compile-time override for DMA colour depth (bits per colour channel). | ||
| // Lower values free DMA-capable SRAM at the cost of colour fidelity (banding). | ||
| // Typical safe values: 3 (saves ~25% SRAM vs 4-bit), 4 (default for large panels). |
There was a problem hiding this comment.
The "typical safe value" is not good. 3 bits per color means that almost any effect will show horrible color banding. If you have too many pixels, use -S3 with PSRAM.
| #else | ||
| // AI: end | ||
| unsigned totalPixels = mxconfig.chain_length * mxconfig.mx_width * mxconfig.mx_height; | ||
| if (totalPixels >= 2048) mxconfig.setPixelColorDepthBits(4); // 4-bit depth saves DMA DRAM for 3200 pixel panels (80x40 1/20 scan) |
There was a problem hiding this comment.
This is totally unnecessary.
| } | ||
| } else if (bc.type == TYPE_HUB75MATRIX_20S) { | ||
| // AI: below section was generated by an AI | ||
| // Only validate panel dimensions; chain_length is not constrained by the ICN2038P hardware. |
There was a problem hiding this comment.
Chain_length is constrained because it influences RAM usage. And because more than 12000 pixels are becoming too slow and too flickering.
softhack007
added
the
AI
|
if this was meant as a serious PR, please do it properly https://github.com/wled/WLED/wiki/How-to-properly-submit-a-PR |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation