Adding support for Waveshare ESP32-S3-RGB-Matrix#5657
Conversation
WalkthroughAdds a PlatformIO environment ChangesWaveshare ESP32-S3 HUB75 Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@platformio.ini`:
- Line 661: Replace the misspelled release name value for the WLED_RELEASE_NAME
build flag: locate the build_flags line that defines -D WLED_RELEASE_NAME and
change the string literal from "ESP32-S3_Waveshave_Matrix" to the correct
"ESP32-S3_Waveshare_Matrix" so the release name spelling is fixed wherever
WLED_RELEASE_NAME is used.
- Line 659: The shared platformio.ini contains a machine-specific setting
upload_port = COM12 which should be removed from the shared config; delete or
comment out the upload_port entry in platformio.ini and place that setting into
platformio_override.ini (or instruct contributors to set their own upload_port
in a local override) so each developer/OS can define their own COM/serial port;
ensure you reference the upload_port key (and keep other shared settings
unchanged) and add a brief comment in platformio_override.ini indicating it is
for machine-specific overrides.
- Around line 652-680: The platformio.ini change touches the protected env
section "env:waveshare_esp32s3_32MB_hub75" and must not be merged without
explicit maintainer approval; before merging, request and record approval from a
WLED maintainer/org member for the platformio.ini modifications (or revert the
changes in the "env:waveshare_esp32s3_32MB_hub75" block until approval is
granted), and ensure the PR description or commit message includes the
approver's GitHub username and a short justification referencing the modified
symbols (e.g., build_flags and lib_deps entries) so reviewers can verify
approval.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9295dff2-bd04-4656-b31f-8af9f01bf741
📒 Files selected for processing (2)
platformio.iniwled00/bus_manager.cpp
| [env:waveshare_esp32s3_32MB_hub75] | ||
| extends = env:esp32S3_wroom2_32MB | ||
| board_build.partitions = ${esp32.extreme_partitions} | ||
| board_build.f_flash = 80000000L | ||
| board_upload.flash_size = 32MB | ||
| monitor_filters = esp32_exception_decoder | ||
| upload_speed = 115200 | ||
| upload_port = COM12 | ||
| build_unflags = ${common.build_unflags} | ||
| build_flags = ${common.build_flags} ${esp32s3.build_flags} -D WLED_RELEASE_NAME=\"ESP32-S3_Waveshave_Matrix\" | ||
| -D WLED_ENABLE_HUB75MATRIX | ||
| -D NO_GFX | ||
| -D NO_FAST_FUNCTIONS | ||
| -D NO_CIE1931 | ||
| -D S3_LCD_DIV_NUM=20 | ||
| -DARDUINO_USB_CDC_ON_BOOT=1 | ||
| -DBOARD_HAS_PSRAM | ||
| -DLOLIN_WIFI_FIX ; seems to work much better with this | ||
| -D WLED_WATCHDOG_TIMEOUT=0 | ||
| -D WAVESHARE_S3_PINOUT | ||
| ; -D WLED_USE_SD_SPI | ||
| ; -D SD_PRINT_HOME_DIR | ||
| ; -D WLED_DEBUG | ||
| -D USERMOD_AUDIOREACTIVE | ||
| -D SR_DMTYPE=0 -D I2S_SDPIN=-1 -D I2S_CKPIN=-1 -D I2S_WSPIN=-1 -D MCLK_PIN=-1 ;; Disable to prevent pin clash | ||
| lib_deps = ${esp32s3.lib_deps} | ||
| ${esp32.AR_lib_deps} | ||
| ${hub75.lib_deps} | ||
|
|
There was a problem hiding this comment.
Maintainer approval is required for this platformio.ini change.
This file is explicitly protected; please ensure a maintainer/WLED org member approves this PR before merge.
As per coding guidelines: “platformio.ini: Modifications to platformio.ini MUST be approved explicitly by a maintainer or WLED organisation Member; always flag such modifications”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@platformio.ini` around lines 652 - 680, The platformio.ini change touches the
protected env section "env:waveshare_esp32s3_32MB_hub75" and must not be merged
without explicit maintainer approval; before merging, request and record
approval from a WLED maintainer/org member for the platformio.ini modifications
(or revert the changes in the "env:waveshare_esp32s3_32MB_hub75" block until
approval is granted), and ensure the PR description or commit message includes
the approver's GitHub username and a short justification referencing the
modified symbols (e.g., build_flags and lib_deps entries) so reviewers can
verify approval.
There was a problem hiding this comment.
@netmindz the platformio buildenv basicially consists of extends = env:esp32S3_wroom2_32MB and -D WAVESHARE_S3_PINOUT.
I'm not sure if this should be in the main platformio.ini, or we move it into platformio_override.ini.sample? The board has a new HUB75 pinout and it seems to be based on a "wroom-2" S3, so it's somewhat special.
We've just managed to clean up the platformio.ini file, not sure if we should continue to populate it with whatever board comes along 🤔 . What do you think?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
Thanks for the PR and this new pinout. That platformio env is a mess however, it's overriding values unnecessarily, setting flags that are for previous versions of WLED etc. Please strip that back to only what is actually required |
Updated build flags for the waveshare_esp32s3_32MB_hub75 environment, removing some flags and adding new ones.
Removed flags for fast functions and CIE1931 from build configuration.
Removed upload speed and port settings for the waveshare_esp32s3_32MB_hub75 environment.
Removed unused build unflags.
* un-set previous RELEASE_NAME before setting a new one * align REALEASE_NAME with naming scheme used by other HUB75 builds
|
@Will-wastelander i've moved your new buildenv into the HUB75 section, and corrected a few problems - was faster to do it, rather than describing what I want. @netmindz we should still decide if we want the new buildenv in the main platformio.ini, or in platformio_override.sample.ini? |
|
@softhack007 Thanks for the assistance! |
Adding basic support for Waveshare ESP32-S3-RGB-Matrix
https://www.waveshare.com/esp32-s3-rgb-matrix.htm
https://docs.waveshare.com/ESP32-S3-RGB-Matrix
Matrix Panel is working. Microphone(s), SD card, and SHTC3 still need work.
Summary by CodeRabbit