ci: discover and build custom PlatformIO envs from usermods platformi…#5649
ci: discover and build custom PlatformIO envs from usermods platformi…#5649netmindz wants to merge 17 commits into
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.
|
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}
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
| pull_request: | ||
| paths: | ||
| - usermods/** | ||
| push: | ||
| paths: | ||
| - usermods/** |
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.
| 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/**".
| 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) |
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:.
| 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 }} |
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.
| [env:usermod_LDR_Dusk_Dawn_esp32dev] | ||
| extends = env:esp32dev | ||
| custom_usermods = ${env:esp32dev.custom_usermods} | ||
| LDR_Dusk_Dawn |
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).
| // 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 |
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.
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?
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.
2 participants
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