Skip to content

Commit eb23527

Browse files
committed
align AI instructions with upstream
some updates coming from ongoing work
1 parent a8463fa commit eb23527

5 files changed

Lines changed: 62 additions & 26 deletions

File tree

.github/agent-build.instructions.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ Use these timeout values when running builds:
1111

1212
| Command | Typical Time | Minimum Timeout | Notes |
1313
|---|---|---|---|
14-
| `npm run build` | ~3 s | 30 s | Web UI → `wled00/html_*.h` headers |
14+
| `npm run build` | ~3 s | 30 s | Web UI → `wled00/html_*.h` `wled00/js_*.h` headers |
1515
| `npm test` | ~40 s | 2 min | Validates build system |
1616
| `npm run dev` | continuous || Watch mode, auto-rebuilds on changes |
1717
| `pio run -e <env>` | 15–20 min | 30 min | First build downloads toolchains; subsequent builds are faster |
@@ -23,13 +23,13 @@ Use these timeout values when running builds:
2323
### Web UI Changes
2424

2525
1. Edit files in `wled00/data/`
26-
2. Run `npm run build` to regenerate `wled00/html_*.h` headers
26+
2. Run `npm run build` to regenerate `wled00/html_*.h` `wled00/js_*.h` headers
2727
3. Test with local HTTP server (see Manual Testing below)
2828
4. Run `npm test` to validate
2929

3030
### Firmware Changes
3131

32-
1. Edit files in `wled00/` (but **never** `html_*.h` files)
32+
1. Edit files in `wled00/` (but **never** `html_*.h` and `js_*.h` files)
3333
2. Ensure web UI is built first: `npm run build`
3434
3. Build firmware: `pio run -e esp32_4MB_V4_M` (set timeout ≥ 30 min)
3535
4. Flash to device: `pio run -e [target] --target upload`
@@ -85,8 +85,8 @@ Test these scenarios after every web UI change:
8585
### Recovery Steps
8686

8787
- **Force web UI rebuild**: `npm run build -- -f`
88-
- **Clear generated files**: `rm -f wled00/html_*.h` then `npm run build`
89-
- **Clean PlatformIO cache**: `pio run --target clean`
88+
- **Clear generated files**: `rm -f wled00/html_*.h wled00/js_*.h` then `npm run build`
89+
- **Clean PlatformIO build artifacts**: `pio run --target clean`
9090
- **Reinstall Node deps**: `rm -rf node_modules && npm ci`
9191

9292
## CI/CD Validation
@@ -106,7 +106,8 @@ Match this workflow in local development to catch failures before pushing.
106106

107107
## Important Reminders
108108

109-
- **Never edit or commit** `wled00/html_*.h` — auto-generated from `wled00/data/`
109+
- Always **commit source code**
110+
- **Never edit or commit** `wled00/html_*.h` and `wled00/js_*.h` — auto-generated from `wled00/data/`
110111
- Web UI rebuild is part of the PlatformIO firmware compilation pipeline
111112
- 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`
112113
- List all PlatformIO targets: `pio run --list-targets`

.github/cicd.instructions.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ schedule:
7171

7272
## Security
7373

74+
Important: Several current workflows still violate parts of the baseline below - migration is in progress.
75+
7476
### Permissions — Least Privilege
7577

7678
Declare explicit `permissions:` blocks. The default token permissions are broad; scope them to the minimum required:

.github/copilot-instructions.md

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,22 +40,19 @@ Always reference these instructions first and fallback to search or bash command
4040

4141
<!-- HUMAN_ONLY_END -->
4242

43-
**Always run `npm ci; npm run build` before `pio run`.** The web UI build generates `wled00/html_*.h` header files required by firmware compilation.
44-
**Build firmware to validate code changes**: `pio run -e esp32_4MB_V4_M` — must succeed, never skip this step.
45-
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)
43+
- **Always run `npm ci; npm run build` before `pio run`.** The web UI build generates `wled00/html_*.h` header files required by firmware compilation.
44+
- **Build firmware to validate code changes**: `pio run -e esp32_4MB_V4_M` — must succeed, never skip this step.
45+
- 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)
4646

4747
For detailed build timeouts, development workflows, troubleshooting, and validation steps, see [agent-build.instructions.md](agent-build.instructions.md).
4848

4949
## Repository Structure
5050

5151
tl;dr:
52-
* Firmware source: `wled00/` (C++).
53-
* Build targets: `platformio.ini`.
54-
* Web UI source: `wled00/data/`.
55-
* Auto-generated headers: `wled00/html_*.h`**never edit or commit**.
56-
* ArduinoJSON + AsyncJSON: `wled00/src/dependencies/json`
52+
* Firmware source: `wled00/` (C++). Web UI source: `wled00/data/`. Build targets: `platformio.ini`.
53+
* Auto-generated headers: `wled00/html_*.h` and `wled00/js_*.h`**never edit or commit**.
54+
* ArduinoJSON + AsyncJSON: `wled00/src/dependencies/json`. CI/CD: `.github/workflows/`.
5755
* Usermods: `usermods/` (`.h` files, included via `usermods_list.cpp`).
58-
* CI/CD: `.github/workflows/`.
5956

6057
Main development trunk: `mdev` branch. Make PRs against this branch.
6158

@@ -81,27 +78,30 @@ tools/cdata-test.js # Test suite
8178
package.json # Node.js scripts and release ID
8279
.github/workflows/ # CI/CD pipelines
8380
```
84-
<!-- HUMAN_ONLY_END -->
8581

82+
<!-- HUMAN_ONLY_END -->
8683
## General Guidelines
8784

88-
- **Never edit or commit** `wled00/html_*.h` — auto-generated from `wled00/data/`.
8985
- **Repository language is English.** Suggest translations for non-English content.
9086
- **Use VS Code with PlatformIO extension** for best development experience.
87+
- **Never edit or commit** `wled00/html_*.h` and `wled00/js_*.h` — auto-generated from `wled00/data/`.
88+
- If updating Web UI files in `wled00/data/`, **make use of common functions in `wled00/data/common.js` whenever possible**.
9189
- **When unsure, say so.** Gather more information rather than guessing.
9290
- **Acknowledge good patterns** when you see them. Summarize good practices as part of your review - positive feedback always helps.
9391
- **Provide references** when making analyses or recommendations. Base them on the correct branch or PR.
94-
- **Look for user-visible breaking changes and ripple effects**. Ask for confirmation that these were introduced intentionally.
92+
- **Highlight user-visible breaking changes and ripple effects**. Ask for confirmation that these were introduced intentionally.
9593
- **Unused / dead code must be justified or removed**. This helps to keep the codebase clean, maintainable and readable.
9694
- **C++ formatting available**: `clang-format` is installed but not in CI
97-
- 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.
95+
- No automated linting is configured — match existing code style in files you edit.
96+
97+
See `cpp.instructions.md`, `esp-idf.instructions.md` and `web.instructions.md` for language-specific conventions, and `cicd.instructions.md` for GitHub Actions workflows.
9898

9999
### Attribution for AI-generated code
100100
Using AI-generated code can hide the source of the inspiration / knowledge / sources it used.
101101
- Document attribution of inspiration / knowledge / sources used in the code, e.g. link to GitHub repositories or other websites describing the principles / algorithms used.
102102
- 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).
103103
- 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.
104-
- 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.
104+
- 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.
105105

106106
### Pull Request Expectations
107107

.github/cpp.instructions.md

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ See also: [CONTRIBUTING.md](../CONTRIBUTING.md) for general style guidelines tha
1818
- Space between keyword and parenthesis: `if (...)`, `for (...)`. No space between function name and parenthesis: `doStuff(a)`
1919
- No enforced line-length limit; wrap when a line exceeds your editor width
2020

21-
<!-- HUMAN_ONLY_START -->
2221
## Naming
2322

2423
- **camelCase** for functions and variables: `setValuesFromMainSeg()`, `effectCurrent`
2524
- **PascalCase** for classes and structs: `PinManagerClass`, `BusConfig`
2625
- **UPPER_CASE** for macros and constants: `WLED_MAX_USERMODS`, `DEFAULT_CLIENT_SSID`
2726

27+
<!-- HUMAN_ONLY_START -->
2828
## Header Guards
2929

