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! |
| [env:waveshare_esp32s3_32MB_hub75] | ||
| ;; Waveshare ESP32-S3-RGB-Matrix (memory_type: opi_opi); see https://docs.waveshare.com/ESP32-S3-RGB-Matrix | ||
| extends = env:esp32S3_wroom2_32MB | ||
| monitor_filters = esp32_exception_decoder |
|
To actually be included in a release it should also be referenced in platformio_release.ini.template If we are going to add support, that should really be for all features, not just the pinout. I think @troyhacks has support for audio Happy to merge as hub75 support only, then we have new issue for proper support of the board, which is when it would get added to the release bins |
@netmindz This was just initial support, to get the ball rolling for it. I'm trying to research getting the MICs working, along w/ SHTC3, IMU, RTC, and SD Card. The Build ENV I did has the build flags needed for the SD Card, just need to add the UM to the build flags for SD Card support. I plan on testing this soon, but not exactly sure what the SD Card would be used for when it comes to WLED. |
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