From eb2352774dcb3999d8d5c7b90f82ffb376ad61fb Mon Sep 17 00:00:00 2001 From: Frank <91616163+softhack007@users.noreply.github.com> Date: Mon, 6 Apr 2026 17:14:40 +0200 Subject: [PATCH] align AI instructions with upstream some updates coming from ongoing work --- .github/agent-build.instructions.md | 13 +++++----- .github/cicd.instructions.md | 2 ++ .github/copilot-instructions.md | 28 ++++++++++---------- .github/cpp.instructions.md | 40 ++++++++++++++++++++++++++--- .github/web.instructions.md | 5 ++-- 5 files changed, 62 insertions(+), 26 deletions(-) diff --git a/.github/agent-build.instructions.md b/.github/agent-build.instructions.md index 6873303d..4c0e2ded 100644 --- a/.github/agent-build.instructions.md +++ b/.github/agent-build.instructions.md @@ -11,7 +11,7 @@ Use these timeout values when running builds: | Command | Typical Time | Minimum Timeout | Notes | |---|---|---|---| -| `npm run build` | ~3 s | 30 s | Web UI → `wled00/html_*.h` headers | +| `npm run build` | ~3 s | 30 s | Web UI → `wled00/html_*.h` `wled00/js_*.h` headers | | `npm test` | ~40 s | 2 min | Validates build system | | `npm run dev` | continuous | — | Watch mode, auto-rebuilds on changes | | `pio run -e ` | 15–20 min | 30 min | First build downloads toolchains; subsequent builds are faster | @@ -23,13 +23,13 @@ Use these timeout values when running builds: ### Web UI Changes 1. Edit files in `wled00/data/` -2. Run `npm run build` to regenerate `wled00/html_*.h` headers +2. Run `npm run build` to regenerate `wled00/html_*.h` `wled00/js_*.h` headers 3. Test with local HTTP server (see Manual Testing below) 4. Run `npm test` to validate ### Firmware Changes -1. Edit files in `wled00/` (but **never** `html_*.h` files) +1. Edit files in `wled00/` (but **never** `html_*.h` and `js_*.h` files) 2. Ensure web UI is built first: `npm run build` 3. Build firmware: `pio run -e esp32_4MB_V4_M` (set timeout ≥ 30 min) 4. Flash to device: `pio run -e [target] --target upload` @@ -85,8 +85,8 @@ Test these scenarios after every web UI change: ### Recovery Steps - **Force web UI rebuild**: `npm run build -- -f` -- **Clear generated files**: `rm -f wled00/html_*.h` then `npm run build` -- **Clean PlatformIO cache**: `pio run --target clean` +- **Clear generated files**: `rm -f wled00/html_*.h wled00/js_*.h` then `npm run build` +- **Clean PlatformIO build artifacts**: `pio run --target clean` - **Reinstall Node deps**: `rm -rf node_modules && npm ci` ## CI/CD Validation @@ -106,7 +106,8 @@ Match this workflow in local development to catch failures before pushing. ## Important Reminders -- **Never edit or commit** `wled00/html_*.h` — auto-generated from `wled00/data/` +- Always **commit source code** +- **Never edit or commit** `wled00/html_*.h` and `wled00/js_*.h` — auto-generated from `wled00/data/` - Web UI rebuild is part of the PlatformIO firmware compilation pipeline - Common firmware environments: `esp32_4MB_V4_M`, `esp32_16MB_V4_S_HUB75`, `esp32S3_8MB_PSRAM_M_qspi`, `esp32_16MB_V4_M_eth`, `esp8266_4MB_S` (deprecated), `esp32dev_compat` - List all PlatformIO targets: `pio run --list-targets` diff --git a/.github/cicd.instructions.md b/.github/cicd.instructions.md index be0793dc..8dfb9c9a 100644 --- a/.github/cicd.instructions.md +++ b/.github/cicd.instructions.md @@ -71,6 +71,8 @@ schedule: ## Security +Important: Several current workflows still violate parts of the baseline below - migration is in progress. + ### Permissions — Least Privilege Declare explicit `permissions:` blocks. The default token permissions are broad; scope them to the minimum required: diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 5c1fe3c6..008af37e 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -40,22 +40,19 @@ Always reference these instructions first and fallback to search or bash command -**Always run `npm ci; npm run build` before `pio run`.** The web UI build generates `wled00/html_*.h` header files required by firmware compilation. -**Build firmware to validate code changes**: `pio run -e esp32_4MB_V4_M` — must succeed, never skip this step. -Common firmware environments: `esp32_4MB_V4_M`, `esp32_16MB_V4_S_HUB75`, `esp32S3_8MB_PSRAM_M_qspi`, `esp32_16MB_V4_M_eth`, `esp32dev_compat`, `esp8266_4MB_S` (deprecated) +- **Always run `npm ci; npm run build` before `pio run`.** The web UI build generates `wled00/html_*.h` header files required by firmware compilation. +- **Build firmware to validate code changes**: `pio run -e esp32_4MB_V4_M` — must succeed, never skip this step. +- Common firmware environments: `esp32_4MB_V4_M`, `esp32_16MB_V4_S_HUB75`, `esp32S3_8MB_PSRAM_M_qspi`, `esp32_16MB_V4_M_eth`, `esp32dev_compat`, `esp8266_4MB_S` (deprecated) 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++). -* Build targets: `platformio.ini`. -* Web UI source: `wled00/data/`. -* Auto-generated headers: `wled00/html_*.h` — **never edit or commit**. -* ArduinoJSON + AsyncJSON: `wled00/src/dependencies/json` +* Firmware source: `wled00/` (C++). Web UI source: `wled00/data/`. Build targets: `platformio.ini`. +* Auto-generated headers: `wled00/html_*.h` and `wled00/js_*.h` — **never edit or commit**. +* ArduinoJSON + AsyncJSON: `wled00/src/dependencies/json`. CI/CD: `.github/workflows/`. * Usermods: `usermods/` (`.h` files, included via `usermods_list.cpp`). -* CI/CD: `.github/workflows/`. Main development trunk: `mdev` branch. Make PRs against this branch. @@ -81,27 +78,30 @@ tools/cdata-test.js # Test suite package.json # Node.js scripts and release ID .github/workflows/ # CI/CD pipelines ``` - + ## General Guidelines -- **Never edit or commit** `wled00/html_*.h` — auto-generated from `wled00/data/`. - **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. 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 and ripple effects**. Ask for confirmation that these were introduced intentionally. +- **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. - **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. +- No automated linting is configured — match existing code style in files you edit. + +See `cpp.instructions.md`, `esp-idf.instructions.md` and `web.instructions.md` for language-specific conventions, and `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). - 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. +- 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 diff --git a/.github/cpp.instructions.md b/.github/cpp.instructions.md index 2ada2aca..b4ff9950 100644 --- a/.github/cpp.instructions.md +++ b/.github/cpp.instructions.md @@ -18,13 +18,13 @@ 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` - **PascalCase** for classes and structs: `PinManagerClass`, `BusConfig` - **UPPER_CASE** for macros and constants: `WLED_MAX_USERMODS`, `DEFAULT_CLIENT_SSID` + ## Header Guards Most headers use `#ifndef` / `#define` guards. Some newer headers add `#pragma once` before the guard: @@ -64,6 +64,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 @@ -77,7 +78,6 @@ 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. ## Preprocessor & Feature Flags @@ -106,12 +106,38 @@ 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 - Memory efficiency matters, but is less critical on boards with PSRAM +Heap fragmentation is a concern: + + - Fragmentation can lead to crashes, even when the overall amount of available heap is still good. The C++ runtime doesn't do any "garbage collection". + + - Avoid frequent `d_malloc` and `d_free` inside a function, especially for small sizes. + - Avoid frequent creation / destruction of objects. + - Allocate buffers early, and try to re-use them. + - Instead of incrementally appending to a `String`, reserve the expected max buffer upfront by using the `reserve()` method. + + +```cpp + String result; + result.reserve(65); // pre-allocate to avoid realloc fragmentation +``` + +```cpp + // prefer DRAM; falls back gracefully and enforces MIN_HEAP_SIZE guard + _ledsDirty = (byte*) d_malloc(getBitArrayBytes(_len)); +``` + +```cpp + _mode.reserve(_modeCount); // allocate memory to prevent initial fragmentation - does not increase size() + _modeData.reserve(_modeCount); // allocate memory to prevent initial fragmentation - does not increase size() +``` + + ## `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. @@ -175,6 +201,12 @@ static_assert(WLED_MAX_BUSSES <= 32, "WLED_MAX_BUSSES exceeds hard limit"); #error "WLED_MAX_BUSSES exceeds hard limit" #endif ``` + +```cpp + // using static_assert() to validate enumerated types (zero cost at runtime) + static_assert(0u == static_cast(PinOwner::None), + "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. @@ -530,7 +562,7 @@ Prefer blocking FreeRTOS primitives (`xQueueReceive`, `ulTaskNotifyTake`, `vTask - 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 -- **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/C6) can produce different results due to clamping. Cast through a signed integer first: +- **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 // Undefined behavior — avoid: uint8_t angle = 40.74f * atan2f(dy, dx); // negative float → uint8_t is UB diff --git a/.github/web.instructions.md b/.github/web.instructions.md index 3713fdae..4a3d760f 100644 --- a/.github/web.instructions.md +++ b/.github/web.instructions.md @@ -24,5 +24,6 @@ applyTo: "wled00/data/**" ## Build Integration -Files in this directory are processed by `tools/cdata.js` into `wled00/html_*.h` headers. -Run `npm run build` after any change. **Never edit the generated `html_*.h` files directly.** +Files in this directory are processed by `tools/cdata.js` into generated headers +(`wled00/html_*.h`, `wled00/js_*.h`). +Run `npm run build` after any change. **Never edit generated headers directly.**