Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 106 additions & 10 deletions .github/workflows/usermods.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ on:
pull_request:
paths:
- usermods/**
push:
paths:
- usermods/**
Comment on lines 4 to +9

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.ini

As 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/**".


env:
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true
Expand All @@ -12,28 +15,40 @@ jobs:

get_usermod_envs:
# Only run for pull requests from forks (not from branches within wled/WLED)
if: github.event.pull_request.head.repo.full_name != github.repository
if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository
name: Gather Usermods
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: '3.12'
cache: 'pip'
- name: Install PlatformIO
run: pip install -r requirements.txt
- name: Get default environments
fetch-depth: 0
- name: Get changed usermod environments
id: envs
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)
Comment on lines 27 to +30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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:.


# All usermods with a library.json (excluding known-incompatible ones)
all=$(find usermods/ -name library.json \
| xargs dirname | xargs -n 1 basename \
| grep -v PWM_fan | grep -v BME68X_v2 | grep -v pixels_dice_tray \
| sort || true)

if [ -z "$changed" ] || [ -z "$all" ]; then
echo "usermods=[]" >> $GITHUB_OUTPUT
else
usermods=$(comm -12 <(echo "$all") <(echo "$changed") | jq -R | jq --slurp -c)
echo "usermods=$usermods" >> $GITHUB_OUTPUT
fi
outputs:
usermods: ${{ steps.envs.outputs.usermods }}


build:
# Only run for pull requests from forks (not from branches within wled/WLED)
if: github.event.pull_request.head.repo.full_name != github.repository
# Skip when no changed usermods were found (e.g. only non-library changes)
if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && needs.get_usermod_envs.outputs.usermods != '[]'
name: Build Enviornments
runs-on: ubuntu-latest
needs: get_usermod_envs
Expand Down Expand Up @@ -74,4 +89,85 @@ jobs:
cat platformio_override.ini

- name: Build firmware
run: pio run -e ${{ matrix.environment }}
run: pio run -e ${{ matrix.environment }}


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 }}
Comment on lines +95 to +173

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

31 changes: 2 additions & 29 deletions platformio_override.sample.ini
Original file line number Diff line number Diff line change
Expand Up @@ -509,42 +509,15 @@ lib_deps = ${esp8266.lib_deps}

# ------------------------------------------------------------------------------
# EleksTube-IPS
# See usermods/EleksTube_IPS/platformio_override.ini.sample
# ------------------------------------------------------------------------------
[env:elekstube_ips]
extends = esp32 ;; use default esp32 platform
board = esp32dev
upload_speed = 921600
custom_usermods = ${env:esp32dev.custom_usermods} RTC EleksTube_IPS
build_flags = ${common.build_flags} ${esp32.build_flags} -D WLED_DISABLE_BROWNOUT_DET -D WLED_DISABLE_INFRARED
-D DATA_PINS=12
-D RLYPIN=27
-D BTNPIN=34
-D PIXEL_COUNTS=6
# Display config
-D ST7789_DRIVER
-D TFT_WIDTH=135
-D TFT_HEIGHT=240
-D CGRAM_OFFSET
-D TFT_SDA_READ
-D TFT_MOSI=23
-D TFT_SCLK=18
-D TFT_DC=25
-D TFT_RST=26
-D SPI_FREQUENCY=40000000
-D USER_SETUP_LOADED
monitor_filters = esp32_exception_decoder


# ------------------------------------------------------------------------------
# Usermod examples
# ------------------------------------------------------------------------------

# 433MHz RF remote example for esp32dev
[env:esp32dev_usermod_RF433]
extends = env:esp32dev
custom_usermods =
${env:esp32dev.custom_usermods}
RF433
# 433MHz RF remote example: see usermods/usermod_v2_RF433/platformio_override.ini.sample

# External usermod from a git repository.
# The library's `library.json` must include `"build": {"libArchive": false}`.
Expand Down
5 changes: 0 additions & 5 deletions usermods/AHT10_v2/platformio_override.ini

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
; USERMOD_DHT_MQTT - publish measurements to the MQTT broker
; USERMOD_DHT_STATS - For debug, report delay stats

[env:d1_mini_usermod_dht_C]
extends = env:d1_mini
custom_usermods = ${env:d1_mini.custom_usermods} DHT
build_flags = ${env:d1_mini.build_flags} -D USERMOD_DHT_CELSIUS
[env:esp8266_2m_usermod_dht_C]
extends = env:esp8266_2m
custom_usermods = ${env:esp8266_2m.custom_usermods} DHT
build_flags = ${env:esp8266_2m.build_flags} -D USERMOD_DHT_CELSIUS

[env:custom32_LEDPIN_16_usermod_dht_C]
extends = env:custom32_LEDPIN_16
custom_usermods = ${env:custom32_LEDPIN_16.custom_usermods} DHT
build_flags = ${env:custom32_LEDPIN_16.build_flags} -D USERMOD_DHT_CELSIUS -D USERMOD_DHT_STATS
[env:esp32dev_LEDPIN_16_usermod_dht_C]
extends = env:esp32dev
custom_usermods = ${env:esp32dev.custom_usermods} DHT
build_flags = ${env:esp32dev.build_flags} -D LEDPIN=16 -D USERMOD_DHT_CELSIUS -D USERMOD_DHT_STATS

6 changes: 0 additions & 6 deletions usermods/INA226_v2/platformio_override.ini

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
; Options
; -------
; USERMOD_SN_PHOTORESISTOR - define this to have this user mod included wled00\usermods_list.cpp
; USERMOD_SN_PHOTORESISTOR_MEASUREMENT_INTERVAL - the number of milliseconds between measurements, defaults to 60 seconds
; USERMOD_SN_PHOTORESISTOR_FIRST_MEASUREMENT_AT - the number of milliseconds after boot to take first measurement, defaults to 20 seconds
; USERMOD_SN_PHOTORESISTOR_REFERENCE_VOLTAGE - the voltage supplied to the sensor, defaults to 5v
; USERMOD_SN_PHOTORESISTOR_ADC_PRECISION - the ADC precision is the number of distinguishable ADC inputs, defaults to 1024.0 (10 bits)
; USERMOD_SN_PHOTORESISTOR_RESISTOR_VALUE - the resistor size, defaults to 10000.0 (10K hms)
; USERMOD_SN_PHOTORESISTOR_OFFSET_VALUE - the offset value to report on, defaults to 25
;
[env:usermod_sn_photoresistor_d1_mini]
extends = env:d1_mini
[env:usermod_sn_photoresistor_esp8266_2m]
extends = env:esp8266_2m
custom_usermods = ${env:esp8266_2m.custom_usermods} SN_Photoresistor
build_flags =
${common.build_flags_esp8266}
-D USERMOD_SN_PHOTORESISTOR
lib_deps = ${env.lib_deps}
${env:esp8266_2m.build_flags}
-D USERMOD_SN_PHOTORESISTOR_MEASUREMENT_INTERVAL=60
lib_deps = ${env:esp8266_2m.lib_deps}
8 changes: 0 additions & 8 deletions usermods/TTGO-T-Display/platformio_override.ini

This file was deleted.

5 changes: 0 additions & 5 deletions usermods/Temperature/platformio_override.ini

This file was deleted.

6 changes: 3 additions & 3 deletions usermods/pixels_dice_tray/platformio_override.ini.sample
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ board_build.partitions = ${esp32.large_partitions}
board_build.f_flash = 80000000L
board_build.flash_mode = qio
monitor_filters = esp32_exception_decoder
build_flags = ${common.build_flags} ${esp32s3.build_flags} -D WLED_RELEASE_NAME=T-QT-PRO-8MB
build_flags = ${common.build_flags} ${esp32s3.build_flags} -D WLED_RELEASE_NAME=\"T-QT-PRO-8MB_dice\"
-D CONFIG_LITTLEFS_FOR_IDF_3_2 -D WLED_WATCHDOG_TIMEOUT=0
-D ARDUINO_USB_CDC_ON_BOOT=1 -D ARDUINO_USB_MODE=1 ;; for boards with USB-OTG connector only (USBCDC or "TinyUSB")

Expand Down Expand Up @@ -75,7 +75,7 @@ board_build.partitions = ${esp32.large_partitions}
board_build.f_flash = 80000000L
board_build.flash_mode = qio
monitor_filters = esp32_exception_decoder
build_flags = ${common.build_flags} ${esp32s3.build_flags} -D WLED_RELEASE_NAME=ESP32-S3_8MB_qspi
build_flags = ${common.build_flags} ${esp32s3.build_flags} -D WLED_RELEASE_NAME=\"ESP32-S3_8MB_qspi_dice\"
-D CONFIG_LITTLEFS_FOR_IDF_3_2 -D WLED_WATCHDOG_TIMEOUT=0
-D ARDUINO_USB_CDC_ON_BOOT=1 -D ARDUINO_USB_MODE=1 ;; for boards with USB-OTG connector only (USBCDC or "TinyUSB")

Expand Down Expand Up @@ -105,7 +105,7 @@ lib_deps = ${esp32s3.lib_deps}
# https://github.com/wled-dev/WLED/issues/1382
; [env:esp32dev_dice]
; extends = env:esp32dev
; build_flags = ${common.build_flags} ${esp32.build_flags} -D WLED_RELEASE_NAME=ESP32
; build_flags = ${common.build_flags} ${esp32.build_flags} -D WLED_RELEASE_NAME=\"ESP32_dice\"
; ; Enable Pixels dice mod
; -D USERMOD_PIXELS_DICE_TRAY
; lib_deps = ${esp32.lib_deps}
Expand Down
6 changes: 6 additions & 0 deletions usermods/sht/sht.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,13 @@ void ShtUsermod::appendDeviceToMqttDiscoveryMessage(JsonDocument& root) {
device[F("ids")] = escapedMac.c_str();
device[F("name")] = serverDescription;
device[F("sw")] = versionString;
// 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
Comment on lines +146 to +152

@coderabbitai coderabbitai Bot May 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for ESP.getChipModel usages in the codebase
rg -n "getChipModel" --type cpp -C3

Length 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.

device[F("mf")] = F("espressif");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
default_envs = esp32dev_fld

[env:esp32dev_fld]
extends = env:esp32dev_V4
custom_usermods = ${env:esp32dev_V4.custom_usermods} four_line_display_ALT
extends = env:esp32dev
custom_usermods = ${env:esp32dev.custom_usermods} four_line_display_ALT
Comment thread
softhack007 marked this conversation as resolved.
build_flags =
${env:esp32dev_V4.build_flags}
${env:esp32dev.build_flags}
-D FLD_TYPE=SH1106
-D I2CSCLPIN=27
-D I2CSDAPIN=26
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
default_envs = esp32dev_re

[env:esp32dev_re]
extends = env:esp32dev_V4
custom_usermods = ${env:esp32dev_V4.custom_usermods} rotary_encoder_ui_ALT
extends = env:esp32dev
custom_usermods = ${env:esp32dev.custom_usermods} rotary_encoder_ui_ALT
Comment thread
softhack007 marked this conversation as resolved.
build_flags =
${env:esp32dev_V4.build_flags}
${env:esp32dev.build_flags}
-D USERMOD_ROTARY_ENCODER_GPIO=INPUT
-D ENCODER_DT_PIN=21
-D ENCODER_CLK_PIN=23
Expand Down
Loading