From 04336053f355cb84735046b304c05e0aa5c54b79 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Tue, 14 Apr 2026 14:48:43 +0200 Subject: [PATCH 1/5] sync c++ and generic instructions with upstream --- .github/copilot-instructions.md | 15 +++++++-- docs/cpp.instructions.md | 60 ++++++++++++++++++++------------- 2 files changed, 49 insertions(+), 26 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b8d481d75a..4b88c39043 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -90,23 +90,32 @@ docs/ # Contributor docs: coding guidelines and design doc - **Never edit or commit** `wled00/html_*.h` and `wled00/js_*.h` — auto-generated from `wled00/data/`. - If updating Web UI files in `wled00/data/`, **make use of common functions in `wled00/data/common.js` whenever possible**. - **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. +- **Acknowledge good patterns** when you see them. Positive feedback always helps. - **Provide references** when making analyses or recommendations. Base them on the correct branch or PR. - **Highlight 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. +- **Verify feature-flag names.** Every `WLED_ENABLE_*` / `WLED_DISABLE_*` flag must exactly match one of the names below — misspellings are silently ignored by the preprocessor (e.g. `WLED_IR_DISABLE` instead of `WLED_DISABLE_INFRARED`), causing silent build variations. Flag unrecognised names as likely typos and suggest the correct spelling. +
**`WLED_DISABLE_*`**: `2D`, `ADALIGHT`, `ALEXA`, `BROWNOUT_DET`, `ESPNOW`, `FILESYSTEM`, `HUESYNC`, `IMPROV_WIFISCAN`, `INFRARED`, `LOXONE`, `MQTT`, `OTA`, `PARTICLESYSTEM1D`, `PARTICLESYSTEM2D`, `PIXELFORGE`, `WEBSOCKETS` +
**`WLED_ENABLE_*`**: `ADALIGHT`, `AOTA`, `DMX`, `DMX_INPUT`, `DMX_OUTPUT`, `FS_EDITOR`, `GIF`, `HUB75MATRIX`, `JSONLIVE`, `LOXONE`, `MQTT`, `PIXART`, `PXMAGIC`, `USERMOD_PAGE`, `WEBSOCKETS`, `WPA_ENTERPRISE` - **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 `docs/cpp.instructions.md`, `docs/esp-idf.instructions.md` and `docs/web.instructions.md` for language-specific conventions, and `docs/cicd.instructions.md` for GitHub Actions workflows. +Refer to `docs/cpp.instructions.md`, `docs/esp-idf.instructions.md` and `docs/web.instructions.md` for language-specific conventions, and `docs/cicd.instructions.md` for GitHub Actions workflows. ### Attribution for AI-generated code 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). +- When a larger block of code is generated by an AI tool, embed it into `// AI: below section was generated by an AI` ... `// AI: end` comments (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. - AI-generated code must be well documented with meaningful comments that explain intent, assumptions, and non-obvious logic. Do not rephrase source code; explain concepts and reasoning. ### Pull Request Expectations - **No force-push on open PRs.** Once a pull request is open and being reviewed, do not force-push (`git push --force`) to the branch. Force-pushing rewrites history that reviewers may have already commented on, making it impossible to track incremental changes. Use regular commits or `git merge` to incorporate feedback; the branch will be squash-merged when it is accepted. +- **Modifications to ``platformio.ini`` MUST be approved explicitly** by a *maintainer* or *WLED organisation Member*. Modifications to the global build environment may break github action builds. Always flag them. - **Document your changes in the PR.** Every pull request should include a clear description of *what* changed and *why*. If the change affects user-visible behavior, describe the expected impact. Link to related issues where applicable. Provide screenshots to showcase new features. + +### Supporting Reviews and Discussions +- **For "is it worth doing?" debates** about proposed reliability, safety, or data-integrity mechanisms (CRC checks, backups, power-loss protection): suggest a software **FMEA** (Failure Mode and Effects Analysis). + Clarify the main feared events, enumerate failure modes, assess each mitigation's effectiveness per failure mode, note common-cause failures, and rate credibility for the typical WLED use case. + diff --git a/docs/cpp.instructions.md b/docs/cpp.instructions.md index e44459dbb2..5dd4dbd8f0 100644 --- a/docs/cpp.instructions.md +++ b/docs/cpp.instructions.md @@ -25,8 +25,16 @@ See also: [CONTRIBUTING.md](../CONTRIBUTING.md) for general style guidelines tha - **camelCase** for functions and variables: `setValuesFromMainSeg()`, `effectCurrent` - **PascalCase** for classes and structs: `PinManagerClass`, `BusConfig` +- **PascalCase** for enum values: `PinOwner::BusDigital` - **UPPER_CASE** for macros and constants: `WLED_MAX_USERMODS`, `DEFAULT_CLIENT_SSID` +## General + +- Follow the existing style in the file you are editing +- If possible, use `static` for local (C-style) variables and functions (keeps the global namespace clean) +- Avoid unexplained "magic numbers". Prefer named constants (`constexpr`) or C-style `#define` constants for repeated numbers that have the same meaning +- Include `"wled.h"` as the primary project header where needed + ## Header Guards @@ -61,7 +69,7 @@ Most headers use `#ifndef` / `#define` guards. Some newer headers add `#pragma o void calculateCRC(const uint8_t* data, size_t len) { ... } -// AI: end of AI-generated section +// AI: end ``` 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. @@ -142,25 +150,28 @@ Heap fragmentation is a concern: ## `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. -### `const` locals +`constexpr` allows to define constants that are *guaranteed* to be evaluated by the compiler (zero run-time costs). + + +- For function parameters that are read-only, prefer `const &` or `const`. +### `const` locals + * Adding `const` to a local variable that is only assigned once is optional, but *not* strictly necessary. -* In hot-path code, `const` on cached locals may help the compiler keep values in registers: + +* In hot-path code, `const` on cached locals may help the compiler keep values in registers. ```cpp const uint_fast16_t cols = vWidth(); const uint_fast16_t rows = vHeight(); ``` - ### `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 objects by `const &` (or `&`) instead of copying them implicitly. +- Use `const &` (or `&`) inside loops - This avoids constructing temporary objects on every access. ```cpp @@ -174,11 +185,14 @@ For function parameters that are read-only, prefer `const &`: BusDigital(BusConfig &bc, uint8_t nr, const ColorOrderMap &com); ``` +- Class **Data Members:** Avoid reference data members (`T&` or `const T&`) in a class. + A reference member can outlive the object it refers to, causing **dangling reference** bugs that are hard to diagnose. Prefer value storage or use a pointer and document the expected lifetime. -### `constexpr` over `#define` + +### `constexpr` over `#define` -Prefer `constexpr` for compile-time constants. Unlike `#define`, `constexpr` respects scope and type safety, keeping the global namespace clean: +- Prefer `constexpr` for compile-time constants. Unlike `#define`, `constexpr` respects scope and type safety, keeping the global namespace clean. ```cpp // Prefer: @@ -190,11 +204,14 @@ constexpr int WLED_MAX_BUSSES = WLED_MAX_DIGITAL_CHANNELS + WLED_MAX_ANALOG_CHAN ``` Note: `#define` is still needed for conditional compilation guards (`#ifdef`), platform macros, and values that must be overridable from build flags. + ### `static_assert` over `#error` -Use `static_assert` instead of the C-style `#if … #error … #endif` pattern when validating compile-time constants. It provides a clear message and works with `constexpr` values: +- Use `static_assert` instead of the C-style `#if … #error … #endif` pattern when validating compile-time constants. It provides a clear message and works with `constexpr` values. +- `#define` and `#if ... #else ... #endif` is still needed for conditional-compilation guards and build-flag-overridable values. + ```cpp // Prefer: constexpr int WLED_MAX_BUSSES = WLED_MAX_DIGITAL_CHANNELS + WLED_MAX_ANALOG_CHANNELS; @@ -212,10 +229,6 @@ static_assert(WLED_MAX_BUSSES <= 32, "WLED_MAX_BUSSES exceeds hard limit"); "PinOwner::None must be zero, so default array initialization works as expected"); ``` - -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 #### `const` member functions @@ -425,7 +438,7 @@ Move invariant calculations before the loop. Pre-compute reciprocals to replace ```cpp const uint_fast16_t cols = virtualWidth(); const uint_fast16_t rows = virtualHeight(); -uint_fast8_t fadeRate = (255 - rate) >> 1; +uint_fast8_t fadeRate = (255U - rate) >> 1; float mappedRate_r = 1.0f / (float(fadeRate) + 1.1f); // reciprocal — avoid division inside loop ``` @@ -444,7 +457,8 @@ 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. -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 targets (ESP32-C3/C6/P4) and ESP8266, prefer explicit bit-shifts for power-of-two arithmetic — the compiler does **not** always convert divisions to shifts. +Always use unsigned operands for right shifts; signed right-shift is implementation-defined. On RISC-V-based boards (ESP32-C3, ESP32-C6, ESP32-C5) explicit shifts can be beneficial. @@ -488,6 +502,9 @@ if (lastKelvin != kelvin) { ``` + +--- + ## 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: @@ -525,7 +542,7 @@ Always pair every `esp32SemTake` with a matching `esp32SemGive`. Choose a timeou * On ESP32, `delay(ms)` calls `vTaskDelay(ms / portTICK_PERIOD_MS)`, which **suspends only the calling task**. The FreeRTOS scheduler immediately runs all other ready tasks. * The Arduino `loop()` function runs inside `loopTask`. Calling `delay()` there does *not* block the network stack, audio FFT, LED DMA, nor any other FreeRTOS task. -* This differs from ESP8266, where `delay()` stalled the entire system unless `yield()` was called inside. +* This differs from ESP8266, where `delay()` stalls the entire system unless `yield()` was called inside. - On ESP32, `delay()` is generally allowed, as it helps to efficiently manage CPU usage of all tasks. @@ -565,12 +582,9 @@ void myTask(void*) { - Prefer blocking FreeRTOS primitives (`xQueueReceive`, `ulTaskNotifyTake`, `vTaskDelayUntil`) over `delay(1)` polling where precise timing or event-driven behaviour is needed. - **Watchdog note.** WLED-MM disables the Task Watchdog by default (`WLED_WATCHDOG_TIMEOUT 0` in `wled.h`). When enabled, `esp_task_wdt_reset()` is called at the end of each `loop()` iteration. Long blocking operations inside `loop()` — such as OTA downloads or slow file I/O — must call `esp_task_wdt_reset()` periodically, or be restructured so the main loop is not blocked for longer than the configured timeout. -## General +## Caveats and Pitfalls -- Follow the existing style in the file you are editing -- If possible, use `static` for local (C-style) variables and functions (keeps the global namespace clean) -- Avoid unexplained "magic numbers". Prefer named constants (`constexpr`) or C-style `#define` constants for repeated numbers that have the same meaning -- Include `"wled.h"` as the primary project header where needed +- **LittleFS filenames**: File paths passed to `file.open()` must not exceed 255 bytes (`LFS_NAME_MAX`). Validate constructed paths (e.g., `/ledmap_` + segment name + `.json`) stay within this limit (assume standard configurations, like WLED_MAX_SEGNAME_LEN = 64). - **Float-to-unsigned conversion is undefined behavior when the value is out of range.** Converting a negative `float` directly to an unsigned integer type (`uint8_t`, `uint16_t`, …) is UB per the C++ standard — the Xtensa (ESP32) toolchain may silently wrap, but RISC-V (ESP32-C3/C5/C6/P4) can produce different results due to clamping. Cast through a signed integer first: ```cpp From fba4c4b8696b90d8ee025484ed8c6e544b5ef484 Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Tue, 14 Apr 2026 15:00:08 +0200 Subject: [PATCH 2/5] minor fixes --- docs/cpp.instructions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/cpp.instructions.md b/docs/cpp.instructions.md index 5dd4dbd8f0..7e8aa078f2 100644 --- a/docs/cpp.instructions.md +++ b/docs/cpp.instructions.md @@ -171,7 +171,7 @@ This pattern enables optimizations and makes intent clear to reviewers. ### `const` references to avoid copies - Pass objects by `const &` (or `&`) instead of copying them implicitly. -- Use `const &` (or `&`) inside loops - This avoids constructing temporary objects on every access. +- Use `const &` (or `&`) inside loops - This avoids constructing temporary objects on every access. ```cpp @@ -454,7 +454,7 @@ uint32_t wg = (((c1 >> 8) & TWO_CHANNEL_MASK) * amount) & ~TWO_CHANNEL_MASK; return rb | wg; ``` -### Bit Shifts Over Division (mainly for RISC-V boards) +### Bit Shifts Over Division (mainly for RISC-V and ESP8266 boards) ESP32 and ESP32-S3 (Xtensa core) have a fast "integer divide" instruction, so manual shifts rarely help. On RISC-V targets (ESP32-C3/C6/P4) and ESP8266, prefer explicit bit-shifts for power-of-two arithmetic — the compiler does **not** always convert divisions to shifts. From 94e67c45961a3ff0200ac78644f21c2b461c964c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frank=20M=C3=B6hle?= <91616163+softhack007@users.noreply.github.com> Date: Wed, 15 Apr 2026 01:17:44 +0200 Subject: [PATCH 3/5] Enhance documentation for feature flags Moved feature flags into separate sub-section added code example --- .github/copilot-instructions.md | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 4b88c39043..5d60185d86 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -94,14 +94,31 @@ docs/ # Contributor docs: coding guidelines and design doc - **Provide references** when making analyses or recommendations. Base them on the correct branch or PR. - **Highlight 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. -- **Verify feature-flag names.** Every `WLED_ENABLE_*` / `WLED_DISABLE_*` flag must exactly match one of the names below — misspellings are silently ignored by the preprocessor (e.g. `WLED_IR_DISABLE` instead of `WLED_DISABLE_INFRARED`), causing silent build variations. Flag unrecognised names as likely typos and suggest the correct spelling. -
**`WLED_DISABLE_*`**: `2D`, `ADALIGHT`, `ALEXA`, `BROWNOUT_DET`, `ESPNOW`, `FILESYSTEM`, `HUESYNC`, `IMPROV_WIFISCAN`, `INFRARED`, `LOXONE`, `MQTT`, `OTA`, `PARTICLESYSTEM1D`, `PARTICLESYSTEM2D`, `PIXELFORGE`, `WEBSOCKETS` -
**`WLED_ENABLE_*`**: `ADALIGHT`, `AOTA`, `DMX`, `DMX_INPUT`, `DMX_OUTPUT`, `FS_EDITOR`, `GIF`, `HUB75MATRIX`, `JSONLIVE`, `LOXONE`, `MQTT`, `PIXART`, `PXMAGIC`, `USERMOD_PAGE`, `WEBSOCKETS`, `WPA_ENTERPRISE` - **C++ formatting available**: `clang-format` is installed but not in CI - No automated linting is configured — match existing code style in files you edit. Refer to `docs/cpp.instructions.md`, `docs/esp-idf.instructions.md` and `docs/web.instructions.md` for language-specific conventions, and `docs/cicd.instructions.md` for GitHub Actions workflows. +### Feature Flags +- **Verify feature-flag names.** Every `WLED_ENABLE_*` / `WLED_DISABLE_*` flag must exactly match one of the names below — misspellings are silently ignored by the preprocessor (e.g. `WLED_IR_DISABLE` instead of `WLED_DISABLE_INFRARED`), causing silent build variations. Flag unrecognised names as likely typos and suggest the correct spelling. + +**`WLED_DISABLE_*`**: `2D`, `ADALIGHT`, `ALEXA`, `BROWNOUT_DET`, `ESPNOW`, `FILESYSTEM`, `HUESYNC`, `IMPROV_WIFISCAN`, `INFRARED`, `LOXONE`, `MQTT`, `OTA`, `PARTICLESYSTEM1D`, `PARTICLESYSTEM2D`, `PIXELFORGE`, `WEBSOCKETS` + +**`WLED_ENABLE_*`**: `ADALIGHT`, `AOTA`, `DMX`, `DMX_INPUT`, `DMX_OUTPUT`, `FS_EDITOR`, `GIF`, `HUB75MATRIX`, `JSONLIVE`, `LOXONE`, `MQTT`, `PIXART`, `PXMAGIC`, `USERMOD_PAGE`, `WEBSOCKETS`, `WPA_ENTERPRISE` + + +```cpp +#ifdef WLED_DMX_ENABLED // wrong flag - initialization silently skipped + initDMX(); +#endif + +#ifdef WLED_ENABLE_DMX // correct flag - initialization performed only when the build supports DMX + initDMX(); +#endif +``` + + + ### Attribution for AI-generated code 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. From 48e06cbeb242d6157e8db1abba409d5ce2a8bcc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frank=20M=C3=B6hle?= <91616163+softhack007@users.noreply.github.com> Date: Wed, 15 Apr 2026 01:20:42 +0200 Subject: [PATCH 4/5] Update instruction on explicit shifts for RISC-V boards Clarified the recommendation for using explicit shifts on RISC-V and ESP8266 boards. --- docs/cpp.instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/cpp.instructions.md b/docs/cpp.instructions.md index 7e8aa078f2..69d6ac3685 100644 --- a/docs/cpp.instructions.md +++ b/docs/cpp.instructions.md @@ -461,7 +461,7 @@ On RISC-V targets (ESP32-C3/C6/P4) and ESP8266, prefer explicit bit-shifts for p Always use unsigned operands for right shifts; signed right-shift is implementation-defined. -On RISC-V-based boards (ESP32-C3, ESP32-C6, ESP32-C5) explicit shifts can be beneficial. +Explicit shifts can be beneficial on RISC-V-based boards (ESP32-C3, ESP32-C6, ESP32-C5) and on ESP8266 boards. ```cpp position >> 3 // instead of position / 8 (255U - rate) >> 1 // instead of (255 - rate) / 2 From c3839370e5da10070e2356cc5636aa317fa078a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frank=20M=C3=B6hle?= <91616163+softhack007@users.noreply.github.com> Date: Wed, 15 Apr 2026 01:32:57 +0200 Subject: [PATCH 5/5] Remove instruction on using common functions for updates Removed instruction about using common functions in Web UI updates. --- .github/copilot-instructions.md | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 5d60185d86..3b8fab0579 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -88,7 +88,6 @@ docs/ # Contributor docs: coding guidelines and design doc - **Repository language is English.** Suggest translations for non-English content. - **Use VS Code with PlatformIO extension** for best development experience. - **Never edit or commit** `wled00/html_*.h` and `wled00/js_*.h` — auto-generated from `wled00/data/`. -- If updating Web UI files in `wled00/data/`, **make use of common functions in `wled00/data/common.js` whenever possible**. - **When unsure, say so.** Gather more information rather than guessing. - **Acknowledge good patterns** when you see them. Positive feedback always helps. - **Provide references** when making analyses or recommendations. Base them on the correct branch or PR.