Add HUB75 support #3777
Add HUB75 support #3777
Conversation
|
(I'm not a WLED maintainer. My comments mean nothing.) Wow! I'm shocked at how cleanly that slots in. Nice. I have both a Matrix S3 and a Mesmerizer (nightdriverled.com) so I may try spinning a build. What kind of frame rates are you seeing on the 2D animations? (We have a lot of the same effects in NightDriver and I've flirted with using the Faptastic HUB75 layer, too...) Why are ESP32_FORUM_PINOUT, ARDUINO_ADAFRUIT_MATRIXPORTAL_ESP32S3, etc. compile-time flags (and not defined in a derivation of esp32dev_hub75) and not runtme-selectable flags like APA102 vs. WS2814 vs WS2812, etc.? Given how differently -S3 and -Nothing have to be built, can you really rely on board = esp32dev? Or is this a "draw the rest of the owl" that needs to have another custom_platform_mps3.ini written that derives from an appropriate S3 with the 8MB flash, 2MB PSRAM combination? (https://github.com/platformio/platform-espressif32/blob/master/boards/adafruit_matrixportal_esp32s3.json ?) Do you need help with that, perhaps? Would this be likely to plan nicely with the Virtual Display (chained panels - limited only by color depth and refresh rates) and UDP/DDP inputs and such? You can see I'm pretty excited to see this as I've traveled a lot of similar paths. I'm a bit engaged in the short term, but may be able to help land this if you need extra eyes and fingers on it. |
platformio.ini
Outdated
| lib_deps = ${esp32s2.lib_deps} | ||
|
|
||
|
|
||
| [env:esp32dev_hub75] |
There was a problem hiding this comment.
I will require this to be moved into sample file. I want platformio.ini clean and only used for default official builds.
|
IMO, I'm OK with custom boards requiring source changes. Nobody wants a
screen to manually configure a dozen plus pins.
I Do think it's preferable to be able to choose between a trinity and a
matrixs3 from a menu option, just as we choose between WS2812 and SK6218
from a single binary build. So I'd vote for the #ifdefss to ve moved to a
runtime selection but I'm perfectly happy to treat the collection of pins
like a molecule (we COULD split it, but let's not) instead of an atom (with
16 different runtime choices). The level of granularity I'd like to see is
menu option for a board with the fallback that if cracked open a soldering
iron (OK, breadboard) and wired one up yourself that you're capable editing
source and rebuilding.
Also, if you have PSRAM (and what new ESP32 doesn't these days?) are you
really limited to one panel? The ESP32-S3s always have octal PSRAM and can
DMA to and from it, lifting two major limitations from the ESP32-Nothings
where PSRAM could be too slow to feep the panels and still have goofy rules
where you effectively needed bounce buffers (hello ISA network cards in
system with > 16MB of memory where you had to allocate in 'low memory', DMA
to/from, and then copy to the real destination). Faptastic (Mr. Fap?) has
some writeups on that and the ESP32-Nothings were pretty terrible for
multi-panel use where the -S3's are less terrible.
Then again, if we say a single panel is all we want to mess with, this is
still a super cool develoment!
…On Tue, Feb 27, 2024 at 4:44 AM netmindz ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In wled00/bus_manager.cpp
<#3777 (comment)>:
> + mxconfig.gpio.b = 19;
+ mxconfig.gpio.c = 5;
+ mxconfig.gpio.d = 17;
+ mxconfig.gpio.e = 18;
+
+#endif
+
+
+ DEBUG_PRINTF("MatrixPanel_I2S_DMA config - %ux%u length: %u\n", mxconfig.mx_width, mxconfig.mx_height, mxconfig.chain_length);
+
+ // OK, now we can create our matrix object
+ display = new MatrixPanel_I2S_DMA(mxconfig);
+
+ this->_len = (display->width() * display->height());
+
+ pinManager.allocatePin(mxconfig.gpio.r1, true, PinOwner::HUB75);
Yeah, now I've swapped to this library for the implementation, it supports
config of pins by array so I can flip to that and then use the multi call
—
Reply to this email directly, view it on GitHub
<#3777 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACCSD3YZGDB7QMJWNR6U5ALYVW2HZAVCNFSM6AAAAABD2ZGHFOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMBTGE2TQOJQHA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
PSRAM is slow. Too slow for anything LED related. |
|
That was true on ESP32-Nothing.
All the ESP32-S3's using the ESP-WROOM modules (breaking their own naming
convention. Again.) like the N8R2 and N16R8 are populated with Octal
PSRAM. (Boards doing their own thing like LUATOS or some of the
Sparkfun/Adafruit class boards will often use QSPI and, well, they are what
they are.) 120Mhz configurations are "soon" with existing hardware
according to
https://docs.espressif.com/projects/esp-idf/en/stable/esp32s3/api-guides/flash_psram_config.html#all-supported-modes-and-speeds
, but 80Mhz is here now and is the norm on $5 boards.
Waiting for the bus arbiter to swing has a cost, so you don't want to
traverse a linked list to another linked list in psram if you can help it,
but block copies like a frame buffer work really well and you really can
get about 80-90MB/sec in the real world doing a single-directional block
move (not a RAM->RAM copy, but shoving it out a DMAC) (DDR * 1b/xfer *
leading + trailing edge would say 160, but I've not seen that.)
It's not as awesome as having LPDDR5 or something and it's admittedly a
house of cards to hit those rates, but it's less terrible than it used to
be. We know that HUB75 can burn bandwidth at high frame rates and color
depths, so there are still some tradeoffs, but don't dismiss the idea based
on being burned by the ESP32-Nothings.
I think that even at QSPI (like on the LUATOS boards I have) that the
wheels come off pretty easily...
…On Tue, Feb 27, 2024 at 5:04 AM Blaž Kristan ***@***.***> wrote:
Also, if you have PSRAM (and what new ESP32 doesn't these days?) are you
really limited to one panel?
PSRAM is slow. Too slow for anything LED related.
—
Reply to this email directly, view it on GitHub
<#3777 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACCSD32TRKTCZTJ52BLX7L3YVW4TJAVCNFSM6AAAAABD2ZGHFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRWGMYDGOJYHA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
I'm having real issues with wifi with the Matrix S3, so will be interesting to see if you see the same.
If you are using the matrix portal, you can't change the pins, so no point asking the user to enter 14 pins. For other boards, just lazy ;)
The example env just shows what options you need to enable this feature. If you aren't using a regular esp32 then you need to add the extra options to the env for your board
In theory yes you could use DDP then Hub75 output or mix of Hub75 and other output types
Great |
You are not limited to single panels. At the moment it will allow you to select up to 4 panels of 32x32 in horizontal configuration. We can add other layouts later
The library being used does allow PSRAM use |
|
Rats. The email gateway ate my response: I finally tried to check this tree out and build it for MatrixS3. I may fumble with WLED and GitHub, but I'm not an inexperienced dev, so please stick with me. $ gh repo clone netmindz/WLED hub75-wled $ pio run -e adafruit_matrixportal_s3_hub75 npm run build
node:internal/modules/cjs/loader:1146 Error: Cannot find module 'inliner'
I don't do Node, so I'm stumped on that one. (I have built WLED directly for another S3 board from teh main repo about a month ago...) If I restart it, it starts to chow down on the CPP code: I agree. There is an assert.h there and it's trying to include an <sdkconfig.h>, but none is in the system header path. If I look at all the sdkconfig.h's on my entire system, I think the pattern is "projects built with idf.py, not platformio", but I'm not certain of that pattern. If I change strategies to: and the C++ compilles until it gets to But that same assert.h is still trying to include a sdkconfig.h that still doesn't exist, so there must be something else going on. I suspect we're getting the wrong assert.h I see adafruit_matrixportal_esp32s3.json in .platformio/platforms/espressif32/boards so I don't think this board is too new to be supported by platformio, but it would be totally on brand for them to not print "something not found" and just blunder on with the wrong one if they did. (My relationship with PlatformIO is strained.) OTOH, it's clearly finding at JSON because it's expanding out the full name: So I guess this is a three part question:
As a sanity check $pio run -e esp32dev_hub75 has the same issue with html_ui.h and with node, but does find a happy assert.h. So I think I just have startup blues. Should this be expected to build from this basic recipe? Is this perhaps divergence from the two wled trees? Thanx for the hand. |
|
The simple option @robertlipe would just be to use this ready-made bin https://github.com/netmindz/WLED/actions/runs/8056668690/artifacts/1276931005 |
|
If you are trying to build from source, you need to run
To have the right modules installed needed for |
|
Thanx.
npm install helped get through the first issue. The actual cpp code still
fails.
<command-line>: warning: "ARDUINO_USB_CDC_ON_BOOT" redefined
<command-line>: note: this is the location of the previous definition
<command-line>: warning: "ARDUINO_USB_CDC_ON_BOOT" redefined
<command-line>: note: this is the location of the previous definition
Compiling
.pio/build/adafruit_matrixportal_s3_hub75/lib7c9/FastLED/hsv2rgb.cpp.o
In file included from
~/.platformio/packages/toolchain-xtensa-esp32s3/xtensa-esp32s3-elf/sys-include/sys/reent.h:503,
from
~.platformio/packages/framework-arduinoespressif32/tools/sdk/esp32s3/include/newlib/platform_include/sys/reent.h:17,
from
~/.platformio/packages/toolchain-xtensa-esp32s3/xtensa-esp32s3-elf/sys-include/stdio.h:60,
from
~/.platformio/packages/framework-arduinoespressif32/cores/esp32/esp32-hal.h:25,
from
.pio/libdeps/adafruit_matrixportal_s3_hub75/FastLED/src/platforms/esp/32/led_sysdefs_esp32.h:2,
from
.pio/libdeps/adafruit_matrixportal_s3_hub75/FastLED/src/led_sysdefs.h:42,
from
.pio/libdeps/adafruit_matrixportal_s3_hub75/FastLED/src/FastLED.h:51,
from
.pio/libdeps/adafruit_matrixportal_s3_hub75/FastLED/src/FastLED.cpp:2:
~/.platformio/packages/framework-arduinoespressif32/tools/sdk/esp32s3/include/newlib/platform_include/assert.h:20:10:
fatal error: sdkconfig.h: No such file or directory
Does pio run -e adafruit_matrixportal_s3_hub75 work for you?
pio run -e esp32dev_hub75 runs to completion.
It's definitely the board = adafruit_matrixportal_esp32s3 line that's
croaking it. Changing that to match the other makes it happy, so there's
something about this board configuratino tha tmakes platformio grumpy,
though it's clearly recognizing it.
Will test the prebuilt binary soon...
…On Mon, Mar 4, 2024 at 6:20 PM netmindz ***@***.***> wrote:
If you are trying to build from source, you need to run
npm install
—
Reply to this email directly, view it on GitHub
<#3777 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACCSD3YRLQ5OLGPUJE25NNLYWUFWVAVCNFSM6AAAAABD2ZGHFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZXG4YTQMRQGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@robertlipe - I suspect the wrong version of the platform is being used. We have the option to build on V4, which is what I use for the MoonModules#108 |
|
Thanks @netmindz. I also have Adafruit MatrixPortal S3 board. I will try this. |
Should I use the following command to flash Adafruit MatrixPortal S3 board?
|
Not sure about that, I just tend to goto the manual ota page of an existing WLED install and upload the bin |
Which firmware file did you use for WLED install before going to the manual ota page and upload your HUB75 version?
|
Ah, for the matrix3 I didn't actually try a regular install, just went directly to building from source. Does the web installer give a basic working setup? https://install.wled.me/ |
|
@netmindz |
|
Any news about this PR? I have 5 pcs led matrix board |
|
There is support for hub75 in the latest MoonModules builds, just use V4_S or V4_M builds As for support in AirCookie, @blazoncek said he would be updating the PR to address an issue he thought was a blocker to accpeting |
blazoncek
added
enhancement
keep
|
@netmindz I've tried to come up with a better solution regarding conditional UI elements for setting options for Hub panels. I can see 2 ways forward as a result of this: a) make Hub 75 panels regular option and not a conditional compile or b) not support Hub 75 at all For a) that would probably increase code size which most users would rather see used by other usermods instead (current flash utilisation is already above 95% while we are still with ESP32 core 3.5.0, newer cores add substantial amount to this). There is of course still possibility to keep PR as is and hide Hub panels in 99.9% of installations at the expense of few wasted bytes for never used UI elements. I cannot decide either way so I'll leave the decision to @Aircoookie. I have marked the PR with For anyone else interested please upvote this PR so we can see how much interest is there and decide upon it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
pio-scripts/load_usermods.py (1)
80-103: Fix indentation issues in the refactored code.The refactoring logic is sound - collecting WLED modules, validating
lib_archivesettings, and storing them inWLED_MODULESfor validation. However, there are indentation inconsistencies that need to be addressed.Apply this diff to fix the indentation (should be 4 spaces):
- wled_deps = [dep for dep in result.depbuilders if is_wled_module(dep)] + wled_deps = [dep for dep in result.depbuilders if is_wled_module(dep)] broken_usermods = [] - for dep in wled_deps: - # Add the wled folder to the include path - dep.env.PrependUnique(CPPPATH=str(wled_dir)) - # Add WLED's own dependencies - for dir in extra_include_dirs: - dep.env.PrependUnique(CPPPATH=str(dir)) - # Enforce that libArchive is not set; we must link them directly to the executable - if dep.lib_archive: - broken_usermods.append(dep) + for dep in wled_deps: + # Add the wled folder to the include path + dep.env.PrependUnique(CPPPATH=str(wled_dir)) + # Add WLED's own dependencies + for dir in extra_include_dirs: + dep.env.PrependUnique(CPPPATH=str(dir)) + # Enforce that libArchive is not set; we must link them directly to the executable + if dep.lib_archive: + broken_usermods.append(dep) - # Save the depbuilders list for later validation - xenv.Replace(WLED_MODULES=wled_deps) + # Save the depbuilders list for later validation + xenv.Replace(WLED_MODULES=wled_deps)
♻️ Duplicate comments (7)
wled00/bus_manager.cpp (7)
28-35: Add boundary validation for array access.The
getBitFromArrayfunction lacks validation that could lead to out-of-bounds access ifpositionexceeds the array size.Consider adding a boundary check or pass the array size as a parameter:
-bool getBitFromArray(const uint8_t* byteArray, size_t position) { +bool getBitFromArray(const uint8_t* byteArray, size_t position, size_t maxBits = SIZE_MAX) { + if (!byteArray || (maxBits != SIZE_MAX && position >= maxBits)) return false; size_t byteIndex = position / 8;
39-47: Add boundary validation for setBitInArray.Similar to
getBitFromArray, this function needs boundary checks to prevent buffer overflows.Add validation before array access:
void setBitInArray(uint8_t* byteArray, size_t position, bool value) { + if (!byteArray) return; + // Add boundary check if array size is known size_t byteIndex = position / 8;
53-58: Add parameter validation for setBitArray.The function validates
byteArraybut could benefit from additional safety checks.The null check is good, but consider adding validation for the array size calculation:
void setBitArray(uint8_t* byteArray, size_t numBits, bool value) { if (byteArray == nullptr) return; + if (numBits == 0) return; size_t len = getBitArrayBytes(numBits);
785-832: Fix initialization order and dereference of uninitialized pointer.The code has a critical issue where
virtualDispis constructed using(*display)beforedisplayis initialized, causing undefined behavior.Reorder the initialization to create
displayfirst:+ // Create display first + display = new MatrixPanel_I2S_DMA(mxconfig); + if (!display) { + DEBUGBUS_PRINTLN("DMA allocation failed"); + return; + } + if (bc.type == TYPE_HUB75MATRIX_HS) { - virtualDisp = new VirtualMatrixPanel((*display), bc.pins[3], bc.pins[4], mxconfig.mx_width, mxconfig.mx_height, CHAIN_BOTTOM_LEFT_UP); + virtualDisp = new VirtualMatrixPanel(*display, bc.pins[3], bc.pins[4], mxconfig.mx_width, mxconfig.mx_height, CHAIN_BOTTOM_LEFT_UP);
834-842: Fix usage of uninitialized chain_length.The code uses
mxconfig.chain_lengthbefore it's assigned on line 842, which could lead to incorrect behavior.Reorder the code to initialize
chain_lengthbefore using it:+ mxconfig.chain_length = max((u_int8_t) 1, min(bc.pins[2], (u_int8_t) 4)); + #if defined(CONFIG_IDF_TARGET_ESP32) || defined(CONFIG_IDF_TARGET_ESP32S2) if (mxconfig.mx_height >= 64) { if (mxconfig.chain_length * mxconfig.mx_width > 192) mxconfig.setPixelColorDepthBits(3); - mxconfig.chain_length = max((u_int8_t) 1, min(bc.pins[2], (u_int8_t) 4));
897-902: Verify pin allocation and handle failure properly.The pin allocation should be checked for success and handle failures gracefully.
Add proper error handling:
if (!PinManager::allocateMultiplePins(pins, PIN_COUNT, PinOwner::HUB75, true)) { DEBUGBUS_PRINTLN("Failed to allocate pins for HUB75"); + deallocatePins(); // Clean up any partially allocated pins return; }
926-931: Fix undeclared variable causing compilation error.The variable
lastHeapis used but not declared, which will cause compilation errors.Declare the variable before use:
+ size_t lastHeap = ESP.getFreeHeap(); display = new MatrixPanel_I2S_DMA(mxconfig); if (display == nullptr) { DEBUGBUS_PRINTLN("****** MatrixPanel_I2S_DMA !KABOOM! driver allocation failed ***********"); DEBUGBUS_PRINT(F("heap usage: ")); DEBUGBUS_PRINTLN(lastHeap - ESP.getFreeHeap());
🧹 Nitpick comments (6)
.github/workflows/usermods.yml (2)
25-25: Consider using a more maintainable approach for excluded usermods.The current approach of using multiple
grep -vcommands to exclude specific usermods is hard to maintain. Consider using a configuration file or environment variable.- echo "usermods=$(find usermods/ -name library.json | xargs dirname | xargs -n 1 basename | jq -R | grep -v PWM_fan | grep -v BME68X_v2| grep -v pixels_dice_tray | jq --slurp -c)" >> $GITHUB_OUTPUT + EXCLUDED_USERMODS="PWM_fan BME68X_v2 pixels_dice_tray" + ALL_USERMODS=$(find usermods/ -name library.json | xargs dirname | xargs -n 1 basename) + FILTERED_USERMODS=$(echo "$ALL_USERMODS" | grep -v -E "$(echo $EXCLUDED_USERMODS | tr ' ' '|')") + echo "usermods=$(echo "$FILTERED_USERMODS" | jq -R | jq --slurp -c)" >> $GITHUB_OUTPUT
31-31: Fix typo in job name.- name: Build Enviornments + name: Build Environmentswled00/FX.cpp (2)
6119-6121: Zero-padding detection may have false positives.The current logic sets
zero = truefor any '0' character found in the token, not just as a suffix. This could incorrectly enable zero-padding if a token happens to contain '0' elsewhere.Consider checking only the last character before the null terminator:
- if(token[j] == '0') - zero = true; // 0 suffix found. Note: there is an edge case where a '0' could be part of a trailing text and not the token, handling it is not worth the effort + // Check for '0' suffix after token is completeThen after the loop:
token[j] = '\0'; + // Check if last character before null is '0' + if (j > 1 && SEGMENT.name[i + j - 1] == '0' && SEGMENT.name[i + j - 2] != '0') { + zero = true; + token[j - 1] = '\0'; // Remove the '0' suffix from token + }
6148-6151: Consider using safer string copy function.While the length check prevents buffer overflow, using
strcpyis generally discouraged in favor of safer alternatives.Consider using
memcpysince the length is already known:- strcpy(text + result_pos, temp); + memcpy(text + result_pos, temp, temp_len);wled00/wled_server.cpp (1)
414-419: Consider more descriptive error messages for access denial.The current implementation correctly enforces subnet restrictions, but users might not understand why their OTA update was denied. Consider providing more specific error messages.
- if (((otaSameSubnet && !inSameSubnet(client)) && !strlen(settingsPIN)) || (!otaSameSubnet && !inLocalSubnet(client))) { - DEBUG_PRINTLN(F("Attempted OTA update from different/non-local subnet!")); - request->send(401, FPSTR(CONTENT_TYPE_PLAIN), FPSTR(s_accessdenied)); + if ((otaSameSubnet && !inSameSubnet(client)) && !strlen(settingsPIN)) { + DEBUG_PRINTLN(F("Attempted OTA update from different subnet without PIN!")); + request->send(401, FPSTR(CONTENT_TYPE_PLAIN), F("OTA update requires same subnet or PIN authentication")); + return; + } + if (!otaSameSubnet && !inLocalSubnet(client)) { + DEBUG_PRINTLN(F("Attempted OTA update from non-local subnet!")); + request->send(401, FPSTR(CONTENT_TYPE_PLAIN), F("OTA update only allowed from local network")); return; }wled00/data/update.htm (1)
9-19: Consider using window.location for navigation.The revert functionality implementation is solid with a good two-step confirmation pattern. However, consider using
window.location.hrefinstead ofwindow.openfor cleaner navigation.- window.open(getURL("/update?revert"),"_self"); + window.location.href = getURL("/update?revert");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.github/workflows/usermods.yml(1 hunks)pio-scripts/load_usermods.py(2 hunks)pio-scripts/validate_modules.py(1 hunks)requirements.txt(1 hunks)usermods/LD2410_v2/readme.md(1 hunks)usermods/Si7021_MQTT_HA/library.json(1 hunks)usermods/Si7021_MQTT_HA/readme.md(1 hunks)usermods/buzzer/buzzer.cpp(1 hunks)usermods/platformio_override.usermods.ini(1 hunks)wled00/FX.cpp(2 hunks)wled00/FX.h(5 hunks)wled00/FX_fcn.cpp(7 hunks)wled00/bus_manager.cpp(6 hunks)wled00/bus_manager.h(6 hunks)wled00/bus_wrapper.h(2 hunks)wled00/cfg.cpp(5 hunks)wled00/data/settings_sec.htm(1 hunks)wled00/data/update.htm(2 hunks)wled00/fcn_declare.h(1 hunks)wled00/pin_manager.cpp(3 hunks)wled00/pin_manager.h(3 hunks)wled00/set.cpp(2 hunks)wled00/udp.cpp(1 hunks)wled00/wled.cpp(2 hunks)wled00/wled.h(2 hunks)wled00/wled_server.cpp(8 hunks)wled00/xml.cpp(4 hunks)
✅ Files skipped from review due to trivial changes (4)
- requirements.txt
- usermods/LD2410_v2/readme.md
- usermods/Si7021_MQTT_HA/readme.md
- usermods/platformio_override.usermods.ini
🚧 Files skipped from review as they are similar to previous changes (6)
- wled00/pin_manager.h
- wled00/set.cpp
- wled00/wled.h
- wled00/cfg.cpp
- wled00/xml.cpp
- wled00/pin_manager.cpp
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
Learnt from: blazoncek
PR: wled/WLED#4700
File: wled00/wled_server.cpp:409-414
Timestamp: 2025-05-26T16:09:34.325Z
Learning: In wled00/wled_server.cpp, the OTA update access control logic intentionally allows OTA updates from different subnets when otaSameSubnet is true AND a PIN is set. This was a conscious design decision by blazoncek to provide flexibility for remote OTA updates with PIN protection, though they acknowledged it may not be the optimal security model.
usermods/buzzer/buzzer.cpp (5)
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
Learnt from: netmindz
PR: wled/WLED#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.
Learnt from: netmindz
PR: wled/WLED#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.
Learnt from: KrX3D
PR: wled/WLED#4237
File: usermods/INA219_v2/INA219_v2.cpp:265-276
Timestamp: 2025-04-18T22:27:58.634Z
Learning: When implementing MQTT message handling in WLED usermods, use `strstr(topic, "/specific/path")` instead of `strcmp_P(topic, PSTR("/specific/path"))` to properly match topics that include the device prefix. The full MQTT topic typically follows the pattern `<mqttDeviceTopic>/specific/path`.
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
wled00/data/settings_sec.htm (1)
Learnt from: blazoncek
PR: wled/WLED#4700
File: wled00/wled_server.cpp:409-414
Timestamp: 2025-05-26T16:09:34.325Z
Learning: In wled00/wled_server.cpp, the OTA update access control logic intentionally allows OTA updates from different subnets when otaSameSubnet is true AND a PIN is set. This was a conscious design decision by blazoncek to provide flexibility for remote OTA updates with PIN protection, though they acknowledged it may not be the optimal security model.
wled00/wled.cpp (5)
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Learnt from: KrX3D
PR: wled/WLED#4664
File: wled00/syslog.cpp:50-51
Timestamp: 2025-04-26T12:51:04.993Z
Learning: In the WLED codebase, syslog configuration variables (syslogHost, syslogPort, syslogProtocol, syslogFacility, syslogSeverity) are defined as global variables in wled.h and are accessed directly in the SyslogPrinter implementation, rather than being class members.
Learnt from: KrX3D
PR: wled/WLED#4237
File: usermods/INA219_v2/INA219_v2.cpp:265-276
Timestamp: 2025-04-18T22:27:58.634Z
Learning: When implementing MQTT message handling in WLED usermods, use `strstr(topic, "/specific/path")` instead of `strcmp_P(topic, PSTR("/specific/path"))` to properly match topics that include the device prefix. The full MQTT topic typically follows the pattern `<mqttDeviceTopic>/specific/path`.
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
Learnt from: blazoncek
PR: wled/WLED#4700
File: wled00/wled_server.cpp:409-414
Timestamp: 2025-05-26T16:09:34.325Z
Learning: In wled00/wled_server.cpp, the OTA update access control logic intentionally allows OTA updates from different subnets when otaSameSubnet is true AND a PIN is set. This was a conscious design decision by blazoncek to provide flexibility for remote OTA updates with PIN protection, though they acknowledged it may not be the optimal security model.
pio-scripts/validate_modules.py (1)
Learnt from: willmmiles
PR: wled/WLED#4669
File: pio-scripts/validate_modules.py:16-19
Timestamp: 2025-05-27T02:17:37.883Z
Learning: In pio-scripts/validate_modules.py, the read_lines function doesn't include error handling because the script itself generates the map file it reads via linker flags. File read errors would indicate fundamental build platform issues or unsupported modifications, making additional error handling not worth the maintenance overhead according to the author willmmiles.
pio-scripts/load_usermods.py (1)
Learnt from: willmmiles
PR: wled/WLED#4623
File: usermods/word-clock-matrix/word-clock-matrix.cpp:332-332
Timestamp: 2025-03-29T01:22:54.617Z
Learning: In the WLED project, hardcoded usermod IDs (like 500 in the WordClockMatrix's getId() method) are intentionally used as part of a strategy to avoid modifying core code. There are plans to remove numeric usermod IDs entirely in the future.
wled00/FX.h (11)
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Learnt from: willmmiles
PR: wled/WLED#4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Learnt from: netmindz
PR: wled/WLED#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.
Learnt from: netmindz
PR: wled/WLED#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.
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:343-347
Timestamp: 2025-02-19T12:41:22.676Z
Learning: The setName() method in WLED's Segment class (FX_fcn.cpp) handles all necessary validation internally, including:
1. Null pointer checks
2. Length validation against WLED_MAX_SEGNAME_LEN
3. Memory allocation/reallocation
No additional validation or name clearing is needed before calling setName().
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs by falling back to the main segment ID, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Learnt from: DedeHai
PR: wled/WLED#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.
Learnt from: DedeHai
PR: wled/WLED#4750
File: usermods/audioreactive/audio_reactive.cpp:311-321
Timestamp: 2025-06-27T06:04:44.652Z
Learning: In WLED's AudioReactive usermod, FFT buffer memory (valFFT, windowFFT) is allocated once during initialization and never deallocated, following typical embedded system design where core functionality buffers persist for the application lifetime. Raw buffer pointers for aligned allocations don't need to be tracked in the current implementation since deallocation is never performed.
wled00/wled_server.cpp (9)
Learnt from: blazoncek
PR: wled/WLED#4700
File: wled00/wled_server.cpp:409-414
Timestamp: 2025-05-26T16:09:34.325Z
Learning: In wled00/wled_server.cpp, the OTA update access control logic intentionally allows OTA updates from different subnets when otaSameSubnet is true AND a PIN is set. This was a conscious design decision by blazoncek to provide flexibility for remote OTA updates with PIN protection, though they acknowledged it may not be the optimal security model.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
Learnt from: KrX3D
PR: wled/WLED#4237
File: usermods/INA219_v2/INA219_v2.cpp:265-276
Timestamp: 2025-04-18T22:27:58.634Z
Learning: When implementing MQTT message handling in WLED usermods, use `strstr(topic, "/specific/path")` instead of `strcmp_P(topic, PSTR("/specific/path"))` to properly match topics that include the device prefix. The full MQTT topic typically follows the pattern `<mqttDeviceTopic>/specific/path`.
Learnt from: KrX3D
PR: wled/WLED#4664
File: wled00/syslog.cpp:50-51
Timestamp: 2025-04-26T12:51:04.993Z
Learning: In the WLED codebase, syslog configuration variables (syslogHost, syslogPort, syslogProtocol, syslogFacility, syslogSeverity) are defined as global variables in wled.h and are accessed directly in the SyslogPrinter implementation, rather than being class members.
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.
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:430-435
Timestamp: 2025-04-28T20:51:29.773Z
Learning: In WLED, `bri` is a global variable used for brightness control.
Learnt from: blazoncek
PR: wled/WLED#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.
Learnt from: netmindz
PR: wled/WLED#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.
Learnt from: netmindz
PR: wled/WLED#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.
wled00/FX_fcn.cpp (10)
Learnt from: willmmiles
PR: wled/WLED#4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Learnt from: blazoncek
PR: wled/WLED#4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the `getSegment()` function guards against out-of-bounds segment IDs by falling back to the main segment ID, and `getFirstSelectedSegId()` falls back to `getMainSegmentId()` if no segments are selected, ensuring no crashes when used through the `setValuesFromFirstSelectedSeg()` macro.
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Learnt from: netmindz
PR: wled/WLED#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.
Learnt from: netmindz
PR: wled/WLED#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.
Learnt from: DedeHai
PR: wled/WLED#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.
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dx*dx + dy*dy to be a practical concern in particle system effects.
Learnt from: DedeHai
PR: wled/WLED#4750
File: usermods/audioreactive/audio_reactive.cpp:311-321
Timestamp: 2025-06-27T06:04:44.652Z
Learning: In WLED's AudioReactive usermod, FFT buffer memory (valFFT, windowFFT) is allocated once during initialization and never deallocated, following typical embedded system design where core functionality buffers persist for the application lifetime. Raw buffer pointers for aligned allocations don't need to be tracked in the current implementation since deallocation is never performed.
wled00/bus_manager.h (10)
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:165-173
Timestamp: 2025-05-05T18:49:09.398Z
Learning: In the UsermodSSDR module, the `umSSDRNumbers` array is designed to handle digits 0-9 only, and any access with indices >= 10 requires bounds checking to prevent out-of-bounds memory access.
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-137
Timestamp: 2025-04-27T10:06:22.545Z
Learning: In the Seven Segment Display Reloaded usermod, the dimension mismatch between the default array (umSSDRNumbers[11][7]) and the override (umSSDRNumbers[11][10] = umSSDR_NUMBERS) is intentional by design, allowing for different use cases with varying numbers of segments per digit.
Learnt from: DedeHai
PR: wled/WLED#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.
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.
Learnt from: KrX3D
PR: wled/WLED#4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.cpp:430-435
Timestamp: 2025-04-28T20:51:29.773Z
Learning: In WLED, `bri` is a global variable used for brightness control.
Learnt from: netmindz
PR: wled/WLED#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.
Learnt from: netmindz
PR: wled/WLED#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.
Learnt from: KrX3D
PR: wled/WLED#4664
File: wled00/syslog.cpp:50-51
Timestamp: 2025-04-26T12:51:04.993Z
Learning: In the WLED codebase, syslog configuration variables (syslogHost, syslogPort, syslogProtocol, syslogFacility, syslogSeverity) are defined as global variables in wled.h and are accessed directly in the SyslogPrinter implementation, rather than being class members.
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dx*dx + dy*dy to be a practical concern in particle system effects.
wled00/bus_manager.cpp (8)
Learnt from: DedeHai
PR: wled/WLED#4682
File: wled00/FX.cpp:9016-9039
Timestamp: 2025-05-09T18:48:21.296Z
Learning: WLED does not support matrices large enough (≥1024 px) for 32-bit overflow in dx*dx + dy*dy to be a practical concern in particle system effects.
Learnt from: blazoncek
PR: wled/WLED#4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
Learnt from: blazoncek
PR: wled/WLED#4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like `if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize))` is unnecessary and could be problematic, as it bypasses proper size verification.
Learnt from: DedeHai
PR: wled/WLED#4750
File: usermods/audioreactive/audio_reactive.cpp:311-321
Timestamp: 2025-06-27T06:04:44.652Z
Learning: In WLED's AudioReactive usermod, FFT buffer memory (valFFT, windowFFT) is allocated once during initialization and never deallocated, following typical embedded system design where core functionality buffers persist for the application lifetime. Raw buffer pointers for aligned allocations don't need to be tracked in the current implementation since deallocation is never performed.
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.
Learnt from: willmmiles
PR: wled/WLED#4712
File: wled00/FX_fcn.cpp:992-993
Timestamp: 2025-06-07T15:58:42.579Z
Learning: In WLED segments, the backing pixel buffer is always allocated to the full physical segment size, but effect rendering only uses a subset when mirror or grouping are enabled. For 2D segments, this subset is vWidth() * vHeight(), and for 1D segments it's vLength(). Color processing operations like fade_out should iterate over this rendering subset, not the full allocated buffer.
Learnt from: netmindz
PR: wled/WLED#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.
Learnt from: netmindz
PR: wled/WLED#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.
wled00/FX.cpp (12)
undefined
<retrieved_learning>
Learnt from: netmindz
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: netmindz
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: KrX3D
PR: #4585
File: usermods/seven_segment_display_reloaded_v2/seven_segment_display_reloaded_v2.h:121-136
Timestamp: 2025-04-27T09:37:28.415Z
Learning: Using PROGMEM for the seven-segment font array (umSSDRNumbers) in the WLED SSDR usermod causes compilation problems, so it should be left as a regular array.
</retrieved_learning>
<retrieved_learning>
Learnt from: blazoncek
PR: #4482
File: wled00/udp.cpp:147-149
Timestamp: 2025-02-19T12:43:34.199Z
Learning: ESP8266 and ESP32 platforms have different maximum segment name lengths in WLED, which can cause truncation when syncing segment names between devices. This platform difference affects the user experience when using the segment name sync feature.
</retrieved_learning>
<retrieved_learning>
Learnt from: blazoncek
PR: #4667
File: usermods/user_fx/user_fx.cpp:27-30
Timestamp: 2025-04-30T05:41:03.633Z
Learning: In WLED, the Segment::allocateData() method already includes optimization to check if data is allocated and sufficiently sized, handling buffer reuse to reduce memory fragmentation. Adding an external check like if (SEGENV.data == nullptr && !SEGENV.allocateData(dataSize)) is unnecessary and could be problematic, as it bypasses proper size verification.
</retrieved_learning>
<retrieved_learning>
Learnt from: blazoncek
PR: #4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the getSegment() function guards against out-of-bounds segment IDs, and getFirstSelectedSegId() falls back to getMainSegmentId() if no segments are selected, ensuring no crashes when used through the setValuesFromFirstSelectedSeg() macro.
</retrieved_learning>
<retrieved_learning>
Learnt from: blazoncek
PR: #4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the getSegment() function guards against out-of-bounds segment IDs, and getFirstSelectedSegId() falls back to getMainSegmentId() if no segments are selected, ensuring no crashes when used through the setValuesFromFirstSelectedSeg() macro.
</retrieved_learning>
<retrieved_learning>
Learnt from: blazoncek
PR: #4658
File: wled00/led.cpp:90-91
Timestamp: 2025-04-24T09:31:06.879Z
Learning: In the WLED codebase, the getSegment() function guards against out-of-bounds segment IDs by falling back to the main segment ID, and getFirstSelectedSegId() falls back to getMainSegmentId() if no segments are selected, ensuring no crashes when used through the setValuesFromFirstSelectedSeg() macro.
</retrieved_learning>
<retrieved_learning>
Learnt from: blazoncek
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: blazoncek
PR: #4482
File: wled00/udp.cpp:343-347
Timestamp: 2025-02-19T12:41:22.676Z
Learning: The setName() method in WLED's Segment class (FX_fcn.cpp) handles all necessary validation internally, including:
- Null pointer checks
- Length validation against WLED_MAX_SEGNAME_LEN
- Memory allocation/reallocation
No additional validation or name clearing is needed before calling setName().
</retrieved_learning>
<retrieved_learning>
Learnt from: DedeHai
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: blazoncek
PR: #4482
File: wled00/udp.cpp:343-347
Timestamp: 2025-02-19T12:41:22.676Z
Learning: The setName() method in WLED handles segment name validation internally. No additional length checks or name clearing is needed before calling setName().
</retrieved_learning>
🧬 Code Graph Analysis (6)
wled00/wled.cpp (2)
wled00/cfg.cpp (4)
deserializeConfigFromFS(778-799)deserializeConfigFromFS(778-778)serializeConfigToFS(801-818)serializeConfigToFS(801-801)wled00/fcn_declare.h (2)
deserializeConfigFromFS(28-28)serializeConfigToFS(31-31)
pio-scripts/load_usermods.py (1)
pio-scripts/validate_modules.py (1)
is_wled_module(9-13)
wled00/FX.h (2)
wled00/bus_manager.cpp (2)
d_free(77-77)d_free(77-77)wled00/fcn_declare.h (2)
d_free(562-562)d_free(562-562)
wled00/bus_wrapper.h (1)
wled00/bus_manager.h (16)
pix(95-95)pix(130-130)pix(133-133)pix(133-133)pix(164-164)pix(164-164)pix(250-250)pix(252-252)pix(298-298)pix(299-299)pix(326-326)pix(327-327)pix(347-347)pix(348-348)pix(368-368)pix(369-369)
wled00/FX_fcn.cpp (6)
wled00/FX.h (25)
i(539-539)i(539-539)i(540-540)i(540-540)i(654-654)i(654-654)i(692-692)i(692-692)i(694-694)i(695-695)i(695-695)i(696-696)i(696-696)i(698-698)i(699-699)i(891-891)blend(558-558)blend(558-558)c(703-703)c(764-764)c(764-764)c(897-897)c(897-897)o(663-663)setCCT(877-877)wled00/bus_manager.h (11)
cct(199-199)cct(199-199)c(129-129)c(129-129)c(208-208)c(237-237)c(249-249)c(278-287)c(278-278)setSegmentCCT(504-504)setPixelColor(496-496)wled00/fcn_declare.h (4)
color_blend(173-173)c(450-450)c(450-450)d_malloc(559-559)wled00/colors.cpp (2)
color_blend(11-21)color_blend(11-11)wled00/bus_manager.cpp (14)
d_malloc(74-74)setSegmentCCT(1327-1334)setSegmentCCT(1327-1327)setPixelColor(300-325)setPixelColor(300-300)setPixelColor(483-517)setPixelColor(483-483)setPixelColor(664-672)setPixelColor(664-664)setPixelColor(725-734)setPixelColor(725-725)setPixelColor(999-999)setPixelColor(1320-1325)setPixelColor(1320-1320)wled00/util.cpp (2)
d_malloc(653-661)d_malloc(653-653)
wled00/bus_manager.cpp (2)
wled00/bus_manager.h (49)
_type(160-160)_cctBlend(200-200)BusHub75Matrix(367-367)BusHub75Matrix(376-378)_valid(161-161)_hasRgb(144-144)_hasWhite(145-145)setPixelColor(496-496)pix(95-95)pix(130-130)pix(133-133)pix(133-133)pix(164-164)pix(164-164)pix(250-250)pix(252-252)pix(298-298)pix(299-299)pix(326-326)pix(327-327)pix(347-347)pix(348-348)pix(368-368)pix(369-369)c(129-129)c(129-129)c(208-208)c(237-237)c(249-249)c(278-287)c(278-278)b(131-131)b(131-131)b(201-207)b(201-201)b(248-248)b(371-371)getPixelColor(497-497)setBrightness(501-501)setBrightness(501-501)show(498-498)deallocatePins(304-304)pinArray(134-134)pinArray(134-134)pinArray(254-254)pinArray(300-300)pinArray(328-328)pinArray(349-349)pinArray(372-372)wled00/pin_manager.cpp (8)
allocateMultiplePins(92-140)allocateMultiplePins(92-92)allocateMultiplePins(142-146)allocateMultiplePins(142-142)deallocateMultiplePins(45-83)deallocateMultiplePins(45-45)deallocateMultiplePins(85-90)deallocateMultiplePins(85-85)
🪛 Flake8 (7.2.0)
pio-scripts/load_usermods.py
[error] 80-80: indentation is not a multiple of 4
(E111)
[error] 82-82: indentation is not a multiple of 4
(E111)
[error] 83-83: indentation is not a multiple of 4
(E111)
[error] 88-88: indentation is not a multiple of 4
(E111)
[error] 91-91: indentation is not a multiple of 4
(E111)
[error] 101-101: indentation is not a multiple of 4 (comment)
(E114)
[error] 102-102: indentation is not a multiple of 4
(E111)
🪛 YAMLlint (1.37.1)
.github/workflows/usermods.yml
[error] 8-8: trailing spaces
(trailing-spaces)
[warning] 15-15: wrong indentation: expected 6 but found 4
(indentation)
[warning] 40-40: wrong indentation: expected 6 but found 4
(indentation)
[warning] 59-59: wrong indentation: expected 8 but found 10
(indentation)
[error] 71-71: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: wled_build / Build Environments (esp32S3_PSRAM_HUB75)
- GitHub Check: wled_build / Build Environments (esp32dev_hub75)
- GitHub Check: wled_build / Build Environments (esp32dev_hub75_forum_pinout)
- GitHub Check: wled_build / Build Environments (adafruit_matrixportal_esp32s3)
🔇 Additional comments (44)
usermods/Si7021_MQTT_HA/library.json (1)
6-8: LGTM! Dependencies align with enhanced platform support.The addition of SPI and Adafruit BusIO dependencies is appropriate for supporting the expanded ESP32 platform configurations introduced in this PR. These dependencies provide necessary hardware abstraction for I2C/SPI operations across different board variants.
pio-scripts/validate_modules.py (1)
56-57: LGTM! Refactoring improves code organization.The simplification to use
env['WLED_MODULES'](set byload_usermods.py) instead of dynamic filtering centralizes module management and reduces code duplication. This change maintains the same validation functionality while improving maintainability..github/workflows/usermods.yml (1)
34-38: LGTM! Matrix strategy provides good coverage.The matrix strategy building each usermod across multiple ESP32 variants (esp32, esp32c3, esp32s2, esp32s3) provides excellent test coverage for the enhanced platform support introduced in this PR.
wled00/fcn_declare.h (1)
28-28: LGTM: Function signature correctly updated.The function declaration now properly matches the implementation in
cfg.cppthat returns a boolean indicating whether configuration needs to be saved.wled00/udp.cpp (1)
603-604: LGTM: Correctly enables dnrgb and dnrgbw realtime protocols.The range extension from
< 4to< 6properly enables the existing handlers for packet types 4 (dnrgb) and 5 (dnrgbw) that are already implemented in lines 637-651.usermods/buzzer/buzzer.cpp (1)
8-12: LGTM: Improves platform compatibility with conditional pin selection.The platform-dependent GPIO selection enhances compatibility across different ESP32 variants by preferring
GPIO_NUM_32when available and falling back to GPIO 21 otherwise.wled00/wled.cpp (2)
426-426: LGTM: Correctly captures configuration save requirement.Capturing the return value from
deserializeConfigFromFS()properly implements the deferred configuration save pattern introduced in this PR.
446-447: LGTM: Proper timing for conditional configuration save.Saving configuration after usermod setup and strip initialization ensures that any new parameters required by usermods are saved only when needed and after the system is fully initialized.
wled00/data/settings_sec.htm (1)
60-62: LGTM: Clear UI for OTA subnet restriction with appropriate warnings.The new checkbox provides intuitive control over the OTA subnet restriction feature. The warning text helpfully explains the implications for users with complex network configurations (VLANs) and clearly states the security trade-offs.
wled00/FX.h (4)
766-766: Good consistency fix - removing constexpr from is2D().Removing
constexprfrom theis2D()method maintains consistency with the 2D version (line 727) which likely cannot beconstexprdue to calling non-constexpr methods likewidth()andheight().
832-832: Proper initialization of new _pixelCCT member.The new
_pixelCCTpointer is correctly initialized tonullptrin the constructor's member initialization list, following RAII principles.
861-861: Correct memory cleanup in destructor.The
_pixelCCTpointer is properly freed in the destructor using thed_freefunction, which callsheap_caps_free. The "just in case" comment suggests defensive programming, which is appropriate for memory management.
1009-1009: Well-structured addition of _pixelCCT member.The new
uint8_t *_pixelCCTmember is appropriately placed in the private section alongside the existing_pixelspointer, supporting the per-pixel CCT functionality described in the AI summary.wled00/bus_wrapper.h (4)
865-865: LGTM: Parameter consistency for RGBW bus.The change ensures both parallel and non-parallel I2S branches pass
coldirectly for the 4-channel NEO_4 bus type, which is consistent with the RMT equivalent (line 851) and correct for RGBW LEDs.
867-867: LGTM: Parameter consistency for TM1814 RGBW bus.The change aligns both I2S branches to pass
coldirectly for the 4-channel TM1814 bus, matching the RMT equivalent (line 853) and maintaining proper RGBW color handling.
869-870: LGTM: Consistent high bit-depth color handling.Both UCS bus cases now consistently use the same parameter types regardless of parallel I2S mode:
- UCS_3:
Rgb48Color(RgbColor(col))for 48-bit RGB- UCS_4:
Rgbw64Color(col)for 64-bit RGBWThis matches the RMT equivalents (lines 855-856) and eliminates unnecessary conditional differences.
1426-1426: LGTM: Accurate comment clarification.The comment update correctly clarifies that offset 1 corresponds to I2S1 (used by Audioreactive), which is more accurate than the previous reference to I2S0.
wled00/FX.cpp (2)
6094-6094: Good choice of variable type and initialization.Using
size_tfor buffer position tracking is appropriate and follows best practices.
6106-6162: Well-implemented token parsing with good buffer overflow protection.The implementation correctly:
- Validates buffer boundaries before copying
- Handles both known and unknown tokens gracefully
- Provides flexible formatting with zero-padding option
- Maintains compatibility with existing empty segment name behavior
The token variety covers common date/time display needs for matrix displays.
wled00/wled_server.cpp (3)
3-7: LGTM!The conditional includes correctly handle platform differences between ESP8266 and ESP32.
604-608: Good security enhancement for settings protection.Restricting settings POST requests to local subnet access is a sensible security measure that's appropriately more restrictive than OTA updates.
40-42: Endianness handling in IPAddress comparisons is safeThe Arduino IPAddress class’s
operator uint32_tand its matchingIPAddress(uint32_t)constructor use a consistent byte-order reinterpret cast, so(uint32_t)ip & (uint32_t)maskyields the correct subnet on both little- and big-endian platforms. No changes required.Likely an incorrect or invalid review comment.
wled00/bus_manager.h (6)
5-11: LGTM! Clean conditional compilation for HUB75 support.The conditional compilation structure properly isolates HUB75-specific includes and follows WLED's patterns for optional features.
191-191: LGTM! Consistent with existing type checking methods.The
isHub75method follows the same pattern as other bus type checking methods likeisPWM,isDigital, etc.
200-203: LGTM! Improved precision with proper rounding.The addition of rounding in the CCT blend conversion methods improves precision. The math is correct:
getCCTBlend(): adds 64 for rounding before division (64 ≈ 127/2)setCCTBlend(): adds 50 for rounding before division (50 ≈ 100/2)
364-394: LGTM! Well-structured HUB75 matrix bus class declaration.The
BusHub75Matrixclass properly inherits fromBusand provides all necessary virtual method overrides. The private members appropriately manage matrix configuration, display objects, and optimization buffers.
406-406: LGTM! Proper expansion of pins array for flexible pin configurations.Expanding the pins array from a fixed size of 5 to
OUTPUT_MAX_PINSis necessary to support HUB75 matrices which require significantly more GPIO pins (typically 14).
424-424: LGTM! Consistent use of OUTPUT_MAX_PINS constant.Replacing the magic number with the proper constant ensures consistency with the array declaration and prevents potential out-of-bounds access.
wled00/bus_manager.cpp (7)
577-577: LGTM! Direct access to static member is appropriate.Accessing
Bus::_cctBlenddirectly instead of throughgetCCTBlend()is more efficient and appropriate in this context where we need the raw 0-127 scale value.
779-783: LGTM! Proper platform validation for HUB75 support.The compile-time checks correctly prevent HUB75 compilation on ESP8266 and provide a clear warning about the experimental nature of the feature.
999-1031: LGTM! Efficient pixel color handling with dirty bit optimization.The implementation properly uses dirty bit tracking to minimize updates and handles both buffered and direct modes appropriately. The gamma correction handling is also correct.
1033-1039: LGTM! Appropriate getPixelColor implementation.The method correctly handles both buffered and non-buffered modes, scaling brightness appropriately and providing reasonable fallback behavior for the non-buffered case.
1084-1097: LGTM! Proper cleanup implementation.The cleanup method correctly stops DMA output, deallocates pins, and frees allocated memory. The order of operations is appropriate.
1177-1180: LGTM! Clean integration into BusManager factory.The HUB75 bus creation is properly integrated into the bus factory pattern, following the same structure as other bus types.
1212-1214: LGTM! Consistent integration into LED types JSON.The HUB75 LED types are properly included in the JSON generation for the UI, maintaining consistency with other bus types.
wled00/data/update.htm (3)
29-29: Good refactoring to use generic toggle function.The change from custom
U()function to generictoggle('upd')improves code reusability and maintainability.
36-37: Well-structured UI additions.The horizontal rule provides good visual separation, and the revert button is properly configured with
type="button"to prevent form submission.
40-40: Consistent implementation with toggle functionality.The div ID and class changes align well with the new toggle-based approach for showing/hiding the update progress message.
wled00/FX_fcn.cpp (6)
682-684: Improved virtual strip index handling.The change from early return to truncation with subsequent bounds checking provides more consistent behavior for virtual strip handling. The approach correctly masks the virtual strip index while ensuring the pixel index remains within bounds.
1321-1322: Correct CCT value retrieval for blending.The code properly retrieves the transitioned CCT value using
currentCCT(), which handles CCT blending during transitions.
1394-1409: Proper CCT handling for 2D mirrored pixels.The implementation correctly applies CCT values to all mirrored pixel positions, maintaining consistency with the color blending logic. The null check for
_pixelCCTprevents potential crashes.
1481-1486: Consistent CCT handling for 1D segments.The implementation correctly applies CCT to both mirrored and main pixels in 1D segments, maintaining consistency with the 2D implementation.
1594-1600: Well-designed CCT buffer allocation.The conditional allocation logic correctly identifies when per-pixel CCT tracking is needed. The initialization to neutral CCT (127) provides a good default, and the memory allocation uses the appropriate helper function.
1618-1633: No explicit null check needed for d_freeThe suggestion to guard the call to
d_free(_pixelCCT)withif (_pixelCCT)is unnecessary—d_freeis already safe to call on a null pointer (it’s even invoked on the null-initialized member in the constructor’s initializer list). You can leave the existing cleanup as is.Likely an incorrect or invalid review comment.
| cat platformio_override.ini | ||
| - name: Build firmware | ||
| run: pio run -e ${{ matrix.environment }} |
There was a problem hiding this comment.
Fix trailing spaces.
- run: pio run -e ${{ matrix.environment }}
+ run: pio run -e ${{ matrix.environment }}📝 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.
| run: pio run -e ${{ matrix.environment }} | |
| run: pio run -e ${{ matrix.environment }} |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 71-71: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In .github/workflows/usermods.yml at line 71, remove the trailing spaces at the
end of the line containing "run: pio run -e ${{ matrix.environment }}". Ensure
there are no extra spaces after the command to maintain clean formatting.
| usermod: ${{ fromJSON(needs.get_usermod_envs.outputs.usermods) }} | ||
| environment: [usermods_esp32, usermods_esp32c3, usermods_esp32s2, usermods_esp32s3] | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
Fix YAML indentation.
- - uses: actions/checkout@v4
+ - uses: actions/checkout@v4📝 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.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v4 |
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 40-40: wrong indentation: expected 6 but found 4
(indentation)
🤖 Prompt for AI Agents
In .github/workflows/usermods.yml at line 40, the YAML indentation for the line
using actions/checkout@v4 is incorrect. Adjust the indentation to align properly
with the surrounding YAML structure, ensuring consistent spacing to maintain
valid YAML syntax.
| name: Gather Usermods | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
Fix YAML indentation.
- - uses: actions/checkout@v4
+ - uses: actions/checkout@v4📝 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.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v4 |
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 15-15: wrong indentation: expected 6 but found 4
(indentation)
🤖 Prompt for AI Agents
In .github/workflows/usermods.yml at line 15, the YAML indentation for the line
using "actions/checkout@v4" is incorrect. Adjust the indentation to align
properly with the surrounding YAML structure, ensuring it follows the expected
nesting level for the "uses" key within the workflow steps.
| - name: Set up Python | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: '3.12' |
There was a problem hiding this comment.
Fix YAML indentation.
- python-version: '3.12'
- cache: 'pip'
+ python-version: '3.12'
+ cache: 'pip'📝 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.
| python-version: '3.12' | |
| python-version: '3.12' | |
| cache: 'pip' |
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 59-59: wrong indentation: expected 8 but found 10
(indentation)
🤖 Prompt for AI Agents
In .github/workflows/usermods.yml at line 59, the indentation for the
python-version key is incorrect. Adjust the indentation to align properly with
the surrounding YAML structure, ensuring it uses consistent spaces to maintain
valid YAML formatting.
| paths: | ||
| - usermods/** | ||
| - .github/workflows/usermods.yml | ||
|
|
There was a problem hiding this comment.
Fix trailing spaces.
- - .github/workflows/usermods.yml
-
+ - .github/workflows/usermods.yml📝 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.
| - .github/workflows/usermods.yml |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 8-8: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In .github/workflows/usermods.yml at line 8, there are trailing spaces that need
to be removed. Edit the line to delete any extra spaces at the end to ensure
clean formatting and avoid potential YAML parsing issues.
| <head> | ||
| <meta content='width=device-width' name='viewport'> | ||
| <title>WLED Update</title> | ||
| <script src="common.js" async type="text/javascript"></script> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify that async loading won't cause race conditions.
The async loading of common.js improves page load performance, but could cause issues if inline scripts depend on functions from this file being available immediately.
🏁 Script executed:
#!/bin/bash
# Check if common.js contains critical functions used by inline scripts
cat wled00/data/common.js | head -50
# Check for any immediate usage of functions that might be defined in common.js
rg -A 5 "getURL|gId|toggle" wled00/data/Length of output: 161130
🏁 Script executed:
#!/bin/bash
# Display the first 50 lines of update.htm to verify script order
sed -n '1,50p' wled00/data/update.htmLength of output: 1523
Avoid async here to prevent race conditions
The inline script in update.htm calls gId(), getURL() and toggle(), which are defined in common.js. Loading common.js with async does not guarantee it executes before that inline script, risking “undefined” errors.
• File: wled00/data/update.htm
Location: line 6
Suggested change:
- <script src="common.js" async type="text/javascript"></script>
+ <script src="common.js" defer type="text/javascript"></script>This ensures common.js is parsed and executed before any inline code that depends on it.
📝 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.
| <script src="common.js" async type="text/javascript"></script> | |
| <script src="common.js" defer type="text/javascript"></script> |
🤖 Prompt for AI Agents
In wled00/data/update.htm at line 6, the script tag loading common.js uses the
async attribute, which can cause race conditions because the inline script
depends on functions defined in common.js. Remove the async attribute from the
script tag to ensure common.js is loaded and executed before the inline script
runs, preventing undefined errors.
|
I was just testing this PR and i was trying to figure out if i can use two 64x32 panels but look like when ever i set the chain length to 2 the esp is going in the boot loop Debug logIn the log when setting chain to two i notice only this different Setting to just one 64x32 panels is working correct ... no crashes and |
|
I'm AFK so missing details but in your Platformio add a filter for
esp32monitor, I think it's called.nThis will translate that pile of numbers
in the crash to a decoded symbolic stack with function names, arguments,
and line numbers to aid in debugging.
…On Tue, Jul 15, 2025, 6:34 AM spiro-c ***@***.***> wrote:
*spiro-c* left a comment (wled/WLED#3777)
<#3777 (comment)>
I was just testing this PR and i was trying to figure out if i can use two
64x32 panels but look like when ever i set the chain length to 2 the esp is
going in the boot loop
Debug log
---DEBUG INFO---
Runtime: 30000
Unix time: 30,001
Free heap: 211640
TX power: 34/34
Wifi state: 3
ESP-NOW state: 0
State time: 0
NTP last sync: 999000000
Client IP: 192.168.10.177
Loops/sec: 1471
Loop time[ms]: 0/664
UM time[ms]: 0/3
Strip time[ms]:0/32
Segments: 1 -> 196/0B
Seg: 30,1 [A=1, 2D=0, RGB=1, W=0, CCT=0]
Modes: 4*218=872B
Data: 4*218=872B
Map: 2*0=0B
Web server status: WS client disconnected.
settings resp 0
Loop delayed more than 8ms.
settings resp 2
JSON buffer locked. (6)
JSON buffer released. (6)
Bus: Config (0-30, type:65, CO:0, rev:0, skip:0, AW:0 kHz:0, mA:55/0)
# of buses: 2
PIN ALLOC: Pin 0 successfully allocated by 0x85.
PIN ALLOC: 0x0000000000000lX.
Loop delayed more than 17ms.
Re-init busses.
Removing all.
Digital Cleanup.
Image playback end called
Removing all.
Maximum LEDs on a bus: 0
Digital buses: 0
Bus: Adding bus (p:0 v:0)
Huidu HD-WF2 ESP32-S3 pinout
PIN ALLOC: Pin 2 allocated by 0x8E.
PIN ALLOC: Pin 6 allocated by 0x8E.
PIN ALLOC: Pin 10 allocated by 0x8E.
PIN ALLOC: Pin 3 allocated by 0x8E.
PIN ALLOC: Pin 7 allocated by 0x8E.
PIN ALLOC: Pin 11 allocated by 0x8E.
PIN ALLOC: Pin 39 allocated by 0x8E.
PIN ALLOC: Pin 38 allocated by 0x8E.
PIN ALLOC: Pin 37 allocated by 0x8E.
PIN ALLOC: Pin 36 allocated by 0x8E.
PIN ALLOC: Pin 21 allocated by 0x8E.
PIN ALLOC: Pin 33 allocated by 0x8E.
PIN ALLOC: Pin 35 allocated by 0x8E.
PIN ALLOC: Pin 34 allocated by 0x8E.
PIN ALLOC: 0x0000000000000lX.
MatrixPanel_I2S_DMA = unsupported color order 0
MatrixPanel_I2S_DMA config - 64x32 length: 2
R1_PIN=2, G1_PIN=6, B1_PIN=10, R2_PIN=3, G2_PIN=7, B2_PIN=11, A_PIN=39, B_PIN=38, C_PIN=37, D_PIN=36, E_PIN=21, LAT_PIN=33, OE_PIN=35, CLK_PIN=34
Length: 4096
MatrixPanel_I2S_DMA created
heap usage: 252
MatrixPanel_I2S_DMA begin ok
heap usage: 37244
MatrixPanel_I2S_DMA clear ok
MatrixPanel_I2S_DMA allocate memory
MatrixPanel_I2S_DMA allocate memory ok
MatrixPanel_I2S_DMA started, width=16320, 4096 pixels.
MatrixPanel_I2S_DMA LEDS buffer enabled.
MatrixPanel_I2S_DMA LEDS dirty bit optimization enabled.
MatrixPanel_I2S_DMA LEDS buffer uses 12800 bytes.
Heap after buses: 162896
Loading custom palettes
Loading custom ledmaps
strip buffer size: 16384B
Heap after strip init: 146632B
Segment geometry: 0,4096 -> 0,1 [1,0]
-- Started transition: S=0x3fced458 T(0x3fced3d0) O[0x3fcb82cc] OP[0x3fcb8328]
Writing settings to /wsec.json...
JSON buffer locked. (4)
JSON buffer released. (4)
Writing settings to /cfg.json...
JSON buffer locked. (2)
Cfg: Saving bus #0
(0-4096, type:65, CO:1, rev:0, skip:0, AW:0 kHz:0, mA:0/0)
JSON buffer released. (2)
Loop took 315ms.
Usermods took 0ms.
Strip took 8ms.
Image playback end called
Guru Meditation Error: Core 1 panic'ed (LoadProhibited). Exception was unhandled.
Core 1 register dump:
PC : 0x40375def PS : 0x00060e30 A0 : 0x82021ad5 A1 : 0x3fcebcf0
=> 0x40375def: MatrixPanel_I2S_DMA::updateMatrixDMABuffer(unsigned short, unsigned short, unsigned char, unsigned char, unsigned char) at .pio/libdeps/HD-WF2_ESP32_S3/ESP32 HUB75 LED MATRIX PANEL DMA Display/src/ESP32-HUB75-MatrixPanel-I2S-DMA.cpp:330
A2 : 0x00000000 A3 : 0x00003fbf A4 : 0x0000001f A5 : 0x000000ff
A6 : 0x000000a0 A7 : 0x00000000 A8 : 0x00003fbf A9 : 0x3fced36c
A10 : 0xfffffffe A11 : 0x0000001f A12 : 0x00000000 A13 : 0xffc13fbf
A14 : 0x00003fc0 A15 : 0x00000000 SAR : 0x00000000 EXCCAUSE: 0x0000001c
EXCVADDR: 0x000000a0 LBEG : 0x40056f5c LEND : 0x40056f72 LCOUNT : 0x00000000
=> 0x40056f5c: ?? ??:0
=> 0x40056f72: ?? ??:0
Backtrace: 0x40375dec:0x3fcebcf0 0x42021ad2:0x3fcebd20 0x42022143:0x3fcebd50 0x4202324f:0x3fcebd80 0x4201d513:0x3fcebda0 0x4201d6f0:0x3fcebe00 0x4204c901:0x3fcebe30 0x4204cdfe:0x3fcebef0 0x42067805:0x3fcebf10
=> 0x40375dec: MatrixPanel_I2S_DMA::updateMatrixDMABuffer(unsigned short, unsigned short, unsigned char, unsigned char, unsigned char) at .pio/libdeps/HD-WF2_ESP32_S3/ESP32 HUB75 LED MATRIX PANEL DMA Display/src/ESP32-HUB75-MatrixPanel-I2S-DMA.cpp:329
=> 0x42021ad2: MatrixPanel_I2S_DMA::drawPixelRGB888(short, short, unsigned char, unsigned char, unsigned char) at .pio/libdeps/HD-WF2_ESP32_S3/ESP32 HUB75 LED MATRIX PANEL DMA Display/src/ESP32-HUB75-MatrixPanel-I2S-DMA.h:926
(inlined by) VirtualMatrixPanel::drawPixelRGB888(short, short, unsigned char, unsigned char, unsigned char) at .pio/libdeps/HD-WF2_ESP32_S3/ESP32 HUB75 LED MATRIX PANEL DMA Display/src/ESP32-VirtualMatrixPanel-I2S-DMA.h:465
=> 0x42022143: BusHub75Matrix::show() at wled00/bus_manager.cpp:1074
(inlined by) BusHub75Matrix::show() at wled00/bus_manager.cpp:1052
=> 0x4202324f: BusManager::show() at wled00/bus_manager.cpp:1321 (discriminator 2)
=> 0x4201d513: WS2812FX::show() at wled00/FX_fcn.cpp:1638
=> 0x4201d6f0: WS2812FX::service() at wled00/FX_fcn.cpp:1261
=> 0x4204c901: WLED::loop() at wled00/wled.cpp:127 (discriminator 5)
=> 0x4204cdfe: loop() at wled00/wled_main.cpp:23
=> 0x42067805: loopTask(void*) at ***@***.***/cores/esp32/main.cpp:50
ELF file SHA256: 0000000000000000
Rebooting...
ESP-ROM:esp32s3-20210327
In the log when setting chain to two i notice only this different MatrixPanel_I2S_DMA
started, width=16320, 4096 pixels. what is setting the width of the panel
to 16320=64*255 so i dont know if that is issue
Setting to just one 64x32 panels is working correct ... no crashes and MatrixPanel_I2S_DMA
started, width=64, 2048 pixels. is correct
—
Reply to this email directly, view it on GitHub
<#3777 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACCSD34DITAGTCKB7DFFYZD3ITRNZAVCNFSM6AAAAABD2ZGHFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTANZTGI2TOOJRGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
platformio_override.ini
Outdated
| [platformio] | ||
| default_envs = esp32dev_hub75, adafruit_matrixportal_esp32s3, esp32S3_PSRAM_HUB75, esp32dev_hub75_forum_pinout | ||
|
|
||
| [env:esp32dev_hub75] |
There was a problem hiding this comment.
TODO: move this to the platform_override_sample.ini once testing complete
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wled00/set.cpp (1)
143-147: Initialize widened pins[] to 0xFF to preserve "unused" sentinelExpanding to OUTPUT_MAX_PINS but initializing only 5 elements leaves the rest as 0, which can be misinterpreted as valid GPIO 0 by downstream code.
Apply this diff:
- uint8_t pins[OUTPUT_MAX_PINS] = {255, 255, 255, 255, 255}; + uint8_t pins[OUTPUT_MAX_PINS]; + memset(pins, 0xFF, sizeof(pins)); // ensure all entries default to 255 ("unused")
♻️ Duplicate comments (4)
platformio_override.sample.ini (3)
585-602: Duplicate environment name [env:esp32dev_hub75] – will collide when copiedThere are two active envs with the same name. Rename or comment this example block to avoid conflicts.
Apply this diff:
-[env:esp32dev_hub75] +[env:example_esp32dev_hub75]
604-620: Sample env duplicates real env and mismatched board id
- Duplicate env name: second [env:adafruit_matrixportal_esp32s3] conflicts with the earlier one; make it an example.
- The env name implies Adafruit MatrixPortal S3 but board is lolin_s3_mini. Consider renaming the env or switching to the official board ID for clarity.
Apply this diff:
-[env:adafruit_matrixportal_esp32s3] +[env:example_adafruit_matrixportal_esp32s3] @@ -build_flags = ${common.build_flags} ${esp32s3.build_flags} -D WLED_RELEASE_NAME=ESP32-S3_4M_qspi +build_flags = ${common.build_flags} ${esp32s3.build_flags} -D WLED_RELEASE_NAME=\"ESP32-S3_4M_qspi\"
545-556: Env name vs board mismatch can confuse usersEnv is named adafruit_matrixportal_esp32s3 but uses board = lolin_s3_mini. Align the name or board to reduce confusion for users.
Suggested tweak:
- Either rename to [env:lolin_s3_mini_hub75], or
- Use the official PlatformIO board id for MatrixPortal S3 if available.
wled00/bus_manager.cpp (1)
820-845: VirtualMatrixPanel constructed with uninitialized 'display' (fatal UB)virtualDisp is created using (*display) before 'display' is allocated. Instantiate MatrixPanel_I2S_DMA first, then create VirtualMatrixPanel.
Apply this diff to defer VirtualMatrixPanel creation until after 'display' is created:
- if (bc.type == TYPE_HUB75MATRIX_HS) { - mxconfig.mx_width = min((u_int8_t) 64, bc.pins[0]); - mxconfig.mx_height = min((u_int8_t) 64, bc.pins[1]); - if(bc.pins[2] > 1 && bc.pins[3] > 0 && bc.pins[4]) { - virtualDisp = new VirtualMatrixPanel((*display), bc.pins[3], bc.pins[4], mxconfig.mx_width, mxconfig.mx_height, CHAIN_BOTTOM_LEFT_UP); - } - } else if (bc.type == TYPE_HUB75MATRIX_QS) { - mxconfig.mx_width = min((u_int8_t) 64, bc.pins[0]) * 2; - mxconfig.mx_height = min((u_int8_t) 64, bc.pins[1]) / 2; - virtualDisp = new VirtualMatrixPanel((*display), 1, 1, bc.pins[0], bc.pins[1]); - virtualDisp->setRotation(0); - switch(bc.pins[1]) { - case 16: - virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_16PX_HIGH); - break; - case 32: - virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_32PX_HIGH); - break; - case 64: - virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_64PX_HIGH); - break; - default: - DEBUGBUS_PRINTLN("Unsupported height"); - return; - } - } else { + if (bc.type == TYPE_HUB75MATRIX_HS) { + mxconfig.mx_width = min((uint8_t)64, bc.pins[0]); + mxconfig.mx_height = min((uint8_t)64, bc.pins[1]); + } else if (bc.type == TYPE_HUB75MATRIX_QS) { + mxconfig.mx_width = min((uint8_t)64, bc.pins[0]) * 2; + mxconfig.mx_height = min((uint8_t)64, bc.pins[1]) / 2; + } else { DEBUGBUS_PRINTLN("Unknown type"); return; }…and then, right after successfully creating 'display' (after the new MatrixPanel_I2S_DMA(mxconfig) block), add:
+ // Create virtual panel wrapper if requested + if (bc.type == TYPE_HUB75MATRIX_HS) { + if (bc.pins[2] > 1 && bc.pins[3] > 0 && bc.pins[4]) { + virtualDisp = new VirtualMatrixPanel(*display, bc.pins[3], bc.pins[4], + mxconfig.mx_width, mxconfig.mx_height, + CHAIN_BOTTOM_LEFT_UP); + } + } else if (bc.type == TYPE_HUB75MATRIX_QS) { + virtualDisp = new VirtualMatrixPanel(*display, 1, 1, bc.pins[0], bc.pins[1]); + virtualDisp->setRotation(0); + switch (bc.pins[1]) { + case 16: virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_16PX_HIGH); break; + case 32: virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_32PX_HIGH); break; + case 64: virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_64PX_HIGH); break; + default: DEBUGBUS_PRINTLN("Unsupported height"); return; + } + }
🧹 Nitpick comments (6)
wled00/bus_manager.h (2)
5-11: Avoid heavy FastLED include in headerOnly a pointer to CRGB is needed here; forward-declare to reduce compile-time and coupling. Include FastLED.h in the .cpp instead.
Apply this diff:
-#include <FastLED.h> +struct CRGB; // forward declaration, include FastLED.h in .cpp
416-438: Ensure BusConfig.pins trailing entries are initialized to 0xFFIf callers don't fill all OUTPUT_MAX_PINS entries, trailing zeros may slip through. Initialize to 0xFF before copying.
Apply this diff:
{ refreshReq = (bool) GET_BIT(busType,7); type = busType & 0x7F; // bit 7 may be/is hacked to include refresh info (1=refresh in off state, 0=no refresh) - size_t nPins = OUTPUT_MAX_PINS; - for (size_t i = 0; i < nPins; i++) pins[i] = ppins[i]; + memset(pins, 0xFF, sizeof(pins)); + size_t nPins = OUTPUT_MAX_PINS; + for (size_t i = 0; i < nPins; i++) pins[i] = ppins[i];wled00/bus_manager.cpp (4)
37-43: getBitFromArray lacks nullptr/bounds guardAdd a simple nullptr check to avoid UB if called defensively; callers already constrain indices, so keep it lightweight.
Apply this diff:
-bool getBitFromArray(const uint8_t* byteArray, size_t position) { // get bit value +bool getBitFromArray(const uint8_t* byteArray, size_t position) { // get bit value + if (!byteArray) return false; size_t byteIndex = position / 8; unsigned bitIndex = position % 8; uint8_t byteValue = byteArray[byteIndex]; return (byteValue >> bitIndex) & 1; }
45-52: setBitInArray has commented-out null checkUncomment the guard to prevent accidental writes on null buffers.
Apply this diff:
-void setBitInArray(uint8_t* byteArray, size_t position, bool value) { // set bit - with error handling for nullptr - //if (byteArray == nullptr) return; +void setBitInArray(uint8_t* byteArray, size_t position, bool value) { // set bit - with error handling for nullptr + if (byteArray == nullptr) return;
821-828: Prefer uint8_t over u_int8_t for portabilityKeep integer types consistent with the codebase and Arduino headers.
Apply this diff:
- mxconfig.mx_width = min((u_int8_t) 64, bc.pins[0]); - mxconfig.mx_height = min((u_int8_t) 64, bc.pins[1]); + mxconfig.mx_width = min((uint8_t)64, bc.pins[0]); + mxconfig.mx_height = min((uint8_t)64, bc.pins[1]); @@ - mxconfig.mx_width = min((u_int8_t) 64, bc.pins[0]) * 2; - mxconfig.mx_height = min((u_int8_t) 64, bc.pins[1]) / 2; + mxconfig.mx_width = min((uint8_t)64, bc.pins[0]) * 2; + mxconfig.mx_height = min((uint8_t)64, bc.pins[1]) / 2; @@ - mxconfig.chain_length = max((u_int8_t) 1, min(bc.pins[2], (u_int8_t) 4)); // prevent bad data preventing boot due to low memory + mxconfig.chain_length = max((uint8_t)1, min(bc.pins[2], (uint8_t)4)); // prevent bad data preventing boot due to low memoryAlso applies to: 858-858
1057-1066: Use setBrightness8 for library compatibility (or verify API)Library commonly exposes setBrightness8(). Using setBrightness() may fail on some versions. Either switch to setBrightness8 or confirm both exist across pinned commits.
Apply this diff:
- display->setBrightness(_bri); + display->setBrightness8(_bri); @@ - display->setBrightness(_bri); + display->setBrightness8(_bri);Would you like me to scan the pinned library versions to confirm method names?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
platformio_override.sample.ini(3 hunks)wled00/bus_manager.cpp(4 hunks)wled00/bus_manager.h(5 hunks)wled00/cfg.cpp(1 hunks)wled00/const.h(1 hunks)wled00/data/settings_leds.htm(6 hunks)wled00/set.cpp(1 hunks)wled00/wled.h(1 hunks)wled00/xml.cpp(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- wled00/const.h
- wled00/wled.h
- wled00/data/settings_leds.htm
- wled00/cfg.cpp
- wled00/xml.cpp
🧰 Additional context used
📓 Path-based instructions (1)
wled00/{*.cpp,!(html_*)*.h}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use spaces (2 per level) for C++ source and header files
Files:
wled00/set.cppwled00/bus_manager.cppwled00/bus_manager.h
🧠 Learnings (2)
📚 Learning: 2025-08-28T08:09:20.588Z
Learnt from: mval-sg
PR: wled/WLED#4876
File: wled00/xml.cpp:0-0
Timestamp: 2025-08-28T08:09:20.588Z
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/set.cppwled00/bus_manager.cpp
📚 Learning: 2025-08-29T00:26:15.770Z
Learnt from: ksedgwic
PR: wled/WLED#4883
File: usermods/usermod_v2_skystrip/rest_json_client.h:6-14
Timestamp: 2025-08-29T00:26:15.770Z
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/bus_manager.cpp
🧬 Code graph analysis (2)
wled00/bus_manager.cpp (1)
wled00/bus_manager.h (7)
BusHub75Matrix(375-403)show(247-253)show(306-307)show(333-334)show(354-358)show(380-388)isHub75(192-192)
wled00/bus_manager.h (1)
wled00/bus_manager.cpp (68)
BusHub75Matrix(801-1013)setPixelColor(293-330)setPixelColor(293-293)setPixelColor(488-519)setPixelColor(488-488)setPixelColor(666-671)setPixelColor(666-666)setPixelColor(728-737)setPixelColor(728-728)setPixelColor(1015-1015)setPixelColor(1343-1348)setPixelColor(1343-1343)getPixelColor(333-354)getPixelColor(333-333)getPixelColor(522-540)getPixelColor(522-522)getPixelColor(673-676)getPixelColor(673-673)getPixelColor(739-743)getPixelColor(739-739)getPixelColor(1049-1055)getPixelColor(1049-1049)getPixelColor(1359-1365)getPixelColor(1359-1359)show(274-277)show(274-274)show(542-609)show(542-542)show(678-681)show(678-678)show(745-750)show(745-745)show(1062-1098)show(1062-1062)show(1336-1341)show(1336-1336)setBrightness(1057-1060)setBrightness(1057-1057)getPins(356-360)getPins(356-356)getPins(611-616)getPins(611-611)getPins(683-687)getPins(683-683)getPins(752-755)getPins(752-752)getPins(1128-1133)getPins(1128-1128)deallocatePins(630-644)deallocatePins(630-630)deallocatePins(1115-1119)deallocatePins(1115-1115)cleanup(403-411)cleanup(403-403)cleanup(785-791)cleanup(785-785)cleanup(1100-1113)cleanup(1100-1100)getLEDTypes(373-396)getLEDTypes(373-373)getLEDTypes(619-628)getLEDTypes(619-619)getLEDTypes(690-694)getLEDTypes(690-690)getLEDTypes(771-783)getLEDTypes(771-771)getLEDTypes(1121-1126)getLEDTypes(1121-1121)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: wled_build / Build Environments (esp32dev_hub75_forum_pinout)
- GitHub Check: wled_build / Build Environments (esp32dev_hub75)
- GitHub Check: wled_build / Build Environments (esp32S3_PSRAM_HUB75)
- GitHub Check: wled_build / Build Environments (adafruit_matrixportal_esp32s3)
🔇 Additional comments (1)
wled00/bus_manager.h (1)
192-193: Add HUB75 type helper looks good
|
The main issue I am aware of at the moment is while the value you enter for the panel width is saved and applied, it's not loaded back properly for the settings page so always shows as empty and has to be re-entered every time |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (4)
wled00/bus_manager.cpp (4)
820-845: Fatal: display is dereferenced before it is initialized.virtualDisp is constructed with (*display) while display is still null. Instantiate display first, then create VirtualMatrixPanel.
if (bc.type == TYPE_HUB75MATRIX_HS) { - mxconfig.mx_width = min((u_int8_t) 64, bc.pins[0]); - mxconfig.mx_height = min((u_int8_t) 64, bc.pins[1]); - if(bc.pins[2] > 1 && bc.pins[3] > 0 && bc.pins[4]) { - virtualDisp = new VirtualMatrixPanel((*display), bc.pins[3], bc.pins[4], mxconfig.mx_width, mxconfig.mx_height, CHAIN_BOTTOM_LEFT_UP); - } + mxconfig.mx_width = min((uint8_t)64, bc.pins[0]); + mxconfig.mx_height = min((uint8_t)64, bc.pins[1]); } else if (bc.type == TYPE_HUB75MATRIX_QS) { - mxconfig.mx_width = min((u_int8_t) 64, bc.pins[0]) * 2; - mxconfig.mx_height = min((u_int8_t) 64, bc.pins[1]) / 2; - virtualDisp = new VirtualMatrixPanel((*display), 1, 1, bc.pins[0], bc.pins[1]); - virtualDisp->setRotation(0); - switch(bc.pins[1]) { - case 16: - virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_16PX_HIGH); - break; - case 32: - virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_32PX_HIGH); - break; - case 64: - virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_64PX_HIGH); - break; - default: - DEBUGBUS_PRINTLN("Unsupported height"); - return; - } + mxconfig.mx_width = min((uint8_t)64, bc.pins[0]) * 2; + mxconfig.mx_height = min((uint8_t)64, bc.pins[1]) / 2; } else { DEBUGBUS_PRINTLN("Unknown type"); return; }Place the VirtualMatrixPanel construction after successful display->begin(), before caching _panelWidth:
// insert after setting _valid = true, before computing _panelWidth if (bc.type == TYPE_HUB75MATRIX_HS) { if (bc.pins[2] > 1 && bc.pins[3] > 0 && bc.pins[4] > 0) { virtualDisp = new VirtualMatrixPanel(*display, bc.pins[3], bc.pins[4], mxconfig.mx_width, mxconfig.mx_height, CHAIN_BOTTOM_LEFT_UP); } } else if (bc.type == TYPE_HUB75MATRIX_QS) { virtualDisp = new VirtualMatrixPanel(*display, 1, 1, bc.pins[0], bc.pins[1]); virtualDisp->setRotation(0); switch (bc.pins[1]) { case 16: virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_16PX_HIGH); break; case 32: virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_32PX_HIGH); break; case 64: virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_64PX_HIGH); break; default: DEBUGBUS_PRINTLN("Unsupported height"); return; } }
850-856: mxconfig.chain_length used before initialization. Reorder.Compute chain_length first, then decide bit‑depth based on it.
-#if defined(CONFIG_IDF_TARGET_ESP32) || defined(CONFIG_IDF_TARGET_ESP32S2)// classic esp32, or esp32-s2: reduce bitdepth for large panels - if (mxconfig.mx_height >= 64) { - if (mxconfig.chain_length * mxconfig.mx_width > 192) mxconfig.setPixelColorDepthBits(3); - else if (mxconfig.chain_length * mxconfig.mx_width > 64) mxconfig.setPixelColorDepthBits(4); - else mxconfig.setPixelColorDepthBits(8); - } else mxconfig.setPixelColorDepthBits(8); -#endif - - mxconfig.chain_length = max((u_int8_t) 1, min(bc.pins[2], (u_int8_t) 4)); // prevent bad data preventing boot due to low memory + mxconfig.chain_length = max((uint8_t)1, min(bc.pins[2], (uint8_t)4)); // constrain chain length +#if defined(CONFIG_IDF_TARGET_ESP32) || defined(CONFIG_IDF_TARGET_ESP32S2) + if (mxconfig.mx_height >= 64) { + if (mxconfig.chain_length * mxconfig.mx_width > 192) mxconfig.setPixelColorDepthBits(3); + else if (mxconfig.chain_length * mxconfig.mx_width > 64) mxconfig.setPixelColorDepthBits(4); + else mxconfig.setPixelColorDepthBits(8); + } else { + mxconfig.setPixelColorDepthBits(8); + } +#endifAlso applies to: 858-858
35-42: Add nullptr guard to avoid UB in getBitFromArray().Defensive check prevents accidental deref when called from other modules.
bool getBitFromArray(const uint8_t* byteArray, size_t position) { // get bit value + if (!byteArray) return false; size_t byteIndex = position / 8; unsigned bitIndex = position % 8; uint8_t byteValue = byteArray[byteIndex]; return (byteValue >> bitIndex) & 1; }
44-52: Enforce the nullptr check in setBitInArray().Comment says “with error handling for nullptr” but the check is commented out.
void setBitInArray(uint8_t* byteArray, size_t position, bool value) { // set bit - with error handling for nullptr - //if (byteArray == nullptr) return; + if (!byteArray) return; size_t byteIndex = position / 8; unsigned bitIndex = position % 8; if (value) byteArray[byteIndex] |= (1 << bitIndex); else byteArray[byteIndex] &= ~(1 << bitIndex); }
🧹 Nitpick comments (1)
wled00/bus_manager.cpp (1)
801-807: Minor: prefer uint8_t over non‑standard u_int8_t.Use standard fixed-width types for portability.
- mxconfig.double_buff = false; // Use our own memory-optimised buffer rather than the driver's own double-buffer + mxconfig.double_buff = false; // Use our own memory-optimised buffer rather than the driver's own double-buffer(Apply similarly to all casts in this file: (u_int8_t) -> (uint8_t).)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wled00/bus_manager.cpp(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/*.{cpp,h}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use spaces (2 per level) for C++ firmware files
Files:
wled00/bus_manager.cpp
🧠 Learnings (2)
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
PR: wled/WLED#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/bus_manager.cpp
📚 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:
wled00/bus_manager.cpp
🧬 Code graph analysis (1)
wled00/bus_manager.cpp (2)
wled00/bus_manager.h (7)
BusHub75Matrix(375-403)show(247-253)show(306-307)show(333-334)show(354-358)show(380-388)isHub75(192-192)wled00/pin_manager.cpp (8)
allocateMultiplePins(92-140)allocateMultiplePins(92-92)allocateMultiplePins(142-146)allocateMultiplePins(142-142)deallocateMultiplePins(45-83)deallocateMultiplePins(45-45)deallocateMultiplePins(85-90)deallocateMultiplePins(85-85)
🔇 Additional comments (3)
wled00/bus_manager.cpp (3)
1128-1135: LGTM: getPins now null-guards output buffer.Matches other buses; avoids nullptr writes.
1195-1198: LGTM: HUB75 bus construction gated and integrated.Nice conditional integration via WLED_ENABLE_HUB75MATRIX.
1231-1232: LGTM: HUB75 LED types appended conditionally.Keeps JSON minimal when feature is disabled.
| if(this->_len >= MAX_LEDS) { | ||
| DEBUGBUS_PRINTLN("MatrixPanel_I2S_DMA Too many LEDS - playing safe"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Clean up on MAX_LEDS overflow path.
Currently returns without releasing resources.
if(this->_len >= MAX_LEDS) {
DEBUGBUS_PRINTLN("MatrixPanel_I2S_DMA Too many LEDS - playing safe");
- return;
+ delete display; display = nullptr;
+ deallocatePins();
+ return;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In wled00/bus_manager.cpp around lines 951 to 954, the early return on exceeding
MAX_LEDS skips necessary cleanup; modify this overflow path to release any
allocated resources and reset state before returning — e.g., stop/disable DMA or
transmission, free or null out any frame buffers or dynamically allocated
memory, release any held locks or semaphores, and set relevant state variables
(such as _len or buffer pointers) to safe values so the object remains
consistent after the error return.
| if (_ledsDirty == nullptr) { | ||
| display->stopDMAoutput(); | ||
| delete display; display = nullptr; | ||
| _valid = false; | ||
| DEBUGBUS_PRINTLN(F("MatrixPanel_I2S_DMA not started - not enough memory for dirty bits!")); | ||
| DEBUGBUS_PRINT(F("heap usage: ")); DEBUGBUS_PRINTLN(lastHeap - ESP.getFreeHeap()); | ||
| return; // fail is we cannot get memory for the buffer | ||
| } |
There was a problem hiding this comment.
Also deallocate pins when dirty-bits allocation fails.
Pins stay reserved on this error path.
if (_ledsDirty == nullptr) {
display->stopDMAoutput();
delete display; display = nullptr;
_valid = false;
DEBUGBUS_PRINTLN(F("MatrixPanel_I2S_DMA not started - not enough memory for dirty bits!"));
DEBUGBUS_PRINT(F("heap usage: ")); DEBUGBUS_PRINTLN(lastHeap - ESP.getFreeHeap());
+ deallocatePins();
return; // fail is we cannot get memory for the buffer
}🤖 Prompt for AI Agents
In wled00/bus_manager.cpp around lines 982 to 989, the error path that handles
failure to allocate _ledsDirty currently stops DMA, deletes the display and
marks _valid false but does not release reserved pins; update this block to
deallocate/unreserve any GPIOs or pin reservations acquired earlier (e.g., call
the routine that releases pins or explicitly set pinMode to INPUT/disable
PWM/stop I2S and free any pin tracking structures) before returning, ensuring
all resources reserved for the display are cleaned up on allocation failure.
There was a problem hiding this comment.
no need to release the pins as it's not like you can reuse them for any other purpose if they are connected to the hub75 panel
There was a problem hiding this comment.
@netmindz You make a good point about the hardware reality - pins physically wired to a HUB75 panel indeed can't be meaningfully reused for other purposes during runtime.
From a software resource management perspective, there's still some value in keeping the PinManager's internal state consistent (avoiding scenarios where pins are marked "allocated" but the bus failed to initialize), but I understand that in embedded systems with fixed hardware configurations like this, the practical impact is minimal.
Your approach prioritizes simplicity and acknowledges the physical constraints of the hardware setup, which is a reasonable design decision for this use case.
✏️ Learnings added
Learnt from: netmindz
PR: wled/WLED#3777
File: wled00/bus_manager.cpp:982-989
Timestamp: 2025-09-13T13:13:36.069Z
Learning: In the WLED HUB75 implementation, when HUB75 matrix initialization fails after pin allocation, pins are intentionally not deallocated because they're physically connected to the HUB75 panel and cannot be reused for other purposes. This design choice prioritizes simplicity over strict software resource management consistency.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| void BusHub75Matrix::cleanup() { | ||
| if (display && _valid) display->stopDMAoutput(); // terminate DMA driver (display goes black) | ||
| _valid = false; | ||
| _panelWidth = 0; | ||
| deallocatePins(); | ||
| DEBUGBUS_PRINTLN("HUB75 output ended."); | ||
|
|
||
| //if (virtualDisp != nullptr) delete virtualDisp; // warning: deleting object of polymorphic class type 'VirtualMatrixPanel' which has non-virtual destructor might cause undefined behavior | ||
| delete display; | ||
| display = nullptr; | ||
| virtualDisp = nullptr; | ||
| if (_ledBuffer != nullptr) free(_ledBuffer); _ledBuffer = nullptr; | ||
| if (_ledsDirty != nullptr) free(_ledsDirty); _ledsDirty = nullptr; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Leak: virtualDisp is never deleted.
Deleting a VirtualMatrixPanel via its own type is safe. Free it in cleanup().
void BusHub75Matrix::cleanup() {
if (display && _valid) display->stopDMAoutput(); // terminate DMA driver (display goes black)
_valid = false;
_panelWidth = 0;
deallocatePins();
DEBUGBUS_PRINTLN("HUB75 output ended.");
- //if (virtualDisp != nullptr) delete virtualDisp; // warning: deleting object of polymorphic class type 'VirtualMatrixPanel' which has non-virtual destructor might cause undefined behavior
+ if (virtualDisp) delete virtualDisp;
delete display;
display = nullptr;
virtualDisp = nullptr;
if (_ledBuffer != nullptr) free(_ledBuffer); _ledBuffer = nullptr;
if (_ledsDirty != nullptr) free(_ledsDirty); _ledsDirty = nullptr;
}📝 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.
| void BusHub75Matrix::cleanup() { | |
| if (display && _valid) display->stopDMAoutput(); // terminate DMA driver (display goes black) | |
| _valid = false; | |
| _panelWidth = 0; | |
| deallocatePins(); | |
| DEBUGBUS_PRINTLN("HUB75 output ended."); | |
| //if (virtualDisp != nullptr) delete virtualDisp; // warning: deleting object of polymorphic class type 'VirtualMatrixPanel' which has non-virtual destructor might cause undefined behavior | |
| delete display; | |
| display = nullptr; | |
| virtualDisp = nullptr; | |
| if (_ledBuffer != nullptr) free(_ledBuffer); _ledBuffer = nullptr; | |
| if (_ledsDirty != nullptr) free(_ledsDirty); _ledsDirty = nullptr; | |
| } | |
| void BusHub75Matrix::cleanup() { | |
| if (display && _valid) display->stopDMAoutput(); // terminate DMA driver (display goes black) | |
| _valid = false; | |
| _panelWidth = 0; | |
| deallocatePins(); | |
| DEBUGBUS_PRINTLN("HUB75 output ended."); | |
| if (virtualDisp) delete virtualDisp; | |
| delete display; | |
| display = nullptr; | |
| virtualDisp = nullptr; | |
| if (_ledBuffer != nullptr) free(_ledBuffer); _ledBuffer = nullptr; | |
| if (_ledsDirty != nullptr) free(_ledsDirty); _ledsDirty = nullptr; | |
| } |
🤖 Prompt for AI Agents
In wled00/bus_manager.cpp around lines 1100 to 1113, virtualDisp is never
deleted causing a memory leak; update cleanup() to check if virtualDisp is
non-null, delete it, and then set it to nullptr (e.g., if (virtualDisp) { delete
virtualDisp; virtualDisp = nullptr; }) so the allocated VirtualMatrixPanel is
freed safely before leaving cleanup().
There was a problem hiding this comment.
Pull Request Overview
This pull request adds comprehensive support for driving HUB75-compatible LED matrix panels in WLED, providing an affordable entry point for 2D LED effects. The implementation includes specialized driver integration, GPIO validation improvements, and build configurations for multiple ESP32 variants.
Key Changes
- Added HUB75 matrix panel driver support with optimized memory management and pixel handling
- Increased internal pin capacity from 5 to OUTPUT_MAX_PINS for future extensibility
- Added build presets and board support for MatrixPortal, MOONHUB, and forum-style pinouts
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| wled00/const.h | Defines HUB75 LED types and increases OUTPUT_MAX_PINS constant |
| wled00/bus_manager.h | Adds BusHub75Matrix class and isHub75() type checking |
| wled00/bus_manager.cpp | Implements complete HUB75 driver with pixel buffer optimization |
| wled00/data/settings_leds.htm | Updates web UI to handle HUB75 panel configuration |
| wled00/pin_manager.h | Adds HUB75 pin owner type and multi-pin allocation method |
| wled00/xml.cpp, wled00/set.cpp, wled00/cfg.cpp | Updates pin arrays to use OUTPUT_MAX_PINS |
| platformio_override.sample.ini | Provides HUB75 build examples for various ESP32 boards |
| if (bc.type == TYPE_HUB75MATRIX_HS) { | ||
| mxconfig.mx_width = min((u_int8_t) 64, bc.pins[0]); | ||
| mxconfig.mx_height = min((u_int8_t) 64, bc.pins[1]); | ||
| if(bc.pins[2] > 1 && bc.pins[3] > 0 && bc.pins[4]) { |
There was a problem hiding this comment.
The condition checking bc.pins[4] will always be true for non-zero values, but should probably check bc.pins[4] > 0 to be consistent with the other conditions and handle the case where pins[4] is 0.
| if(bc.pins[2] > 1 && bc.pins[3] > 0 && bc.pins[4]) { | |
| if(bc.pins[2] > 1 && bc.pins[3] > 0 && bc.pins[4] > 0) { |
| gId("off"+n).innerText = off; | ||
| // secondary pins show/hide (type string length is equivalent to number of pins used; except for network and on/off) | ||
| let pins = Math.max(gT(t).t.length,1) + 3*isNet(t); // fixes network pins to 4 | ||
| let pins = Math.max(gT(t).t.length,1) + 3*isNet(t) + 2*isHub75(t); // fixes network pins to 4 |
There was a problem hiding this comment.
The magic number 2 for HUB75 pins should be defined as a constant or have a comment explaining why HUB75 requires 2 additional pins beyond the base type length.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wled00/bus_manager.h (1)
416-438: Critical: BusConfig copies too few “pins” for HUB75 meta-fieldsBusHub75Matrix consumes pins[0..4] (width, height, chain, virt-cols, virt-rows), but BusConfig copies only getNumberOfPins(type). For HUB75, getNumberOfPins() currently resolves to 1, so pins[1..4] retain defaults (often 255), causing wrong geometry, unintended VirtualMatrix creation, and crashes.
Copy at least 5 meta-values for HUB75.
- size_t nPins = Bus::getNumberOfPins(type); - for (size_t i = 0; i < nPins; i++) pins[i] = ppins[i]; + // HUB75 uses 5 meta fields in pins[]: W,H,chain,virtCols,virtRows + size_t nPins = Bus::isHub75(type) ? 5 : Bus::getNumberOfPins(type); + for (size_t i = 0; i < nPins && i < OUTPUT_MAX_PINS; i++) pins[i] = ppins[i];
♻️ Duplicate comments (5)
wled00/bus_manager.cpp (5)
820-845: Fatal: dereferencing ‘display’ before it is constructedvirtualDisp is created with (*display) while display is still nullptr. Move VirtualMatrixPanel creation until after MatrixPanel_I2S_DMA is allocated and begun. Keep the width/height selection here; defer only the VirtualMatrixPanel instantiation.
- if (bc.type == TYPE_HUB75MATRIX_HS) { - mxconfig.mx_width = min((u_int8_t) 64, bc.pins[0]); - mxconfig.mx_height = min((u_int8_t) 64, bc.pins[1]); - if(bc.pins[2] > 1 && bc.pins[3] > 0 && bc.pins[4]) { - virtualDisp = new VirtualMatrixPanel((*display), bc.pins[3], bc.pins[4], mxconfig.mx_width, mxconfig.mx_height, CHAIN_BOTTOM_LEFT_UP); - } - } else if (bc.type == TYPE_HUB75MATRIX_QS) { - mxconfig.mx_width = min((u_int8_t) 64, bc.pins[0]) * 2; - mxconfig.mx_height = min((u_int8_t) 64, bc.pins[1]) / 2; - virtualDisp = new VirtualMatrixPanel((*display), 1, 1, bc.pins[0], bc.pins[1]); - virtualDisp->setRotation(0); - switch(bc.pins[1]) { + if (bc.type == TYPE_HUB75MATRIX_HS) { + mxconfig.mx_width = min((uint8_t)64, bc.pins[0]); + mxconfig.mx_height = min((uint8_t)64, bc.pins[1]); + } else if (bc.type == TYPE_HUB75MATRIX_QS) { + mxconfig.mx_width = min((uint8_t)64, bc.pins[0]) * 2; + mxconfig.mx_height = min((uint8_t)64, bc.pins[1]) / 2; + switch (bc.pins[1]) { case 16: virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_16PX_HIGH); break; case 32: virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_32PX_HIGH); break; case 64: virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_64PX_HIGH); break; default: DEBUGBUS_PRINTLN("Unsupported height"); return; } } else { DEBUGBUS_PRINTLN("Unknown type"); return; }Follow-up: instantiate VirtualMatrixPanel after begin() as shown in the later diff.
1107-1113: Delete virtualDisp in cleanup()Leak on repeated init/cleanup. Deleting by concrete type is safe.
- //if (virtualDisp != nullptr) delete virtualDisp; // warning: deleting object of polymorphic class type 'VirtualMatrixPanel' which has non-virtual destructor might cause undefined behavior + if (virtualDisp) delete virtualDisp; delete display; display = nullptr; virtualDisp = nullptr;
850-864: chain_length used before initialization in bit-depth decisionSet chain_length first, then compute color depth. Also prefer uint8_t over u_int8_t.
-#if defined(CONFIG_IDF_TARGET_ESP32) || defined(CONFIG_IDF_TARGET_ESP32S2)// classic esp32, or esp32-s2: reduce bitdepth for large panels - if (mxconfig.mx_height >= 64) { - if (mxconfig.chain_length * mxconfig.mx_width > 192) mxconfig.setPixelColorDepthBits(3); - else if (mxconfig.chain_length * mxconfig.mx_width > 64) mxconfig.setPixelColorDepthBits(4); - else mxconfig.setPixelColorDepthBits(8); - } else mxconfig.setPixelColorDepthBits(8); -#endif - - mxconfig.chain_length = max((u_int8_t) 1, min(bc.pins[2], (u_int8_t) 4)); // prevent bad data preventing boot due to low memory + mxconfig.chain_length = max((uint8_t)1, min(bc.pins[2], (uint8_t)4)); // constrain chain length +#if defined(CONFIG_IDF_TARGET_ESP32) || defined(CONFIG_IDF_TARGET_ESP32S2) // classic esp32, or esp32-s2 + if (mxconfig.mx_height >= 64) { + if (mxconfig.chain_length * mxconfig.mx_width > 192) mxconfig.setPixelColorDepthBits(3); + else if (mxconfig.chain_length * mxconfig.mx_width > 64) mxconfig.setPixelColorDepthBits(4); + else mxconfig.setPixelColorDepthBits(8); + } else { + mxconfig.setPixelColorDepthBits(8); + } +#endif
960-969: Also free driver on begin() failureMemory leak if begin() fails.
if( not display->begin() ) { DEBUGBUS_PRINTLN("****** MatrixPanel_I2S_DMA !KABOOM! I2S memory allocation failed ***********"); DEBUGBUS_PRINT(F("heap usage: ")); DEBUGBUS_PRINTLN(lastHeap - ESP.getFreeHeap()); - return; + delete display; display = nullptr; + return; } else {
969-1004: Instantiate VirtualMatrixPanel after successful begin()Create virtualDisp only after display is valid.
else { DEBUGBUS_PRINTLN("MatrixPanel_I2S_DMA begin ok"); DEBUGBUS_PRINT(F("heap usage: ")); DEBUGBUS_PRINTLN(lastHeap - ESP.getFreeHeap()); delay(18); // experiment - give the driver a moment (~ one full frame @ 60hz) to settle _valid = true; display->clearScreen(); // initially clear the screen buffer DEBUGBUS_PRINTLN("MatrixPanel_I2S_DMA clear ok"); + // Defer VirtualMatrixPanel creation until display is valid + if (bc.type == TYPE_HUB75MATRIX_HS) { + if (bc.pins[2] > 1 && bc.pins[3] > 0 && bc.pins[4] > 0) { + virtualDisp = new VirtualMatrixPanel(*display, bc.pins[3], bc.pins[4], + mxconfig.mx_width, mxconfig.mx_height, + CHAIN_BOTTOM_LEFT_UP); + } + } else if (bc.type == TYPE_HUB75MATRIX_QS) { + virtualDisp = new VirtualMatrixPanel(*display, 1, 1, bc.pins[0], bc.pins[1]); + virtualDisp->setRotation(0); + switch (bc.pins[1]) { + case 16: virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_16PX_HIGH); break; + case 32: virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_32PX_HIGH); break; + case 64: virtualDisp->setPhysicalPanelScanRate(FOUR_SCAN_64PX_HIGH); break; + default: DEBUGBUS_PRINTLN("Unsupported height"); _valid = false; return; + } + }
🧹 Nitpick comments (9)
platformio_override.sample.ini (3)
555-591: Align HUB75 DMA library version across envsesp32dev_hub75 pins to 3.0.11 while S3 envs pin to aa28e2a. For reproducible testing and fewer cross-env surprises, use the same known-good ref (prefer the S3 fix commit if it also works on ESP32), or add a short comment explaining the split.
- https://github.com/mrfaptastic/ESP32-HUB75-MatrixPanel-DMA.git#3.0.11 + # NOTE: align with S3 env for consistency (fix includes S3_LCD_DIV_NUM) + https://github.com/mrfaptastic/ESP32-HUB75-MatrixPanel-DMA.git#aa28e2a
592-621: Consider larger partition scheme for HUB75 buildsMatrices plus PSRAM-heavy libs can make OTA tight. Consider extended/big partitions on HUB75 envs (esp32dev_hub75, S3 variants), or document when to switch.
-board_build.partitions = ${esp32.default_partitions} +# HUB75 builds can benefit from more app space; switch if you hit space errors +board_build.partitions = ${esp32.default_partitions} ; or ${esp32.extended_partitions}
570-587: S3_WIFI perf guardrailS3_LCD_DIV_NUM=20 is critical for Wi‑Fi when panel is active. Add a one-line comment on symptom/workaround to help users porting to other boards.
- -D S3_LCD_DIV_NUM=20 ;; Attempt to fix wifi performance issue when panel active with S3 chips + -D S3_LCD_DIV_NUM=20 ;; mitigates severe Wi-Fi throughput drops when HUB75 is active on ESP32-S3wled00/bus_manager.h (1)
397-403: Prefer constexpr for constant color sentinelsMake IS_BLACK/IS_DARKGREY static constexpr to keep them in flash and clarify immutability.
- uint32_t IS_BLACK = 0x000000; - uint32_t IS_DARKGREY = 0x333333; + static constexpr uint32_t IS_BLACK = 0x000000; + static constexpr uint32_t IS_DARKGREY = 0x333333;wled00/bus_manager.cpp (5)
35-63: Bit-array helpers: add null/bounds guards (low-cost safety)These run hot but a single nullptr guard prevents hard faults during error paths.
-bool getBitFromArray(const uint8_t* byteArray, size_t position) { // get bit value +bool getBitFromArray(const uint8_t* byteArray, size_t position) { // get bit value + if (!byteArray) return false; size_t byteIndex = position / 8; unsigned bitIndex = position % 8; uint8_t byteValue = byteArray[byteIndex]; return (byteValue >> bitIndex) & 1; } -void setBitInArray(uint8_t* byteArray, size_t position, bool value) { // set bit - with error handling for nullptr - //if (byteArray == nullptr) return; +void setBitInArray(uint8_t* byteArray, size_t position, bool value) { // set bit + if (!byteArray) return; size_t byteIndex = position / 8; unsigned bitIndex = position % 8;
821-829: Normalize to uint8_t castsUse standard uint8_t instead of u_int8_t for portability across cores/toolchains.
- mxconfig.mx_width = min((u_int8_t) 64, bc.pins[0]) * 2; - mxconfig.mx_height = min((u_int8_t) 64, bc.pins[1]) / 2; + mxconfig.mx_width = min((uint8_t)64, bc.pins[0]) * 2; + mxconfig.mx_height = min((uint8_t)64, bc.pins[1]) / 2;
979-995: Prefer PSRAM for large buffers on S3/PSRAM targetsUsing p_malloc/p_calloc reduces DRAM pressure. Consider allocating _ledBuffer/_ledsDirty via p_calloc/p_malloc where available.
Would you like a guarded patch that uses p_calloc/p_malloc when PSRAM is present and falls back otherwise?
1049-1055: getPixelColor semantics (direct-write mode)Returning DARKGREY/BLACK proxy diverges from BusDigital/NPB semantics (expected actual color). If acceptable as interim, please document; otherwise consider minimal buffering or caching recent color per pixel.
913-918: Note: Not deallocating pins on init failure is intentionalThe early return keeps HUB75 pins reserved; this matches the stated design (physically dedicated pins). Consider adding a one-line comment here to prevent “fix” churn.
if (!PinManager::allocateMultiplePins(pins, PIN_COUNT, PinOwner::HUB75, true)) { DEBUGBUS_PRINTLN("Failed to allocate pins for HUB75"); + // Pins are physically dedicated; do not deallocate on early failure by design. return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
platformio_override.sample.ini(3 hunks)wled00/bus_manager.cpp(4 hunks)wled00/bus_manager.h(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
wled00/**/*.{cpp,h}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use spaces (2 per level) for C++ firmware files
Files:
wled00/bus_manager.cppwled00/bus_manager.h
🧠 Learnings (5)
📓 Common learnings
Learnt from: netmindz
PR: wled/WLED#3777
File: wled00/bus_manager.cpp:982-989
Timestamp: 2025-09-13T13:13:36.069Z
Learning: In the WLED HUB75 implementation, when HUB75 matrix initialization fails after pin allocation, pins are intentionally not deallocated because they're physically connected to the HUB75 panel and cannot be reused for other purposes. This design choice prioritizes simplicity over strict software resource management consistency.
Learnt from: netmindz
PR: wled/WLED#3777
File: wled00/bus_manager.cpp:941-947
Timestamp: 2025-09-13T13:13:35.786Z
Learning: For HUB75 matrix panels in WLED, pins are physically dedicated to the matrix hardware and don't need to be deallocated on driver initialization failure since they cannot be reused for other purposes without hardware changes.
📚 Learning: 2025-09-13T13:13:36.069Z
Learnt from: netmindz
PR: wled/WLED#3777
File: wled00/bus_manager.cpp:982-989
Timestamp: 2025-09-13T13:13:36.069Z
Learning: In the WLED HUB75 implementation, when HUB75 matrix initialization fails after pin allocation, pins are intentionally not deallocated because they're physically connected to the HUB75 panel and cannot be reused for other purposes. This design choice prioritizes simplicity over strict software resource management consistency.
Applied to files:
wled00/bus_manager.cppwled00/bus_manager.h
📚 Learning: 2025-09-13T13:13:35.786Z
Learnt from: netmindz
PR: wled/WLED#3777
File: wled00/bus_manager.cpp:941-947
Timestamp: 2025-09-13T13:13:35.786Z
Learning: For HUB75 matrix panels in WLED, pins are physically dedicated to the matrix hardware and don't need to be deallocated on driver initialization failure since they cannot be reused for other purposes without hardware changes.
Applied to files:
wled00/bus_manager.cppwled00/bus_manager.h
📚 Learning: 2025-08-28T08:09:20.630Z
Learnt from: mval-sg
PR: wled/WLED#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/bus_manager.cpp
📚 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:
wled00/bus_manager.cpp
🧬 Code graph analysis (2)
wled00/bus_manager.cpp (2)
wled00/bus_manager.h (7)
BusHub75Matrix(375-403)show(247-253)show(306-307)show(333-334)show(354-358)show(380-388)isHub75(192-192)wled00/pin_manager.cpp (8)
allocateMultiplePins(92-140)allocateMultiplePins(92-92)allocateMultiplePins(142-146)allocateMultiplePins(142-142)deallocateMultiplePins(45-83)deallocateMultiplePins(45-45)deallocateMultiplePins(85-90)deallocateMultiplePins(85-85)
wled00/bus_manager.h (1)
wled00/bus_manager.cpp (59)
BusHub75Matrix(801-1013)hot(1015-1047)setPixelColor(293-330)setPixelColor(293-293)setPixelColor(488-519)setPixelColor(488-488)setPixelColor(666-671)setPixelColor(666-666)setPixelColor(728-737)setPixelColor(728-728)setPixelColor(1015-1015)setPixelColor(1345-1350)setPixelColor(1345-1345)getPixelColor(333-354)getPixelColor(333-333)getPixelColor(522-540)getPixelColor(522-522)getPixelColor(673-676)getPixelColor(673-673)getPixelColor(739-743)getPixelColor(739-739)getPixelColor(1049-1055)getPixelColor(1049-1049)getPixelColor(1361-1367)getPixelColor(1361-1361)show(274-277)show(274-274)show(542-609)show(542-542)show(678-681)show(678-678)show(745-750)show(745-745)show(1062-1098)show(1062-1062)show(1338-1343)show(1338-1338)setBrightness(1057-1060)setBrightness(1057-1057)deallocatePins(630-644)deallocatePins(630-630)deallocatePins(1115-1119)deallocatePins(1115-1115)cleanup(403-411)cleanup(403-403)cleanup(785-791)cleanup(785-785)cleanup(1100-1113)cleanup(1100-1100)getLEDTypes(373-396)getLEDTypes(373-373)getLEDTypes(619-628)getLEDTypes(619-619)getLEDTypes(690-694)getLEDTypes(690-690)getLEDTypes(771-783)getLEDTypes(771-771)getLEDTypes(1121-1126)getLEDTypes(1121-1121)
🔇 Additional comments (3)
wled00/bus_manager.h (2)
192-193: Classify HUB75 in helpers – good additionAdding isHub75(type) keeps type checks readable and localizes future HUB75 range changes.
374-404: Add virtual panel members behind compile guard – OKIncludes and members gated by WLED_ENABLE_HUB75MATRIX are correct; avoids pull-in when HUB75 is disabled.
wled00/bus_manager.cpp (1)
941-954: Free driver on early-return paths (keep pins reserved by design)On length overflow and begin() failure, display is leaked. Free it before returning; per design, do not deallocate HUB75 pins on init failure.
[Suggest_essential_refactor]display = new MatrixPanel_I2S_DMA(mxconfig); if (display == nullptr) { DEBUGBUS_PRINTLN("****** MatrixPanel_I2S_DMA !KABOOM! driver allocation failed ***********"); DEBUGBUS_PRINT(F("heap usage: ")); DEBUGBUS_PRINTLN(lastHeap - ESP.getFreeHeap()); return; } this->_len = (display->width() * display->height()); DEBUGBUS_PRINTF("Length: %u\n", _len); if(this->_len >= MAX_LEDS) { DEBUGBUS_PRINTLN("MatrixPanel_I2S_DMA Too many LEDS - playing safe"); + delete display; display = nullptr; return; }
|
am i missing something? How do I enable the HUB75 support. In the MM release it was a setting of the LED, but I cant find that setting in v16. Tried a couple of different f/w builds on esp32 |
|
@rhamblen see the sample environments in the platformio override example file |
|
thanks |
12 participants
Add support to drive HUB75-compatible LED matrix panels. It's a great way to get started with 2D effects in WLED at very low prices.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores