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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,25 +88,50 @@ 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. 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.
- **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.

### 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`

<!-- HUMAN_ONLY_START -->
```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
```
<!-- HUMAN_ONLY_END -->


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

64 changes: 39 additions & 25 deletions docs/cpp.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

<!-- HUMAN_ONLY_START -->
## Header Guards

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -142,25 +150,28 @@ Heap fragmentation is a concern:
<!-- HUMAN_ONLY_END -->

## `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.

<!-- HUMAN_ONLY_START -->
`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).

<!-- HUMAN_ONLY_END -->
- For function parameters that are read-only, prefer `const &` or `const`.

### `const` locals
<!-- HUMAN_ONLY_START -->
* 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:
<!-- HUMAN_ONLY_END -->
* 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();
```

<!-- HUMAN_ONLY_END -->
### `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.

<!-- HUMAN_ONLY_START -->
```cpp
Expand All @@ -174,11 +185,14 @@ For function parameters that are read-only, prefer `const &`:
BusDigital(BusConfig &bc, uint8_t nr, const ColorOrderMap &com);
```
<!-- HUMAN_ONLY_END -->
- 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`
<!-- HUMAN_ONLY_START -->
<!-- hidden from AI for now - codebase is not compliant to this rule (slowly migrating) -->
### `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:
Expand All @@ -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.
<!-- HUMAN_ONLY_END -->

### `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.

<!-- HUMAN_ONLY_START -->
```cpp
// Prefer:
constexpr int WLED_MAX_BUSSES = WLED_MAX_DIGITAL_CHANNELS + WLED_MAX_ANALOG_CHANNELS;
Expand All @@ -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");
```
<!-- HUMAN_ONLY_END -->

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
Expand Down Expand Up @@ -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
```

Expand All @@ -441,13 +454,14 @@ 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), 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.

<!-- HUMAN_ONLY_START -->
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
Expand Down Expand Up @@ -488,6 +502,9 @@ if (lastKelvin != kelvin) {
```

<!-- HUMAN_ONLY_END -->

---

## 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:
Expand Down Expand Up @@ -525,7 +542,7 @@ Always pair every `esp32SemTake` with a matching `esp32SemGive`. Choose a timeou
<!-- HUMAN_ONLY_START -->
* 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.
<!-- HUMAN_ONLY_END -->

- On ESP32, `delay()` is generally allowed, as it helps to efficiently manage CPU usage of all tasks.
Expand Down Expand Up @@ -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
Expand Down
Loading