ci: build HUB75 envs in release workflow#5558
Conversation
Move the four HUB75 envs from platformio_override.sample.ini into platformio.ini (refactored with a shared [hub75] section and extends= to deduplicate), and gate them behind a release-only config template so nightly/PR CI keep building only the standard env matrix. The release workflow now copies platformio_release.ini.template into place, which extends default_envs with the HUB75 envs.
WalkthroughThe PR adds HUB75 LED matrix display support across the build system and firmware. It introduces a release CI workflow that conditionally builds HUB75-specific board variants, defines five new PlatformIO environments with centralized HUB75 configuration, adds pinout handling for ESP32-S3 HUB75 boards, updates LED panel validation to permit multi-panel setups when PSRAM is available, and adjusts GPIO pin validation to skip HUB75 buses. ChangesHUB75 Display Support Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
This includes the latest version of the library 3.0.14 @DedeHai @softhack007 this does have the S3 fix Yes — aa28e2a ("Update gdma_lcd_parallel16.cpp") landed in 3.0.12 and is included in 3.0.13 and 3.0.14. Our pin (3.0.14, commit f17fb7fe) has the S3 fix. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (2)
.github/workflows/release.yml (1)
1-21:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd an explicit least-privilege
permissionsblock to this workflow.This release workflow currently relies on repository defaults for token scope.
As per coding guidelines: “Declare explicit permissions: blocks scoped to least privilege.”🔐 Minimal explicit permissions example
name: WLED Release CI +permissions: + contents: write🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 1 - 21, Add an explicit top-level permissions block (instead of relying on repo defaults) to this workflow to scope the GITHUB_TOKEN to least privilege; under the existing env section insert a permissions map granting only what's needed for creating releases and downstream jobs (for example: contents: write, packages: write, id-token: write, checks: read, actions: read) and keep other permissions unset or set to "none"; ensure the new block applies to both the wled_build and release jobs and adjust any job-specific overrides if present..github/workflows/build.yml (1)
1-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefine explicit
permissionsfor the reusable build workflow.The workflow currently has no scoped token permissions and inherits defaults.
As per coding guidelines: “Declare explicit permissions: blocks scoped to least privilege.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build.yml around lines 1 - 15, Add an explicit top-level permissions block to the reusable workflow (located under the "name: WLED Build" workflow definition) and scope it to least privilege for the build use-case — e.g., set permissions.contents to read and any other specific permissions required (packages or id-token) only if your downstream jobs need them (do not leave defaults). Ensure the new permissions block appears alongside the existing on: and env: sections so the workflow no longer inherits default token scopes.
🧹 Nitpick comments (1)
platformio_override.sample.ini (1)
194-199: ⚡ Quick winClarify the “opt-in” usage with an explicit
[env:*]snippet.The instruction “Add
${Speed_Flags.build_flags}/${Speed_Flags.build_unflags}to your own env” is a bit ambiguous about where/how to append them relative to existingbuild_flags/build_unflags. Adding a short concrete example will reduce misconfiguration risk (especially since most envs already setbuild_flags = ${common.build_flags} ...andbuild_unflags = ${common.build_unflags}).🛠️ Suggested edit
# ------------------------------------------------------------------------------ # Optional: build flags for speed, instead of optimising for size. -# Add ${Speed_Flags.build_flags} / ${Speed_Flags.build_unflags} to your own env -# in platformio_override.ini to opt in. +# To opt in for a specific env section, append these (example): +# build_unflags = ${common.build_unflags} ${Speed_Flags.build_unflags} +# build_flags = ${common.build_flags} ${esp8266.build_flags} ${Speed_Flags.build_flags} +# (Use ${esp32.build_flags} instead of ${esp8266.build_flags} for ESP32-based envs.) # ------------------------------------------------------------------------------🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platformio_override.sample.ini` around lines 194 - 199, Add a concrete opt-in example showing how to append ${Speed_Flags.build_flags} and ${Speed_Flags.build_unflags} inside an env section so users know where to place them: provide a short `[env:*]` snippet that demonstrates appending `${Speed_Flags.build_flags}` to an existing `build_flags` entry and `${Speed_Flags.build_unflags}` to an existing `build_unflags` entry (preserving any `${common.build_flags}` / `${common.build_unflags}` already present), and mention to put this in their platformio_override.ini to opt in; reference the `${Speed_Flags.build_flags}` / `${Speed_Flags.build_unflags}` tokens and the `build_flags` / `build_unflags` keys so reviewers can locate the intended replacement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platformio.ini`:
- Around line 39-41: This change touches global build config symbols
extra_configs and HUB75 shared sections/environments; per policy these must be
approved by a maintainer—either revert the edits to extra_configs and the HUB75
sections in this commit or obtain explicit maintainer/organization approval and
document that approval in the PR description; if approval is granted, add a
brief comment above the modified blocks referencing the approver (e.g.,
“approved by `@maintainer`”) and leave only the minimal intended changes to the
extra_configs/HUB75 sections so CI behavior is explicit.
In `@wled00/data/settings_leds.htm`:
- Line 42: The fetch handler sets hasPSRAM from the free-bytes field info.psram,
causing false negatives; change the assignment so hasPSRAM is derived from the
PSRAM capability/size field (e.g., info.psram_total, info.psram_size or a
boolean like info.has_psram / info.psram_capable) instead of info.psram > 0, and
keep the free-bytes check only as a fallback; ensure the HUB75 gating logic that
uses hasPSRAM (the check that rejects multi-panel HUB75 setups) now relies on
this capability field so PSRAM-capable boards aren’t erroneously rejected.
---
Outside diff comments:
In @.github/workflows/build.yml:
- Around line 1-15: Add an explicit top-level permissions block to the reusable
workflow (located under the "name: WLED Build" workflow definition) and scope it
to least privilege for the build use-case — e.g., set permissions.contents to
read and any other specific permissions required (packages or id-token) only if
your downstream jobs need them (do not leave defaults). Ensure the new
permissions block appears alongside the existing on: and env: sections so the
workflow no longer inherits default token scopes.
In @.github/workflows/release.yml:
- Around line 1-21: Add an explicit top-level permissions block (instead of
relying on repo defaults) to this workflow to scope the GITHUB_TOKEN to least
privilege; under the existing env section insert a permissions map granting only
what's needed for creating releases and downstream jobs (for example: contents:
write, packages: write, id-token: write, checks: read, actions: read) and keep
other permissions unset or set to "none"; ensure the new block applies to both
the wled_build and release jobs and adjust any job-specific overrides if
present.
---
Nitpick comments:
In `@platformio_override.sample.ini`:
- Around line 194-199: Add a concrete opt-in example showing how to append
${Speed_Flags.build_flags} and ${Speed_Flags.build_unflags} inside an env
section so users know where to place them: provide a short `[env:*]` snippet
that demonstrates appending `${Speed_Flags.build_flags}` to an existing
`build_flags` entry and `${Speed_Flags.build_unflags}` to an existing
`build_unflags` entry (preserving any `${common.build_flags}` /
`${common.build_unflags}` already present), and mention to put this in their
platformio_override.ini to opt in; reference the `${Speed_Flags.build_flags}` /
`${Speed_Flags.build_unflags}` tokens and the `build_flags` / `build_unflags`
keys so reviewers can locate the intended replacement.
🪄 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: 3a391272-cdb9-4873-bfba-525ba6c33a81
📒 Files selected for processing (7)
.github/workflows/build.yml.github/workflows/release.yml.gitignoreplatformio.iniplatformio_override.sample.iniplatformio_release.ini.templatewled00/data/settings_leds.htm
Free PSRAM (info.psram) can be transiently 0 on PSRAM-capable boards, which would wrongly reject multi-panel HUB75 setups in the LED settings validation. Prefer info.psrSz (total PSRAM size) and fall back to info.psram only if psrSz is unavailable. Suggested in PR wled#5558 review.
…sage in getNumberOfPins logic
|
@coderabbitai - why did you add an "Awaiting Testing" label? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
It's likely that label was added by a human reviewer, another bot/automation in the repository, or a GitHub Action configured for this repo. You may want to check the PR's activity timeline to see exactly who or what applied it. If you'd like me to check the current CodeRabbit configuration for this repository to confirm there are no label rules set up, I can do that. 🛠 Debug Information Gathered
Looking at the configuration, If you'd like to prevent CodeRabbit from automatically applying labels in the future, you can set reviews:
auto_apply_labels: falseThis will still allow CodeRabbit to suggest labels in the walkthrough without actually applying them. Alternatively, if you want to remove the label entirely from this PR, I can do that for you — just let me know. |
|
@coderabbitai - please update the PR description to include all the changes |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1038 🏁 Script executed: Length of output: 678
SummaryMove the four HUB75 build environments from ChangesCI / Workflow (
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
platformio.ini (1)
743-748: ⚡ Quick win
-DBOARD_HAS_PSRAMin shareds3_build_flagsconflicts with the no-PSRAM HD-WF2 env — verifybuild_unflagsstrips it correctly.
hub75.s3_build_flagsexplicitly adds-DBOARD_HAS_PSRAM(Line 745). Theesp32s3dev_4MB_qspi_hub75env — which targets the HD-WF2 board that has no PSRAM — includes those flags via${hub75.s3_build_flags}and then attempts to strip the flag withbuild_unflags = -DBOARD_HAS_PSRAM(Line 772).A confirmed PlatformIO issue (
#4309) shows thatbuild_unflagsremoves flags that were also added in the same env'sbuild_flags. This means the flag should be stripped, but this is a non-obvious, counter-intuitive interaction. If the flag isn't stripped successfully, the DMA library would try to allocate only in SPIRAM, causing a boot failure on hardware without PSRAM — exactly the scenario described in the inline comment.A clearer and more maintainable approach is to move
-DBOARD_HAS_PSRAMout ofs3_build_flags(since it is not common to all S3 HUB75 boards) and add it explicitly only to the envs that actually have PSRAM:♻️ Proposed refactor
s3_build_flags = -DARDUINO_USB_CDC_ON_BOOT=1 ;; -DARDUINO_USB_MODE=1 - -DBOARD_HAS_PSRAM -DLOLIN_WIFI_FIX -D WLED_WATCHDOG_TIMEOUT=0 -D S3_LCD_DIV_NUM=20Then add
-DBOARD_HAS_PSRAMexplicitly to every PSRAM-bearing env (all exceptesp32s3dev_4MB_qspi_hub75):[env:adafruit_matrixportal_esp32s3] build_flags = ${common.build_flags} ${esp32s3.build_flags} ${hub75.build_flags} ${hub75.s3_build_flags} ${hub75.i2s_disable_flags} -D WLED_RELEASE_NAME=\"ESP32-S3_Adafruit_Matrixportal\" -D ARDUINO_ADAFRUIT_MATRIXPORTAL_ESP32S3 + -DBOARD_HAS_PSRAM[env:esp32s3dev_16MB_opi_hub75] build_flags = ${common.build_flags} ${esp32s3.build_flags} ${hub75.build_flags} ${hub75.s3_build_flags} ... + -DBOARD_HAS_PSRAMAnd drop the now-unnecessary
build_unflagsfromesp32s3dev_4MB_qspi_hub75:[env:esp32s3dev_4MB_qspi_hub75] -build_unflags = -DBOARD_HAS_PSRAM build_flags = ${common.build_flags} ${esp32s3.build_flags} ${hub75.build_flags} ${hub75.s3_build_flags} ${hub75.i2s_disable_flags}Also applies to: 767-777
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platformio.ini` around lines 743 - 748, The shared symbol s3_build_flags currently contains -DBOARD_HAS_PSRAM which conflicts with the esp32s3dev_4MB_qspi_hub75 env that has no PSRAM; remove -DBOARD_HAS_PSRAM from s3_build_flags (and from hub75.s3_build_flags if duplicated), add -DBOARD_HAS_PSRAM explicitly to only the envs that actually have PSRAM, and delete the build_unflags = -DBOARD_HAS_PSRAM line from esp32s3dev_4MB_qspi_hub75; finally scan for any other uses of -DBOARD_HAS_PSRAM to ensure no leftover references remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@platformio.ini`:
- Around line 743-748: The shared symbol s3_build_flags currently contains
-DBOARD_HAS_PSRAM which conflicts with the esp32s3dev_4MB_qspi_hub75 env that
has no PSRAM; remove -DBOARD_HAS_PSRAM from s3_build_flags (and from
hub75.s3_build_flags if duplicated), add -DBOARD_HAS_PSRAM explicitly to only
the envs that actually have PSRAM, and delete the build_unflags =
-DBOARD_HAS_PSRAM line from esp32s3dev_4MB_qspi_hub75; finally scan for any
other uses of -DBOARD_HAS_PSRAM to ensure no leftover references remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b47ee380-0242-4bf8-bb1f-da5c620b3685
📒 Files selected for processing (5)
platformio.iniplatformio_release.ini.templatewled00/bus_manager.cppwled00/bus_manager.hwled00/cfg.cpp
✅ Files skipped from review due to trivial changes (2)
- wled00/bus_manager.h
- platformio_release.ini.template
|
picked into 16_x |
Free PSRAM (info.psram) can be transiently 0 on PSRAM-capable boards, which would wrongly reject multi-panel HUB75 setups in the LED settings validation. Prefer info.psrSz (total PSRAM size) and fall back to info.psram only if psrSz is unavailable. Suggested in PR #5558 review.
1 participant
Summary
Move the four HUB75 build environments from
platformio_override.sample.iniintoplatformio.ini, refactor them with a shared[hub75]section usingextends=to deduplicate configuration, and gate them behind a release-only config template so nightly/PR CI continues to build only the standard env matrix.Changes
CI / Workflow (
.github/workflows/)build.yml: Added areleaseboolean workflow input (default:false). Whentrue, the build job copiesplatformio_release.ini.template→platformio_release.inibefore invoking PlatformIO. The PlatformIO cache key now also hashesplatformio_release.ini.template.release.yml: Thewled_buildjob now passesrelease: trueinto the reusablebuild.ymlworkflow, enabling HUB75 envs only on tagged releases.PlatformIO Configuration
platformio.ini:platformio_release.initoextra_configsso the release override is picked up automatically when present.[hub75]section containing common build flags and the pinned ESP32-HUB75-MatrixPanel-DMA library (f17fb7fe, v3.0.14 — includes the S3 fix).extends = env:esp32dev, hub75(or the S3 equivalent):env:esp32dev_hub75env:esp32dev_hub75_forum_pinoutenv:adafruit_matrixportal_esp32s3env:esp32s3dev_8MB_opi_HUB75platformio_release.ini.template(new): Template copied toplatformio_release.iniby the release workflow. Overrides[platformio].default_envsto append all four HUB75 environments to the standard release build matrix.platformio_override.sample.ini: Removed the four HUB75 environments (now inplatformio.ini). Added an expanded documentation block describing the optionalSpeed_Flagsbuild flags/unflags that users can copy into their ownplatformio_override.ini..gitignore: Addedplatformio_release.iniso the generated release config is not committed.UI / Settings (
wled00/data/settings_leds.htm)hasPSRAMflag populated asynchronously from/json/infoduring page init, usinginfo.psrSz(total PSRAM size) with a fallback toinfo.psram.!hasPSRAM, allowing multi-panel setups when PSRAM is detected. This avoids false rejections caused by transiently zero free-PSRAM readings on PSRAM-capable boards.Library Version
The HUB75 environments are pinned to ESP32-HUB75-MatrixPanel-DMA v3.0.14 (commit
f17fb7fe), which includes commitaa28e2a(Update gdma_lcd_parallel16.cpp) — the S3 fix that landed in v3.0.12.