esp32/machine_i2s: Integrate new I2S IDF driver. #13727
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #13727 +/- ##
=======================================
Coverage 98.39% 98.39%
=======================================
Files 161 161
Lines 21078 21078
=======================================
Hits 20739 20739
Misses 339 339 ☔ View full report in Codecov by Sentry. |
|
Code size report: |
|
Thanks - Just tried this with SEEED XIAO ESP32-C3 - and it works. I ran your easy_wav_player using your mono and stereo wav files pulled from the local memory (so only 16-bit versions). I used pcm5102 for I2S to audio out. |
|
@ricksorensen Fantastic ! Thank you for testing this PR. |
extmod/machine_i2s.c
Outdated
| @@ -600,6 +600,7 @@ STATIC mp_uint_t machine_i2s_stream_write(mp_obj_t self_in, const void *buf_in, | |||
|
|
|||
| return size; | |||
| } else { // blocking or asyncio mode | |||
|
|
|||
There was a problem hiding this comment.
I think this blank line can be removed, to keep coding style consistent (eg with the "if" part of this statement).
extmod/machine_i2s.c
Outdated
| @@ -623,6 +625,7 @@ STATIC mp_uint_t machine_i2s_ioctl(mp_obj_t self_in, mp_uint_t request, uintptr_ | |||
|
|
|||
| if (flags & MP_STREAM_POLL_RD) { | |||
| if (self->mode != MICROPY_PY_MACHINE_I2S_CONSTANT_RX) { | |||
|
|
|||
extmod/machine_i2s.c
Outdated
| } | ||
| #endif | ||
| } | ||
|
|
||
| if (flags & MP_STREAM_POLL_WR) { | ||
|
|
extmod/machine_i2s.c
Outdated
| } | ||
| #endif | ||
| } | ||
| } else { | ||
|
|
extmod/machine_i2s.c
Outdated
| // indicating that audio samples can be written to the I2S object | ||
| ret |= MP_STREAM_POLL_WR; | ||
| } | ||
|
|
ports/esp32/machine_i2s.c
Outdated
| .on_send_q_ovf = NULL, | ||
| }; | ||
|
|
||
|
|
There was a problem hiding this comment.
only one blank line needed here (for consistency with the rest of this file)
ports/esp32/machine_i2s.c
Outdated
| } | ||
|
|
||
|
|
|
Thanks for this, it looks good! I think the line |
|
@dpgeorge thanks for reviewing this PR. I removed all the unnecessary blank lines. The macro could be removed after I included the header file for the new driver. |
|
This is an automated heads-up that we've just merged a Pull Request See #13763 A search suggests this PR might apply the STATIC macro to some C code. If it Although this is an automated message, feel free to @-reply to me directly if |
|
I'm currently rebasing this on latest master and about to merge it. |
The legacy I2S "shim" is removed and replaced by the new I2S driver. The new driver fixes a bug where mono audio plays only in one channel. Application code size is reduced by 2672 bytes with this change. Tested on ESP32, ESP32+spiram, ESP32-S3 using example code from https://github.com/miketeachman/micropython-i2s-examples Signed-off-by: Mike Teachman <mike.teachman@gmail.com>
7ed5b34 to
0b145fd
Compare
The legacy I2S "shim" is removed and replaced by the new I2S driver. The new driver fixes a bug where mono audio plays only in one channel.
Application code size is reduced by 2672 bytes with this change. Tested on ESP32, ESP32+spiram, ESP32-S3 using example code from https://github.com/miketeachman/micropython-i2s-examples