docs(esp-idf): add millis/micros internals, precision-wait pattern, PDM 16-bit note, ESP_ERROR_CHECK_WITHOUT_ABORT

[Copilot]

Incorporates four user-reported suggestions into esp-idf.instructions.md, each verified against official Espressif docs and arduino-esp32 source before inclusion.

Timer section

• millis()/micros() internals: Both are thin wrappers around esp_timer_get_time() in arduino-esp32 (ARDUINO_ISR_ATTR-marked, same monotonic source). The existing guidance to prefer esp_timer_get_time() directly still stands — for the full 64-bit value and to avoid truncation.

• New subsection — Precision waiting: coarse delay then spin-poll: Avoid spinning the full wait duration; yield to FreeRTOS while ≥2 ms remain, then busy-poll only the tail:

  long time_to_wait = (long)(target_us - micros());
  while (time_to_wait > 2000) {   // coarse: let other tasks run
    vTaskDelay(1);
    time_to_wait = (long)(target_us - micros());
  }
  while ((long)(target_us - micros()) > 0) { /* spin */ }  // fine: µs accuracy

  The troyhacks/WLED P4_experimental implementation is a working example.

PDM caveats (I2S Audio section)

The S3 "very low amplitude" issue is likely a bit-width mismatch. Espressif's IDF docs confirm PDM data is always 16-bit regardless of bits_per_sample. If the driver is configured for 32-bit, apparent amplitude drops to ~1/65536 of expected. Fix: set I2S_BITS_PER_SAMPLE_16BIT or interpret the read buffer as int16_t.

Error handling section

Added ESP_ERROR_CHECK_WITHOUT_ABORT() as the middle-ground option: same structured log output (file, line, error name) as ESP_ERROR_CHECK, but returns the error code instead of calling abort() — suitable for non-fatal runtime driver calls.

Summary by CodeRabbit

Release Notes

• Documentation
  • Clarified PDM microphone behavior: data unit width is effectively 16-bit in PDM mode and guidance on settings linked to low-amplitude symptom
  • Updated microsecond timing with Arduino-ESP32 note about direct timer usage
  • Added "Precision waiting" subsection: coarse delay then busy-spin for microsecond accuracy
  • Expanded error-handling docs with non-aborting check example
  • Adjusted logging example presentation for human readers

[coderabbitai]

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2dc13e1d-5842-4197-99e9-c7f8c8c3907a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• ✅ Review completed - (🔄 Check again to review again)

📝 Walkthrough

Walkthrough

Documentation updates to ESP-IDF guidance covering PDM microphone behavior, microsecond timing accuracy, precision waiting patterns, and error handling practices. Changes include addition of new subsections, clarification notes, and code examples wrapped with human-reader markers.

Changes

Cohort / File(s)	Summary
ESP-IDF Documentation .github/esp-idf.instructions.md	Added PDM microphone clarification regarding fixed 16-bit data width; inserted timing notes about millis()/micros() wrapper behavior; introduced new "Precision waiting: coarse delay then spin-poll" subsection with hybrid delay strategy; expanded error handling guidance with ESP_ERROR_CHECK_WITHOUT_ABORT() examples; wrapped logging example with HUMAN_ONLY markers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Revise *.instructions.md: separate AI-relevant special knowledge from background information for contributors  #354: Modifies the same .github/esp-idf.instructions.md file with related HUMAN_ONLY section additions and ESP-IDF guidance expansions.
• Revise copilot-instructions.md: consolidate, shorten, and extract language-specific rules, take 2 #353: Updates the same documentation file covering ESP-IDF timing, PDM/I2S, and error-handling guidance topics.

Poem

🐰 Hopping through the docs with glee,
PDM widths and timers set free,
Spin-polls and delays align,
Error notes now crystal fine,
Clear guidance hops its way to thee! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes all four main additions: millis/micros internals documentation, precision-wait pattern, PDM 16-bit note, and ESP_ERROR_CHECK_WITHOUT_ABORT guidance.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch copilot/validate-user-feedback-guidelines

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

.github/esp-idf.instructions.md (1)

498-505: Optional: add a paired HUMAN_ONLY snippet for 16-bit PDM buffer handling.

You added new AI-facing PDM guidance, but there isn’t a matching HUMAN_ONLY reference example here yet (e.g., int16_t interpretation/config snippet).

As per coding guidelines, "If new AI-facing rules were added without updating a related HUMAN_ONLY reference section, note this as a suggestion (not a required fix)."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/esp-idf.instructions.md around lines 498 - 505, Add a HUMAN_ONLY
example showing how to interpret 16-bit PDM buffers (use int16_t) and how it
maps to the documented bits_per_sample behavior for PDM mode; include a short
snippet demonstrating reading the PDM buffer as int16_t samples and the config
note that I2S_CKPIN = -1 enables PDM mode, so developers know to treat the
buffer as 16-bit units despite bits_per_sample settings. Ensure the snippet is
placed in the HUMAN_ONLY examples section adjacent to the new PDM guidance and
references int16_t, bits_per_sample, PDM, and I2S_CKPIN so readers can find the
matching human-facing example.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.github/esp-idf.instructions.md:
- Around line 498-505: Add a HUMAN_ONLY example showing how to interpret 16-bit
PDM buffers (use int16_t) and how it maps to the documented bits_per_sample
behavior for PDM mode; include a short snippet demonstrating reading the PDM
buffer as int16_t samples and the config note that I2S_CKPIN = -1 enables PDM
mode, so developers know to treat the buffer as 16-bit units despite
bits_per_sample settings. Ensure the snippet is placed in the HUMAN_ONLY
examples section adjacent to the new PDM guidance and references int16_t,
bits_per_sample, PDM, and I2S_CKPIN so readers can find the matching
human-facing example.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 881a4758-2c39-487c-90f0-00ccadfb42ec

📥 Commits

Reviewing files that changed from the base of the PR and between 231373a and e8ea676.

📒 Files selected for processing (1)

• .github/esp-idf.instructions.md

[softhack007]

OK, looks good to me 👍

[softhack007]

@coderabbitai just worth a note: I haven't added the "solution" of casting sample buffers to uint16_t or using BITS_PER_SAMPLE_16BIT. There is more to it - 16BIT mode does not work at all when building audiorective with esp-idf V4.4.8. Deeper investigation will be necessary, so let's not quickly pusblish a solution that possibly causes more problems.

#333 (comment)

[coderabbitai]

@softhack007, noted — thanks for the important clarification. The current text in the diff looks correct as-is: it documents the cause (PDM data is always 16-bit internally, 32-bit config causes low amplitude on S3) and links the upstream issue, but deliberately stops short of prescribing a fix. That's the right call given that BITS_PER_SAMPLE_16BIT mode is broken in audioreactive under IDF v4.4.8 and needs deeper investigation first.

I'll record this for future reviews to avoid incorrectly suggesting the 16-bit workaround as a ready solution.

---

✏️ Learnings added

Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 357
File: .github/esp-idf.instructions.md:504-504
Timestamp: 2026-04-06T17:17:42.196Z
Learning: In MoonModules/WLED-MM audioreactive (audio_source.h), the PDM S3 low-amplitude symptom is caused by configuring `bits_per_sample = I2S_BITS_PER_SAMPLE_32BIT` in PDM mode (IDF internally treats PDM data as always 16-bit). However, switching to `I2S_BITS_PER_SAMPLE_16BIT` or casting sample buffers to `int16_t` is NOT a ready solution: 16BIT mode does not work at all when building audioreactive with ESP-IDF v4.4.8. Deeper investigation is needed (tracked in issue `#333`) before recommending any fix. Do not suggest `BITS_PER_SAMPLE_16BIT` or `int16_t` casting as a PDM S3 fix.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

Learnt from: DedeHai
Repo: MoonModules/WLED-MM PR: 253
File: usermods/audioreactive/audio_source.h:1256-1261
Timestamp: 2025-07-12T04:20:14.546Z
Learning: In WLED AudioReactive usermod, DC offset removal optimization in DMAadcSource::getSamples() should be implemented after PR `#248` merges, which will change the sample buffer from float to int16. This eliminates the need for temporary integer buffers and allows direct integer arithmetic on the native buffer type for better memory efficiency on resource-constrained MCUs like C3 and S2.

---

Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: usermods/usermod_v2_rotary_encoder_ui_ALT/usermod_v2_rotary_encoder_ui_ALT.h:155-157
Timestamp: 2025-10-22T21:50:25.839Z
Learning: In WLED-MM PR `#270`, only Effect (Mode) IDs are migrated to 16-bit; Palette IDs and counts remain 8-bit. Usermod code should widen mode-related indices/loops to uint16_t while keeping palette-related indices/loops as uint8_t.

---

Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 353
File: .github/cpp.instructions.md:418-425
Timestamp: 2026-04-03T13:44:26.400Z
Learning: In MoonModules/WLED-MM, the guidance for float-to-unsigned conversion is: casting a potentially-negative float through `int` first (e.g., `uint8_t x = int(someFloat)`) is defined and intentional when the float value is known to be within `int`'s representable range. This achieves consistent, repeatable modulo-256 wrapping across Xtensa (ESP32) and RISC-V (ESP32-C3/C6) MCUs — avoiding the UB that occurs when a negative float is cast directly to an unsigned type. The comment `// float→int (defined), int→uint8_t (defined)` in cpp.instructions.md is correct for expressions like `40.74f * atan2f(dy, dx)` whose result is bounded within int's range. Do not suggest clamping as a "fix" for this pattern, as it would change the intended wrapping semantics.

---

Learnt from: DedeHai
Repo: MoonModules/WLED-MM PR: 248
File: usermods/audioreactive/audio_reactive.h:199-200
Timestamp: 2025-06-30T17:53:37.232Z
Learning: In WLED AudioReactive usermod, when using ArduinoFFT (enabled via UM_AUDIOREACTIVE_USE_ARDUINO_FFT), it's acceptable and necessary to redefine the global sqrt macro to sqrtf within the conditional compilation block for performance optimization. This is a specific requirement for ArduinoFFT optimization that can provide 10-50% performance improvement on ESP32.

---

Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 293
File: wled00/udp.cpp:668-669
Timestamp: 2025-12-21T22:26:03.261Z
Learning: In MoonModules/WLED-MM on 32-bit ESP32 platforms, changing pixel index variables to `int` type to align with the `setPixelColor(int, ...)` API signature is intentional and safe. Maximum pixel addresses are bounded by UINT16_MAX, and `setPixelColor()` performs internal overflow checks.

---

Learnt from: DedeHai
Repo: MoonModules/WLED-MM PR: 248
File: usermods/audioreactive/audio_reactive.h:1121-1121
Timestamp: 2025-06-30T17:58:27.987Z
Learning: In WLED AudioReactive usermod's runMicFilter function, the different cutoff frequencies between float (80Hz) and integer (90Hz) FFT implementations are intentional. The higher 90Hz cutoff for integer math helps preserve precision by avoiding overly aggressive filtering where integer arithmetic limitations could compound errors. The 10Hz difference between 80Hz and 90Hz is negligible from an audio performance standpoint but provides better precision for integer calculations.

---

Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 270
File: wled00/fcn_declare.h:403-406
Timestamp: 2025-10-22T21:59:24.331Z
Learning: In MoonModules/WLED-MM PR `#270`, the out-of-bounds null-terminator write in wled00/util.cpp (extractModeSlider) is deferred and tracked in Issue `#272`; do not address it within PR `#270`.

---

Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2026-04-02T22:23:41.534Z
Learning: In MoonModules/WLED-MM, the approved VLA guidance for cpp.instructions.md reads: "Avoid Variable Length Arrays (VLAs): GCC/Clang support VLAs as an extension (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 PR."

---

Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 0
File: :0-0
Timestamp: 2025-12-22T20:31:17.991Z
Learning: In MoonModules/WLED-MM on ESP32/ESP8266 platforms with limited heap memory, when reallocating buffers (e.g., LED buffers), always free the old buffer before allocating the new one. Allocating the new buffer first would temporarily require double the memory and could cause malloc failures when available RAM is low. The free-then-allocate pattern is preferred for resource-constrained embedded devices, even if it creates a brief nullptr window that must be protected by proper synchronization.

---

Learnt from: softhack007
Repo: MoonModules/WLED-MM PR: 308
File: platformio.ini:560-560
Timestamp: 2026-01-07T23:30:25.779Z
Learning: In platformio.ini, ESP32-S3 environments intentionally have WLED_USE_SHARED_RMT commented out (keeping RMTHI driver enabled) for testing purposes, while ESP32-S2 has it enabled (using shared RMT driver) to avoid boot loops. This inconsistency is temporary while investigating why RMTHI causes interrupt watchdog resets on S3. Once the S3 issue is understood, both S2 and S3 will use the same driver flags.

---

Learnt from: CR
Repo: MoonModules/WLED-MM PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-04-05T15:46:36.895Z
Learning: Document PR changes with a clear description of what changed and why — include expected impact on user-visible behavior and link related issues