Adding new audio source DMA ADC: analog input support for C3, S2, S3 #253
Conversation
WalkthroughThe changes expand analog microphone support for ESP32 microcontrollers, adding DMA ADC sampling for ESP32-S2, ESP32-C3, and ESP32-S3 targets using ESP-IDF 4.4.0 or later. Conditional compilation and type checks are updated, a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AudioReactive
participant I2SAdcSource
participant DMAadcSource
User->>AudioReactive: Configure analog microphone input
AudioReactive->>AudioReactive: Check target (classic ESP32 or S2/C3/S3)
alt Classic ESP32
AudioReactive->>I2SAdcSource: initialize(audioPin)
I2SAdcSource-->>AudioReactive: Ready for sampling
else DMA ADC Supported Target
AudioReactive->>DMAadcSource: initialize(audioPin)
DMAadcSource-->>AudioReactive: Ready for sampling
end
AudioReactive->>AudioReactive: Start FFT processing with selected source
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsusermods/audioreactive/audio_source.h (3)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
🔇 Additional comments (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
usermods/audioreactive/audio_reactive.h (4)
69-73: Consider simplifying the target check with a helper macro.The condition for enabling DMA ADC sampling could be more readable with a helper macro to avoid repeating the target checks throughout the codebase.
+// Helper macro for ESP32 variants that support DMA ADC +#define ESP32_VARIANT_WITH_DMA_ADC (defined(CONFIG_IDF_TARGET_ESP32S2) || defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32S3)) + // Analog mic with DMA ADC sampling is only supported on ESP32-S2, ESP32-C3 and ESP32-S3 on IDF >= 4.4.0 -#if !defined(CONFIG_IDF_TARGET_ESP32) && (ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 4, 0) && (defined(CONFIG_IDF_TARGET_ESP32S2) || defined(CONFIG_IDF_TARGET_ESP32C3) || defined(CONFIG_IDF_TARGET_ESP32S3))) +#if !defined(CONFIG_IDF_TARGET_ESP32) && (ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(4, 4, 0)) && ESP32_VARIANT_WITH_DMA_ADC #define AR_DMA_ADC_SAMPLING #endif
918-918: Update the comment to reflect the new type check.The comment above this line still references
Type_I2SAdc, but the code now checks for the more genericType_ADC.Update the comment on line 918 to mention
Type_ADCinstead ofType_I2SAdcto maintain consistency with the code change.
1244-1246: Reduce #ifdef complexity with a unified macro for analog support.The pattern
defined(CONFIG_IDF_TARGET_ESP32) || defined(AR_DMA_ADC_SAMPLING)is repeated throughout the file. Consider defining a helper macro to improve readability and maintainability.Add this near the top of the file after defining
AR_DMA_ADC_SAMPLING:+// Helper macro to check if analog microphone is supported +#define SUPPORTS_ANALOG_MIC (defined(CONFIG_IDF_TARGET_ESP32) || defined(AR_DMA_ADC_SAMPLING))Then replace all occurrences:
-#if defined(CONFIG_IDF_TARGET_ESP32) || defined(AR_DMA_ADC_SAMPLING) +#if SUPPORTS_ANALOG_MICAlso applies to: 2813-2816
69-73: Consider a more modular approach to handle platform-specific code.The numerous
#ifdefconditionals make the code difficult to follow, as you noted. Consider refactoring platform-specific code into separate initialization functions or even separate files:
- Create platform-specific initialization functions (e.g.,
initAnalogMicESP32(),initAnalogMicDMA())- Use a single conditional block to select the appropriate function
- This would reduce nesting and make the code flow clearer
Additionally, consider documenting the supported configurations in a table format in the code comments to help developers understand which features are available on which platforms.
Also applies to: 1972-2110
usermods/audioreactive/audio_source.h (1)
1177-1177: Consider making ADC attenuation configurable.The ADC attenuation is hardcoded to
ADC_ATTEN_DB_11(≈0-2.5V range). Different microphones or audio sources might require different voltage ranges for optimal signal quality.Consider adding an attenuation parameter to the constructor or making it configurable:
+ DMAadcSource(SRate_t sampleRate, int blockSize, float sampleScale = 1.0f, adc_atten_t attenuation = ADC_ATTEN_DB_11) : AudioSource(sampleRate, blockSize, sampleScale, false) { // ... adcpattern = { - .atten = ADC_ATTEN_DB_11, // approx. 0-2.5V input range + .atten = attenuation, // configurable input range .channel = 0, // ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
usermods/audioreactive/audio_reactive.h(11 hunks)usermods/audioreactive/audio_source.h(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: DedeHai
PR: MoonModules/WLED-MM#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.
usermods/audioreactive/audio_reactive.h (3)
undefined
<retrieved_learning>
Learnt from: DedeHai
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: DedeHai
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: netmindz
PR: #248
File: platformio.ini:1613-1613
Timestamp: 2025-07-02T23:22:57.175Z
Learning: In WLED platformio.ini, the particle system disable flags (WLED_DISABLE_PARTICLESYSTEM1D and WLED_DISABLE_PARTICLESYSTEM2D) are intentionally varied across different build environments based on platform memory constraints. More memory-limited platforms (like ESP8266, ESP32-C3, ESP32-S2) disable both 1D and 2D particle systems, while platforms with more available memory (like esp32_4MB_V4_S with 4MB flash) may only disable the 1D version to preserve flash space while keeping 2D functionality available.
</retrieved_learning>
usermods/audioreactive/audio_source.h (1)
Learnt from: DedeHai
PR: MoonModules/WLED-MM#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.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Builds (esp32c3dev_4MB_M)
- GitHub Check: Builds (esp8266_2MB_S)
- GitHub Check: Builds (seeed_esp32c3_4MB_S)
- GitHub Check: Builds (esp8266_4MB_S)
- GitHub Check: Builds (esp32c3mini_dio_4MB_M)
- GitHub Check: Builds (esp32S3_16MB_PSRAM_M_HUB75)
- GitHub Check: Builds (esp32S3_WROOM-2_M)
- GitHub Check: Builds (esp32s2_PSRAM_S)
- GitHub Check: Builds (esp32S3_8MB_PSRAM_M)
- GitHub Check: Builds (esp32s2_PSRAM_M)
- GitHub Check: Builds (esp32S3_4MB_PSRAM_M)
- GitHub Check: Builds (esp32S3_4MB_PSRAM_S)
- GitHub Check: Builds (esp32S3_8MB_M)
- GitHub Check: Builds (esp32S3_4MB_S)
- GitHub Check: Builds (esp32S3_8MB_S)
- GitHub Check: Builds (esp32_16MB_V4_M)
- GitHub Check: Builds (esp32_16MB_V4_S)
- GitHub Check: Builds (esp32_4MB_V4_S)
- GitHub Check: Builds (esp32_4MB_PSRAM_S)
- GitHub Check: Builds (esp32_4MB_V4_HUB75_forum)
🔇 Additional comments (5)
usermods/audioreactive/audio_reactive.h (2)
2099-2107: Verify the intentional difference in buffer sizes between ADC sources.The code uses different buffer sizes for different ADC sources:
I2SAdcSourceusesBLOCK_SIZE(line 2101)DMAadcSourceusessamplesFFT(line 2106)Is this difference intentional? If so, consider adding a comment explaining why different buffer sizes are needed.
2105-2107: DMAadcSource implementation confirmed and Type_ADC mapping is correct.
- usermods/audioreactive/audio_source.h defines
class DMAadcSourceinheriting fromAudioSource.- The
AudioSourceTypeenum includesType_ADC.- Both
I2SAdcSource::getType()andDMAadcSource::getType()returnType_ADC.- No lingering
Type_I2SAdcreferences remain.No changes needed.
usermods/audioreactive/audio_source.h (3)
167-167: LGTM!The rename from
Type_I2SAdctoType_ADCis appropriate as it's more generic and covers both I2S ADC and DMA ADC implementations.
986-986: LGTM!The type update is consistent with the enum rename.
1238-1254: DMA buffer flush via reset_DMA_ADC is in place and functioningThe
reset_DMA_ADC()routine (usermods/audioreactive/audio_source.h lines 1303–1308) stops, deinitializes, reinitializes and restarts the ADC DMA—effectively flushing the buffer after an overflow. No change is strictly required to “flush the DMA buffer completely.”• reset_DMA_ADC implementation:
• stops DMA (adc_digi_stop())
• deinitializes (adc_digi_deinitialize())
• reinitializes (init_adc_continuous())
• restarts sampling (adc_digi_start())Optional enhancements (if you see remaining glitches):
- Add a simple frame-sync marker or phase counter in the sample stream to detect mis-alignment
- Instrument a counter or debug log for each
ESP_ERR_INVALID_STATEoccurrence to monitor frequency of buffer overruns
|
not sure why the seeed env on C3 fails, it is not failing locally. maybe IDF version check for ADC support needs to be increased to 4.4.4. |
|
Hi, |
|
@drlovedotcom you are mistaken. |
|
the gif button is not merged yet #240 |
3 participants
please thouroughly check the
#ifdefs, there are just too many variants... I tested with modfied env on S2 only.The driver itself is tested and working on C3, S2 and S3 (with AC code, see wled#4761)
The platformio.ini is a bit messy and inconsistent for S2 and C3, IMHO its in desperate need of a cleanup for these.
Summary by CodeRabbit
New Features
Improvements