ci: discover and build custom PlatformIO envs from usermods platformi… - #5649
Merged
Conversation
…o_override.ini.sample files Adds two new jobs to the Usermod CI workflow to address issue wled#5648: - get_custom_build_envs: scans all usermods/*/platformio_override.ini.sample files and emits a matrix of {usermod, env} pairs by extracting [env:*] section names - build_custom: builds each discovered environment by copying the .ini.sample as platformio_override.ini and running pio run -e <env> This allows PRs introducing usermods with custom build environments (such as pixels_dice_tray) to have those environments validated in CI without committing platformio_override.ini to the repository.
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
…de.ini.sample files Move usermod-specific PlatformIO environments out of the root platformio_override.sample.ini and into dedicated files within each usermod's own directory, making them discoverable by CI: - Moved elekstube_ips env → usermods/EleksTube_IPS/ - Moved esp32dev_usermod_RF433 env → usermods/usermod_v2_RF433/ Extracted build environments from README docs into new sample files for: AHT10_v2, BME280_v2, INA226_v2, LD2410_v2 (fix LD2140_v2 typo), LDR_Dusk_Dawn_v2 (fix invalid inline comment), MAX17048_v2, Temperature, mpu6050_imu, sht (esp32 + d1_mini envs), usermod_v2_klipper_percentage (convert -D flag to custom_usermods) These files are now picked up by the get_custom_build_envs / build_custom CI jobs added in the previous commit.
Instead of building every usermod with a library.json on each PR, use git diff to identify which usermods/ subdirectories were actually touched and intersect that with the known-good library.json list. This reduces CI time significantly for PRs that only modify one or two usermods (previously every PR triggered ~40 usermods × 4 chipsets). Also removes the unnecessary PlatformIO install from get_usermod_envs (the step only uses shell/jq, not pio) and adds fetch-depth: 0 to ensure the base branch is available for the diff.
github.base_ref is empty on push events, causing 'ambiguous argument
origin/...HEAD'. Fix by:
- Adding github.event_name == 'pull_request' guard to both jobs so
they never run on push events where pull_request context is absent
- Replacing origin/${{ github.base_ref }}...HEAD with
${{ github.event.pull_request.base.sha }} HEAD which uses the
concrete base commit SHA provided by GitHub directly
- Rename *.ini (gitignored) and *.sample.ini (wrong extension order) to the correct platformio_override.ini.sample convention so CI discovers them - Fix AHT10_v2: custom_usermods AHT10 → AHT10_v2 (match library.json name) - Fix INA226_v2: custom_usermods INA226 → INA226_v2 (both envs) - Fix TTGO-T-Display: replace direct [env:esp32dev] override with a named env that extends esp32dev; add note that library.json is absent so custom_usermods is not available for this usermod Affected usermods: AHT10_v2, DHT, INA226_v2, SN_Photoresistor, TTGO-T-Display, Temperature, four_line_display_ALT, rotary_encoder_ui_ALT
On pull_request events, get_custom_build_envs now only scans usermod directories that changed in the PR (matching the behaviour of the get_usermod_envs job). On push events (e.g. merging a PR) it still scans all usermods to validate the full set.
- BME280_v2: fix default_envs typo (usermod_bme280_esp8266_2m →
usermod_esp8266_2m to match actual [env:] section name)
- DHT: replace extends env:custom32_LEDPIN_16 (not in this fork) with
env:esp32dev + -D LEDPIN=16; rename env accordingly
- SN_Photoresistor: replace ${common.build_flags_esp8266} (key absent in
this fork) with ${env:esp8266_2m.build_flags} / lib_deps equivalent
- sht: guard ESP.getChipModel() behind #ifdef ARDUINO_ARCH_ESP32 — method
does not exist on ESP8266 (fixes custom_esp8266_2m_usermod_sht build)
- EleksTube_IPS: delete platformio_override.ini.sample — library.json is
disabled so the env cannot build; delete rather than ship a broken sample
- pixels_dice_tray: exclude from build_custom matrix (same as standard
matrix) — BLE library incompatibility with current IDF causes build failure
- workflow: add PWM_fan / BME68X_v2 to exclusion list in get_custom_build_envs
for consistency with get_usermod_envs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
usermods/sht/sht.cpp (2)
150-150: ⚖️ Poor tradeoffUse
ESP.getChipId()on ESP8266 instead of hardcoded"ESP8266"formdl
ESP.getChipModel()-style model strings aren’t available in the ESP8266 Arduino core; the closest runtime identifier isESP.getChipId()(32-bit, format as hex) if you wantdevice["mdl"]to be more informative than the fixed"ESP8266". Otherwise, leaving"ESP8266"is fine.device[F("mdl")] = F("ESP8266");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@usermods/sht/sht.cpp` at line 150, Replace the hardcoded model string assignment device[F("mdl")] = F("ESP8266") with a runtime identifier using ESP.getChipId(); specifically, call ESP.getChipId(), format the 32-bit value as hex (e.g. into a String or char buffer) and assign that formatted hex string to device[F("mdl")] so the entry contains the chip ID instead of the literal "ESP8266"; locate the assignment in sht.cpp and update it to use ESP.getChipId() when compiling for ESP8266.
147-151: ⚡ Quick win
#elseis consistent with existing WLED ESP32/ESP8266 handling
The sht usermod (and corewled00/xml.cpp) uses the same convention:#if defined(ARDUINO_ARCH_ESP32)forESP.getChipModel()and a broad#elsethat assumes the remaining supported target is ESP8266. With WLED’s current PlatformIO environments and the usermod readme, this#elsepath shouldn’t be hit for other architectures, so the “unsupported platform” risk is low.ESP.getChipModel()is also used across the codebase behindARDUINO_ARCH_ESP32, so it’s a stable pattern.Optional clarity improvement
`#ifdef` ARDUINO_ARCH_ESP32 device[F("mdl")] = ESP.getChipModel(); -#else +#elif defined(ESP8266) device[F("mdl")] = F("ESP8266"); +#else +#error "Unsupported platform" `#endif`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@usermods/sht/sht.cpp` around lines 147 - 151, Replace the preprocessor conditional to match the project's convention: change the "`#ifdef` ARDUINO_ARCH_ESP32" check around the device[F("mdl")] assignment to use "`#if` defined(ARDUINO_ARCH_ESP32)" while keeping the existing else branch that sets device[F("mdl")] = F("ESP8266") and the ESP.getChipModel() use; this aligns the conditional style with other files (e.g., xml.cpp) and preserves behavior.
🤖 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 @.github/workflows/usermods.yml:
- Around line 27-30: The run step injects github.event.* values directly into
the shell (the assignment to the changed variable), which is forbidden; instead
add env entries (e.g., BASE_SHA and any other github.event.* used later) to the
step and reference those env vars inside the run script (use "$BASE_SHA" when
computing changed), and repeat the same fix for the other run step that
currently interpolates github.event.* (the block around where changed is
computed again); ensure all github.event.* references are passed via env: not
embedded in run:.
- Around line 4-9: The path filters under the pull_request and push triggers in
.github/workflows/usermods.yml currently only include "usermods/**" which allows
changes to the workflow file or related config to bypass the workflow; update
the triggers (the pull_request: paths: and push: paths: entries) to also include
the workflow file itself (".github/workflows/usermods.yml") and the
configuration sample ("platformio_override.sample.ini") so changes to those
files will run the workflow; adjust the paths list entries accordingly to
include these two file patterns alongside "usermods/**".
- Around line 95-173: Add an explicit permissions block (scoped to least
privilege) to this workflow so jobs no longer inherit default GITHUB_TOKEN
rights; for example add a top-level or per-job permissions: entry (before the
job definitions or before the job `get_custom_build_envs` / `build_custom`)
declaring only required scopes such as permissions: contents: read (and any
other minimal scopes your jobs need) to satisfy the repo rule requiring explicit
least-privilege permissions.
In `@usermods/LDR_Dusk_Dawn_v2/platformio_override.ini.sample`:
- Around line 7-10: Update the custom_usermods entry in
platformio_override.ini.sample so it matches the usermod name declared in
library.json: replace the currently listed LDR_Dusk_Dawn with LDR_Dusk_Dawn_v2
under the env [env:usermod_LDR_Dusk_Dawn_esp32dev] so custom_usermods includes
the exact symbol LDR_Dusk_Dawn_v2 (ensure the key custom_usermods remains intact
and preserves the existing ${env:esp32dev.custom_usermods} merge).
In `@usermods/sht/sht.cpp`:
- Around line 146-152: Add a brief descriptive comment above the AI-generated
conditional that explains its intent and assumptions: state that it populates
the device["mdl"] field with the ESP chip model when compiled for ESP32 (using
the ARDUINO_ARCH_ESP32 macro and ESP.getChipModel()) and falls back to the
hard-coded "ESP8266" string for other builds; mention the assumption that
non-ESP32 builds are ESP8266 and note that ESP.getChipModel() is only available
on ESP32 to justify the conditional and avoid confusion for future maintainers.
In `@usermods/usermod_v2_four_line_display_ALT/platformio_override.ini.sample`:
- Around line 5-6: The custom_usermods entry is using the wrong name; update the
env:esp32dev.custom_usermods value from "four_line_display_ALT" to the actual
usermod directory name "usermod_v2_four_line_display_ALT" so the build enables
the correct usermod (edit the extends = env:esp32dev block and the
custom_usermods assignment that references env:esp32dev.custom_usermods and
four_line_display_ALT).
In `@usermods/usermod_v2_rotary_encoder_ui_ALT/platformio_override.ini.sample`:
- Around line 5-6: The custom_usermods entry is pointing to the wrong folder
name so the usermod never builds; update the custom_usermods value from
"rotary_encoder_ui_ALT" to the actual directory name
"usermod_v2_rotary_encoder_ui_ALT" in the platformio_override.ini sample (keep
the existing extends = env:esp32dev line), ensuring the custom_usermods key
references the exact usermod folder name so the rotary encoder ALT usermod is
compiled.
---
Nitpick comments:
In `@usermods/sht/sht.cpp`:
- Line 150: Replace the hardcoded model string assignment device[F("mdl")] =
F("ESP8266") with a runtime identifier using ESP.getChipId(); specifically, call
ESP.getChipId(), format the 32-bit value as hex (e.g. into a String or char
buffer) and assign that formatted hex string to device[F("mdl")] so the entry
contains the chip ID instead of the literal "ESP8266"; locate the assignment in
sht.cpp and update it to use ESP.getChipId() when compiling for ESP8266.
- Around line 147-151: Replace the preprocessor conditional to match the
project's convention: change the "`#ifdef` ARDUINO_ARCH_ESP32" check around the
device[F("mdl")] assignment to use "`#if` defined(ARDUINO_ARCH_ESP32)" while
keeping the existing else branch that sets device[F("mdl")] = F("ESP8266") and
the ESP.getChipModel() use; this aligns the conditional style with other files
(e.g., xml.cpp) and preserves behavior.
🪄 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: c17b7160-d452-469c-aaa3-91cfcc2bf71b
📒 Files selected for processing (24)
.github/workflows/usermods.ymlplatformio_override.sample.iniusermods/AHT10_v2/platformio_override.iniusermods/AHT10_v2/platformio_override.ini.sampleusermods/BME280_v2/platformio_override.ini.sampleusermods/DHT/platformio_override.ini.sampleusermods/INA226_v2/platformio_override.iniusermods/INA226_v2/platformio_override.ini.sampleusermods/LD2410_v2/platformio_override.ini.sampleusermods/LDR_Dusk_Dawn_v2/platformio_override.ini.sampleusermods/MAX17048_v2/platformio_override.ini.sampleusermods/SN_Photoresistor/platformio_override.ini.sampleusermods/TTGO-T-Display/platformio_override.iniusermods/TTGO-T-Display/platformio_override.ini.sampleusermods/Temperature/platformio_override.iniusermods/Temperature/platformio_override.ini.sampleusermods/mpu6050_imu/platformio_override.ini.sampleusermods/pixels_dice_tray/platformio_override.ini.sampleusermods/sht/platformio_override.ini.sampleusermods/sht/sht.cppusermods/usermod_v2_RF433/platformio_override.ini.sampleusermods/usermod_v2_four_line_display_ALT/platformio_override.ini.sampleusermods/usermod_v2_klipper_percentage/platformio_override.ini.sampleusermods/usermod_v2_rotary_encoder_ui_ALT/platformio_override.ini.sample
💤 Files with no reviewable changes (4)
- usermods/AHT10_v2/platformio_override.ini
- usermods/TTGO-T-Display/platformio_override.ini
- usermods/INA226_v2/platformio_override.ini
- usermods/Temperature/platformio_override.ini
Comment on lines
4
to
+9
| pull_request: | ||
| paths: | ||
| - usermods/** | ||
| push: | ||
| paths: | ||
| - usermods/** |
Contributor
There was a problem hiding this comment.
Broaden the path filter to cover the files that define this CI behavior.
Right now this workflow only fires for usermods/**, so changes to .github/workflows/usermods.yml or platformio_override.sample.ini can skip the very checks they affect.
Suggested change
on:
pull_request:
paths:
- usermods/**
+ - .github/workflows/usermods.yml
+ - platformio_override.sample.ini
push:
paths:
- usermods/**
+ - .github/workflows/usermods.yml
+ - platformio_override.sample.iniAs per coding guidelines, "Follow the CI/CD conventions documented in docs/cicd.instructions.md."
📝 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.
Suggested change
| pull_request: | |
| paths: | |
| - usermods/** | |
| push: | |
| paths: | |
| - usermods/** | |
| pull_request: | |
| paths: | |
| - usermods/** | |
| - .github/workflows/usermods.yml | |
| - platformio_override.sample.ini | |
| push: | |
| paths: | |
| - usermods/** | |
| - .github/workflows/usermods.yml | |
| - platformio_override.sample.ini |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/usermods.yml around lines 4 - 9, The path filters under
the pull_request and push triggers in .github/workflows/usermods.yml currently
only include "usermods/**" which allows changes to the workflow file or related
config to bypass the workflow; update the triggers (the pull_request: paths: and
push: paths: entries) to also include the workflow file itself
(".github/workflows/usermods.yml") and the configuration sample
("platformio_override.sample.ini") so changes to those files will run the
workflow; adjust the paths list entries accordingly to include these two file
patterns alongside "usermods/**".
Comment on lines
27
to
+30
| run: | | ||
| 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 | ||
| # Usermods whose directories changed in this PR | ||
| changed=$(git diff --name-only ${{ github.event.pull_request.base.sha }} HEAD \ | ||
| | grep '^usermods/' | cut -d/ -f2 | sort -u || true) |
Contributor
There was a problem hiding this comment.
Pass github.event.* values through env: instead of embedding them in the shell script.
Both scripts inject github.event.pull_request.base.sha directly into run:. The workflow guidelines explicitly forbid that pattern.
Suggested change
- name: Get changed usermod environments
id: envs
+ env:
+ PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
run: |
# Usermods whose directories changed in this PR
- changed=$(git diff --name-only ${{ github.event.pull_request.base.sha }} HEAD \
+ changed=$(git diff --name-only "$PR_BASE_SHA" HEAD \
| grep '^usermods/' | cut -d/ -f2 | sort -u || true) - name: Find usermods with custom build environments
id: custom_envs
+ env:
+ PR_BASE_SHA: ${{ github.event.pull_request.base.sha }}
run: |
# On PRs: only scan usermods whose directories changed.
# On push: scan all usermods (validates the full set on merge).
if [ "${{ github.event_name }}" = "pull_request" ]; then
- changed=$(git diff --name-only ${{ github.event.pull_request.base.sha }} HEAD \
+ changed=$(git diff --name-only "$PR_BASE_SHA" HEAD \
| grep '^usermods/' | cut -d/ -f2 | sort -u || true)As per coding guidelines, "Never interpolate github.event.* values directly into run: steps — pass them through an env: variable to prevent script injection."
Also applies to: 107-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/usermods.yml around lines 27 - 30, The run step injects
github.event.* values directly into the shell (the assignment to the changed
variable), which is forbidden; instead add env entries (e.g., BASE_SHA and any
other github.event.* used later) to the step and reference those env vars inside
the run script (use "$BASE_SHA" when computing changed), and repeat the same fix
for the other run step that currently interpolates github.event.* (the block
around where changed is computed again); ensure all github.event.* references
are passed via env: not embedded in run:.
Comment on lines
+95
to
+173
| get_custom_build_envs: | ||
| name: Gather Custom Build Environments | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| - name: Find usermods with custom build environments | ||
| id: custom_envs | ||
| run: | | ||
| # On PRs: only scan usermods whose directories changed. | ||
| # On push: scan all usermods (validates the full set on merge). | ||
| if [ "${{ github.event_name }}" = "pull_request" ]; then | ||
| changed=$(git diff --name-only ${{ github.event.pull_request.base.sha }} HEAD \ | ||
| | grep '^usermods/' | cut -d/ -f2 | sort -u || true) | ||
| if [ -z "$changed" ]; then | ||
| echo "matrix=[]" >> $GITHUB_OUTPUT | ||
| exit 0 | ||
| fi | ||
| samples=$(for mod in $changed; do | ||
| f="usermods/$mod/platformio_override.ini.sample" | ||
| [ -f "$f" ] && echo "$f" | ||
| done | sort) | ||
| else | ||
| samples=$(find usermods/ -name "platformio_override.ini.sample" | sort) | ||
| fi | ||
|
|
||
| result='[]' | ||
| for sample in $samples; do | ||
| usermod=$(dirname "$sample" | xargs basename) | ||
| # Skip usermods known to be incompatible (same list as get_usermod_envs) | ||
| case "$usermod" in PWM_fan|BME68X_v2|pixels_dice_tray) continue ;; esac | ||
| envs=$(grep -E '^\[env:[^]]+\]' "$sample" | sed 's/^\[env:\(.*\)\]$/\1/') | ||
| for env in $envs; do | ||
| result=$(echo "$result" | jq --arg u "$usermod" --arg e "$env" '. + [{usermod: $u, env: $e}]') | ||
| done | ||
| done | ||
| echo "matrix=$(echo "$result" | jq -c '.')" >> $GITHUB_OUTPUT | ||
| outputs: | ||
| matrix: ${{ steps.custom_envs.outputs.matrix }} | ||
|
|
||
|
|
||
| build_custom: | ||
| name: Build Custom Env (${{ matrix.usermod }} / ${{ matrix.env }}) | ||
| runs-on: ubuntu-latest | ||
| needs: get_custom_build_envs | ||
| if: needs.get_custom_build_envs.outputs.matrix != '[]' | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| include: ${{ fromJSON(needs.get_custom_build_envs.outputs.matrix) }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Set up Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version-file: '.nvmrc' | ||
| cache: 'npm' | ||
| - run: npm ci | ||
| - name: Cache PlatformIO | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: | | ||
| ~/.platformio/.cache | ||
| ~/.buildcache | ||
| build_output | ||
| key: pio-${{ runner.os }}-${{ matrix.env }}-${{ hashFiles('platformio.ini', 'pio-scripts/output_bins.py') }}-${{ hashFiles('wled00/**', 'usermods/**') }} | ||
| restore-keys: pio-${{ runner.os }}-${{ matrix.env }}-${{ hashFiles('platformio.ini', 'pio-scripts/output_bins.py') }}- | ||
| - name: Set up Python | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: '3.12' | ||
| cache: 'pip' | ||
| - name: Install PlatformIO | ||
| run: pip install -r requirements.txt | ||
| - name: Apply custom build environment | ||
| run: cp -v "usermods/${{ matrix.usermod }}/platformio_override.ini.sample" platformio_override.ini | ||
| - name: Build firmware | ||
| run: pio run -e ${{ matrix.env }} |
Contributor
There was a problem hiding this comment.
Add an explicit permissions: block before expanding this workflow further.
These new jobs currently inherit the default GITHUB_TOKEN permissions. The repo workflow rules require least-privilege permissions to be declared explicitly.
Suggested change
+permissions:
+ contents: read
+
jobs:As per coding guidelines, "Declare explicit permissions: blocks scoped to least privilege."
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 99-101: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 147-147: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 95-134: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[error] 99-99: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 147-147: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 149-149: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 155-155: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 164-164: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/usermods.yml around lines 95 - 173, Add an explicit
permissions block (scoped to least privilege) to this workflow so jobs no longer
inherit default GITHUB_TOKEN rights; for example add a top-level or per-job
permissions: entry (before the job definitions or before the job
`get_custom_build_envs` / `build_custom`) declaring only required scopes such as
permissions: contents: read (and any other minimal scopes your jobs need) to
satisfy the repo rule requiring explicit least-privilege permissions.
Comment on lines
+7
to
+10
| [env:usermod_LDR_Dusk_Dawn_esp32dev] | ||
| extends = env:esp32dev | ||
| custom_usermods = ${env:esp32dev.custom_usermods} | ||
| LDR_Dusk_Dawn |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
jq -r '.name' usermods/LDR_Dusk_Dawn_v2/library.jsonRepository: wled/WLED
Length of output: 70
Fix custom_usermods to match the v2 usermod name (LDR_Dusk_Dawn_v2)
usermods/LDR_Dusk_Dawn_v2/library.json sets .name = LDR_Dusk_Dawn_v2, but usermods/LDR_Dusk_Dawn_v2/platformio_override.ini.sample currently uses LDR_Dusk_Dawn, so this env may not enable the intended usermod (lines 7-10).
[env:usermod_LDR_Dusk_Dawn_esp32dev]
extends = env:esp32dev
custom_usermods = ${env:esp32dev.custom_usermods}
LDR_Dusk_Dawn_v2🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@usermods/LDR_Dusk_Dawn_v2/platformio_override.ini.sample` around lines 7 -
10, Update the custom_usermods entry in platformio_override.ini.sample so it
matches the usermod name declared in library.json: replace the currently listed
LDR_Dusk_Dawn with LDR_Dusk_Dawn_v2 under the env
[env:usermod_LDR_Dusk_Dawn_esp32dev] so custom_usermods includes the exact
symbol LDR_Dusk_Dawn_v2 (ensure the key custom_usermods remains intact and
preserves the existing ${env:esp32dev.custom_usermods} merge).
Comment on lines
+146
to
+152
| // AI: below section was generated by an AI | ||
| #ifdef ARDUINO_ARCH_ESP32 | ||
| device[F("mdl")] = ESP.getChipModel(); | ||
| #else | ||
| device[F("mdl")] = F("ESP8266"); | ||
| #endif | ||
| // AI: end |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add a descriptive comment explaining the conditional.
The AI-generated block lacks a comment describing what it does and why the conditional is needed. As per coding guidelines, AI-generated code must have a brief comment explaining intent and assumptions.
📝 Suggested documentation
// AI: below section was generated by an AI
+// Set device model: ESP.getChipModel() is ESP32-only; ESP8266 uses hardcoded string
`#ifdef` ARDUINO_ARCH_ESP32
device[F("mdl")] = ESP.getChipModel();As per coding guidelines: AI-generated code must be well documented with meaningful comments explaining intent, assumptions, and non-obvious logic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@usermods/sht/sht.cpp` around lines 146 - 152, Add a brief descriptive comment
above the AI-generated conditional that explains its intent and assumptions:
state that it populates the device["mdl"] field with the ESP chip model when
compiled for ESP32 (using the ARDUINO_ARCH_ESP32 macro and ESP.getChipModel())
and falls back to the hard-coded "ESP8266" string for other builds; mention the
assumption that non-ESP32 builds are ESP8266 and note that ESP.getChipModel() is
only available on ESP32 to justify the conditional and avoid confusion for
future maintainers.
Member
There was a problem hiding this comment.
please double-check if ESP.getChipModel() is indeed not available in the 8266 arduino framework. Maybe the intention was to always show "ESP8266" instead of actual model variant names?
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for ESP.getChipModel usages in the codebase
rg -n "getChipModel" --type cpp -C3Length of output: 4271
🌐 Web query:
ESP8266 Arduino framework ESP.getChipModel() availability EspClass
Length of output: 11721
@softhack007, confirmed: ESP.getChipModel() does not exist in the ESP8266 Arduino framework at all — it is absent from EspClass in cores/esp8266/Esp.h. The guard is therefore necessary to prevent a compile error on ESP8266 builds.
As for the intent: since the ESP8266 framework provides no equivalent API, hardcoding "ESP8266" is the only viable option. Interestingly, all ESP8266 devices use the same chip (ESP8266EX), so a generic "ESP8266" string is both accurate and sufficient here.
This pattern is actually already established in the WLED codebase — wled00/xml.cpp uses exactly the same guard:
`#if` defined(ARDUINO_ARCH_ESP32)
ESP.getChipModel()
`#else`
"ESP8266"
`#endif`So the AI-generated code in sht.cpp correctly mirrors an existing project convention. My original comment about adding a descriptive intent-explaining comment still stands as a minor improvement, but the logic itself is sound.
✏️ Learnings added
Learnt from: softhack007
Repo: wled/WLED PR: 5649
File: usermods/sht/sht.cpp:146-152
Timestamp: 2026-05-29T13:10:25.470Z
Learning: In the ESP8266 Arduino framework (`esp8266/Arduino`), `EspClass` does NOT have a `getChipModel()` method. This method is ESP32-only (`espressif/arduino-esp32`). When writing cross-platform WLED code that needs chip model info, use `#ifdef ARDUINO_ARCH_ESP32` (or `#if defined(ARDUINO_ARCH_ESP32)`) to guard `ESP.getChipModel()`, with a hardcoded `"ESP8266"` string in the `#else` branch. This is the established pattern already used in `wled00/xml.cpp` and `wled00/improv.cpp`.
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.
🧠 Learnings used
Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:08.677Z
Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).
Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35
Timestamp: 2026-03-27T12:33:48.499Z
Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.
Learnt from: softhack007
Repo: wled/WLED PR: 4843
File: usermods/elastic_collisions/Elastic_Collisions.cpp:1-1
Timestamp: 2026-05-06T22:10:27.517Z
Learning: In C/C++, do not flag precedence issues for expressions that mix additive operators (`+`, `-`) with shift operators (`<<`, `>>`) based on the assumption that shifts bind tighter. Per C/C++ operator precedence, `+`/`-` have higher precedence than `<<`/`>>` (e.g., `x - edge0 << 8` parses as `(x - edge0) << 8`). When reviewing WLED (and other) fixed-point/bit-manipulation code, confirm the intended parse using cppreference before reporting a precedence bug for mixed `-`/`+` and `<<`/`>>` expressions.
Learnt from: softhack007
Repo: wled/WLED PR: 5599
File: usermods/audioreactive/audio_reactive.cpp:1303-1308
Timestamp: 2026-05-11T16:00:13.574Z
Learning: When targeting arduino-esp32 v3.x (ESP-IDF 5.x), treat `WiFiUDP::flush()` as deprecated and ineffective for RX draining; it should not be used to clear/drain the UDP receive buffer. Instead, use `WiFiUDP::clear()`, which was introduced in arduino-esp32 v3.0 and is marked as the deprecated replacement via `NetworkUdp.h`.
If the code must compile and behave correctly across both arduino-esp32 v2.x (ESP-IDF < 5) and v3.x (ESP-IDF >= 5), use conditional compilation:
- for `ESP_IDF_VERSION_MAJOR < 5`: use `udp.flush()`
- for `ESP_IDF_VERSION_MAJOR >= 5`: use `udp.clear()`
Do not flag `WiFiUDP::clear()` usages for ESP-IDF >= 5 as non-existent—this is the documented correct replacement.
netmindz
marked this pull request as ready for review
June 11, 2026 07:49
This was referenced Jul 3, 2026
aenertia
pushed a commit
to aenertia/WLED
that referenced
this pull request
Aug 18, 2026
wled#5649) * ci: discover and build custom PlatformIO envs from usermods platformio_override.ini.sample files Adds two new jobs to the Usermod CI workflow to address issue wled#5648: - get_custom_build_envs: scans all usermods/*/platformio_override.ini.sample files and emits a matrix of {usermod, env} pairs by extracting [env:*] section names - build_custom: builds each discovered environment by copying the .ini.sample as platformio_override.ini and running pio run -e <env> This allows PRs introducing usermods with custom build environments (such as pixels_dice_tray) to have those environments validated in CI without committing platformio_override.ini to the repository. * ci: consolidate usermod build envs into per-usermod platformio_override.ini.sample files Move usermod-specific PlatformIO environments out of the root platformio_override.sample.ini and into dedicated files within each usermod's own directory, making them discoverable by CI: * run on push * Fix AHT10_v2 example * no d1_mini env * no d1_mini env * ci: filter usermod matrix to only build changed usermod directories Instead of building every usermod with a library.json on each PR, use git diff to identify which usermods/ subdirectories were actually touched and intersect that with the known-good library.json list. This reduces CI time significantly for PRs that only modify one or two usermods (previously every PR triggered ~40 usermods × 4 chipsets). Also removes the unnecessary PlatformIO install from get_usermod_envs (the step only uses shell/jq, not pio) and adds fetch-depth: 0 to ensure the base branch is available for the diff. * SN_Photoresistor * fix(ci): use PR base SHA for diff and guard jobs against push events github.base_ref is empty on push events, causing 'ambiguous argument origin/...HEAD'. Fix by: - Adding github.event_name == 'pull_request' guard to both jobs so they never run on push events where pull_request context is absent - Replacing origin/${{ github.base_ref }}...HEAD with ${{ github.event.pull_request.base.sha }} HEAD which uses the concrete base commit SHA provided by GitHub directly * ESP32 builds all V4 * fix: rename/fix platformio_override sample files across usermods - Rename *.ini (gitignored) and *.sample.ini (wrong extension order) to the correct platformio_override.ini.sample convention so CI discovers them - Fix AHT10_v2: custom_usermods AHT10 → AHT10_v2 (match library.json name) - Fix INA226_v2: custom_usermods INA226 → INA226_v2 (both envs) - Fix TTGO-T-Display: replace direct [env:esp32dev] override with a named env that extends esp32dev; add note that library.json is absent so custom_usermods is not available for this usermod Affected usermods: AHT10_v2, DHT, INA226_v2, SN_Photoresistor, TTGO-T-Display, Temperature, four_line_display_ALT, rotary_encoder_ui_ALT * perf: filter custom build matrix to changed usermods on PRs On pull_request events, get_custom_build_envs now only scans usermod directories that changed in the PR (matching the behaviour of the get_usermod_envs job). On push events (e.g. merging a PR) it still scans all usermods to validate the full set. * fix release name * fix release name * fix: resolve 8 CI build failures in custom usermod environments - BME280_v2: fix default_envs typo (usermod_bme280_esp8266_2m → usermod_esp8266_2m to match actual [env:] section name) - DHT: replace extends env:custom32_LEDPIN_16 (not in this fork) with env:esp32dev + -D LEDPIN=16; rename env accordingly - SN_Photoresistor: replace ${common.build_flags_esp8266} (key absent in this fork) with ${env:esp8266_2m.build_flags} / lib_deps equivalent - sht: guard ESP.getChipModel() behind #ifdef ARDUINO_ARCH_ESP32 — method does not exist on ESP8266 (fixes custom_esp8266_2m_usermod_sht build) - EleksTube_IPS: delete platformio_override.ini.sample — library.json is disabled so the env cannot build; delete rather than ship a broken sample - pixels_dice_tray: exclude from build_custom matrix (same as standard matrix) — BLE library incompatibility with current IDF causes build failure - workflow: add PWM_fan / BME68X_v2 to exclusion list in get_custom_build_envs for consistency with get_usermod_envs * Do not add platformio_override.ini.sample just because example was in readme --------- Co-authored-by: Frank Möhle <91616163+softhack007@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request adds automated support for discovering and building custom PlatformIO environments defined by usermods in the CI workflow. It introduces new jobs to gather usermod-specific build environments and builds them dynamically, ensuring that custom configurations are properly tested.
Custom build environment automation:
get_custom_build_envsjob to scanusermods/forplatformio_override.ini.samplefiles, extract custom environment names, and output them as a build matrix.build_customjob that uses the generated matrix to build firmware for each usermod/environment combination, applying the corresponding override file before building.#5648
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores