From 9ecc34d1e3615dacd34e462429fcac4f37c99640 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 16:02:00 +0200 Subject: [PATCH 01/16] inreoduce / this allows to separate background knowledge (good for contributors, but too repetitive for AI review bots) from AI-relevant decision rules, special cases and repo-specific conventions. --- .coderabbit.yaml | 16 +++++++++++ .github/cicd.instructions.md | 7 +++++ .github/cpp.instructions.md | 21 +++++++++++++- .github/esp-idf.instructions.md | 49 +++++++++++++++++++++++++++++++-- 4 files changed, 90 insertions(+), 3 deletions(-) diff --git a/.coderabbit.yaml b/.coderabbit.yaml index f941bec839..5dbb79d2f6 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -66,3 +66,19 @@ reviews: scoped to least privilege. Never interpolate github.event.* values directly into run: steps — pass them through an env: variable to prevent script injection. Do not use pull_request_target unless fully justified. + + - path: ".github/*.instructions.md" + instructions: | + This file contains both AI-facing rules and human-only reference sections. + Human-only sections are enclosed in `` / + `` HTML comment markers and should not be used as + actionable review criteria. + + When this file is modified in a PR, perform the following alignment check: + 1. For each ` ... ` block, + verify that its examples and guidance are consistent with (and do not + contradict) the AI-facing rules stated in the same file. + 2. Flag any HUMAN_ONLY section whose content has drifted from the surrounding + AI-facing rules due to edits introduced in this PR. + 3. If new AI-facing rules were added without updating a related HUMAN_ONLY + reference section, note this as a suggestion (not a required fix). \ No newline at end of file diff --git a/.github/cicd.instructions.md b/.github/cicd.instructions.md index b67a3a2863..be0793dc30 100644 --- a/.github/cicd.instructions.md +++ b/.github/cicd.instructions.md @@ -3,6 +3,12 @@ applyTo: ".github/workflows/*.yml,.github/workflows/*.yaml" --- # CI/CD Conventions — GitHub Actions Workflows +> **Note for AI review tools**: sections enclosed in +> `` / `` HTML comments contain +> contributor reference material. Do **not** use that content as actionable review +> criteria — treat it as background context only. + + ## YAML Style - Indent with **2 spaces** (no tabs) @@ -60,6 +66,7 @@ schedule: - Name artifacts with enough context to be unambiguous (e.g., `firmware-${{ matrix.environment }}`) - Avoid uploading artifacts that will never be consumed downstream + --- ## Security diff --git a/.github/cpp.instructions.md b/.github/cpp.instructions.md index cca620dd45..3d10a2ea57 100644 --- a/.github/cpp.instructions.md +++ b/.github/cpp.instructions.md @@ -3,6 +3,11 @@ applyTo: "**/*.cpp,**/*.h,**/*.hpp, **/*.ino" --- # C++ Coding Conventions +> **Note for AI review tools**: sections enclosed in +> `` / `` HTML comments contain +> contributor reference material. Do **not** use that content as actionable review +> criteria — treat it as background context only. + See also: [CONTRIBUTING.md](../CONTRIBUTING.md) for general style guidelines that apply to all contributors. ## Formatting @@ -13,6 +18,7 @@ See also: [CONTRIBUTING.md](../CONTRIBUTING.md) for general style guidelines tha - Space between keyword and parenthesis: `if (...)`, `for (...)`. No space between function name and parenthesis: `doStuff(a)` - No enforced line-length limit; wrap when a line exceeds your editor width + ## Naming - **camelCase** for functions and variables: `setValuesFromMainSeg()`, `effectCurrent` @@ -29,18 +35,21 @@ Most headers use `#ifndef` / `#define` guards. Some newer headers add `#pragma o // ... #endif // WLED_EXAMPLE_H ``` + ## Comments - `//` for inline comments, `/* ... */` for block comments. Always put a space after `//` - Mark WLED-MM-specific changes with `// WLEDMM` or `// WLEDMM: description`: + ```cpp // WLEDMM: increased max bus count for larger installs #ifndef WLED_MAX_BUSSES #define WLED_MAX_BUSSES 20 // WLEDMM default (upstream: 10) #endif ``` + - **AI attribution:** When a larger block of code is generated by an AI tool, mark it with an `// AI:` comment so reviewers know to scrutinize it: @@ -113,6 +122,7 @@ const uint_fast16_t rows = virtualHeight(); ### `const` references to avoid copies + Pass and store objects by `const &` (or `&`) instead of copying them implicitly. This avoids constructing temporary objects on every access — especially important in loops: ```cpp @@ -125,6 +135,7 @@ For function parameters that are read-only, prefer `const &`: ```cpp BusDigital(BusConfig &bc, uint8_t nr, const ColorOrderMap &com); ``` + ### `constexpr` over `#define` @@ -176,6 +187,7 @@ Declare every getter, query, or inspection method `const`. If you need to mark a #### `static` member functions + A `static` member function has no implicit `this` pointer. This has two distinct advantages: 1. **Smaller code, faster calls**: no `this` is passed in a register. On Xtensa and RISC-V, this removes one register argument from every call site and prevents the compiler from emitting `this`-preservation code around inlined blocks. @@ -191,8 +203,9 @@ static BusConfig fromJson(JsonObject obj); static uint8_t gamma8(uint8_t val); static uint32_t colorBalance(uint32_t color, uint8_t r, uint8_t g, uint8_t b); ``` + -`static` also communicates intent clearly: a reviewer immediately knows the method is stateless and safe to call without a fully constructed object. +`static` communicates intent clearly: a reviewer immediately knows the method is stateless and safe to call without a fully constructed object. > **Rule of thumb**: if a method does not read or write any member variable, make it `static`. If it only reads member variables, make it `const`. Note: `static` methods cannot also be `const`-qualified because there is no implicit `this` pointer to be const — just use `static`. Both qualifiers reduce coupling and improve generated code on all ESP32 targets. @@ -225,6 +238,7 @@ Example signature: void IRAM_ATTR_YN WLED_O2_ATTR __attribute__((hot)) Segment::setPixelColor(int i, uint32_t col) ``` + ### Use `uint_fast` Types for Locals Use `uint_fast8_t` and `uint_fast16_t` for loop counters, indices, and temporary calculations in hot paths. These let the compiler pick the CPU's native word size (32-bit on ESP32), avoiding unnecessary narrow-type masking: @@ -249,6 +263,7 @@ for (uint_fast8_t i = 0; i < count; i++) { ... } ``` + ### Unsigned Range Check @@ -310,6 +325,7 @@ else Each callback is a small, single-purpose function with no internal branching — the decoder's per-pixel loop never re-evaluates which strategy to use. + ### Template Specialization (Advanced) Templates can eliminate runtime decisions by generating separate code paths at compile time. For example, a pixel setter could be templated on color order or channel count so the compiler removes dead branches and produces tight, specialized machine code: @@ -359,6 +375,7 @@ const uint_fast16_t rows = virtualHeight(); uint_fast8_t fadeRate = (255 - rate) >> 1; float mappedRate_r = 1.0f / (float(fadeRate) + 1.1f); // reciprocal — avoid division inside loop ``` + ### Parallel Channel Processing @@ -406,6 +423,7 @@ if (lastKelvin != kelvin) { - Use `static inline` for file-local helpers - On ESP32 with `WLEDMM_FASTPATH`, color utilities are inlined from `colorTools.hpp`; on ESP8266 or `WLEDMM_SAVE_FLASH`, they fall back to `colors.cpp` + ### Colors - Store and pass colors as `uint32_t` (0xWWRRGGBB) @@ -415,6 +433,7 @@ if (lastKelvin != kelvin) { ```cpp uint16_t r1 = R(color1); // 16-bit intermediate keeps the multiply result in 32 bits, avoiding 64-bit promotion ``` + ## Multi-Task Synchronization diff --git a/.github/esp-idf.instructions.md b/.github/esp-idf.instructions.md index b196940c92..56915b41fb 100644 --- a/.github/esp-idf.instructions.md +++ b/.github/esp-idf.instructions.md @@ -7,8 +7,14 @@ WLED-MM runs on the Arduino-ESP32 framework, which wraps ESP-IDF. Understanding > **Scope**: This file is an optional review guideline. It applies when touching chip-specific code, peripheral drivers, memory allocation, or platform conditionals. +> **Note for AI review tools**: sections enclosed in +> `` / `` HTML comments contain +> contributor reference material. Do **not** use that content as actionable review +> criteria — treat it as background context only. + --- + ## Identifying the Build Target: `CONFIG_IDF_TARGET_*` Use `CONFIG_IDF_TARGET_*` macros to gate chip-specific code at compile time. These are set by the build system and are mutually exclusive — exactly one is defined per build. @@ -23,8 +29,9 @@ Use `CONFIG_IDF_TARGET_*` macros to gate chip-specific code at compile time. The | `CONFIG_IDF_TARGET_ESP32C6` | ESP32-C6 | RISC-V single-core | Wi-Fi 6, Thread/Zigbee. Future target | | `CONFIG_IDF_TARGET_ESP32P4` | ESP32-P4 | RISC-V dual-core | High performance. Future target | -### Build-time validation + +### Build-time validation WLED validates at compile time that exactly one target is defined and that it is a supported chip (`wled.cpp` lines 39–61). Follow this pattern when adding new chip-specific branches: ```cpp @@ -53,6 +60,7 @@ WLED validates at compile time that exactly one target is defined and that it is `SOC_*` macros (from `soc/soc_caps.h`) describe what the current chip supports. They are the correct way to check for peripheral features — they stay accurate when new chips are added, unlike `CONFIG_IDF_TARGET_*` checks. ### Important `SOC_*` macros used in WLED-MM + | Macro | Type | Used in | Purpose | |---|---|---|---| @@ -64,7 +72,11 @@ WLED validates at compile time that exactly one target is defined and that it is | `SOC_ADC_CHANNEL_NUM(unit)` | `int` | `pin_manager.cpp` | ADC channels per unit | | `SOC_UART_NUM` | `int` | `dmx_input.cpp` | Number of UART peripherals | | `SOC_DRAM_LOW` / `SOC_DRAM_HIGH` | `addr` | `util.cpp` | DRAM address boundaries for validation | + +Common pitfall: `SOC_ADC_MAX_BITWIDTH` (ADC resolution 12 or 13 bits) was renamed to `CONFIG_SOC_ADC_RTC_MAX_BITWIDTH` in IDF v5. + + ### Less commonly used but valuable | Macro | Purpose | @@ -76,6 +88,8 @@ WLED validates at compile time that exactly one target is defined and that it is | `SOC_SPIRAM_SUPPORTED` | Whether PSRAM interface exists | | `SOC_CPU_CORES_NUM` | Core count (1 or 2) — useful for task pinning decisions | + + ### Best practices ```cpp @@ -92,6 +106,7 @@ WLED validates at compile time that exactly one target is defined and that it is #endif ``` + ### PSRAM capability macros For PSRAM presence, mode, and DMA access patterns: @@ -105,6 +120,8 @@ For PSRAM presence, mode, and DMA access patterns: | `CONFIG_SOC_PSRAM_DMA_CAPABLE` | PSRAM buffers can be used with DMA (ESP32-S3 with octal PSRAM) | | `CONFIG_SOC_MEMSPI_FLASH_PSRAM_INDEPENDENT` | SPI flash and PSRAM on separate buses (no speed contention) | + + #### Detecting octal/hex flash On ESP32-S3 modules with OPI flash (e.g. N8R8 modules where the SPI flash itself runs in Octal-PI mode), the build system sets: @@ -129,6 +146,7 @@ On ESP32-S3 modules with OPI flash (e.g. N8R8 modules where the SPI flash itself ## ESP-IDF Version Conditionals + ### Checking the IDF version ```cpp @@ -142,6 +160,7 @@ On ESP32-S3 modules with OPI flash (e.g. N8R8 modules where the SPI flash itself // Legacy IDF v3/v4.x path #endif ``` + ### Key ESP-IDF version thresholds for WLED-MM @@ -280,6 +299,7 @@ Legacy `i2s_driver_install()` + `i2s_read()` API is deprecated. The new API uses | `i2s_set_clk()` | `i2s_channel_reconfig_std_clk()` | Reconfigure running channel | | `i2s_config_t` | `i2s_std_config_t` | Separate config for each mode | + **Migration pattern** (from Espressif examples): ```cpp #if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0) @@ -301,6 +321,7 @@ Legacy `i2s_driver_install()` + `i2s_read()` API is deprecated. The new API uses ``` **WLED impact**: The audioreactive usermod (`audio_source.h`) heavily uses legacy I2S. Migration requires rewriting the `I2SSource` class for channel-based API. + #### ADC (Analog-to-Digital Converter) @@ -329,6 +350,7 @@ WLED already has a compatibility shim in `ota_update.cpp` that maps old names to | `gpio_pad_select_gpio()` | `esp_rom_gpio_pad_select_gpio()` (or use `gpio_config()`) | | `gpio_set_direction()` + `gpio_set_pull_mode()` | `gpio_config()` with `gpio_config_t` struct | + ### Features disabled in IDF v5 builds The upstream `V5-C6` branch explicitly disables features with incompatible library dependencies: @@ -341,6 +363,8 @@ The upstream `V5-C6` branch explicitly disables features with incompatible libra -D WLED_USE_SHARED_RMT # Use new shared RMT driver for NeoPixel output ``` + + ### Migration checklist for new code 1. **Never use a removed API without a version guard.** Always provide both old and new paths, or disable the feature on IDF v5. @@ -354,6 +378,7 @@ The upstream `V5-C6` branch explicitly disables features with incompatible libra ESP32 has multiple memory regions with different capabilities. Using the right allocator is critical for performance and stability. + ### Memory regions | Region | Flag | Speed | DMA | Size | Use for | @@ -363,6 +388,8 @@ ESP32 has multiple memory regions with different capabilities. Using the right a | PSRAM (SPIRAM) | `MALLOC_CAP_SPIRAM \| MALLOC_CAP_8BIT` | Slower | Chip-dependent | 2–16 MB | Large buffers, JSON documents, image data | | RTC RAM | `MALLOC_CAP_RTCRAM` | Moderate | No | 8 KB | Data surviving deep sleep; small persistent buffers | + + ### WLED-MM allocation wrappers WLED-MM provides convenience wrappers with automatic fallback. **Always prefer these over raw `heap_caps_*` calls**: @@ -385,8 +412,9 @@ WLED-MM provides convenience wrappers with automatic fallback. **Always prefer t ``` - **Fragmentation**: PSRAM allocations fragment less than DRAM because the region is larger. But avoid mixing small and large allocations in PSRAM — small allocations waste the MMU page granularity. - **Heap validation**: use `d_measureHeap()` and `d_measureContiguousFreeHeap()` to monitor remaining DRAM. Allocations that would drop free DRAM below `MIN_HEAP_SIZE` should go to PSRAM instead. -- **Performance**: PSRAM access is 3–10× slower than DRAM on ESP32/S2 (quad-SPI bus). On ESP32-S3 with octal PSRAM (`CONFIG_SPIRAM_MODE_OCT`), the penalty is smaller (~2×) because the 8-line DTR bus runs at up to 80 MHz (120 MHz is possible with CONFIG_SPIRAM_SPEED_120M, which requires enabling experimental ESP-IDF features). On ESP32-P4 with hex PSRAM (`CONFIG_SPIRAM_MODE_HEX`), the 16-line bus runs at 200 MHz, further reducing the gap. Keep hot-path data in DRAM regardless. +- **Performance**: PSRAM access is up to 15× slower than DRAM on ESP32, 3–10× slower than DRAM on ESP32-S3/-S2 with quad-SPI bus. On ESP32-S3 with octal PSRAM (`CONFIG_SPIRAM_MODE_OCT`), the penalty is smaller (~2×) because the 8-line DTR bus can transfer 8 bits in parallel at 80MHz (120 MHz is possible with CONFIG_SPIRAM_SPEED_120M, which requires enabling experimental ESP-IDF features). On ESP32-P4 with hex PSRAM (`CONFIG_SPIRAM_MODE_HEX`), the 16-line bus runs at 200 MHz which brings it on-par with DRAM. Keep hot-path data in DRAM regardless, but consider that esp32 often crashes when the largest DRAM chunk is below 10Kb. + ### Pattern: preference-based allocation When you need a buffer that works on boards with or without PSRAM: @@ -400,6 +428,7 @@ uint8_t* buf = (uint8_t*)heap_caps_malloc_prefer(bufSize, 2, uint8_t* buf = (uint8_t*)p_malloc(bufSize); ``` + --- ## I2S Audio: Best Practices @@ -499,6 +528,7 @@ The driver dynamically reduces color depth for larger displays to stay within DM --- + ## GPIO Best Practices ### Prefer `gpio_config()` over individual calls @@ -518,6 +548,7 @@ gpio_config(&io_conf); gpio_set_direction(pin, GPIO_MODE_OUTPUT); gpio_set_pull_mode(pin, GPIO_FLOATING); ``` + ### Pin manager integration @@ -543,6 +574,7 @@ For high-resolution timing, prefer `esp_timer_get_time()` (microsecond resolutio int64_t now_us = esp_timer_get_time(); // monotonic, not affected by NTP ``` + ### Periodic timers For periodic tasks with sub-millisecond precision, use `esp_timer`: @@ -558,6 +590,7 @@ esp_timer_create_args_t args = { esp_timer_create(&args, &timer); esp_timer_start_periodic(timer, 1000); // 1 ms period ``` + Always prefer `ESP_TIMER_TASK` dispatch over `ESP_TIMER_ISR` unless you need ISR-level latency — ISR callbacks have severe restrictions (no logging, no heap allocation, no FreeRTOS API calls). @@ -565,6 +598,7 @@ Always prefer `ESP_TIMER_TASK` dispatch over `ESP_TIMER_ISR` unless you need ISR ## ADC Best Practices + ### Version-aware ADC code ADC is one of the most fragmented APIs across IDF versions: @@ -585,6 +619,7 @@ ADC is one of the most fragmented APIs across IDF versions: int raw = adc1_get_raw(ADC1_CHANNEL_0); #endif ``` + ### Bit width portability @@ -611,6 +646,7 @@ WLED-MM's `util.cpp` uses the IDF v4 form (`SOC_ADC_MAX_BITWIDTH`) — this will --- + ## RMT Best Practices ### Current usage in WLED @@ -643,6 +679,7 @@ if (err != ESP_OK) { return; } ``` + ### Logging @@ -666,6 +703,7 @@ ESP_LOGE(TAG, "Failed to allocate %u bytes", size); ### Task creation and pinning + On dual-core chips (ESP32, S3, P4), pin latency-sensitive tasks to a specific core: ```cpp @@ -679,6 +717,7 @@ xTaskCreatePinnedToCore( 0 // core ID (0 = protocol core, 1 = app core) ); ``` + Guidelines: - Pin network/protocol tasks to core 0 (where Wi-Fi runs). @@ -699,6 +738,7 @@ Guidelines: FreeRTOS on ESP32 is **preemptive** — all tasks are scheduled by priority regardless of `yield()` calls. This is fundamentally different from ESP8266 cooperative multitasking. + | Call | What it does | Reaches IDLE (priority 0)? | |---|---|---| | `delay(ms)` / `vTaskDelay(ticks)` | Suspends calling task; scheduler runs all other ready tasks | ✅ Yes | @@ -706,11 +746,14 @@ FreeRTOS on ESP32 is **preemptive** — all tasks are scheduled by priority rega | `taskYIELD()` | Same as `vTaskDelay(0)` | ❌ No | | Blocking API (`xQueueReceive`, `ulTaskNotifyTake`, `vTaskDelayUntil`) | Suspends task until event or timeout; IDLE runs freely | ✅ Yes | + + **`delay()` in `loopTask` is safe.** Arduino's `loop()` runs inside `loopTask`. Calling `delay()` suspends only `loopTask` — all other FreeRTOS tasks (Wi-Fi stack, audio FFT, LED DMA) continue uninterrupted on either core. **`yield()` does not yield to IDLE.** Any task that loops with only `yield()` calls will starve the IDLE task, causing the IDLE watchdog to fire. Always use `delay(1)` (or a blocking FreeRTOS call) in tight task loops. Note: WLED-MM redefines `yield()` as an empty macro on ESP32 WLEDMM_FASTPATH builds — see `cpp.instructions.md`. #### Why the IDLE task is not optional + The FreeRTOS IDLE task (one per core on dual-core ESP32 and ESP32-S3; single instance on single-core chips) is not idle in the casual sense — it performs essential system housekeeping: @@ -719,6 +762,8 @@ The FreeRTOS IDLE task (one per core on dual-core ESP32 and ESP32-S3; single ins - **Implements tickless idle / light sleep**: on battery-powered devices, IDLE is the entry point for low-power sleep. A permanently starved IDLE task disables light sleep entirely. - **Runs registered idle hooks**: ESP-IDF components register callbacks via `esp_register_freertos_idle_hook()` (e.g., Wi-Fi background maintenance, Bluetooth housekeeping). These only fire when IDLE runs. + + In short: **starving IDLE corrupts memory cleanup, breaks background activities, disables low-power sleep, and prevents Wi-Fi/BT maintenance.** The IDLE watchdog panic is a symptom — the real damage happens before the watchdog fires. ### Watchdog management From 1108c56c7283f89173de475252298b2af7addb8f Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 16:29:16 +0200 Subject: [PATCH 02/16] optimizations as suggested by the rabbit --- .github/cpp.instructions.md | 3 +++ .github/esp-idf.instructions.md | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/cpp.instructions.md b/.github/cpp.instructions.md index 3d10a2ea57..d063ca7d5f 100644 --- a/.github/cpp.instructions.md +++ b/.github/cpp.instructions.md @@ -63,6 +63,7 @@ void calculateCRC(const uint8_t* data, size_t len) { Single-line AI-assisted edits do not need the marker — use it when the AI produced a contiguous block that a human did not write line-by-line. + - **Function & feature comments:** Every non-trivial function should have a brief comment above it describing what it does. Include a note about each parameter when the names alone are not self-explanatory: ```cpp @@ -74,6 +75,8 @@ void calculateCRC(const uint8_t* data, size_t len) { ***** */ uint8_t gammaCorrect(uint8_t value, float gamma); ``` + + Short accessor-style functions (getters/setters, one-liners) may skip this if their purpose is obvious from the name. diff --git a/.github/esp-idf.instructions.md b/.github/esp-idf.instructions.md index 56915b41fb..33fab7d28a 100644 --- a/.github/esp-idf.instructions.md +++ b/.github/esp-idf.instructions.md @@ -412,7 +412,7 @@ WLED-MM provides convenience wrappers with automatic fallback. **Always prefer t ``` - **Fragmentation**: PSRAM allocations fragment less than DRAM because the region is larger. But avoid mixing small and large allocations in PSRAM — small allocations waste the MMU page granularity. - **Heap validation**: use `d_measureHeap()` and `d_measureContiguousFreeHeap()` to monitor remaining DRAM. Allocations that would drop free DRAM below `MIN_HEAP_SIZE` should go to PSRAM instead. -- **Performance**: PSRAM access is up to 15× slower than DRAM on ESP32, 3–10× slower than DRAM on ESP32-S3/-S2 with quad-SPI bus. On ESP32-S3 with octal PSRAM (`CONFIG_SPIRAM_MODE_OCT`), the penalty is smaller (~2×) because the 8-line DTR bus can transfer 8 bits in parallel at 80MHz (120 MHz is possible with CONFIG_SPIRAM_SPEED_120M, which requires enabling experimental ESP-IDF features). On ESP32-P4 with hex PSRAM (`CONFIG_SPIRAM_MODE_HEX`), the 16-line bus runs at 200 MHz which brings it on-par with DRAM. Keep hot-path data in DRAM regardless, but consider that esp32 often crashes when the largest DRAM chunk is below 10Kb. +- **Performance**: PSRAM access is up to 15× slower than DRAM on ESP32, 3–10× slower than DRAM on ESP32-S3/-S2 with quad-SPI bus. On ESP32-S3 with octal PSRAM (`CONFIG_SPIRAM_MODE_OCT`), the penalty is smaller (~2×) because the 8-line DTR bus can transfer 8 bits in parallel at 80MHz (120 MHz is possible with CONFIG_SPIRAM_SPEED_120M, which requires enabling experimental ESP-IDF features). On ESP32-P4 with hex PSRAM (`CONFIG_SPIRAM_MODE_HEX`), the 16-line bus runs at 200 MHz which brings it on-par with DRAM. Keep hot-path data in DRAM regardless, but consider that esp32 often crashes when the largest DRAM chunk gets below 10 KB. ### Pattern: preference-based allocation From fe4501d46878f931d5b83afef5132471025f6012 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 19:02:50 +0200 Subject: [PATCH 03/16] AI trimming of cpp.instructions.md --- .github/cpp.instructions.md | 50 ++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/.github/cpp.instructions.md b/.github/cpp.instructions.md index d063ca7d5f..0799694ab0 100644 --- a/.github/cpp.instructions.md +++ b/.github/cpp.instructions.md @@ -63,7 +63,7 @@ void calculateCRC(const uint8_t* data, size_t len) { Single-line AI-assisted edits do not need the marker — use it when the AI produced a contiguous block that a human did not write line-by-line. - + - **Function & feature comments:** Every non-trivial function should have a brief comment above it describing what it does. Include a note about each parameter when the names alone are not self-explanatory: ```cpp @@ -104,13 +104,18 @@ uint8_t gammaCorrect(uint8_t value, float gamma); ## Memory - **PSRAM-aware allocation**: use `d_malloc()` (prefer DRAM), `p_malloc()` (prefer PSRAM) from `util.h` -- **Avoid Variable Length Arrays (VLAs)**: GCC/Clang support VLAs as an extension (they are not part of the C++ standard), so they look like a legitimate feature — but they are allocated on the stack at runtime. On ESP32/ESP8266, FreeRTOS task stacks are typically only 2–8 KB; a VLA whose size depends on a runtime parameter (segment dimensions, pixel counts, etc.) can silently exhaust the stack and cause the program to behave in unexpected ways or crash. Prefer a fixed-size array with a compile-time bound, or heap allocation (`d_malloc` / `p_malloc`) for dynamically sized buffers. **Any VLA must be explicitly justified in the source code or PR.** + +- Variable Length Arrays (VLAs): GCC/Clang support VLAs as an extension (they are not part of the C++ standard), so they look like a legitimate feature — but they are allocated on the stack at runtime. On ESP32/ESP8266. A VLA whose size depends on a runtime parameter (segment dimensions, pixel counts, etc.) can silently exhaust the stack and cause the program to behave in unexpected ways or crash. + + **Avoid Variable Length Arrays (VLAs)**: FreeRTOS task stacks are typically 2–8 KB. A runtime-sized VLA can silently exhaust the stack. Use fixed-size arrays or heap allocation (`d_malloc` / `p_malloc`). Any VLA must be explicitly justified in source or PR. - **Larger buffers** (LED data, JSON documents) should use PSRAM when available and technically feasible - **Hot-path**: some data should stay in DRAM or IRAM for performance reasons - Memory efficiency matters, but is less critical on boards with PSRAM ## `const` and `constexpr` +Add `const` to cached locals in hot-path code (helps the compiler keep values in registers). Pass and store objects by `const&` to avoid copies in loops. + `const` is a promise to the compiler that a value (or object) will not change - a function declared with a `const char* message` parameter is not allowed to modify the content of `message`. This pattern enables optimizations and makes intent clear to reviewers. @@ -122,12 +127,12 @@ Adding `const` to a local variable that is only assigned once is not necessary const uint_fast16_t cols = virtualWidth(); const uint_fast16_t rows = virtualHeight(); ``` - + ### `const` references to avoid copies - Pass and store objects by `const &` (or `&`) instead of copying them implicitly. This avoids constructing temporary objects on every access — especially important in loops: + ```cpp const auto &m = _mappings[i]; // reference, not a copy (bus_manager.cpp) const CRGB& c = ledBuffer[pix]; // alias — avoids creating a temporary CRGB instance @@ -141,6 +146,7 @@ BusDigital(BusConfig &bc, uint8_t nr, const ColorOrderMap &com); ### `constexpr` over `#define` + Prefer `constexpr` for compile-time constants. Unlike `#define`, `constexpr` respects scope and type safety, keeping the global namespace clean: @@ -169,6 +175,10 @@ static_assert(WLED_MAX_BUSSES <= 32, "WLED_MAX_BUSSES exceeds hard limit"); #error "WLED_MAX_BUSSES exceeds hard limit" #endif ``` + + +Prefer `constexpr` over `#define` for typed constants (scope-safe, debuggable). Use `static_assert` instead of `#if … #error` for compile-time validation. +Exception: `#define` is required for conditional-compilation guards and build-flag-overridable values. ### `static` and `const` class methods @@ -180,13 +190,14 @@ Marking a member function `const` tells the compiler that it does not modify the uint16_t length() const { return _len; } bool isActive() const { return _active; } ``` - + Benefits for GCC/Xtensa/RISC-V: - The compiler knows the method cannot write to `this`, so it is free to **keep member values in registers** across the call and avoid reload barriers. - `const` methods can be called on `const` objects and `const` references — essential when passing large objects as `const &` to avoid copying. - `const` allows the compiler to **eliminate redundant loads**: if a caller already has a member value cached, the compiler can prove the `const` call cannot invalidate it. + -Declare every getter, query, or inspection method `const`. If you need to mark a member `mutable` to work around this (e.g. for a cache or counter), document the reason. +Declare getter, query, or inspection methods `const`. If you need to mark a member `mutable` to work around this (e.g. for a cache or counter), document the reason. #### `static` member functions @@ -289,7 +300,7 @@ if (unsigned(i) >= virtualLength()) return; // bounds check (catches negative i ### Avoid Nested Calls — Fast Path / Complex Path Avoid calling non-inline functions or making complex decisions inside per-pixel hot-path code. When a function has both a common simple case and a rare complex case, split it into two variants and choose once per frame rather than per pixel: - + ```cpp // Decision made once per frame in startFrame(), stored in a bool bool simpleSegment = _isSuperSimpleSegment; @@ -306,16 +317,18 @@ The same principle applies to color utilities — `color_add()` accepts a `fast` ```cpp uint32_t color_add(uint32_t c1, uint32_t c2, bool fast=false); ``` + General rules: -- Keep the per-pixel fast path free of non-inline function calls, multi-way branches and complex switch-case decisions. -- Hoist the "which path?" decision out of the inner loop (once per frame or per segment) -- It is acceptable to duplicate some code between fast and complex variants to keep the fast path lean +- Keep fast-path functions free of non-inline calls, multi-way branches, and complex switch-case decisions. +- Hoist per-frame decisions (e.g. simple vs. complex segment) out of the per-pixel loop. +- Code duplication between fast/slow variants is acceptable to keep the fast path lean. ### Function Pointers to Eliminate Repeated Decisions When the same decision (e.g. "which drawing routine?") would be evaluated for every pixel, assign the chosen variant to a function pointer once and let the inner loop call through the pointer. This removes the branch entirely — the calling code (e.g. the GIF decoder loop) only ever invokes one function per frame, with no per-pixel decision. + `image_loader.cpp` demonstrates the pattern: `calculateScaling()` picks the best drawing callback once per frame based on segment dimensions and GIF size, then passes it to the decoder via `setDrawPixelCallback()`: ```cpp @@ -327,7 +340,7 @@ else ``` Each callback is a small, single-purpose function with no internal branching — the decoder's per-pixel loop never re-evaluates which strategy to use. - + ### Template Specialization (Advanced) @@ -368,18 +381,19 @@ if (!guard) return; // another task is already sending This avoids FreeRTOS semaphore overhead and the risk of forgetting `esp32SemGive`. There are no current examples of this pattern in the codebase — consult with maintainers before introducing it in new code, to ensure it aligns with the project's synchronization conventions. + ### Pre-Compute Outside Loops Move invariant calculations before the loop. Pre-compute reciprocals to replace division with multiplication: - + ```cpp const uint_fast16_t cols = virtualWidth(); const uint_fast16_t rows = virtualHeight(); uint_fast8_t fadeRate = (255 - rate) >> 1; float mappedRate_r = 1.0f / (float(fadeRate) + 1.1f); // reciprocal — avoid division inside loop ``` - + ### Parallel Channel Processing Process R+B and W+G channels simultaneously using the two-channel mask pattern: @@ -394,11 +408,10 @@ return rb | wg; ### Bit Shifts Over Division (mainly for RISC-V boards) ESP32 and ESP32-S3 (Xtensa core) have a fast "integer divide" instruction, so manual shifts rarely help. -The compiler already converts power-of-two unsigned divisions to shifts at `-O2`. -On RISC-V-based boards (ESP32-C3, ESP32-C6, ESP32-C5) explicit shifts can be beneficial: - -Prefer bit shifts for power-of-two operations: +On RISC-V targets (ESP32-C3/C6/P4), prefer explicit bit-shifts for power-of-two arithmetic — the compiler does **not** always convert divisions to shifts on RISC-V at `-O2`. Always use unsigned operands; signed right-shift is implementation-defined. + +On RISC-V-based boards (ESP32-C3, ESP32-C6, ESP32-C5) explicit shifts can be beneficial. ```cpp position >> 3 // instead of position / 8 (255U - rate) >> 1 // instead of (255 - rate) / 2 @@ -407,6 +420,7 @@ i & 0x0007 // instead of i % 8 **Important**: The bit-shifted expression should be unsigned. On some MCUs, "signed right-shift" is implemented by an "arithmetic shift right" that duplicates the sign bit: ``0b1010 >> 1 = 0b1101``. + ### Static Caching for Expensive Computations Cache results in static locals when the input rarely changes between calls: @@ -436,8 +450,8 @@ if (lastKelvin != kelvin) { ```cpp uint16_t r1 = R(color1); // 16-bit intermediate keeps the multiply result in 32 bits, avoiding 64-bit promotion ``` - + ## Multi-Task Synchronization ESP32 runs multiple FreeRTOS tasks concurrently (e.g. network handling, LED output, JSON parsing). Use the WLED-MM mutex macros for synchronization — they expand to FreeRTOS recursive semaphore calls on ESP32 and compile to no-ops on ESP8266: From aaf737589a9e2f6d6d01f2f58ce236bd3e2e5f2b Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 19:15:01 +0200 Subject: [PATCH 04/16] small improvment for cpp instructions --- .github/cpp.instructions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/cpp.instructions.md b/.github/cpp.instructions.md index 0799694ab0..345a82ac68 100644 --- a/.github/cpp.instructions.md +++ b/.github/cpp.instructions.md @@ -104,10 +104,10 @@ uint8_t gammaCorrect(uint8_t value, float gamma); ## Memory - **PSRAM-aware allocation**: use `d_malloc()` (prefer DRAM), `p_malloc()` (prefer PSRAM) from `util.h` +- **Avoid Variable Length Arrays (VLAs)**: FreeRTOS task stacks are typically 2–8 KB. A runtime-sized VLA can silently exhaust the stack. Use fixed-size arrays or heap allocation (`d_malloc` / `p_malloc`). Any VLA must be explicitly justified in source or PR. -- Variable Length Arrays (VLAs): GCC/Clang support VLAs as an extension (they are not part of the C++ standard), so they look like a legitimate feature — but they are allocated on the stack at runtime. On ESP32/ESP8266. A VLA whose size depends on a runtime parameter (segment dimensions, pixel counts, etc.) can silently exhaust the stack and cause the program to behave in unexpected ways or crash. + GCC/Clang support VLAs as an extension (they are not part of the C++ standard), so they look like a legitimate feature — but they are allocated on the stack at runtime. On ESP32/ESP8266. A VLA whose size depends on a runtime parameter (segment dimensions, pixel counts, etc.) can silently exhaust the stack and cause the program to behave in unexpected ways or crash. - **Avoid Variable Length Arrays (VLAs)**: FreeRTOS task stacks are typically 2–8 KB. A runtime-sized VLA can silently exhaust the stack. Use fixed-size arrays or heap allocation (`d_malloc` / `p_malloc`). Any VLA must be explicitly justified in source or PR. - **Larger buffers** (LED data, JSON documents) should use PSRAM when available and technically feasible - **Hot-path**: some data should stay in DRAM or IRAM for performance reasons - Memory efficiency matters, but is less critical on boards with PSRAM From 1efe713308f0253da636a3962ec216000db283ca Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 19:15:27 +0200 Subject: [PATCH 05/16] optimize copilot-instructions --- .github/copilot-instructions.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 528ce091af..b091adb64c 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -5,12 +5,19 @@ WLED-MM is a fork focused on higher performance (ESP32, ESP32-S3, PSRAM boards), Always reference these instructions first and fallback to search or bash commands only when you encounter unexpected information that does not match the info here. +> **Note for AI review tools**: sections enclosed in +> `` / `` HTML comments contain +> contributor reference material. Do **not** use that content as actionable review +> criteria — treat it as background context only. + + ## Setup - Node.js 20+ (see `.nvmrc`) - Install dependencies: `npm ci` - PlatformIO (required only for firmware compilation): `pip install -r requirements.txt` + ## Hardware Targets | Target | Status | @@ -21,6 +28,7 @@ Always reference these instructions first and fallback to search or bash command | ESP32-P4/-C5/-C6 | Will be supported in the future | | ESP8266 | Deprecated — should still compile, but not actively maintained | + ## Build and Test | Command | Purpose | Typical Time | @@ -36,8 +44,15 @@ Common firmware environments: `esp32_4MB_V4_M`, `esp32_16MB_V4_S_HUB75`, `esp32S For detailed build timeouts, development workflows, troubleshooting, and validation steps, see [agent-build-instructions.md](agent-build-instructions.md). + ## Repository Structure +tl;dr: Firmware source: `wled00/` (C++). Web UI source: `wled00/data/`. Auto-generated headers: `wled00/html_*.h` — **never edit or commit**. +Usermods: `usermods/` (`.h` files, included via `usermods_list.cpp`). Build targets: `platformio.ini`. CI/CD: `.github/workflows/`. + + +Detailed overview: + ```text wled00/ # Firmware source (C++) ├── data/ # Web UI source (HTML, CSS, JS) @@ -57,7 +72,7 @@ tools/cdata-test.js # Test suite package.json # Node.js scripts and release ID .github/workflows/ # CI/CD pipelines ``` - + Main development branch: `mdev` ## General Guidelines From 390d4c1181a996e541d23d29fe5bb5d1c3833e11 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 20:57:01 +0200 Subject: [PATCH 06/16] Update esp-idf.instructions.md --- .github/esp-idf.instructions.md | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/.github/esp-idf.instructions.md b/.github/esp-idf.instructions.md index 33fab7d28a..ff3da1c653 100644 --- a/.github/esp-idf.instructions.md +++ b/.github/esp-idf.instructions.md @@ -30,10 +30,10 @@ Use `CONFIG_IDF_TARGET_*` macros to gate chip-specific code at compile time. The | `CONFIG_IDF_TARGET_ESP32P4` | ESP32-P4 | RISC-V dual-core | High performance. Future target | - ### Build-time validation WLED validates at compile time that exactly one target is defined and that it is a supported chip (`wled.cpp` lines 39–61). Follow this pattern when adding new chip-specific branches: + ```cpp #if defined(CONFIG_IDF_TARGET_ESP32) // classic ESP32 path @@ -46,6 +46,7 @@ WLED validates at compile time that exactly one target is defined and that it is #endif ``` + ### Guidelines - **Always test on the actual chip** before claiming support. Simulators and cross-compilation can hide peripheral differences. @@ -160,7 +161,6 @@ On ESP32-S3 modules with OPI flash (e.g. N8R8 modules where the SPI flash itself // Legacy IDF v3/v4.x path #endif ``` - ### Key ESP-IDF version thresholds for WLED-MM @@ -174,6 +174,7 @@ On ESP32-S3 modules with OPI flash (e.g. N8R8 modules where the SPI flash itself | **5.1.0** | Matter protocol support; new `esp_flash` API stable | | **5.3+** | arduino-esp32 v3.x compatibility; C6/P4 support | + ### Guidelines - When adding a version guard, **always include a comment** explaining *what* changed and *why* the guard is needed. @@ -193,6 +194,7 @@ On ESP32-S3 modules with OPI flash (e.g. N8R8 modules where the SPI flash itself The jump from IDF v4.4 (arduino-esp32 v2.x) to IDF v5.x (arduino-esp32 v3.x) is the largest API break in ESP-IDF history. This section documents the critical changes and recommended migration patterns based on the upstream WLED `V5-C6` branch (`https://github.com/wled/WLED/tree/V5-C6`). Note: WLED-MM has not yet migrated to IDF v5 — these patterns prepare for the future migration. + ### Compiler changes IDF v5.x ships a much newer GCC toolchain. Key versions: @@ -264,33 +266,41 @@ These work on both IDF v4.4 and v5.x — prefer them now: | Narrowing in aggregate init | Warning | Error | Use explicit cast or wider type | | Implicit `this` capture in lambdas | Accepted in `[=]` | Deprecated warning; error in C++20 mode | Use `[=, this]` or `[&]` | + #### Recommendations - **Do not raise the minimum C++ standard yet.** WLED-MM must still build on IDF v4.4 (GCC 8.4, C++17). Use `#if __cplusplus > 201703L` to gate C++20 features. +- **Mark intentional fallthrough** with `[[fallthrough]]` — GCC 14 warns on unmarked fallthrough by default. + - **Prefer `std::optional` over sentinel values** (e.g., `-1` for "no pin") in new code — it works on both compilers. - **Use `std::string_view`** for read-only string parameters instead of `const char*` or `const String&` — zero-copy and works on GCC 8+. - **Avoid raw `union` type punning** — prefer `memcpy` (GCC 8) or `std::bit_cast` (GCC 13+) for strict-aliasing safety. -- **Mark intentional fallthrough** with `[[fallthrough]]` — GCC 14 warns on unmarked fallthrough by default. + ### Deprecated and removed APIs #### RMT (Remote Control Transceiver) -The legacy `rmt_*` functions are removed in IDF v5. The new API is channel-based: +The legacy `rmt_*` functions are removed in IDF v5. Do not introduce new legacy RMT calls. + +The new API is channel-based: | IDF v4 (legacy) | IDF v5 (new) | Notes | |---|---|---| | `rmt_config()` + `rmt_driver_install()` | `rmt_new_tx_channel()` / `rmt_new_rx_channel()` | Channels are now objects | | `rmt_write_items()` | `rmt_transmit()` with encoder | Requires `rmt_encoder_t` | | `rmt_set_idle_level()` | Configure in channel config | Set at creation time | | `rmt_item32_t` | `rmt_symbol_word_t` | Different struct layout | + **WLED impact**: NeoPixelBus LED output and IR receiver both use legacy RMT. The upstream `V5-C6` branch adds `-D WLED_USE_SHARED_RMT` and disables IR until the library is ported. #### I2S (Inter-IC Sound) -Legacy `i2s_driver_install()` + `i2s_read()` API is deprecated. The new API uses channel handles: +Legacy `i2s_driver_install()` + `i2s_read()` API is deprecated. When touching audio source code, wrap any I2S init in `#if ESP_IDF_VERSION` guards. + + The new API uses channel handles: | IDF v4 (legacy) | IDF v5 (new) | Notes | |---|---|---| | `i2s_driver_install()` | `i2s_channel_init_std_mode()` | Separate STD/PDM/TDM modes | @@ -299,7 +309,6 @@ Legacy `i2s_driver_install()` + `i2s_read()` API is deprecated. The new API uses | `i2s_set_clk()` | `i2s_channel_reconfig_std_clk()` | Reconfigure running channel | | `i2s_config_t` | `i2s_std_config_t` | Separate config for each mode | - **Migration pattern** (from Espressif examples): ```cpp #if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0) @@ -319,10 +328,10 @@ Legacy `i2s_driver_install()` + `i2s_read()` API is deprecated. The new API uses // Legacy i2s_driver_install() path #endif ``` - -**WLED impact**: The audioreactive usermod (`audio_source.h`) heavily uses legacy I2S. Migration requires rewriting the `I2SSource` class for channel-based API. +**WLED impact**: The audioreactive usermod (`audio_source.h`) heavily uses legacy I2S. Migration requires rewriting the `I2SSource` class for channel-based API. + #### ADC (Analog-to-Digital Converter) Legacy `adc1_get_raw()` and `esp_adc_cal_*` are deprecated: @@ -350,7 +359,6 @@ WLED already has a compatibility shim in `ota_update.cpp` that maps old names to | `gpio_pad_select_gpio()` | `esp_rom_gpio_pad_select_gpio()` (or use `gpio_config()`) | | `gpio_set_direction()` + `gpio_set_pull_mode()` | `gpio_config()` with `gpio_config_t` struct | - ### Features disabled in IDF v5 builds The upstream `V5-C6` branch explicitly disables features with incompatible library dependencies: @@ -364,7 +372,6 @@ The upstream `V5-C6` branch explicitly disables features with incompatible libra ``` - ### Migration checklist for new code 1. **Never use a removed API without a version guard.** Always provide both old and new paths, or disable the feature on IDF v5. @@ -785,6 +792,7 @@ esp_task_wdt_add(NULL); // re-register > **IDF v5 note**: In IDF v5, `esp_task_wdt_add()` and `esp_task_wdt_delete()` require an explicit `TaskHandle_t`. Use `xTaskGetCurrentTaskHandle()` instead of `NULL`. + --- ## Quick Reference: IDF v4 → v5 API Mapping @@ -799,3 +807,4 @@ esp_task_wdt_add(NULL); // re-register | GPIO | `driver/gpio.h` | `driver/gpio.h` | `gpio_pad_select_gpio()` removed | | Timer | `driver/timer.h` | `driver/gptimer.h` | General-purpose timer handles | | PCNT | `driver/pcnt.h` | `driver/pulse_cnt.h` | Handle-based API | + From 38861e7c1d68b1ec5dd3f73011a7c6254ec84245 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 21:09:37 +0200 Subject: [PATCH 07/16] Add blank lines around the table block --- .github/esp-idf.instructions.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/esp-idf.instructions.md b/.github/esp-idf.instructions.md index ff3da1c653..0366df9309 100644 --- a/.github/esp-idf.instructions.md +++ b/.github/esp-idf.instructions.md @@ -285,14 +285,15 @@ The legacy `rmt_*` functions are removed in IDF v5. Do not introduce new legacy The new API is channel-based: + | IDF v4 (legacy) | IDF v5 (new) | Notes | |---|---|---| | `rmt_config()` + `rmt_driver_install()` | `rmt_new_tx_channel()` / `rmt_new_rx_channel()` | Channels are now objects | | `rmt_write_items()` | `rmt_transmit()` with encoder | Requires `rmt_encoder_t` | | `rmt_set_idle_level()` | Configure in channel config | Set at creation time | | `rmt_item32_t` | `rmt_symbol_word_t` | Different struct layout | - + **WLED impact**: NeoPixelBus LED output and IR receiver both use legacy RMT. The upstream `V5-C6` branch adds `-D WLED_USE_SHARED_RMT` and disables IR until the library is ported. #### I2S (Inter-IC Sound) From e71a9d4979718d4ef9d0f9ae0d77f0b12f92a312 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 21:24:32 +0200 Subject: [PATCH 08/16] improvements for AI-facing view --- .github/cpp.instructions.md | 6 +++--- .github/esp-idf.instructions.md | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.github/cpp.instructions.md b/.github/cpp.instructions.md index 345a82ac68..b6143a8371 100644 --- a/.github/cpp.instructions.md +++ b/.github/cpp.instructions.md @@ -130,7 +130,7 @@ const uint_fast16_t rows = virtualHeight(); ### `const` references to avoid copies -Pass and store objects by `const &` (or `&`) instead of copying them implicitly. This avoids constructing temporary objects on every access — especially important in loops: +Pass and store objects by `const &` (or `&`) instead of copying them implicitly. This avoids constructing temporary objects on every access — especially important in loops. ```cpp @@ -299,7 +299,7 @@ if (unsigned(i) >= virtualLength()) return; // bounds check (catches negative i ### Avoid Nested Calls — Fast Path / Complex Path -Avoid calling non-inline functions or making complex decisions inside per-pixel hot-path code. When a function has both a common simple case and a rare complex case, split it into two variants and choose once per frame rather than per pixel: +Avoid calling non-inline functions or making complex decisions inside per-pixel hot-path code. When a function has both a common simple case and a rare complex case, split it into two variants and choose once per frame rather than per pixel. ```cpp // Decision made once per frame in startFrame(), stored in a bool @@ -384,7 +384,7 @@ This avoids FreeRTOS semaphore overhead and the risk of forgetting `esp32SemGive ### Pre-Compute Outside Loops -Move invariant calculations before the loop. Pre-compute reciprocals to replace division with multiplication: +Move invariant calculations before the loop. Pre-compute reciprocals to replace division with multiplication. ```cpp const uint_fast16_t cols = virtualWidth(); diff --git a/.github/esp-idf.instructions.md b/.github/esp-idf.instructions.md index 0366df9309..3c36df05ab 100644 --- a/.github/esp-idf.instructions.md +++ b/.github/esp-idf.instructions.md @@ -60,8 +60,8 @@ WLED validates at compile time that exactly one target is defined and that it is `SOC_*` macros (from `soc/soc_caps.h`) describe what the current chip supports. They are the correct way to check for peripheral features — they stay accurate when new chips are added, unlike `CONFIG_IDF_TARGET_*` checks. -### Important `SOC_*` macros used in WLED-MM +### Important `SOC_*` macros used in WLED-MM | Macro | Type | Used in | Purpose | |---|---|---|---| @@ -73,9 +73,10 @@ WLED validates at compile time that exactly one target is defined and that it is | `SOC_ADC_CHANNEL_NUM(unit)` | `int` | `pin_manager.cpp` | ADC channels per unit | | `SOC_UART_NUM` | `int` | `dmx_input.cpp` | Number of UART peripherals | | `SOC_DRAM_LOW` / `SOC_DRAM_HIGH` | `addr` | `util.cpp` | DRAM address boundaries for validation | - -Common pitfall: `SOC_ADC_MAX_BITWIDTH` (ADC resolution 12 or 13 bits) was renamed to `CONFIG_SOC_ADC_RTC_MAX_BITWIDTH` in IDF v5. + +### Key pitfall +`SOC_ADC_MAX_BITWIDTH` (ADC resolution 12 or 13 bits) was renamed to `CONFIG_SOC_ADC_RTC_MAX_BITWIDTH` in IDF v5. ### Less commonly used but valuable From 829a52df0ca7d27fdf92d53dcef37f1b064fbf14 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 21:36:09 +0200 Subject: [PATCH 09/16] html compliant < > --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b091adb64c..18c4c5f473 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -6,7 +6,7 @@ WLED-MM is a fork focused on higher performance (ESP32, ESP32-S3, PSRAM boards), Always reference these instructions first and fallback to search or bash commands only when you encounter unexpected information that does not match the info here. > **Note for AI review tools**: sections enclosed in -> `` / `` HTML comments contain +> `<!-- HUMAN_ONLY_START -->` / `<!-- HUMAN_ONLY_END -->` HTML comments contain > contributor reference material. Do **not** use that content as actionable review > criteria — treat it as background context only. From 0b4404b83d156548a34d22302bc0926745331e35 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 21:37:36 +0200 Subject: [PATCH 10/16] extract decision rules for AI use. --- .github/esp-idf.instructions.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/esp-idf.instructions.md b/.github/esp-idf.instructions.md index 3c36df05ab..a10419be0c 100644 --- a/.github/esp-idf.instructions.md +++ b/.github/esp-idf.instructions.md @@ -299,7 +299,7 @@ The new API is channel-based: #### I2S (Inter-IC Sound) -Legacy `i2s_driver_install()` + `i2s_read()` API is deprecated. When touching audio source code, wrap any I2S init in `#if ESP_IDF_VERSION` guards. +Legacy `i2s_driver_install()` + `i2s_read()` API is deprecated. When touching audio source code, wrap legacy I2S init and reading in `#if ESP_IDF_VERSION_MAJOR < 5` / `#else`. The new API uses channel handles: @@ -421,7 +421,10 @@ WLED-MM provides convenience wrappers with automatic fallback. **Always prefer t ``` - **Fragmentation**: PSRAM allocations fragment less than DRAM because the region is larger. But avoid mixing small and large allocations in PSRAM — small allocations waste the MMU page granularity. - **Heap validation**: use `d_measureHeap()` and `d_measureContiguousFreeHeap()` to monitor remaining DRAM. Allocations that would drop free DRAM below `MIN_HEAP_SIZE` should go to PSRAM instead. -- **Performance**: PSRAM access is up to 15× slower than DRAM on ESP32, 3–10× slower than DRAM on ESP32-S3/-S2 with quad-SPI bus. On ESP32-S3 with octal PSRAM (`CONFIG_SPIRAM_MODE_OCT`), the penalty is smaller (~2×) because the 8-line DTR bus can transfer 8 bits in parallel at 80MHz (120 MHz is possible with CONFIG_SPIRAM_SPEED_120M, which requires enabling experimental ESP-IDF features). On ESP32-P4 with hex PSRAM (`CONFIG_SPIRAM_MODE_HEX`), the 16-line bus runs at 200 MHz which brings it on-par with DRAM. Keep hot-path data in DRAM regardless, but consider that esp32 often crashes when the largest DRAM chunk gets below 10 KB. +- **Performance**: Keep hot-path data in DRAM. Prefer PSRAM for capacity-oriented buffers and monitor contiguous DRAM headroom. + + PSRAM access is up to 15× slower than DRAM on ESP32, 3–10× slower than DRAM on ESP32-S3/-S2 with quad-SPI bus. On ESP32-S3 with octal PSRAM (`CONFIG_SPIRAM_MODE_OCT`), the penalty is smaller (~2×) because the 8-line DTR bus can transfer 8 bits in parallel at 80MHz (120 MHz is possible with CONFIG_SPIRAM_SPEED_120M, which requires enabling experimental ESP-IDF features). On ESP32-P4 with hex PSRAM (`CONFIG_SPIRAM_MODE_HEX`), the 16-line bus runs at 200 MHz which brings it on-par with DRAM. Keep hot-path data in DRAM regardless, but consider that esp32 often crashes when the largest DRAM chunk gets below 10 KB. + ### Pattern: preference-based allocation From f974affbcdf910b0f687e540c9ecfe93a4f66cea Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 21:40:07 +0200 Subject: [PATCH 11/16] revert html < > does not display correctly due to verbatim quotes. --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 18c4c5f473..b091adb64c 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -6,7 +6,7 @@ WLED-MM is a fork focused on higher performance (ESP32, ESP32-S3, PSRAM boards), Always reference these instructions first and fallback to search or bash commands only when you encounter unexpected information that does not match the info here. > **Note for AI review tools**: sections enclosed in -> `<!-- HUMAN_ONLY_START -->` / `<!-- HUMAN_ONLY_END -->` HTML comments contain +> `` / `` HTML comments contain > contributor reference material. Do **not** use that content as actionable review > criteria — treat it as background context only. From fc8faaa0f610e7021e45c93b750fa07e7961bdc1 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 22:03:17 +0200 Subject: [PATCH 12/16] clearer expectations on AI code comments --- .github/copilot-instructions.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b091adb64c..7539cbb261 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -92,7 +92,8 @@ Main development branch: `mdev` Using AI-generated code can hide the source of the inspiration / knowledge / sources it used. - Document attribution of inspiration / knowledge / sources used in the code, e.g. link to GitHub repositories or other websites describing the principles / algorithms used. - When a larger block of code is generated by an AI tool, mark it with an `// AI: below section was generated by an AI` comment (see C++ guidelines). -- Make sure AI-generated code is well documented. +- Every non-trivial AI generated function should have a brief comment describing what it does. Explain parameters when their names alone are not self-explanatory. +- AI-generated code must be well documented; comment-to-code ratio > 15% is expected. Do not rephrase source code, but explain the concepts/logic behind the code. ### Pull Request Expectations From f2fd245414a3cac0f3417a168fe28e6d9c325625 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 22:06:07 +0200 Subject: [PATCH 13/16] grammar fix --- .github/cpp.instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/cpp.instructions.md b/.github/cpp.instructions.md index b6143a8371..7cbf6ab751 100644 --- a/.github/cpp.instructions.md +++ b/.github/cpp.instructions.md @@ -106,7 +106,7 @@ uint8_t gammaCorrect(uint8_t value, float gamma); - **PSRAM-aware allocation**: use `d_malloc()` (prefer DRAM), `p_malloc()` (prefer PSRAM) from `util.h` - **Avoid Variable Length Arrays (VLAs)**: FreeRTOS task stacks are typically 2–8 KB. A runtime-sized VLA can silently exhaust the stack. Use fixed-size arrays or heap allocation (`d_malloc` / `p_malloc`). Any VLA must be explicitly justified in source or PR. - GCC/Clang support VLAs as an extension (they are not part of the C++ standard), so they look like a legitimate feature — but they are allocated on the stack at runtime. On ESP32/ESP8266. A VLA whose size depends on a runtime parameter (segment dimensions, pixel counts, etc.) can silently exhaust the stack and cause the program to behave in unexpected ways or crash. + GCC/Clang support VLAs as an extension (they are not part of the C++ standard), so they look like a legitimate feature — but they are allocated on the stack at runtime. On ESP32/ESP8266, a VLA whose size depends on a runtime parameter (segment dimensions, pixel counts, etc.) can silently exhaust the stack and cause the program to behave in unexpected ways or crash. - **Larger buffers** (LED data, JSON documents) should use PSRAM when available and technically feasible - **Hot-path**: some data should stay in DRAM or IRAM for performance reasons From 2afe9d0b1784395490ffcd3abca9475474495231 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 22:13:31 +0200 Subject: [PATCH 14/16] Look for user-visible breaking changes and ripple effects --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 7539cbb261..00a61a63da 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -83,7 +83,7 @@ Main development branch: `mdev` - **When unsure, say so.** Gather more information rather than guessing. - **Acknowledge good patterns** when you see them. Summarize good practices as part of your review - positive feedback always helps. - **Provide references** when making analyses or recommendations. Base them on the correct branch or PR. -- **Look for user-visible "breaking" changes**. Ask for confirmation that these were made intentionally. +- **Look for user-visible breaking changes and ripple effects**. Ask for confirmation that these were introduced intentionally. - **Unused / dead code must be justified or removed**. This helps to keep the codebase clean, maintainable and readable. - **C++ formatting available**: `clang-format` is installed but not in CI - No automated linting is configured — match existing code style in files you edit. See `cpp.instructions.md` and `web.instructions.md` for language-specific conventions, and `cicd.instructions.md` for GitHub Actions workflows. From d11c8639ef9f9f65e65e1c9bef41ba91118a6531 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 22:17:21 +0200 Subject: [PATCH 15/16] style --- .github/esp-idf.instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/esp-idf.instructions.md b/.github/esp-idf.instructions.md index a10419be0c..ac26679dfe 100644 --- a/.github/esp-idf.instructions.md +++ b/.github/esp-idf.instructions.md @@ -423,7 +423,7 @@ WLED-MM provides convenience wrappers with automatic fallback. **Always prefer t - **Heap validation**: use `d_measureHeap()` and `d_measureContiguousFreeHeap()` to monitor remaining DRAM. Allocations that would drop free DRAM below `MIN_HEAP_SIZE` should go to PSRAM instead. - **Performance**: Keep hot-path data in DRAM. Prefer PSRAM for capacity-oriented buffers and monitor contiguous DRAM headroom. - PSRAM access is up to 15× slower than DRAM on ESP32, 3–10× slower than DRAM on ESP32-S3/-S2 with quad-SPI bus. On ESP32-S3 with octal PSRAM (`CONFIG_SPIRAM_MODE_OCT`), the penalty is smaller (~2×) because the 8-line DTR bus can transfer 8 bits in parallel at 80MHz (120 MHz is possible with CONFIG_SPIRAM_SPEED_120M, which requires enabling experimental ESP-IDF features). On ESP32-P4 with hex PSRAM (`CONFIG_SPIRAM_MODE_HEX`), the 16-line bus runs at 200 MHz which brings it on-par with DRAM. Keep hot-path data in DRAM regardless, but consider that esp32 often crashes when the largest DRAM chunk gets below 10 KB. + PSRAM access is up to 15× slower than DRAM on ESP32, 3–10× slower than DRAM on ESP32-S3/-S2 with quad-SPI bus. On ESP32-S3 with octal PSRAM (`CONFIG_SPIRAM_MODE_OCT`), the penalty is smaller (~2×) because the 8-line DTR bus can transfer 8 bits in parallel at 80 MHz (120 MHz is possible with CONFIG_SPIRAM_SPEED_120M, which requires enabling experimental ESP-IDF features). On ESP32-P4 with hex PSRAM (`CONFIG_SPIRAM_MODE_HEX`), the 16-line bus runs at 200 MHz which brings it on-par with DRAM. Keep hot-path data in DRAM regardless, but consider that ESP32 often crashes when the largest DRAM chunk gets below 10 KB. From 6b38be53e623ac36267c6288cf3170bf32add0c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frank=20M=C3=B6hle?= <91616163+softhack007@users.noreply.github.com> Date: Sat, 4 Apr 2026 22:18:41 +0200 Subject: [PATCH 16/16] Update .github/copilot-instructions.md style Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 00a61a63da..7b4d0e63bb 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -92,7 +92,7 @@ Main development branch: `mdev` Using AI-generated code can hide the source of the inspiration / knowledge / sources it used. - Document attribution of inspiration / knowledge / sources used in the code, e.g. link to GitHub repositories or other websites describing the principles / algorithms used. - When a larger block of code is generated by an AI tool, mark it with an `// AI: below section was generated by an AI` comment (see C++ guidelines). -- Every non-trivial AI generated function should have a brief comment describing what it does. Explain parameters when their names alone are not self-explanatory. +- Every non-trivial AI-generated function should have a brief comment describing what it does. Explain parameters when their names alone are not self-explanatory. - AI-generated code must be well documented; comment-to-code ratio > 15% is expected. Do not rephrase source code, but explain the concepts/logic behind the code. ### Pull Request Expectations