581 lines
27 KiB
Markdown
581 lines
27 KiB
Markdown
---
|
||
applyTo: "**/*.cpp,**/*.h,**/*.hpp,**/*.ino"
|
||
---
|
||
# C++ Coding Conventions
|
||
|
||
> **Note for AI review tools**: sections enclosed in
|
||
> `<!-- 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.
|
||
|
||
See also: [CONTRIBUTING.md](../CONTRIBUTING.md) for general style guidelines that apply to all contributors.
|
||
|
||
## Formatting
|
||
|
||
- Indent with **2 spaces** (no tabs in C++ files)
|
||
- Opening braces on the same line is preferred (K&R style). Brace on a separate line (Allman style) is acceptable
|
||
- Single-statement `if` bodies may omit braces: `if (a == b) doStuff(a);`
|
||
- 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`
|
||
|
||
<!-- HUMAN_ONLY_START -->
|
||
## Header Guards
|
||
|
||
Most headers use `#ifndef` / `#define` guards. Some newer headers add `#pragma once` before the guard:
|
||
|
||
```cpp
|
||
#ifndef WLED_EXAMPLE_H
|
||
#define WLED_EXAMPLE_H
|
||
// ...
|
||
#endif // WLED_EXAMPLE_H
|
||
```
|
||
<!-- HUMAN_ONLY_END -->
|
||
|
||
## Comments
|
||
|
||
- `//` for inline comments, `/* ... */` for block comments. Always put a space after `//`
|
||
- Mark WLED-MM-specific changes with `// WLEDMM` or `// WLEDMM: description`:
|
||
|
||
<!-- HUMAN_ONLY_START -->
|
||
```cpp
|
||
// WLEDMM: increased max bus count for larger installs
|
||
#ifndef WLED_MAX_BUSSES
|
||
#define WLED_MAX_BUSSES 20 // WLEDMM default (upstream: 10)
|
||
#endif
|
||
```
|
||
<!-- HUMAN_ONLY_END -->
|
||
|
||
- **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:
|
||
|
||
```cpp
|
||
// AI: below section was generated by an AI
|
||
void calculateCRC(const uint8_t* data, size_t len) {
|
||
...
|
||
}
|
||
// AI: end of AI-generated section
|
||
```
|
||
|
||
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.
|
||
|
||
<!-- HUMAN_ONLY_START -->
|
||
<!-- hidden from AI for now, as it created too many "please add a description" review findings in my first tests -->
|
||
- **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
|
||
/* *****
|
||
* Apply gamma correction to a single color channel.
|
||
* @param value raw 8-bit channel value (0–255)
|
||
* @param gamma gamma exponent (typically 2.8)
|
||
* @return corrected 8-bit value
|
||
***** */
|
||
uint8_t gammaCorrect(uint8_t value, float gamma);
|
||
```
|
||
<!-- HUMAN_ONLY_END -->
|
||
|
||
Short accessor-style functions (getters/setters, one-liners) may skip this if their purpose is obvious from the name.
|
||
|
||
## Preprocessor & Feature Flags
|
||
|
||
- Prefer compile-time feature flags (`#ifdef` / `#ifndef`) over runtime checks where possible
|
||
- Platform differentiation: `ARDUINO_ARCH_ESP32` vs `ESP8266`
|
||
- WLED-MM fork detection: `_MoonModules_WLED_` (defined in `wled.h`)
|
||
- PSRAM availability: `BOARD_HAS_PSRAM`
|
||
- WLEDMM_FASTPATH is the default path; code under `#ifndef WLEDMM_FASTPATH` is deprecated and will be phased out.
|
||
- Flash-saving mode: `WLEDMM_SAVE_FLASH` (disables aggressive inlining)
|
||
|
||
## Error Handling
|
||
|
||
- `DEBUG_PRINTF()` / `DEBUG_PRINTLN()` for developer diagnostics (compiled out unless `-D WLED_DEBUG`)
|
||
- `USER_PRINTF()` / `USER_PRINTLN()` for user-visible messages (always compiled in)
|
||
- Don't rely on C++ exceptions — use return codes (`-1` / `false` for errors) and global flags (e.g. `errorFlag = ERR_LOW_MEM`). Some builds don't support C++ exceptions.
|
||
|
||
## Strings
|
||
|
||
- (Optional) Use `F("string")` for string constants (stores in PROGMEM, saves RAM)
|
||
- Use `const char*` for temporary/parsed strings
|
||
- Avoid `String` (Arduino heap-allocated string) in hot paths; acceptable in config/setup code
|
||
|
||
## 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.
|
||
<!-- HUMAN_ONLY_START -->
|
||
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.
|
||
<!-- HUMAN_ONLY_END -->
|
||
- **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:
|
||
<!-- HUMAN_ONLY_START -->
|
||
- 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".
|
||
<!-- HUMAN_ONLY_END -->
|
||
- 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.
|
||
<!-- HUMAN_ONLY_START -->
|
||
|
||
```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()
|
||
```
|
||
<!-- 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
|
||
|
||
* 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:
|
||
```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.
|
||
|
||
<!-- HUMAN_ONLY_START -->
|
||
```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
|
||
```
|
||
|
||
For function parameters that are read-only, prefer `const &`:
|
||
|
||
```cpp
|
||
BusDigital(BusConfig &bc, uint8_t nr, const ColorOrderMap &com);
|
||
```
|
||
<!-- HUMAN_ONLY_END -->
|
||
|
||
### `constexpr` over `#define`
|
||
<!-- HUMAN_ONLY_START -->
|
||
|
||
Prefer `constexpr` for compile-time constants. Unlike `#define`, `constexpr` respects scope and type safety, keeping the global namespace clean:
|
||
|
||
```cpp
|
||
// Prefer:
|
||
constexpr uint32_t TWO_CHANNEL_MASK = 0x00FF00FF;
|
||
constexpr int WLED_MAX_BUSSES = WLED_MAX_DIGITAL_CHANNELS + WLED_MAX_ANALOG_CHANNELS;
|
||
|
||
// Avoid (when possible):
|
||
#define TWO_CHANNEL_MASK 0x00FF00FF
|
||
```
|
||
|
||
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:
|
||
|
||
```cpp
|
||
// Prefer:
|
||
constexpr int WLED_MAX_BUSSES = WLED_MAX_DIGITAL_CHANNELS + WLED_MAX_ANALOG_CHANNELS;
|
||
static_assert(WLED_MAX_BUSSES <= 32, "WLED_MAX_BUSSES exceeds hard limit");
|
||
|
||
// Avoid:
|
||
#if (WLED_MAX_BUSSES > 32)
|
||
#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<uint8_t>(PinOwner::None),
|
||
"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
|
||
|
||
Marking a member function `const` tells the compiler that it does not modify the object's state:
|
||
|
||
```cpp
|
||
uint16_t length() const { return _len; }
|
||
bool isActive() const { return _active; }
|
||
```
|
||
<!-- HUMAN_ONLY_START -->
|
||
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.
|
||
<!-- HUMAN_ONLY_END -->
|
||
|
||
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
|
||
|
||
<!-- HUMAN_ONLY_START -->
|
||
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.
|
||
2. **Better inlining**: GCC can inline a `static` method with more certainty because it cannot be overridden by a derived class (no virtual dispatch ambiguity) and has no aliasing concern through `this`.
|
||
|
||
Use `static` for any method that does not need access to instance members:
|
||
|
||
```cpp
|
||
// Factory / utility — no instance needed:
|
||
static BusConfig fromJson(JsonObject obj);
|
||
|
||
// Pure computation helpers:
|
||
static uint8_t gamma8(uint8_t val);
|
||
static uint32_t colorBalance(uint32_t color, uint8_t r, uint8_t g, uint8_t b);
|
||
```
|
||
<!-- HUMAN_ONLY_END -->
|
||
|
||
`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.
|
||
|
||
---
|
||
|
||
## Hot-Path Optimization
|
||
|
||
The hot path is the per-frame pixel pipeline: **Segment → Strip → BusManager → Bus(Digital,HUB75,Network) or PolyBus → LED driver**. Speed is the top priority here. The patterns below are taken from existing hot-path code (`FX_fcn.cpp`, `FX_2Dfcn.cpp`, `bus_manager.cpp`, `colorTools.hpp`) and should be followed when modifying these files.
|
||
|
||
Note: `FX.cpp` (effect functions) is written by many contributors and has diverse styles — that is acceptable. The guidelines below apply starting from pixel set/get operations and below.
|
||
|
||
### Function Attributes
|
||
|
||
Stack the appropriate attributes on hot-path functions. Defined in `const.h`:
|
||
|
||
| Attribute | Meaning | When to use |
|
||
|---|---|---|
|
||
| `__attribute__((hot))` | Branch-prediction hint | hot-path functions with complex logic |
|
||
| `IRAM_ATTR` | Place in fast IRAM (ESP32) | Critical per-pixel functions (e.g. `BusDigital::setPixelColor`) |
|
||
| `IRAM_ATTR_YN` | IRAM on ESP32, no-op on ESP8266 | Hot functions that ESP8266 can't fit in IRAM |
|
||
| `WLED_O2_ATTR` | Force `-O2` optimization | Most hot-path functions |
|
||
| `WLED_O3_ATTR` | Force `-O3,fast-math` | Innermost color math (e.g. `color_blend`) |
|
||
| `[[gnu::hot]] inline` | Modern C++ attribute + inline | Header-defined accessors (e.g. `progress()`, `currentBri()`) |
|
||
|
||
Note: `WLED_O3_ATTR` sometimes causes performance loss compared to `WLED_O2_ATTR`. Choose optimization levels based on test results.
|
||
|
||
Example signature:
|
||
|
||
```cpp
|
||
void IRAM_ATTR_YN WLED_O2_ATTR __attribute__((hot)) Segment::setPixelColor(int i, uint32_t col)
|
||
```
|
||
|
||
<!-- HUMAN_ONLY_START -->
|
||
### 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:
|
||
|
||
```cpp
|
||
uint_fast8_t count = numBusses;
|
||
for (uint_fast8_t i = 0; i < count; i++) { ... }
|
||
```
|
||
|
||
Keep `uint8_t` / `uint16_t` for struct fields and stored data where memory layout matters.
|
||
|
||
### Cache Members to Locals Before Loops
|
||
|
||
Copy class members and virtual-call results to local variables before entering a loop:
|
||
|
||
```cpp
|
||
uint_fast8_t count = numBusses; // avoid repeated member access
|
||
for (uint_fast8_t i = 0; i < count; i++) {
|
||
Bus* const b = busses[i]; // const pointer hints to compiler
|
||
uint_fast16_t bstart = b->getStart();
|
||
uint_fast16_t blen = b->getLength();
|
||
...
|
||
}
|
||
```
|
||
<!-- HUMAN_ONLY_END -->
|
||
|
||
### Unsigned Range Check
|
||
|
||
Replace two-comparison range tests with a single unsigned subtraction:
|
||
|
||
```cpp
|
||
// Instead of: if (pix >= bstart && pix < bstart + blen)
|
||
if ((uint_fast16_t)(pix - bstart) < blen) // also catches negative pix via unsigned underflow
|
||
```
|
||
|
||
### Early Returns
|
||
|
||
Guard every hot-path function with the cheapest necessary checks first:
|
||
|
||
```cpp
|
||
if (!isActive()) return; // inactive segment
|
||
if (unsigned(i) >= virtualLength()) return; // bounds check (catches negative i too)
|
||
```
|
||
|
||
### 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.
|
||
<!-- HUMAN_ONLY_START -->
|
||
```cpp
|
||
// Decision made once per frame in startFrame(), stored in a bool
|
||
bool simpleSegment = _isSuperSimpleSegment;
|
||
|
||
// Per-pixel loop — no complex branching inside
|
||
if (simpleSegment)
|
||
setPixelColorXY_fast(x, y, col, scaled_col, cols, rows); // inline, no bounds checks
|
||
else
|
||
setPixelColorXY_slow(x, y, col); // full validation, grouping, mirroring
|
||
```
|
||
|
||
The same principle applies to color utilities — `color_add()` accepts a `fast` flag so callers can choose saturating adds (no branches) vs. ratio-preserving adds (with division) without an inner-loop decision:
|
||
|
||
```cpp
|
||
uint32_t color_add(uint32_t c1, uint32_t c2, bool fast=false);
|
||
```
|
||
<!-- HUMAN_ONLY_END -->
|
||
|
||
General rules:
|
||
- 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.
|
||
|
||
<!-- HUMAN_ONLY_START -->
|
||
`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
|
||
// calculateScaling() — called once per frame
|
||
if ((perPixelX < 2) && (perPixelY < 2))
|
||
decoder.setDrawPixelCallback(drawPixelCallbackDownScale2D); // downscale-only variant
|
||
else
|
||
decoder.setDrawPixelCallback(drawPixelCallback2D); // full-scaling variant
|
||
```
|
||
|
||
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.
|
||
<!-- HUMAN_ONLY_END -->
|
||
<!-- HUMAN_ONLY_START -->
|
||
### 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:
|
||
|
||
```cpp
|
||
template<bool hasWhite>
|
||
void setChannel(uint8_t* out, uint32_t col) {
|
||
out[0] = R(col); out[1] = G(col); out[2] = B(col);
|
||
if constexpr (hasWhite) out[3] = W(col); // compiled out when hasWhite==false
|
||
}
|
||
```
|
||
|
||
Use sparingly — each instantiation duplicates code in flash. On ESP8266 and small-flash ESP32 boards this can exhaust IRAM/flash. Prefer templates only when the hot path is measurably faster and the number of instantiations is small (2–4).
|
||
|
||
### RAII Lock-Free Synchronization (Advanced)
|
||
|
||
Where contention is rare and the critical section is short, consider replacing mutex-based locking with lock-free techniques using `std::atomic` and RAII scoped guards. A scoped guard sets a flag on construction and clears it on destruction, guaranteeing cleanup even on early return:
|
||
|
||
```cpp
|
||
struct ScopedBusyFlag {
|
||
std::atomic<bool>& flag;
|
||
bool acquired;
|
||
ScopedBusyFlag(std::atomic<bool>& f) : flag(f), acquired(false) {
|
||
bool expected = false;
|
||
acquired = flag.compare_exchange_strong(expected, true);
|
||
}
|
||
~ScopedBusyFlag() { if (acquired) flag.store(false); }
|
||
explicit operator bool() const { return acquired; }
|
||
};
|
||
|
||
// Usage
|
||
static std::atomic<bool> busySending{false};
|
||
ScopedBusyFlag guard(busySending);
|
||
if (!guard) return; // another task is already sending
|
||
// ... do work — flag auto-clears when guard goes out of scope
|
||
```
|
||
|
||
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.
|
||
|
||
<!-- HUMAN_ONLY_END -->
|
||
### Pre-Compute Outside Loops
|
||
|
||
Move invariant calculations before the loop. Pre-compute reciprocals to replace division with multiplication.
|
||
<!-- HUMAN_ONLY_START -->
|
||
```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
|
||
```
|
||
|
||
<!-- HUMAN_ONLY_END -->
|
||
### Parallel Channel Processing
|
||
|
||
Process R+B and W+G channels simultaneously using the two-channel mask pattern:
|
||
|
||
```cpp
|
||
constexpr uint32_t TWO_CHANNEL_MASK = 0x00FF00FF;
|
||
uint32_t rb = (((c1 & TWO_CHANNEL_MASK) * amount) >> 8) & TWO_CHANNEL_MASK;
|
||
uint32_t wg = (((c1 >> 8) & TWO_CHANNEL_MASK) * amount) & ~TWO_CHANNEL_MASK;
|
||
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.
|
||
|
||
<!-- HUMAN_ONLY_START -->
|
||
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
|
||
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``.
|
||
|
||
<!-- HUMAN_ONLY_END -->
|
||
### Static Caching for Expensive Computations
|
||
|
||
Cache results in static locals when the input rarely changes between calls:
|
||
|
||
```cpp
|
||
static uint16_t lastKelvin = 0;
|
||
static byte correctionRGB[4] = {255,255,255,0};
|
||
if (lastKelvin != kelvin) {
|
||
colorKtoRGB(kelvin, correctionRGB); // expensive — only recalculate when input changes
|
||
lastKelvin = kelvin;
|
||
}
|
||
```
|
||
|
||
### Inlining Strategy
|
||
|
||
- Move frequently-called small functions to headers for inlining (e.g. `WS2812FX::setPixelColor` is in `FX.h`)
|
||
- 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`
|
||
|
||
<!-- HUMAN_ONLY_START -->
|
||
### Colors
|
||
|
||
- Store and pass colors as `uint32_t` (0xWWRRGGBB)
|
||
- Extract channels with macros: `R(c)`, `G(c)`, `B(c)`, `W(c)`, compose with `RGBW32(r,g,b,w)`
|
||
- Use `CRGB` (FastLED type) mainly when interfacing with FastLED functions; convert at boundaries
|
||
- Use 16-bit intermediates for channel math to ensure 32-bit (not 64-bit) arithmetic:
|
||
```cpp
|
||
uint16_t r1 = R(color1); // 16-bit intermediate keeps the multiply result in 32 bits, avoiding 64-bit promotion
|
||
```
|
||
|
||
<!-- 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:
|
||
|
||
| Macro | Signature | Description |
|
||
|---|---|---|
|
||
| `esp32SemTake(mux, timeout)` | `mux`: `SemaphoreHandle_t`, `timeout`: milliseconds | Acquire a recursive mutex. Returns `pdTRUE` on success, `pdFALSE` on timeout. |
|
||
| `esp32SemGive(mux)` | `mux`: `SemaphoreHandle_t` | Release a previously acquired recursive mutex. |
|
||
|
||
Pre-defined mutex handles (declared in `wled.h`):
|
||
|
||
| Mutex | Protects |
|
||
|---|---|
|
||
| `busDrawMux` | Concurrent `strip.show()` and `strip.service()` — acquire before writing pixels from background tasks (DDP, E1.31, Art-Net) |
|
||
| `segmentMux` | Segment array modifications — acquire before adding, removing, or iterating segments |
|
||
| `jsonBufferLockMutex` | Shared JSON document buffer |
|
||
| `presetFileMux` | `presets.json` file reads and writes |
|
||
|
||
Usage pattern:
|
||
|
||
```cpp
|
||
if (esp32SemTake(busDrawMux, 200) == pdTRUE) { // wait max 200 ms
|
||
// ... critical section ...
|
||
esp32SemGive(busDrawMux);
|
||
} else {
|
||
// fallback code or error reporting
|
||
}
|
||
```
|
||
|
||
Always pair every `esp32SemTake` with a matching `esp32SemGive`. Choose a timeout appropriate for the operation — typically 200 ms for drawing, up to 2500 ms for file I/O.
|
||
|
||
**Important**: Not every shared resource needs a mutex. Some synchronization is guaranteed by the overall control flow. For example, `volatile bool` flags like `suspendStripService`, `doInitBusses`, `loadLedmap`, and `OTAisRunning` (declared in `wled.h`) are checked sequentially in the main loop (`wled.cpp`), so they serialize access without requiring a semaphore. Use mutexes when true concurrent access from multiple FreeRTOS tasks is possible and race-conditions can lead to unexpected behaviour. Rely on control-flow ordering when operations are sequenced within the same loop iteration.
|
||
|
||
### `delay()` vs `yield()` in FreeRTOS Tasks
|
||
<!-- 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.
|
||
<!-- HUMAN_ONLY_END -->
|
||
|
||
- On ESP32, `delay()` is generally allowed, as it helps to efficiently manage CPU usage of all tasks.
|
||
- On ESP8266, only use `delay()` and `yield()` in the main `loop()` context. If not sure, protect with `if (can_yield()) ...`.
|
||
- Do *not* use `delay()` in effects (FX.cpp) or in the hot pixel path.
|
||
- `delay()` on ``busses`` level is allowed, it might be needed to achieve exact timing in LED drivers.
|
||
- **`yield()` is a no-op in WLED-MM on ESP32.** `WLEDMM_FASTPATH` redefines `yield()` to an empty macro.
|
||
```cpp
|
||
#define yield() {} // WLEDMM: yield() is completely unnecessary on ESP32
|
||
```
|
||
|
||
### IDLE Watchdog and Custom Tasks on ESP32
|
||
|
||
- **Do NOT use `yield()` to pace ESP32 tasks or assume it feeds any watchdog**.
|
||
|
||
- Even in stock arduino-esp32, `yield()` calls `vTaskDelay(0)`, which only switches to tasks at equal or higher priority — the IDLE task (priority 0) is never reached.
|
||
|
||
- **Custom `xTaskCreate()` tasks must call `delay(1)` in their loop, not `yield()`.** Without a real blocking call, the IDLE task is starved. The IDLE watchdog panic is the first visible symptom — but the damage starts earlier: deleted task memory leaks, software timers stop firing, light sleep is disabled, and Wi-Fi/BT idle hooks don't run. See `esp-idf.instructions.md` for a full explanation of what IDLE does. Structure custom tasks like this:
|
||
```cpp
|
||
// WRONG — IDLE task is never scheduled; yield() does not feed the idle task watchdog.
|
||
void myTask(void*) {
|
||
for (;;) {
|
||
doWork();
|
||
yield();
|
||
}
|
||
}
|
||
|
||
// CORRECT — delay(1) suspends the task for ≥1 ms, IDLE task runs, IDLE watchdog is fed
|
||
void myTask(void*) {
|
||
for (;;) {
|
||
doWork();
|
||
delay(1); // DO NOT REMOVE — lets IDLE(0) run and feeds its watchdog
|
||
}
|
||
}
|
||
```
|
||
|
||
- 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
|
||
|
||
- 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
|
||
|
||
- **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
|
||
|
||
// Correct — cast through int first:
|
||
// atan2f returns [-π..+π], scaled ≈ [-128..+128] as int; uint8_t wraps negative ints via 2's complement (e.g. -1 → 255)
|
||
uint8_t angle = int(40.74f * atan2f(dy, dx)); // float→int (defined), int→uint8_t (defined)
|
||
```
|