diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 0000000000..f941bec839 --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,68 @@ +# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json +# +# CodeRabbit configuration — references existing guideline files to avoid +# duplicating conventions. See: +# .github/copilot-instructions.md — project overview & general rules +# .github/cpp.instructions.md — C++ coding conventions +# .github/web.instructions.md — Web UI coding conventions +# .github/cicd.instructions.md — GitHub Actions / CI-CD conventions +# .github/esp-idf.instructions.md — ESP-IDF / chip-specific coding guidelines +# (apply when code directly uses ESP-IDF APIs: +# esp_idf_*, I2S, RMT, ADC, GPIO, heap_caps, etc.) +# +# NOTE: This file must be committed (tracked by git) for CodeRabbit to read +# it from the repository. If it is listed in .gitignore, CodeRabbit will +# not see it and these settings will have no effect. + +language: en-US + +reviews: + path_instructions: + - path: "**/*.{cpp,h,hpp,ino}" + instructions: > + Follow the C++ coding conventions documented in .github/cpp.instructions.md + and the general project guidelines in .github/copilot-instructions.md. + If the code under review directly uses ESP-IDF APIs (e.g. heap_caps_malloc, + I2S, RMT, ADC, GPIO, esp_timer, or any esp_idf_* / soc_* symbols), also + apply the guidelines in .github/esp-idf.instructions.md. + + Key rules: 2-space indentation (no tabs), camelCase functions/variables, + PascalCase classes, UPPER_CASE macros. Mark WLED-MM-specific changes with + `// WLEDMM` comments. No C++ exceptions — use return codes and debug macros. + + Hot-path optimization guidelines (attributes, uint_fast types, caching, + unsigned range checks) apply from pixel set/get operations downward — + NOT to effect functions in FX.cpp, which have diverse contributor styles. + + - path: "wled00/data/**" + instructions: > + Follow the web UI conventions documented in .github/web.instructions.md. + + Key rules: indent HTML and JavaScript with tabs, CSS with tabs or spaces. + Files here are built into wled00/html_*.h by tools/cdata.js — never + edit those generated headers directly. + + - path: "wled00/html_*.h" + instructions: > + These files are auto-generated from wled00/data/ by tools/cdata.js. + They must never be manually edited or committed. Flag any PR that + includes changes to these files. + + - path: "usermods/**" + instructions: > + Usermods are community add-ons following the upstream WLED 0.15.x style. + Each usermod lives in its own directory under usermods/ and is implemented + as a .h file that is pulled in by wled00/usermods_list.cpp (guarded by + #ifdef). Usermods do not use library.json. Follow the same C++ conventions + as the core firmware (.github/cpp.instructions.md). + + - path: ".github/workflows/*.{yml,yaml}" + instructions: > + Follow the CI/CD conventions documented in .github/cicd.instructions.md. + + Key rules: 2-space indentation, descriptive name: on every workflow/job/step. + Third-party actions must be pinned to a specific version tag — branch pins + such as @main or @master are not allowed. Declare explicit permissions: blocks + scoped to least privilege. Never interpolate github.event.* values directly + into run: steps — pass them through an env: variable to prevent script + injection. Do not use pull_request_target unless fully justified. diff --git a/.github/agent-build-instructions.md b/.github/agent-build-instructions.md new file mode 100644 index 0000000000..fe73031447 --- /dev/null +++ b/.github/agent-build-instructions.md @@ -0,0 +1,109 @@ +# Agent-Mode Build & Test Instructions + +Detailed build workflow, timeouts, and troubleshooting for making code changes in agent mode. Always reference these instructions first when running builds or validating changes. + +## Build Timing and Timeouts + +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 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 | + +**NEVER cancel long-running builds.** PlatformIO downloads and compilation require patience. + +## Development Workflow + +### Web UI Changes + +1. Edit files in `wled00/data/` +2. Run `npm run build` to regenerate `wled00/html_*.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) +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` + +### Combined Web + Firmware Changes + +1. Always build web UI first +2. Test web interface manually +3. Then build and test firmware + + +## Before Finishing Work + +**You MUST complete ALL of these before marking work as done:** + +1. **Run tests**: `npm test` — must pass +2. **Build firmware**: `pio run -e esp32_4MB_V4_M` — must succeed + - Set timeout to 30+ minutes, **never cancel** + - Choose `esp32_4MB_V4_M` as a common, representative environment + - If the build fails, fix the issue before proceeding +3. **For web UI changes**: manually test the interface (see below) + +If any step fails, fix the issue. **Do NOT mark work complete with failing builds or tests.** + +## Manual Web UI Testing + +Start a local server: + +```sh +cd wled00/data && python3 -m http.server 8080 +# Open http://localhost:8080/index.htm +``` + +Test these scenarios after every web UI change: + +- **Load**: `index.htm` loads without JavaScript errors (check browser console) +- **Navigation**: switching between main page and settings pages works +- **Color controls**: color picker and brightness controls function correctly +- **Effects**: effect selection and parameter changes work +- **Settings**: form submission and validation work + +## Troubleshooting + +### Common Build Issues + +| Problem | Solution | +|---|---| +| Missing `html_*.h` | Run `npm ci; npm run build` | +| Web UI looks broken | Check browser console for JS errors | +| PlatformIO network errors | Retry — downloads can be flaky | +| Node.js version mismatch | Ensure Node.js 20+ (check `.nvmrc`) | + +### 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` +- **Reinstall Node deps**: `rm -rf node_modules && npm ci` + +## CI/CD Validation + +The GitHub Actions CI workflow will: +1. Install Node.js and Python dependencies +2. Run `npm test` +3. Build web UI (automatic via PlatformIO) +4. Compile firmware for **all** `default_envs` targets + +**To ensure CI success, always validate locally:** +- Run `npm test` and ensure it passes +- Run `pio run -e esp32_4MB_V4_M` (or another common firmware environment, see next section) and ensure it completes successfully +- If either fails locally, it WILL fail in CI + +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/` +- 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 new file mode 100644 index 0000000000..b67a3a2863 --- /dev/null +++ b/.github/cicd.instructions.md @@ -0,0 +1,153 @@ +--- +applyTo: ".github/workflows/*.yml,.github/workflows/*.yaml" +--- +# CI/CD Conventions — GitHub Actions Workflows + +## YAML Style + +- Indent with **2 spaces** (no tabs) +- Every workflow, job, and step must have a `name:` field that clearly describes its purpose +- Group related steps logically; separate unrelated groups with a blank line +- Comments (`#`) are encouraged for non-obvious decisions (e.g., why `fail-fast: false` is set, what a cron expression means) + +## Workflow Structure + +### Triggers + +- Declare `on:` triggers explicitly; avoid bare `on: push` without branch filters on long-running or expensive jobs +- Prefer `workflow_call` for shared build logic (see `build.yml`) to avoid duplicating steps across workflows +- Document scheduled triggers (`cron:`) with a human-readable comment: + +```yaml +schedule: + - cron: '0 2 * * *' # run at 2 AM UTC daily +``` + +### Jobs + +- Express all inter-job dependencies with `needs:` — never rely on implicit ordering +- Use job `outputs:` + step `id:` to pass structured data between jobs (see `get_default_envs` in `build.yml`) +- Set `fail-fast: false` on matrix builds so that a single failing environment does not cancel others + +### Runners + +- Pin to a specific Ubuntu version (`ubuntu-22.04`, `ubuntu-24.04`) rather than `ubuntu-latest` for reproducible builds +- Only use `ubuntu-latest` in jobs where exact environment reproducibility is not required (e.g., trivial download/publish steps) + +### Tool and Language Versions + +- Pin tool versions explicitly: + ```yaml + python-version: '3.12' + ``` +- Do not rely on the runner's pre-installed tool versions — always install via a versioned setup action + +### Caching + +- Always cache package managers and build tool directories when the job installs dependencies: + ```yaml + - uses: actions/cache@v4 + with: + path: ~/.cache/pip + key: ${{ runner.os }}-pip-${{ hashFiles('**/requirements.txt') }} + restore-keys: | + ${{ runner.os }}-pip- + ``` +- Include the environment name or a relevant identifier in cache keys when building multiple targets + +### Artifacts + +- Name artifacts with enough context to be unambiguous (e.g., `firmware-${{ matrix.environment }}`) +- Avoid uploading artifacts that will never be consumed downstream + +--- + +## Security + +### Permissions — Least Privilege + +Declare explicit `permissions:` blocks. The default token permissions are broad; scope them to the minimum required: + +```yaml +permissions: + contents: read # for checkout +``` + +For jobs that publish releases or write to the repository: + +```yaml +permissions: + contents: write # create/update releases +``` + +A common safe baseline for build-only jobs: + +```yaml +permissions: + contents: read +``` + +### Supply Chain — Action Pinning + +**Third-party actions** (anything outside the `actions/` and `github/` namespaces) should be pinned to a specific release tag. Branch pins (`@main`, `@master`) are **not allowed** — they can be updated by the action author at any time without notice: + +```yaml +# ✅ Acceptable — specific version tag. SHA pinning recommended for more security, as @v2 is still a mutable tag. +uses: softprops/action-gh-release@v2 + +# ❌ Not acceptable — mutable branch reference +uses: andelf/nightly-release@main +``` + +SHA pinning (e.g., `uses: someorg/some-action@abc1234`) is the most secure option for third-party actions; it is recommended when auditing supply-chain risk is a priority. At minimum, always use a specific version tag. + +**First-party actions** (`actions/checkout`, `actions/cache`, `actions/upload-artifact`, etc.) pinned to a major version tag (e.g., `@v4`) are acceptable because GitHub maintains and audits these. + +When adding a new third-party action: +1. Check that the action's repository is actively maintained +2. Review the action's source before adding it +3. Prefer well-known, widely-used actions over obscure ones + +### Credentials and Secrets + +- Use `${{ secrets.GITHUB_TOKEN }}` for operations within the same repository — it is automatically scoped and rotated +- Never commit secrets, tokens, or passwords into workflow files or any tracked file +- Never print secrets in `run:` steps, even with `echo` — GitHub masks known secrets but derived values are not automatically masked +- Scope secrets to the narrowest step that needs them using `env:` at the step level, not at the workflow level: + +```yaml +# ✅ Scoped to the step that needs it +- name: Create release + uses: softprops/action-gh-release@v2 + with: + token: ${{ secrets.GITHUB_TOKEN }} + +# ❌ Unnecessarily broad +env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} +``` + +- Personal Access Tokens (PATs, stored as repository secrets) should have the minimum required scopes and should be rotated periodically + +### Script Injection + +`${{ }}` expressions are evaluated before the shell script runs. If an expression comes from untrusted input (PR titles, issue bodies, branch names from forks), it can inject arbitrary shell commands. + +**Never** interpolate `github.event.*` values directly into a `run:` step: + +```yaml +# ❌ Injection risk — PR title is attacker-controlled +- run: echo "${{ github.event.pull_request.title }}" + +# ✅ Safe — value passed through an environment variable +- env: + PR_TITLE: ${{ github.event.pull_request.title }} + run: echo "$PR_TITLE" +``` + +This rule applies to any value that originates outside the repository (issue bodies, labels, comments, commit messages from forks). + +### Pull Request Workflows + +- Workflows triggered by `pull_request` from a fork run with **read-only** token permissions and no access to repository secrets — this is intentional and correct +- Do not use `pull_request_target` unless you fully understand the security implications; it runs in the context of the base branch and *does* have secret access, making it a common attack surface diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index c8c3f5c804..75abd27e01 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,187 +1,84 @@ -# WLED - ESP32/ESP8266 LED Controller Firmware +# WLED-MM — ESP32 LED Controller Firmware -WLED is a fast and feature-rich implementation of an ESP32 and ESP8266 webserver to control NeoPixel (WS2812B, WS2811, SK6812) LEDs and SPI-based chipsets. The project consists of C++ firmware for microcontrollers and a modern web interface. WLED-MM is a fork of WLED that concentrates on higher performance (esp32, esp32-s3, boards with PSRAM), large installs and advanced audio analysis. +WLED is a fast, feature-rich ESP32/ESP8266 webserver for controlling NeoPixel (WS2812B, WS2811, SK6812) LEDs and SPI-based chipsets. +WLED-MM is a fork focused on higher performance (ESP32, ESP32-S3, PSRAM boards), large installs, and advanced audio analysis. Always reference these instructions first and fallback to search or bash commands only when you encounter unexpected information that does not match the info here. -## Working Effectively - -### Initial Setup -- Install Node.js 20+ (specified in `.nvmrc`): Check your version with `node --version` -- Install dependencies: `npm ci` (takes ~5 seconds) -- Install PlatformIO for hardware builds: `pip install -r requirements.txt` (takes ~60 seconds) - -### Build and Test Workflow -- **ALWAYS build web UI first**: `npm run build` -- takes 3 seconds. NEVER CANCEL. -- **Run tests**: `npm test` -- takes 40 seconds. NEVER CANCEL. Set timeout to 2+ minutes. -- **Development mode**: `npm run dev` -- monitors file changes and auto-rebuilds web UI -- **Hardware firmware build**: `pio run -e [environment]` -- takes 15+ minutes. NEVER CANCEL. Set timeout to 30+ minutes. - -### Build Process Details -The build has two main phases: -1. **Web UI Generation** (`npm run build`): - - Processes files in `wled00/data/` (HTML, CSS, JS) - - Minifies and compresses web content - - Generates `wled00/html_*.h` files with embedded web content - - **CRITICAL**: Must be done before any hardware build - -2. **Hardware Compilation** (`pio run`): - - Compiles C++ firmware for various ESP32/ESP8266 targets - - Common environments: `esp32_4MB_V4_M`, `esp32_16MB_V4_S_HUB75`, `esp32S3_8MB_PSRAM_M_qspi`, `esp32_16MB_V4_M_eth`, `esp8266_4MB_S`, `esp32dev_compat` - - List all targets: `pio run --list-targets` - -## Before Finishing Work - -**CRITICAL: You MUST complete ALL of these steps before marking your work as complete:** - -1. **Build at least one hardware environment**: `pio run -e esp32_4MB_V4_M` -- Set timeout to 30+ minutes. NEVER CANCEL. - - Choose `esp32_4MB_V4_M` as it's a common, representative environment - - See "Hardware Compilation" section above for the full list of common environments - - The build MUST complete successfully without errors - - If the build fails, fix the issue before proceeding - - **DO NOT skip this step** - it validates that firmware compiles with your changes - -2. **For web UI changes only**: Manually test the interface - - See "Manual Testing Scenarios" section below - - Verify the UI loads and functions correctly - -**If any of these validation steps fail, you MUST fix the issues before finishing. Do NOT mark work as complete with failing builds or tests.** - -## Validation and Testing - -### Web UI Testing -- **ALWAYS validate web UI changes manually**: - - Start local server: `cd wled00/data && python3 -m http.server 8080` - - Open `http://localhost:8080/index.htm` in browser - - Test basic functionality: color picker, effects, settings pages -- **Check for JavaScript errors** in browser console - -### Code Validation -- **No automated linting configured** - follow existing code style in files you edit -- **Code style**: Use tabs for web files (.html/.css/.js), spaces (2 per level) for C++ files -- **Language**: The repository language is English (british, american, canadian, or australian). If you find other languages, suggest a translation into English. -- **C++ formatting available**: `clang-format` is installed but not in CI -- **Always run tests before finishing**: `npm test` -- **MANDATORY: Always run a hardware build before finishing** (see "Before Finishing Work" section below) +## Setup -### Manual Testing Scenarios -After making changes to web UI, always test: -- **Load main interface**: Verify index.htm loads without errors -- **Navigation**: Test switching between main page and settings pages -- **Color controls**: Verify color picker and brightness controls work -- **Effects**: Test effect selection and parameter changes -- **Settings**: Test form submission and validation +- Node.js 20+ (see `.nvmrc`) +- Install dependencies: `npm ci` +- PlatformIO (required only for firmware compilation): `pip install -r requirements.txt` -## Common Tasks +## Hardware Targets -### Project Branch / Release Structure -``` -mdev # Main development trunk (daily/nightly) 17.7.2-mdev -``` +| Target | Status | +|---|---| +| ESP32 (classic Xtensa dual-core) | **Primary target** | +| ESP32-S3 | **Primary target** — preferred for larger installs and HUB75 matrix | +| ESP32-S2, ESP32-C3 | Supported | +| ESP32-P4/-C5/-C6 | Will be supported in the future | +| ESP8266 | Deprecated — should still compile, but not actively maintained | -### Repository Structure -``` -wled00/ # Main firmware source (C++) "WLED core" - ├── data/ # Web interface files - │ ├── index.htm # Main UI - │ ├── settings*.htm # Settings pages - │ └── *.js/*.css # Frontend resources - ├── *.cpp/*.h # Firmware source files - ├── html_*.h # Auto-generated embedded web files (DO NOT EDIT, DO NOT COMMIT) - ├── src/ # Modules used by the WLED core (C++) - │ ├── fonts/ # Font libraries for scrolling text effect - └ └── dependencies/ # Utility functions - some of them have their own licensing terms -lib/ # Project specific custom libraries. PlatformIO will compile them to separate static libraries and link them -platformio.ini # Hardware build configuration +## Build and Test + +| Command | Purpose | Typical Time | +|---|---|---| +| `npm run build` | Build web UI → generates `wled00/html_*.h` headers | ~3 s | +| `npm test` | Run test suite | ~40 s | +| `npm run dev` | Watch mode — auto-rebuilds web UI on file changes | — | +| `pio run -e ` | Build firmware for a hardware target | 15–20 min | + +**Always run `npm ci; npm run build` before `pio run`.** The web UI build generates `wled00/html_*.h` header files required by firmware compilation. + +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 + +```text +wled00/ # Firmware source (C++) + ├── data/ # Web UI source (HTML, CSS, JS) + ├── src/ # Core modules, fonts, dependencies + ├── html_*.h # Auto-generated (DO NOT EDIT OR COMMIT) + └── wled.h # Main firmware configuration, and global variables +usermods/ # Community addons (.h files, included via usermods_list.cpp) +lib/ # Project specific custom libraries. PlatformIO will compile them to separate static libraries and link them +platformio.ini # Build targets and configuration platformio_override.sample.ini # examples for custom build configurations - entries must be copied into platformio_override.ini to use them. # platformio_override.ini is _not_ stored in the WLED repository! -usermods/ # User-contributed addons to the WLED core, maintained by individual contributors (C++, with individual library.json) -package.json # Node.js dependencies and scripts, release identification -pio-scripts/ # Build tools (platformio) -tools/ # Build tools (Node.js), partition files, and generic utilities - ├── cdata.js # Web UI build script - └── cdata-test.js # Test suite -.github/workflows/ # CI/CD pipelines + +pio-scripts/ # Build tools (platformio) +tools/ # Build tools (Node.js), partition files, and generic utilities +tools/cdata.js # Web UI → header build script +tools/cdata-test.js # Test suite +package.json # Node.js scripts and release ID +.github/workflows/ # CI/CD pipelines ``` -### Key Files and Their Purpose -- `wled00/data/index.htm` - Main web interface -- `wled00/data/settings*.htm` - Configuration pages -- `tools/cdata.js` - Converts web files to C++ headers -- `wled00/wled.h` - Main firmware configuration -- `platformio.ini` - Hardware build targets and settings - -### Development Workflow (applies to agent mode only) -1. **For web UI changes**: - - Edit files in `wled00/data/` - - Run `npm run build` to regenerate headers - - Test with local HTTP server - - Run `npm test` to validate build system - -2. **For firmware changes**: - - Edit files in `wled00/` (but NOT `html_*.h` files) - - Ensure web UI is built first (`npm run build`) - - Build firmware: `pio run -e [target]` - - Flash to device: `pio run -e [target] --target upload` - -3. **For both web and firmware**: - - Always build web UI first - - Test web interface manually - - Build and test firmware if making firmware changes - -## Build Timing and Timeouts - -**IMPORTANT: Use these timeout values when running builds:** - -- **Web UI build** (`npm run build`): 3 seconds typical - Set timeout to 30 seconds minimum -- **Test suite** (`npm test`): 40 seconds typical - Set timeout to 120 seconds (2 minutes) minimum -- **Hardware builds** (`pio run -e [target]`): 15-20 minutes typical for first build - Set timeout to 1800 seconds (30 minutes) minimum - - Subsequent builds are faster due to caching - - First builds download toolchains and dependencies which takes significant time -- **NEVER CANCEL long-running builds** - PlatformIO downloads and compilation require patience - -**When validating your changes before finishing, you MUST wait for the hardware build to complete successfully. Set the timeout appropriately and be patient.** - -## Troubleshooting - -### Common Issues -- **Build fails with missing html_*.h**: Run `npm run build` first -- **Web UI looks broken**: Check browser console for JavaScript errors -- **PlatformIO network errors**: Try again, downloads can be flaky -- **Node.js version issues**: Ensure Node.js 20+ is installed (check `.nvmrc`) - -### When Things Go Wrong -- **Clear generated files**: `rm -f wled00/html_*.h` then rebuild -- **Force web UI rebuild**: `npm run build -- --force` or `npm run build -- -f` -- **Clean PlatformIO cache**: `pio run --target clean` -- **Reinstall dependencies**: `rm -rf node_modules && npm install` - -## Important Notes - -- **Always commit source files** -- **Web UI re-built is part of the platformio firmware compilation** -- **do not commit generated html_*.h files** -- **DO NOT edit `wled00/html_*.h` files** - they are auto-generated. If needed, modify Web UI files in `wled00/data/`. -- **Test web interface manually after any web UI changes** -- When reviewing a PR: the PR author does not need to update/commit generated html_*.h files - these files will be auto-generated when building the firmware binary. -- **If you are not sure about something, just answer that you are not sure.** Gather more information instead of continuing with a wild guess. -- If asked for an analysis, assessment or web research, **provide relevant references** to justify your conclusions. Ensure your recommendations are based on the correct source code branch or PR. -- **Use VS Code with PlatformIO extension for best development experience** -- **Hardware builds require appropriate ESP32/ESP8266 development board** - -## CI/CD Pipeline - -**The GitHub Actions CI workflow will:** -1. Installs Node.js and Python dependencies -2. Runs `npm test` to validate build system -3. Builds web UI with `npm run build` (automatically run by PlatformIO) -4. Compiles firmware for ALL hardware targets listed in `default_envs` (MUST succeed for all) -5. Uploads build artifacts - -**To ensure CI success, you MUST locally:** -- Run `npm test` and ensure it passes -- Run `pio run -e esp32_4MB_V4_M` (or another common environment from "Hardware Compilation" section) and ensure it completes successfully -- If either fails locally, it WILL fail in CI - -**Match this workflow in your local development to ensure CI success. Do not mark work complete until you have validated builds locally.** +Main development branch: `mdev` + +## 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. +- **When unsure, say so.** Gather more information rather than guessing. +- **Acknowledge good patterns** when you see them. +- **Provide references** when making analyses or recommendations. Base them on the correct branch or PR. +- **Look for user-visible "breaking" changes**. Ask for confirmation that these were made intentionally. +- **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. + +### 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). +- Make sure AI-generated code is well documented. + +### 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. +- **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. diff --git a/.github/cpp.instructions.md b/.github/cpp.instructions.md new file mode 100644 index 0000000000..cca620dd45 --- /dev/null +++ b/.github/cpp.instructions.md @@ -0,0 +1,505 @@ +--- +applyTo: "**/*.cpp,**/*.h,**/*.hpp, **/*.ino" +--- +# C++ Coding Conventions + +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` + +## 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 `//` +- Mark WLED-MM-specific changes with `// WLEDMM` or `// WLEDMM: description`: + +```cpp +// WLEDMM: increased max bus count for larger installs +#ifndef WLED_MAX_BUSSES + #define WLED_MAX_BUSSES 20 // WLEDMM default (upstream: 10) +#endif +``` + +- **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. + +- **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` +- 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)**: 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, FreeRTOS task stacks are typically only 2–8 KB; 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. Prefer a fixed-size array with a compile-time bound, or heap allocation (`d_malloc` / `p_malloc`) for dynamically sized buffers. **Any VLA must be explicitly justified in the source code or PR.** +- **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 + +## `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. + +### `const` locals + +Adding `const` to a local variable that is only assigned once is not necessary — but it **is** required when the variable is passed to a function that takes a `const` parameter (pointer or reference). In hot-path code, `const` on cached locals helps the compiler keep values in registers: + +```cpp +const uint_fast16_t cols = virtualWidth(); +const uint_fast16_t rows = virtualHeight(); +``` + +### `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: + +```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); +``` + +### `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 +``` + +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 +``` + +### `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 every getter, query, or inspection method `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` also 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) +``` + +### 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(); + ... +} +``` + +### 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: + +```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); +``` + +General rules: +- Keep the per-pixel fast path free of non-inline function calls, multi-way branches and complex switch-case decisions. +- Hoist the "which path?" decision out of the inner loop (once per frame or per segment) +- It is acceptable to duplicate some code between fast and complex variants 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 `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. + +### 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 = (255 - 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. +The compiler already converts power-of-two unsigned divisions to shifts at `-O2`. +On RISC-V-based boards (ESP32-C3, ESP32-C6, ESP32-C5) explicit shifts can be beneficial: + +Prefer bit shifts for power-of-two operations: + +```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. `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` + +### 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 + ``` + +## 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 + +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. This differs from ESP8266, where `delay()` stalled the entire system unless `yield()` was called inside. + +**`delay()` in `loopTask` is allowed.** The Arduino `loop()` function runs inside `loopTask`. Calling `delay()` there does not block the network stack, audio FFT, LED DMA, or any other FreeRTOS task. + +**`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 +``` + +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. +**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. 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/C6) 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) + ``` diff --git a/.github/esp-idf.instructions.md b/.github/esp-idf.instructions.md new file mode 100644 index 0000000000..b196940c92 --- /dev/null +++ b/.github/esp-idf.instructions.md @@ -0,0 +1,756 @@ +--- +applyTo: "**/*.cpp,**/*.h,**/*.hpp,**/*.ino" +--- +# ESP-IDF Coding Guide (within arduino-esp32) + +WLED-MM runs on the Arduino-ESP32 framework, which wraps ESP-IDF. Understanding the ESP-IDF layer is essential when writing chip-specific code, managing peripherals, or preparing for the IDF v5.x migration. This guide documents patterns already used in the codebase and best practices derived from Espressif's official examples. + +> **Scope**: This file is an optional review guideline. It applies when touching chip-specific code, peripheral drivers, memory allocation, or platform conditionals. + +--- + +## Identifying the Build Target: `CONFIG_IDF_TARGET_*` + +Use `CONFIG_IDF_TARGET_*` macros to gate chip-specific code at compile time. These are set by the build system and are mutually exclusive — exactly one is defined per build. + +| Macro | Chip | Architecture | Notes | +|---|---|---|---| +| `CONFIG_IDF_TARGET_ESP32` | ESP32 (classic) | Xtensa dual-core | Primary target. Has DAC, APLL, I2S ADC mode | +| `CONFIG_IDF_TARGET_ESP32S2` | ESP32-S2 | Xtensa single-core | Limited peripherals. 13-bit ADC | +| `CONFIG_IDF_TARGET_ESP32S3` | ESP32-S3 | Xtensa dual-core | Preferred for large installs. Octal PSRAM, USB-OTG | +| `CONFIG_IDF_TARGET_ESP32C3` | ESP32-C3 | RISC-V single-core | Minimal peripherals. RISC-V clamps out-of-range float→unsigned casts; see `cpp.instructions.md` UB note | +| `CONFIG_IDF_TARGET_ESP32C5` | ESP32-C5 | RISC-V single-core | Wi-Fi 2.4Ghz + 5Ghz, Thread/Zigbee. Future target | +| `CONFIG_IDF_TARGET_ESP32C6` | ESP32-C6 | RISC-V single-core | Wi-Fi 6, Thread/Zigbee. Future target | +| `CONFIG_IDF_TARGET_ESP32P4` | ESP32-P4 | RISC-V dual-core | High performance. Future target | + +### Build-time validation + +WLED validates at compile time that exactly one target is defined and that it is a supported chip (`wled.cpp` lines 39–61). Follow this pattern when adding new chip-specific branches: + +```cpp +#if defined(CONFIG_IDF_TARGET_ESP32) + // classic ESP32 path +#elif defined(CONFIG_IDF_TARGET_ESP32S3) + // S3-specific path +#elif defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32C5) || defined(CONFIG_IDF_TARGET_ESP32C6) + // RISC-V common path +#else + #warning "Untested chip — review peripheral availability" +#endif +``` + +### Guidelines + +- **Always test on the actual chip** before claiming support. Simulators and cross-compilation can hide peripheral differences. +- **Prefer `#elif` chains** over nested `#ifdef` for readability. +- **Do not use `CONFIG_IDF_TARGET_*` for feature detection.** Use `SOC_*` capability macros instead (see next section). For example, use `SOC_I2S_SUPPORTS_ADC` instead of `CONFIG_IDF_TARGET_ESP32` to check for I2S ADC support. +- When a feature must be disabled on certain chips, use explicit `static_assert()` or `#warning` directives so the build clearly reports what is missing. + +--- + +## Hardware Capability Detection: `SOC_*` Macros + +`SOC_*` macros (from `soc/soc_caps.h`) describe what the current chip supports. They are the correct way to check for peripheral features — they stay accurate when new chips are added, unlike `CONFIG_IDF_TARGET_*` checks. + +### Important `SOC_*` macros used in WLED-MM + +| Macro | Type | Used in | Purpose | +|---|---|---|---| +| `SOC_I2S_NUM` | `int` | `audio_source.h` | Number of I2S peripherals (1 or 2) | +| `SOC_I2S_SUPPORTS_ADC` | `bool` | `usermods/audioreactive/audio_source.h` | I2S ADC sampling mode (ESP32 only) | +| `SOC_I2S_SUPPORTS_APLL` | `bool` | `usermods/audioreactive/audio_source.h` | Audio PLL for precise sample rates | +| `SOC_I2S_SUPPORTS_PDM_RX` | `bool` | `usermods/audioreactive/audio_source.h` | PDM microphone input | +| `SOC_ADC_MAX_BITWIDTH` | `int` | `util.cpp` | ADC resolution (12 or 13 bits). Renamed to `CONFIG_SOC_ADC_RTC_MAX_BITWIDTH` in IDF v5 | +| `SOC_ADC_CHANNEL_NUM(unit)` | `int` | `pin_manager.cpp` | ADC channels per unit | +| `SOC_UART_NUM` | `int` | `dmx_input.cpp` | Number of UART peripherals | +| `SOC_DRAM_LOW` / `SOC_DRAM_HIGH` | `addr` | `util.cpp` | DRAM address boundaries for validation | + +### Less commonly used but valuable + +| Macro | Purpose | +|---|---| +| `SOC_RMT_TX_CANDIDATES_PER_GROUP` | Number of RMT TX channels (varies 2–8 by chip) | +| `SOC_LEDC_CHANNEL_NUM` | Number of LEDC (PWM) channels | +| `SOC_GPIO_PIN_COUNT` | Total GPIO pin count | +| `SOC_DAC_SUPPORTED` | Whether the chip has a DAC (ESP32/S2 only) | +| `SOC_SPIRAM_SUPPORTED` | Whether PSRAM interface exists | +| `SOC_CPU_CORES_NUM` | Core count (1 or 2) — useful for task pinning decisions | + +### Best practices + +```cpp +// Good: feature-based detection +#if SOC_I2S_SUPPORTS_PDM_RX + _config.mode = i2s_mode_t(I2S_MODE_MASTER | I2S_MODE_RX | I2S_MODE_PDM); +#else + #warning "PDM microphones not supported on this chip" +#endif + +// Avoid: chip-name-based detection +#if defined(CONFIG_IDF_TARGET_ESP32) || defined(CONFIG_IDF_TARGET_ESP32S3) + // happens to be correct today, but breaks when a new chip adds PDM support +#endif +``` + +### PSRAM capability macros + +For PSRAM presence, mode, and DMA access patterns: + +| Macro | Meaning | +|---|---| +| `CONFIG_SPIRAM` / `BOARD_HAS_PSRAM` | PSRAM is present in the build configuration | +| `CONFIG_SPIRAM_MODE_QUAD` | Quad-SPI PSRAM (standard, used on ESP32 classic and some S2/S3 boards) | +| `CONFIG_SPIRAM_MODE_OCT` | Octal-SPI PSRAM — 8 data lines, DTR mode. Used on ESP32-S3 with octal PSRAM (e.g. N8R8 / N16R8 modules). Reserves GPIO 33–37 for the PSRAM bus — **do not allocate these pins** when this macro is defined. `wled.cpp` uses this to gate GPIO reservation. | +| `CONFIG_SPIRAM_MODE_HEX` | Hex-SPI (16-line) PSRAM — future interface on ESP32-P4 running at up to 200 MHz. Used in `json.cpp` to report the PSRAM mode. | +| `CONFIG_SOC_PSRAM_DMA_CAPABLE` | PSRAM buffers can be used with DMA (ESP32-S3 with octal PSRAM) | +| `CONFIG_SOC_MEMSPI_FLASH_PSRAM_INDEPENDENT` | SPI flash and PSRAM on separate buses (no speed contention) | + +#### Detecting octal/hex flash + +On ESP32-S3 modules with OPI flash (e.g. N8R8 modules where the SPI flash itself runs in Octal-PI mode), the build system sets: + +| Macro | Meaning | +|---|---| +| `CONFIG_ESPTOOLPY_FLASHMODE_OPI` | Octal-PI flash mode. On S3, implies GPIO 33–37 are used by the flash/PSRAM interface — the same GPIO block as octal PSRAM. `wled.cpp` uses `CONFIG_ESPTOOLPY_FLASHMODE_OPI \|\| (CONFIG_SPIRAM_MODE_OCT && BOARD_HAS_PSRAM)` to decide whether to reserve these GPIOs. `json.cpp` uses this to report the flash mode string as `"🚀OPI"`. | +| `CONFIG_ESPTOOLPY_FLASHMODE_HEX` | Hex flash mode (ESP32-P4). Reported as `"🚀🚀HEX"` in `json.cpp`. | + +**Pattern used in WLED-MM** (from `wled.cpp`) to reserve the octal-bus GPIOs on S3: +```cpp +#if defined(CONFIG_IDF_TARGET_ESP32S3) + #if CONFIG_ESPTOOLPY_FLASHMODE_OPI || (CONFIG_SPIRAM_MODE_OCT && defined(BOARD_HAS_PSRAM)) + // S3: GPIO 33-37 are used by the octal PSRAM/flash bus + managed_pin_type pins[] = { {33, true}, {34, true}, {35, true}, {36, true}, {37, true} }; + pinManager.allocateMultiplePins(pins, sizeof(pins)/sizeof(managed_pin_type), PinOwner::SPI_RAM); + #endif +#endif +``` + +--- + +## ESP-IDF Version Conditionals + +### Checking the IDF version + +```cpp +#include + +#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0) + // IDF v5+ code path +#elif ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 4, 0) + // IDF v4.4+ code path +#else + // Legacy IDF v3/v4.x path +#endif +``` + +### Key ESP-IDF version thresholds for WLED-MM + +| Version | What changed | +|---|---| +| **4.0.0** | Filesystem API (`SPIFFS`/`LittleFS`), GPIO driver overhaul | +| **4.2.0** | ADC/GPIO API updates; `esp_adc_cal` introduced | +| **4.4.0** | I2S driver refactored (legacy API remains); `adc_deprecated.h` headers appear for newer targets | +| **4.4.4–4.4.8** | Known I2S channel-swap regression on ESP32 (workaround in `audio_source.h`) | +| **5.0.0** | **Major breaking changes** — RMT, I2S, ADC, SPI flash APIs replaced (see migration section) | +| **5.1.0** | Matter protocol support; new `esp_flash` API stable | +| **5.3+** | arduino-esp32 v3.x compatibility; C6/P4 support | + +### Guidelines + +- When adding a version guard, **always include a comment** explaining *what* changed and *why* the guard is needed. +- Avoid version ranges that silently break — prefer `>=` over exact version matches. +- Known regressions should use explicit range guards: + ```cpp + // IDF 4.4.4–4.4.8 swapped I2S left/right channels (fixed in 4.4.9) + #if (ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 4, 4)) && \ + (ESP_IDF_VERSION <= ESP_IDF_VERSION_VAL(4, 4, 8)) + #define I2S_CHANNELS_SWAPPED + #endif + ``` + +--- + +## Migrating from ESP-IDF v4.4.x to v5.x + +The jump from IDF v4.4 (arduino-esp32 v2.x) to IDF v5.x (arduino-esp32 v3.x) is the largest API break in ESP-IDF history. This section documents the critical changes and recommended migration patterns based on the upstream WLED `V5-C6` branch (`https://github.com/wled/WLED/tree/V5-C6`). Note: WLED-MM has not yet migrated to IDF v5 — these patterns prepare for the future migration. + +### Compiler changes + +IDF v5.x ships a much newer GCC toolchain. Key versions: + +| ESP-IDF | GCC | C++ default | Notes | +|---|---|---|---| +| 4.4.x (current) | **8.4.0** | C++17 (gnu++17) | Xtensa + RISC-V | +| 5.1–5.3 | **13.2** | C++20 (gnu++2b) | Significant warning changes | +| 5.4–5.5 | **14.2** | C++23 (gnu++2b) | Latest; stricter diagnostics | + +Notable behavioral differences: + +| Change | Impact | Action | +|---|---|---| +| Stricter `-Werror=enum-conversion` | Implicit int-to-enum casts now error | Use explicit `static_cast<>` or typed enums | +| C++20/23 features available | `consteval`, `concepts`, `std::span`, `std::expected` | Use judiciously — ESP8266 builds still require GCC 10.x with C++17 | +| `-Wdeprecated-declarations` enforced | Deprecated API calls become warnings/errors | Migrate to new APIs (see below) | +| `-Wdangling-reference` (GCC 13+) | Warns when a reference binds to a temporary that will be destroyed | Fix the lifetime issue; do not suppress the warning | +| `-fno-common` default (GCC 12+) | Duplicate tentative definitions across translation units cause linker errors | Use `extern` declarations in headers, define in exactly one `.cpp` | +| RISC-V codegen improvements | C3/C6/P4 benefit from better register allocation | No action needed — automatic | + +### C++ language features: GCC 8 → GCC 14 + +The jump from GCC 8.4 to GCC 14.2 spans six major compiler releases. This section lists features that become available and patterns that need updating. + +#### Features safe to use after migration + +These work in GCC 13+/14+ but **not** in GCC 8.4. Guard with `#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0)` if the code must compile on both IDF v4 and v5. + +| Feature | Standard | Example | Benefit | +|---|---|---|---| +| Designated initializers (C++20) | C++20 | `gpio_config_t cfg = { .mode = GPIO_MODE_OUTPUT };` | Already used as a GNU extension in GCC 8; becomes standard and portable in C++20 | +| `[[likely]]` / `[[unlikely]]` | C++20 | `if (err != ESP_OK) [[unlikely]] { ... }` | Hints for branch prediction; useful in hot paths | +| `[[nodiscard("reason")]]` | C++20 | `[[nodiscard("leak if ignored")]] void* allocBuffer();` | Enforces checking return values — helpful for `esp_err_t` wrappers | +| `std::span` | C++20 | `void process(std::span buf)` | Safe, non-owning view of contiguous memory — replaces raw pointer + length pairs | +| `consteval` | C++20 | `consteval uint32_t packColor(...)` | Guarantees compile-time evaluation; useful for color constants | +| `constinit` | C++20 | `constinit static int counter = 0;` | Prevents static initialization order fiasco | +| Concepts / `requires` | C++20 | `template requires std::integral` | Clearer constraints than SFINAE; improves error messages | +| Three-way comparison (`<=>`) | C++20 | `auto operator<=>(const Version&) const = default;` | Less boilerplate for comparable types | +| `std::bit_cast` | C++20 | `float f = std::bit_cast(uint32_val);` | Type-safe reinterpretation — replaces `memcpy` or `union` tricks | +| `if consteval` | C++23 | `if consteval { /* compile-time */ } else { /* runtime */ }` | Cleaner than `std::is_constant_evaluated()` | +| `std::expected` | C++23 | `std::expected readSensor()` | Monadic error handling — cleaner than returning error codes | +| `std::to_underlying` | C++23 | `auto val = std::to_underlying(myEnum);` | Replaces `static_cast(myEnum)` | + +#### Features already available in GCC 8 (C++17) + +These work on both IDF v4.4 and v5.x — prefer them now: + +| Feature | Example | Notes | +|---|---|---| +| `if constexpr` | `if constexpr (sizeof(T) == 4) { ... }` | Compile-time branching; already used in WLED-MM (see `cpp.instructions.md`) | +| `std::optional` | `std::optional pin;` | Nullable value without sentinel values like `-1` | +| `std::string_view` | `void log(std::string_view msg)` | Non-owning, non-allocating string reference | +| Structured bindings | `auto [err, value] = readSensor();` | Useful with `std::pair` / `std::tuple` returns | +| Fold expressions | `(addSegment(args), ...);` | Variadic template expansion | +| Inline variables | `inline constexpr int MAX_PINS = 50;` | Avoids ODR issues with header-defined constants | +| `[[maybe_unused]]` | `[[maybe_unused]] int debug_only = 0;` | Suppresses unused-variable warnings cleanly | +| `[[fallthrough]]` | `case 1: doA(); [[fallthrough]]; case 2:` | Documents intentional switch fallthrough | +| Nested namespaces | `namespace wled::audio { }` | Shorter than nested `namespace` blocks | + +#### Patterns that break or change behavior + +| Pattern | GCC 8 behavior | GCC 14 behavior | Fix | +|---|---|---|---| +| `int x; enum E e = x;` | Warning (often ignored) | Error with `-Werror=enum-conversion` | `E e = static_cast(x);` | +| `int g;` in two `.cpp` files | Both compile, linker merges (tentative definition) | Error: multiple definitions (`-fno-common`) | `extern int g;` in header, `int g;` in one `.cpp` | +| `const char* ref = std::string(...).c_str();` | Silent dangling pointer | Warning (`-Wdangling-reference`) | Extend lifetime: store the `std::string` in a local variable | +| `register int x;` | Accepted (ignored) | Warning or error (`register` removed in C++17) | Remove `register` keyword | +| Narrowing in aggregate init | Warning | Error | Use explicit cast or wider type | +| Implicit `this` capture in lambdas | Accepted in `[=]` | Deprecated warning; error in C++20 mode | Use `[=, this]` or `[&]` | + +#### Recommendations + +- **Do not raise the minimum C++ standard yet.** WLED-MM must still build on IDF v4.4 (GCC 8.4, C++17). Use `#if __cplusplus > 201703L` to gate C++20 features. +- **Prefer `std::optional` over sentinel values** (e.g., `-1` for "no pin") in new code — it works on both compilers. +- **Use `std::string_view`** for read-only string parameters instead of `const char*` or `const String&` — zero-copy and works on GCC 8+. +- **Avoid raw `union` type punning** — prefer `memcpy` (GCC 8) or `std::bit_cast` (GCC 13+) for strict-aliasing safety. +- **Mark intentional fallthrough** with `[[fallthrough]]` — GCC 14 warns on unmarked fallthrough by default. + +### Deprecated and removed APIs + +#### RMT (Remote Control Transceiver) + +The legacy `rmt_*` functions are removed in IDF v5. The new API is channel-based: + +| IDF v4 (legacy) | IDF v5 (new) | Notes | +|---|---|---| +| `rmt_config()` + `rmt_driver_install()` | `rmt_new_tx_channel()` / `rmt_new_rx_channel()` | Channels are now objects | +| `rmt_write_items()` | `rmt_transmit()` with encoder | Requires `rmt_encoder_t` | +| `rmt_set_idle_level()` | Configure in channel config | Set at creation time | +| `rmt_item32_t` | `rmt_symbol_word_t` | Different struct layout | + +**WLED impact**: NeoPixelBus LED output and IR receiver both use legacy RMT. The upstream `V5-C6` branch adds `-D WLED_USE_SHARED_RMT` and disables IR until the library is ported. + +#### I2S (Inter-IC Sound) + +Legacy `i2s_driver_install()` + `i2s_read()` API is deprecated. The new API uses channel handles: + +| IDF v4 (legacy) | IDF v5 (new) | Notes | +|---|---|---| +| `i2s_driver_install()` | `i2s_channel_init_std_mode()` | Separate STD/PDM/TDM modes | +| `i2s_set_pin()` | Pin config in `i2s_std_gpio_config_t` | Set at init time | +| `i2s_read()` | `i2s_channel_read()` | Uses channel handle | +| `i2s_set_clk()` | `i2s_channel_reconfig_std_clk()` | Reconfigure running channel | +| `i2s_config_t` | `i2s_std_config_t` | Separate config for each mode | + +**Migration pattern** (from Espressif examples): +```cpp +#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0) + #include "driver/i2s_std.h" + i2s_chan_handle_t rx_handle; + i2s_chan_config_t chan_cfg = I2S_CHANNEL_DEFAULT_CONFIG(I2S_NUM_0, I2S_ROLE_MASTER); + i2s_new_channel(&chan_cfg, NULL, &rx_handle); + + i2s_std_config_t std_cfg = { + .clk_cfg = I2S_STD_CLK_DEFAULT_CONFIG(22050), + .slot_cfg = I2S_STD_MSB_SLOT_DEFAULT_CONFIG(I2S_DATA_BIT_WIDTH_32BIT, I2S_SLOT_MODE_MONO), + .gpio_cfg = { .din = GPIO_NUM_32, .mclk = I2S_GPIO_UNUSED, ... }, + }; + i2s_channel_init_std_mode(rx_handle, &std_cfg); + i2s_channel_enable(rx_handle); +#else + // Legacy i2s_driver_install() path +#endif +``` + +**WLED impact**: The audioreactive usermod (`audio_source.h`) heavily uses legacy I2S. Migration requires rewriting the `I2SSource` class for channel-based API. + +#### ADC (Analog-to-Digital Converter) + +Legacy `adc1_get_raw()` and `esp_adc_cal_*` are deprecated: + +| IDF v4 (legacy) | IDF v5 (new) | Notes | +|---|---|---| +| `adc1_config_width()` + `adc1_get_raw()` | `adc_oneshot_new_unit()` + `adc_oneshot_read()` | Object-based API | +| `esp_adc_cal_characterize()` | `adc_cali_create_scheme_*()` | Calibration is now scheme-based | +| `adc_continuous_*` (old) | `adc_continuous_*` (restructured) | Config struct changes | + +#### SPI Flash + +| IDF v4 (legacy) | IDF v5 (new) | +|---|---| +| `spi_flash_read()` | `esp_flash_read()` | +| `spi_flash_write()` | `esp_flash_write()` | +| `spi_flash_erase_range()` | `esp_flash_erase_region()` | + +WLED already has a compatibility shim in `ota_update.cpp` that maps old names to new ones. + +#### GPIO + +| IDF v4 (legacy) | IDF v5 (recommended) | +|---|---| +| `gpio_pad_select_gpio()` | `esp_rom_gpio_pad_select_gpio()` (or use `gpio_config()`) | +| `gpio_set_direction()` + `gpio_set_pull_mode()` | `gpio_config()` with `gpio_config_t` struct | + +### Features disabled in IDF v5 builds + +The upstream `V5-C6` branch explicitly disables features with incompatible library dependencies: + +```ini +# platformio.ini [esp32_idf_V5] +-D WLED_DISABLE_INFRARED # IR library uses legacy RMT +-D WLED_DISABLE_MQTT # AsyncMqttClient incompatible with IDF v5 +-D ESP32_ARDUINO_NO_RGB_BUILTIN # Prevents RMT driver conflict with built-in LED +-D WLED_USE_SHARED_RMT # Use new shared RMT driver for NeoPixel output +``` + +### Migration checklist for new code + +1. **Never use a removed API without a version guard.** Always provide both old and new paths, or disable the feature on IDF v5. +2. **Test on both IDF v4.4 and v5.x builds** if the code must be backward-compatible. +3. **Prefer the newer API** when writing new code — wrap the old API in an `#else` block. +4. **Mark migration TODOs** with `// TODO(idf5):` so they are easy to find later. + +--- + +## Memory Management: `heap_caps_*` Best Practices + +ESP32 has multiple memory regions with different capabilities. Using the right allocator is critical for performance and stability. + +### Memory regions + +| Region | Flag | Speed | DMA | Size | Use for | +|---|---|---|---|---|---| +| DRAM | `MALLOC_CAP_INTERNAL \| MALLOC_CAP_8BIT` | Fast | Yes (ESP32) | 200–320 KB | Hot-path buffers, task stacks, small allocations | +| IRAM | `MALLOC_CAP_EXEC` | Fastest | No | 32–128 KB | Code (automatic via `IRAM_ATTR`) | +| PSRAM (SPIRAM) | `MALLOC_CAP_SPIRAM \| MALLOC_CAP_8BIT` | Slower | Chip-dependent | 2–16 MB | Large buffers, JSON documents, image data | +| RTC RAM | `MALLOC_CAP_RTCRAM` | Moderate | No | 8 KB | Data surviving deep sleep; small persistent buffers | + +### WLED-MM allocation wrappers + +WLED-MM provides convenience wrappers with automatic fallback. **Always prefer these over raw `heap_caps_*` calls**: + +| Function | Allocation preference | Use case | +|---|---|---| +| `d_malloc(size)` | RTC → DRAM → PSRAM | General-purpose; prefers fast memory | +| `d_calloc(n, size)` | Same as `d_malloc`, zero-initialized | Arrays, structs | +| `p_malloc(size)` | PSRAM → DRAM | Large buffers; prefers abundant memory | +| `p_calloc(n, size)` | Same as `p_malloc`, zero-initialized | Large arrays | +| `d_malloc_only(size)` | RTC → DRAM (no PSRAM fallback) | DMA buffers, time-critical data | + +### PSRAM guidelines + +- **Check availability**: always test `psramFound()` before assuming PSRAM is present. +- **DMA compatibility**: on ESP32 (classic), PSRAM buffers are **not DMA-capable** — use `d_malloc_only()` to allocate DMA buffers in DRAM only. On ESP32-S3 with octal PSRAM (`CONFIG_SPIRAM_MODE_OCT`), PSRAM buffers *can* be used with DMA when `CONFIG_SOC_PSRAM_DMA_CAPABLE` is defined. +- **JSON documents**: use the `PSRAMDynamicJsonDocument` allocator (defined in `wled.h`) to put large JSON documents in PSRAM: + ```cpp + PSRAMDynamicJsonDocument doc(16384); // allocated in PSRAM if available + ``` +- **Fragmentation**: PSRAM allocations fragment less than DRAM because the region is larger. But avoid mixing small and large allocations in PSRAM — small allocations waste the MMU page granularity. +- **Heap validation**: use `d_measureHeap()` and `d_measureContiguousFreeHeap()` to monitor remaining DRAM. Allocations that would drop free DRAM below `MIN_HEAP_SIZE` should go to PSRAM instead. +- **Performance**: PSRAM access is 3–10× slower than DRAM on ESP32/S2 (quad-SPI bus). On ESP32-S3 with octal PSRAM (`CONFIG_SPIRAM_MODE_OCT`), the penalty is smaller (~2×) because the 8-line DTR bus runs at up to 80 MHz (120 MHz is possible with CONFIG_SPIRAM_SPEED_120M, which requires enabling experimental ESP-IDF features). On ESP32-P4 with hex PSRAM (`CONFIG_SPIRAM_MODE_HEX`), the 16-line bus runs at 200 MHz, further reducing the gap. Keep hot-path data in DRAM regardless. + +### Pattern: preference-based allocation + +When you need a buffer that works on boards with or without PSRAM: + +```cpp +// Prefer PSRAM for large buffers, fall back to DRAM +uint8_t* buf = (uint8_t*)heap_caps_malloc_prefer(bufSize, 2, + MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT, // first choice: PSRAM + MALLOC_CAP_DEFAULT); // fallback: any available +// Or simply: +uint8_t* buf = (uint8_t*)p_malloc(bufSize); +``` + +--- + +## I2S Audio: Best Practices + +The audioreactive usermod uses I2S for microphone input. Key patterns: + +### Port selection + +```cpp +constexpr i2s_port_t AR_I2S_PORT = I2S_NUM_0; +// I2S_NUM_1 has limitations: no MCLK routing, no ADC support, no PDM support +``` + +Always use `I2S_NUM_0` unless you have a specific reason and have verified support on all target chips. + +### DMA buffer tuning + +DMA buffer size controls latency vs. reliability: + +| Scenario | `dma_buf_count` | `dma_buf_len` | Latency | Notes | +|---|---|---|---|---| +| With HUB75 matrix | 18 | 128 | ~100 ms | Higher count prevents I2S starvation during matrix DMA | +| Without PSRAM | 24 | 128 | ~140 ms | More buffers compensate for slower interrupt response | +| Default | 8 | 128 | ~46 ms | Acceptable for most setups | + +### Interrupt priority + +Choose interrupt priority based on coexistence with other drivers: + +```cpp +#ifdef WLED_ENABLE_HUB75MATRIX + .intr_alloc_flags = ESP_INTR_FLAG_IRAM | ESP_INTR_FLAG_LEVEL1, // level 1 (lowest) to avoid starving HUB75 +#else + .intr_alloc_flags = ESP_INTR_FLAG_LEVEL2 | ESP_INTR_FLAG_LEVEL3, // accept level 2 or 3 (allocator picks available) +#endif +``` + +### APLL (Audio PLL) usage + +The ESP32 has an audio PLL for precise sample rates. Rules: + +- Enable APLL when an MCLK pin is provided and precision matters. +- **Disable APLL** when Ethernet or HUB75 is active — they also use the APLL. +- APLL is broken on ESP32 revision 0 silicon. +- Not all chips have APLL — gate with `SOC_I2S_SUPPORTS_APLL`. + +```cpp +#if !defined(SOC_I2S_SUPPORTS_APLL) + _config.use_apll = false; +#elif defined(WLED_USE_ETHERNET) || defined(WLED_ENABLE_HUB75MATRIX) + _config.use_apll = false; // APLL conflict +#endif +``` + +### PDM microphone caveats + +- Not supported on ESP32-C3 (`SOC_I2S_SUPPORTS_PDM_RX` not defined). +- ESP32-S3 PDM has known issues: sample rate at 50% of expected, very low amplitude. +- No clock pin (`I2S_CKPIN = -1`) triggers PDM mode in WLED-MM. + +--- + +## HUB75 LED Matrix: Best Practices + +WLED-MM uses the `ESP32-HUB75-MatrixPanel-I2S-DMA` library for HUB75 matrix output. + +### Chip-specific panel limits + +```cpp +#if defined(CONFIG_IDF_TARGET_ESP32S3) && defined(BOARD_HAS_PSRAM) + maxChainLength = 6; // S3 + PSRAM: up to 6 panels (DMA-capable PSRAM) +#elif defined(CONFIG_IDF_TARGET_ESP32S2) + maxChainLength = 2; // S2: limited DMA channels +#else + maxChainLength = 4; // Classic ESP32: default +#endif +``` + +### Color depth vs. pixel count + +The driver dynamically reduces color depth for larger displays to stay within DMA buffer limits: + +| Pixel count | Color depth | Bits per pixel | +|---|---|---| +| ≤ `MAX_PIXELS_10BIT` | 10-bit (30-bit color) | High quality (experimental) | +| ≤ `MAX_PIXELS_8BIT` | 8-bit (24-bit color) | Full quality | +| ≤ `MAX_PIXELS_6BIT` | 6-bit (18-bit color) | Slight banding | +| ≤ `MAX_PIXELS_4BIT` | 4-bit (12-bit color) | Visible banding | +| larger | 3-bit (9-bit color) | Minimal color range | + +### Resource conflicts + +- **APLL**: HUB75 I2S DMA uses the APLL. Disable APLL in the audio I2S driver when HUB75 is active. +- **I2S peripheral**: HUB75 uses `I2S_NUM_1` (or `I2S_NUM_0` on single-I2S chips). Audio must use the other port. +- **Pin count**: HUB75 requires 13–14 GPIO pins. On ESP32-S2 this severely limits remaining GPIO. +- **Reboot required**: on ESP32-S3, changing HUB75 driver options requires a full reboot — the I2S DMA cannot be reconfigured at runtime. + +--- + +## GPIO Best Practices + +### Prefer `gpio_config()` over individual calls + +```cpp +// Preferred: single struct-based configuration +gpio_config_t io_conf = { + .pin_bit_mask = (1ULL << pin), + .mode = GPIO_MODE_OUTPUT, + .pull_up_en = GPIO_PULLUP_DISABLE, + .pull_down_en = GPIO_PULLDOWN_DISABLE, + .intr_type = GPIO_INTR_DISABLE, +}; +gpio_config(&io_conf); + +// Avoid: multiple separate calls (more error-prone, deprecated in IDF v5) +gpio_set_direction(pin, GPIO_MODE_OUTPUT); +gpio_set_pull_mode(pin, GPIO_FLOATING); +``` + +### Pin manager integration + +Always allocate pins through WLED's `pinManager` before using GPIO APIs: + +```cpp +if (!pinManager.allocatePin(myPin, true, PinOwner::UM_MyUsermod)) { + return; // pin in use by another module +} +// Now safe to configure +``` + +--- + +## Timer Best Practices + +### Microsecond timing + +For high-resolution timing, prefer `esp_timer_get_time()` (microsecond resolution, 64-bit) over `millis()` or `micros()`: + +```cpp +#include +int64_t now_us = esp_timer_get_time(); // monotonic, not affected by NTP +``` + +### Periodic timers + +For periodic tasks with sub-millisecond precision, use `esp_timer`: + +```cpp +esp_timer_handle_t timer; +esp_timer_create_args_t args = { + .callback = myCallback, + .arg = nullptr, + .dispatch_method = ESP_TIMER_TASK, // run in timer task (not ISR) + .name = "my_timer", +}; +esp_timer_create(&args, &timer); +esp_timer_start_periodic(timer, 1000); // 1 ms period +``` + +Always prefer `ESP_TIMER_TASK` dispatch over `ESP_TIMER_ISR` unless you need ISR-level latency — ISR callbacks have severe restrictions (no logging, no heap allocation, no FreeRTOS API calls). + +--- + +## ADC Best Practices + +### Version-aware ADC code + +ADC is one of the most fragmented APIs across IDF versions: + +```cpp +#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0) + // IDF v5: oneshot driver + #include "esp_adc/adc_oneshot.h" + #include "esp_adc/adc_cali.h" + adc_oneshot_unit_handle_t adc_handle; + adc_oneshot_unit_init_cfg_t unit_cfg = { .unit_id = ADC_UNIT_1 }; + adc_oneshot_new_unit(&unit_cfg, &adc_handle); +#else + // IDF v4: legacy driver + #include "driver/adc.h" + #include "esp_adc_cal.h" + adc1_config_width(ADC_WIDTH_BIT_12); + int raw = adc1_get_raw(ADC1_CHANNEL_0); +#endif +``` + +### Bit width portability + +Not all chips have 12-bit ADC. `SOC_ADC_MAX_BITWIDTH` reports the maximum resolution (12 or 13 bits). Note that in IDF v5, this macro was renamed to `CONFIG_SOC_ADC_RTC_MAX_BITWIDTH`. Write version-aware guards: + +```cpp +// IDF v4: SOC_ADC_MAX_BITWIDTH IDF v5: CONFIG_SOC_ADC_RTC_MAX_BITWIDTH +#if defined(CONFIG_SOC_ADC_RTC_MAX_BITWIDTH) // IDF v5+ + #define MY_ADC_MAX_BITWIDTH CONFIG_SOC_ADC_RTC_MAX_BITWIDTH +#elif defined(SOC_ADC_MAX_BITWIDTH) // IDF v4 + #define MY_ADC_MAX_BITWIDTH SOC_ADC_MAX_BITWIDTH +#else + #define MY_ADC_MAX_BITWIDTH 12 // safe fallback +#endif + +#if MY_ADC_MAX_BITWIDTH == 13 + adc1_config_width(ADC_WIDTH_BIT_13); // ESP32-S2 +#else + adc1_config_width(ADC_WIDTH_BIT_12); // ESP32, S3, C3, etc. +#endif +``` + +WLED-MM's `util.cpp` uses the IDF v4 form (`SOC_ADC_MAX_BITWIDTH`) — this will need updating when the codebase migrates to IDF v5. + +--- + +## RMT Best Practices + +### Current usage in WLED + +RMT drives NeoPixel LED output (via NeoPixelBus) and IR receiver input. Both use the legacy API that is removed in IDF v5. + +### Migration notes + +- The upstream `V5-C6` branch uses `-D WLED_USE_SHARED_RMT` to switch to the new RMT driver for NeoPixel output. +- IR is disabled on IDF v5 until the IR library is ported. +- New chips (C6, P4) have different RMT channel counts — use `SOC_RMT_TX_CANDIDATES_PER_GROUP` to check availability. +- The new RMT API requires an "encoder" object (`rmt_encoder_t`) to translate data formats — this is more flexible but requires more setup code. + +--- + +## Espressif Best Practices (from official examples) + +### Error handling + +Always check `esp_err_t` return values. Use `ESP_ERROR_CHECK()` in initialization code, but handle errors gracefully in runtime code: + +```cpp +// Initialization — crash early on failure +ESP_ERROR_CHECK(i2s_driver_install(I2S_NUM_0, &config, 0, nullptr)); + +// Runtime — log and recover +esp_err_t err = i2s_read(I2S_NUM_0, buf, len, &bytes_read, portMAX_DELAY); +if (err != ESP_OK) { + DEBUGSR_PRINTF("I2S read failed: %s\n", esp_err_to_name(err)); + return; +} +``` + +### Logging + +WLED-MM uses its own logging macros — **not** `ESP_LOGx()`. For application-level code, always use the WLED-MM macros defined in `wled.h`: + +| Macro family | Defined in | Controlled by | Use for | +|---|---|---|---| +| `USER_PRINT` / `USER_PRINTLN` / `USER_PRINTF` | `wled.h` | Always active | Important messages the user should see (startup, errors, status) | +| `DEBUG_PRINT` / `DEBUG_PRINTLN` / `DEBUG_PRINTF` | `wled.h` | `WLED_DEBUG` build flag | Development/diagnostic output; compiled out in release builds | + +All of these wrap `Serial` output through the `DEBUGOUT` / `DEBUGOUTLN` / `DEBUGOUTF` macros. + +**Exception — low-level driver code**: When writing code that interacts directly with ESP-IDF APIs (e.g., I2S initialization, RMT setup), use `ESP_LOGx()` macros instead. They support tag-based filtering and compile-time log level control: + +```cpp +static const char* TAG = "my_module"; +ESP_LOGI(TAG, "Initialized with %d buffers", count); +ESP_LOGW(TAG, "PSRAM not available, falling back to DRAM"); +ESP_LOGE(TAG, "Failed to allocate %u bytes", size); +``` + +### Task creation and pinning + +On dual-core chips (ESP32, S3, P4), pin latency-sensitive tasks to a specific core: + +```cpp +xTaskCreatePinnedToCore( + audioTask, // function + "audio", // name + 4096, // stack size + nullptr, // parameter + 5, // priority (higher = more important) + &audioTaskHandle, // handle + 0 // core ID (0 = protocol core, 1 = app core) +); +``` + +Guidelines: +- Pin network/protocol tasks to core 0 (where Wi-Fi runs). +- Pin real-time tasks (audio, LED output) to core 1. +- On single-core chips (S2, C3, C5, C6), only core 0 exists — pinning to core 1 will fail. Use `SOC_CPU_CORES_NUM > 1` guards or `tskNO_AFFINITY`. +- Use `SOC_CPU_CORES_NUM` to conditionally pin tasks: + ```cpp + #if SOC_CPU_CORES_NUM > 1 + xTaskCreatePinnedToCore(audioTask, "audio", 4096, nullptr, 5, &handle, 1); + #else + xTaskCreate(audioTask, "audio", 4096, nullptr, 5, &handle); + #endif + ``` + +**Tip: use xTaskCreateUniversal()** - from arduino-esp32 - to avoid the conditional on `SOC_CPU_CORES_NUM`. This function has the same signature as ``xTaskCreatePinnedToCore()``, but automatically falls back to ``xTaskCreate()`` on single-core MCUs. + +### `delay()`, `yield()`, and the IDLE task + +FreeRTOS on ESP32 is **preemptive** — all tasks are scheduled by priority regardless of `yield()` calls. This is fundamentally different from ESP8266 cooperative multitasking. + +| Call | What it does | Reaches IDLE (priority 0)? | +|---|---|---| +| `delay(ms)` / `vTaskDelay(ticks)` | Suspends calling task; scheduler runs all other ready tasks | ✅ Yes | +| `yield()` / `vTaskDelay(0)` | Hint to switch to tasks at **equal or higher** priority only | ❌ No | +| `taskYIELD()` | Same as `vTaskDelay(0)` | ❌ No | +| Blocking API (`xQueueReceive`, `ulTaskNotifyTake`, `vTaskDelayUntil`) | Suspends task until event or timeout; IDLE runs freely | ✅ Yes | + +**`delay()` in `loopTask` is safe.** Arduino's `loop()` runs inside `loopTask`. Calling `delay()` suspends only `loopTask` — all other FreeRTOS tasks (Wi-Fi stack, audio FFT, LED DMA) continue uninterrupted on either core. + +**`yield()` does not yield to IDLE.** Any task that loops with only `yield()` calls will starve the IDLE task, causing the IDLE watchdog to fire. Always use `delay(1)` (or a blocking FreeRTOS call) in tight task loops. Note: WLED-MM redefines `yield()` as an empty macro on ESP32 WLEDMM_FASTPATH builds — see `cpp.instructions.md`. + +#### Why the IDLE task is not optional + +The FreeRTOS IDLE task (one per core on dual-core ESP32 and ESP32-S3; single instance on single-core chips) is not idle in the casual sense — it performs essential system housekeeping: + +- **Frees deleted task memory**: when a task calls `vTaskDelete()`, the IDLE task reclaims its TCB and stack. Without IDLE running, deleted tasks leak memory permanently. +- **Runs the idle hook**: when `configUSE_IDLE_HOOK = 1`, the IDLE task calls `vApplicationIdleHook()` on every iteration — some ESP-IDF components register low-priority background work here. +- **Implements tickless idle / light sleep**: on battery-powered devices, IDLE is the entry point for low-power sleep. A permanently starved IDLE task disables light sleep entirely. +- **Runs registered idle hooks**: ESP-IDF components register callbacks via `esp_register_freertos_idle_hook()` (e.g., Wi-Fi background maintenance, Bluetooth housekeeping). These only fire when IDLE runs. + +In short: **starving IDLE corrupts memory cleanup, breaks background activities, disables low-power sleep, and prevents Wi-Fi/BT maintenance.** The IDLE watchdog panic is a symptom — the real damage happens before the watchdog fires. + +### Watchdog management + +Long-running operations may trigger the task watchdog. Feed it explicitly: + +```cpp +#include +esp_task_wdt_reset(); // feed the watchdog in long loops +``` + +For tasks that intentionally block for extended periods, consider subscribing/unsubscribing from the TWDT: + +```cpp +esp_task_wdt_delete(NULL); // remove current task from TWDT (IDF v4.4) +// ... long blocking operation ... +esp_task_wdt_add(NULL); // re-register +``` + +> **IDF v5 note**: In IDF v5, `esp_task_wdt_add()` and `esp_task_wdt_delete()` require an explicit `TaskHandle_t`. Use `xTaskGetCurrentTaskHandle()` instead of `NULL`. + +--- + +## Quick Reference: IDF v4 → v5 API Mapping + +| Component | IDF v4 Header | IDF v5 Header | Key Change | +|---|---|---|---| +| I2S | `driver/i2s.h` | `driver/i2s_std.h` | Channel-based API | +| ADC (oneshot) | `driver/adc.h` | `esp_adc/adc_oneshot.h` | Unit/channel handles | +| ADC (calibration) | `esp_adc_cal.h` | `esp_adc/adc_cali.h` | Scheme-based calibration | +| RMT | `driver/rmt.h` | `driver/rmt_tx.h` / `rmt_rx.h` | Encoder-based transmit | +| SPI Flash | `spi_flash.h` | `esp_flash.h` | `esp_flash_*` functions | +| GPIO | `driver/gpio.h` | `driver/gpio.h` | `gpio_pad_select_gpio()` removed | +| Timer | `driver/timer.h` | `driver/gptimer.h` | General-purpose timer handles | +| PCNT | `driver/pcnt.h` | `driver/pulse_cnt.h` | Handle-based API | diff --git a/.github/web.instructions.md b/.github/web.instructions.md new file mode 100644 index 0000000000..3713fdae34 --- /dev/null +++ b/.github/web.instructions.md @@ -0,0 +1,28 @@ +--- +applyTo: "wled00/data/**" +--- +# Web UI Coding Conventions + +## Formatting + +- Indent **HTML and JavaScript** with **tabs** +- Indent **CSS** with **tabs** or **spaces** + +## JavaScript Style + +- **camelCase** for functions and variables: `gId()`, `selectedFx`, `currentPreset` +- Abbreviated helpers are common: `d` for `document`, `gId()` for `getElementById()` +- Mark WLED-MM additions with `// WLEDMM` comments + +## Key Files + +- `index.htm` — main interface +- `index.js` — functions that manage / update the main interface +- peek.js, liveview*.htm - live preview in main interface +- `settings*.htm` — configuration pages +- `*.css` — stylesheets (inlined during build) + +## 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.**