--- applyTo: "**/*.cpp,**/*.h,**/*.hpp,**/*.ino" --- # C++ Coding Conventions > **Note for AI review tools**: sections enclosed in > `` / `` 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` - **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 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 ``` ## Comments - `//` for inline comments, `/* ... */` for block comments. Always put a space after `//` - **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 ``` 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 /* ***** * 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); ``` 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` - PSRAM availability: `BOARD_HAS_PSRAM` ## Error Handling - `DEBUG_PRINTF()` / `DEBUG_PRINTLN()` for developer diagnostics (compiled out unless `-D WLED_DEBUG`) - 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 - Use `const char*` for temporary/parsed strings - Avoid `String` (Arduino heap-allocated string) in hot paths; acceptable in config/setup code - Use `F("string")` for string constants (major RAM win on ESP8266; mostly overload/type compatibility on ESP32) - Store repeated strings as `static const char[] PROGMEM` On **ESP8266** this explicitly stores the string in flash (PROGMEM), saving precious RAM — every byte counts on that platform. On **ESP32**, `PROGMEM` is a no-op and string literals already reside in flash/rodata, so `F()` yields little RAM benefit but remains harmless (it satisfies `__FlashStringHelper*` overloads that some APIs expect). ```cpp DEBUG_PRINTLN(F("WS client connected.")); // string stays in flash, not RAM DEBUG_PRINTF_P(PSTR("initE: Ignoring attempt for invalid ethernetType (%d)\n"), ethernetType); // format string stays in flash ``` ## Memory - **PSRAM-aware allocation**: use `d_malloc()` (prefer DRAM), `p_malloc()` (prefer PSRAM) from `fcn_declare.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. - **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` `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. `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. ```cpp const uint_fast16_t cols = vWidth(); const uint_fast16_t rows = vHeight(); ``` ### `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. ```cpp const auto &m = _mappings[i]; // reference, not a copy (bus_manager.cpp) Segment& sourcesegment = strip.getSegment(sourceid); // alias — avoids creating a temporary Segment instance ``` For function parameters that are read-only, prefer `const &`: ```cpp BusManager::add(const BusConfig &bc, bool placeholder) { ``` - 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` - 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 ``` ### `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. - `#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; 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(PinOwner::None), "PinOwner::None must be zero, so default array initialization works as expected"); ``` ### `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; } ``` 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. 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 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); ``` `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, plus ``WS2812FX::show()`` and below**. Speed is the priority here. The patterns below are taken from existing hot-path code (`FX_fcn.cpp`, `FX_2Dfcn.cpp`, `bus_manager.cpp`, `colors.cpp`) 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. ### 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(unsigned i, uint32_t c) ``` ### 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(); ... } ``` ### 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) >= vLength()) 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. 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. `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. ### 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 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& flag; bool acquired; ScopedBusyFlag(std::atomic& 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 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 to return a semaphore. 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. ### Pre-Compute Outside Loops Move invariant calculations before the loop. Pre-compute reciprocals to replace division with multiplication. ```cpp const uint_fast16_t cols = virtualWidth(); const uint_fast16_t rows = virtualHeight(); uint_fast8_t fadeRate = (255U - rate) >> 1; float mappedRate_r = 1.0f / (float(fadeRate) + 1.1f); // reciprocal — avoid division inside loop ``` ### 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) 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. ```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``. ### 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. `Segment::setPixelColorRaw` is in `FX.h`) - Use `static inline` for file-local helpers ### Math & Trigonometric Functions - WLED uses a custom `fastled_slim` library. The old FastLED trig aliases (`sin8`, `cos8`, `sin16`, `cos16`) **no longer exist and cause a compile error** — use `sin8_t()`, `cos8_t()`, `sin16_t()`, `cos16_t()` instead. For float approximations use `sin_approx()` / `cos_approx()` instead of `sinf()` / `cosf()`. Replace FastLED noise aliases (`inoise8`, `inoise16`) with `perlin8`, `perlin16`. | ❌ Do not use (compile error) | ✅ Use instead | Source | |---|---|---| | `sin8()`, `cos8()` | `sin8_t()`, `cos8_t()` | `fastled_slim.h` → `wled_math.cpp` | | `sin16()`, `cos16()` | `sin16_t()`, `cos16_t()` | `fastled_slim.h` → `wled_math.cpp` | | `sinf()`, `cosf()` | `sin_approx()`, `cos_approx()` | `wled_math.cpp` | | `atan2f()`, `atan2()` | `atan2_t()` | `wled_math.cpp` | | `sqrt()` on integers | `sqrt32_bw()` | `fcn_declare.h` → `wled_math.cpp` | | `sqrtf()` on floats | `sqrtf()` (acceptable) | — no WLED replacement | --- ## `delay()` vs `yield()` in ESP32 Tasks * 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()` 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. - 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 the bus-level is allowed, it might be needed to achieve exact timing in LED drivers. ### IDLE Watchdog and Custom Tasks on ESP32 - In arduino-esp32, `yield()` calls `vTaskDelay(0)`, which only switches to tasks at equal or higher priority — the IDLE task (priority 0) is never reached. - **Do not use `yield()` to pace ESP32 tasks or assume it feeds any watchdog**. - **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. 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 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. ## Caveats and Pitfalls - **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 // 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) ```