3030
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) {
6464
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.
6565
6666
<!-- HUMAN_ONLY_START -->
67+
<!-- hidden from AI for now, as it created too many "please add a description" review findings in my first tests -->
6768
- **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:
6869
6970
```cpp
@@ -77,7 +78,6 @@ uint8_t gammaCorrect(uint8_t value, float gamma);
7778
```
7879
<!-- HUMAN_ONLY_END -->
7980

80-
8181
Short accessor-style functions (getters/setters, one-liners) may skip this if their purpose is obvious from the name.
8282

8383
## Preprocessor & Feature Flags
@@ -106,12 +106,38 @@ uint8_t gammaCorrect(uint8_t value, float gamma);
106106
- **PSRAM-aware allocation**: use `d_malloc()` (prefer DRAM), `p_malloc()` (prefer PSRAM) from `util.h`
107107
- **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.
108108
<!-- HUMAN_ONLY_START -->
109-
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.
109+
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.
110110
<!-- HUMAN_ONLY_END -->
111111
- **Larger buffers** (LED data, JSON documents) should use PSRAM when available and technically feasible
112112
- **Hot-path**: some data should stay in DRAM or IRAM for performance reasons
113113
- Memory efficiency matters, but is less critical on boards with PSRAM
114114

115+
Heap fragmentation is a concern:
116+
<!-- HUMAN_ONLY_START -->
117+
- 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".
118+
<!-- HUMAN_ONLY_END -->
119+
- Avoid frequent `d_malloc` and `d_free` inside a function, especially for small sizes.
120+
- Avoid frequent creation / destruction of objects.
121+
- Allocate buffers early, and try to re-use them.
122+
- Instead of incrementally appending to a `String`, reserve the expected max buffer upfront by using the `reserve()` method.
123+
<!-- HUMAN_ONLY_START -->
124+
125+
```cpp
126+
String result;
127+
result.reserve(65); // pre-allocate to avoid realloc fragmentation
128+
```
129+
130+
```cpp
131+
// prefer DRAM; falls back gracefully and enforces MIN_HEAP_SIZE guard
132+
_ledsDirty = (byte*) d_malloc(getBitArrayBytes(_len));
133+
```
134+
135+
```cpp
136+
_mode.reserve(_modeCount); // allocate memory to prevent initial fragmentation - does not increase size()
137+
_modeData.reserve(_modeCount); // allocate memory to prevent initial fragmentation - does not increase size()
138+
```
139+
<!-- HUMAN_ONLY_END -->
140+
115141
## `const` and `constexpr`
116142
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.
117143

@@ -175,6 +201,12 @@ static_assert(WLED_MAX_BUSSES <= 32, "WLED_MAX_BUSSES exceeds hard limit");
175201
#error "WLED_MAX_BUSSES exceeds hard limit"
176202
#endif
177203
```
204+
205+
```cpp
206+
// using static_assert() to validate enumerated types (zero cost at runtime)
207+
static_assert(0u == static_cast<uint8_t>(PinOwner::None),
208+
"PinOwner::None must be zero, so default array initialization works as expected");
209+
```
178210
<!-- HUMAN_ONLY_END -->
179211
180212
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
530562
- If possible, use `static` for local (C-style) variables and functions (keeps the global namespace clean)
531563
- Avoid unexplained "magic numbers". Prefer named constants (`constexpr`) or C-style `#define` constants for repeated numbers that have the same meaning
532564
- Include `"wled.h"` as the primary project header where needed
533-
- **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:
565+
- **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:
534566
```cpp
535567
// Undefined behavior — avoid:
536568
uint8_t angle = 40.74f * atan2f(dy, dx); // negative float → uint8_t is UB

.github/web.instructions.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,6 @@ applyTo: "wled00/data/**"
2424

2525
## Build Integration
2626

27-
Files in this directory are processed by `tools/cdata.js` into `wled00/html_*.h` headers.
28-
Run `npm run build` after any change. **Never edit the generated `html_*.h` files directly.**
27+
Files in this directory are processed by `tools/cdata.js` into generated headers
28+
(`wled00/html_*.h`, `wled00/js_*.h`).
29+
Run `npm run build` after any change. **Never edit generated headers directly.**

0 commit comments

Comments
 (0)