esp32-arch-review

name	esp32-arch-review
description	Review ESP32 FastLED firmware architecture for RTOS safety, DMA correctness, LED driver patterns, memory management, and peripheral safety. Use before merging significant driver changes, new platform ports, or when auditing existing ESP32 FastLED code.
argument-hint	<file, directory, or component to review>
context	fork
agent	esp32-arch-review-agent

Perform a structured architecture review of ESP32 firmware code, with emphasis on FastLED driver patterns, DMA safety, RTOS correctness, and LED-specific constraints.

$ARGUMENTS

What This Skill Reviews

1. FastLED Driver Architecture

Review LED driver implementations under src/platforms/esp/:

• Driver inherits from correct base class (ClocklessController, ClockedController, or equivalent)
• show() method is interrupt-safe and atomic from the LED strip's perspective
• Pixel data encoding is performed before DMA transfer starts (no on-the-fly encoding race)
• Color order (RGB/GRB/BGR) is handled at compile time, not runtime switch
• Gamma correction and color scaling applied before encoding, not after DMA
• leds_data / CRGB array not modified during active DMA transfer
• FastLED's show_ordinal() / beginShow() / endShow() lifecycle respected
• Brightness scaling via FastLED.setBrightness() applied before encoding

2. DMA Safety

• DMA buffers allocated with heap_caps_malloc(size, MALLOC_CAP_INTERNAL | MALLOC_CAP_DMA)
• DMA buffers are 4-byte aligned (use __attribute__((aligned(4))) or heap_caps_aligned_alloc)
• DMA buffer lifetime extends beyond transfer completion (no premature free)
• DMA descriptors reside in internal SRAM (not PSRAM/SPIRAM)
• Cache coherence handled: esp_cache_msync() called or memory marked non-cacheable for DMA regions
• Double-buffering used where needed to allow CPU-side encoding to overlap with DMA transmission
• DMA completion callback or semaphore used to signal transfer end (not busy-wait polling)
• Buffer sizes within hardware limits (RMT: 64 items/channel, I2S/SPI: configurable block size)

3. FreeRTOS Safety

• No blocking calls (vTaskDelay, xSemaphoreTake with timeout) inside ISRs
• ISR functions use FromISR API variants only (xSemaphoreGiveFromISR, xQueueSendFromISR)
• ISR handlers marked IRAM_ATTR (ESP32) or __attribute__((section(".iram1"))) (other)
• No malloc, new, fl::vector allocation inside ISRs
• No printf, FL_DBG, ESP_LOGI or logging inside ISRs
• No flash access from ISRs (all ISR code and data must be in IRAM/DRAM)
• Shared state between ISR and task protected with critical section or ISR-safe primitives
• Task stack sizes adequate for FastLED usage (show() + encoding can be stack-heavy)
• No unbounded spin-wait loops without taskYIELD() (causes watchdog reset)
• Watchdog not starved during long show() calls with high LED counts

4. RMT-Specific Checks (WS2812/SK6812/APA106)

• RMT channel count within hardware limits (ESP32: 8, S2: 4, S3/C3/C6/H2: 4)
• RMT resolution set correctly for target LED protocol (typically 10ns for WS2812)
• RMT item encoding matches LED chipset timing spec (T0H, T0L, T1H, T1L, reset)
• RMT encoder (rmt_encoder_t) properly initialized and not shared between channels
• RMT callback or event queue used for completion, not polling
• RMT loop mode disabled (LED output is one-shot, not continuous)
• RMT end-of-frame reset pulse duration meets minimum spec (≥50µs for WS2812)

5. I2S/SPI-Specific Checks (APA102/SK9822/HD107)

• SPI clock rate set correctly for target LED chipset (APA102 max: 20MHz typical)
• SPI word size matches encoding (32-bit for APA102, 8-bit for raw SPI)
• I2S sample rate matches LED bit rate requirement
• SPI/I2S DMA buffer count adequate (≥2 for double buffering)
• SPI/I2S CS/CLK/DATA pins not conflicting with flash (GPIO 6-11 forbidden on ESP32)
• SPI transaction callback used to notify completion (not blocking transmit)

6. PARLIO-Specific Checks (ESP32-S3 Parallel Output)

• PARLIO clock source and divider correctly configured for target bit rate
• Data bus width matches LED strip count (1, 2, 4, 8, or 16 parallel strips)
• PARLIO DMA buffer aligned to cache line boundaries
• Output enable pin wired or bypassed correctly

7. Memory Management

• LED CRGB buffer allocation checked for NULL (large strips can exhaust heap)
• No large stack-allocated arrays in show() path (> 512 bytes on stack is risky)
• PSRAM usage documented when LED buffers are placed in SPIRAM
• Free heap monitored at startup and logged (alert if < 10KB before show())
• No memory leaks in driver initialization error paths
• String/buffer operations have bounds checking

8. Peripheral Pin Safety

• GPIO 6-11 not used for any output (ESP32 internal flash pins)
• GPIO 34-39 not used for output (input-only on original ESP32)
• Strapping pins (0, 2, 12, 15) not used without awareness of boot implications
• ADC2 channels not used alongside Wi-Fi (GPIO 0, 2, 4, 12-15, 25-27)
• Pin assignments match board schematic or documented LED strip wiring

9. Error Handling

• All ESP-IDF API return values checked (esp_err_t, esp_err check macros)
• Driver initialization failures reported with meaningful error messages
• Graceful degradation: driver failure doesn't crash the whole application
• Timeout values defined for all blocking operations
• Error paths clean up allocated resources (channels, DMA buffers, callbacks)

10. Code Quality & Maintainability

• Platform guards (#ifdef FASTLED_ESP32, #ifdef CONFIG_IDF_TARGET_ESP32S3, etc.) correct
• No magic numbers — timing constants named and commented with source (datasheet)
• Driver variant selection at compile time, not runtime (no dynamic dispatch overhead)
• Header includes minimal (no platform headers leaked into public FastLED API)
• Porting guide updated if new platform port added (see PORTING.md)

Output Format

# ESP32 Architecture Review Report

## Scope
- **Files reviewed**: [list]
- **Driver type**: [RMT / I2S / SPI / PARLIO / Other]
- **Target chip**: [ESP32 / ESP32-S2 / ESP32-S3 / ESP32-C3 / ESP32-C6 / ESP32-H2]

## Overall Assessment
[PASS / PASS WITH NOTES / FAIL]

## Critical Issues (must fix before merge)
- [ ] [Issue description + file:line reference]

## Warnings (should fix)
- [ ] [Issue description]

## Recommendations (consider for future)
- [ ] [Suggestion]

## Checklist Results
[Filled checklist from sections above — mark each PASS / FAIL / N/A]

Common FastLED ESP32 Architecture Anti-Patterns

Anti-Pattern	Problem	Fix
show() in ISR	Blocks ISR with long DMA wait	Move to task, use semaphore from ISR
Stack-allocated encode buffer	Stack overflow on high LED counts	Use heap or static allocation
Polling DMA completion	Wastes CPU, may block IDLE	Use DMA completion callback/semaphore
Shared RMT channel for multiple strips	Data corruption	One channel per strip
malloc() without MALLOC_CAP_DMA	DMA transfer fails silently	Always use heap_caps_malloc for DMA buffers
Timing constants without datasheet reference	Hard to audit or adapt	Add comments citing datasheet section
Runtime color order switch	Branch overhead per LED	Use compile-time template parameter