Extend support for ESP32 targets#1187
Conversation
|
Thanks for working on this @pera! I appreciate the thought behind making the code more flexible. Logic issue: The pin_config parameter is being overwritten inside the constructor by the #if blocks, so custom configurations passed in won't actually be used when board defines are set. Missing default: The original #else case with working default pins (bck: 33, ws: 25, data_out: 26, data_in: 27) was removed. Now DEFAULT_I2S_PIN_CONFIG is all I2S_PIN_NO_CHANGE, which won't work for boards without a specific define. Design concern: I'm not sure this flexibility is needed in practice. Pin configuration is really a compile-time concern - you set it once for your hardware and it never changes. You're not swapping codecs at runtime. The current pattern with board defines makes it clear what pins are being used and works well for the use case. |
|
Hi @thopman thanks very much for reviewing and your comments. Given that there are a few things happening in this PR I will list them here:
Maybe a better option for point 2 could be to make GPIO pins configurable through Kconfig and sdkconfig. What do you think? My intention with this PR is to make it as easy as possible for people wanting to use an esp32-s3 together with pcm5100 or pcm5102 breakout boards, DACs that don't require codec initialisation code. I think this combination is very common because you can buy them for about £10 on many shops. |
|
Thanks for clarifying @pera. Why runtime configuration doesn't work: Why Kconfig/sdkconfig isn't better: The real problem you're solving is "I have an ESP32-S3 with a PCM5102 and the current defaults don't work for me." That's legitimate, but it's better solved with: Either editing the default in your local faust install and use that to compile your faust dsp. Something like: My suggestion: |
|
Thanks very much for your thoughts @thopman, I have created a new PR for the Regarding the PIN defaults:
|
|
Hi everyone, I've been working on a project using Faust with a LilyGO T-Display-S3 and a PCM5102A DAC and ran into the same compilation issue described here. I already use this kind of pin mapping in other projects with this board, like this example I put together: https://github.com/Xinyuan-LilyGO/T-Display-S3/tree/main/examples/T-Display-S3-Music-Explorer While getting it to work I ended up extracting the pin configuration into a separate header ( The change to The code is working and tested here. I'm available to open a PR if you think this direction makes sense — wanted to check first before going ahead. |
This PR extends the current support for other ESP32 targets (e.g. ESP32-S3), allows for custom I2S pin configurations and also simplifies integration with DACs that don't require a codec driver (e.g. PCM5100).