Added audioio to espressif #9995
Conversation
| @@ -10,3 +10,5 @@ CIRCUITPY_ESP_FLASH_SIZE = 8MB | |||
| CIRCUITPY_ESP_PSRAM_SIZE = 2MB | |||
| CIRCUITPY_ESP_PSRAM_MODE = qio | |||
| CIRCUITPY_ESP_PSRAM_FREQ = 40m | |||
|
|
|||
| CIRCUITPY_AUDIOIO = 1 | |||
There was a problem hiding this comment.
we'll probably want to default enable this on any supported mcu, but that's fine to do as a subsequent PR.
There was a problem hiding this comment.
i couldn't figure out where to make that change. could you point me in the right direction?
There was a problem hiding this comment.
in ports/espressif/mpconfigport.h there's a section for each different IDF_TARGET. So the needed change might be
diff --git a/ports/espressif/mpconfigport.mk b/ports/espressif/mpconfigport.mk
index 06f8c87c57..310558b867 100644
--- a/ports/espressif/mpconfigport.mk
+++ b/ports/espressif/mpconfigport.mk
@@ -85,6 +85,7 @@ CIRCUITPY__EVE ?= 1
ifeq ($(IDF_TARGET),esp32)
# Modules
CIRCUITPY_ALARM_TOUCH = 1
+CIRCUITPY_AUDIOIO = 1
CIRCUITPY_RGBMATRIX = 0
# SDMMC not supported yet
@@ -244,6 +245,7 @@ CIRCUITPY_ESPCAMERA = 0
else ifeq ($(IDF_TARGET),esp32s2)
# Modules
CIRCUITPY_ALARM_TOUCH = 1
+CIRCUITPY_AUDIOIO = 1
# No BLE in hw
CIRCUITPY_BLEIO = 0
If you want to leave this PR as enabling the module on just a single board, that's fine too.
There was a problem hiding this comment.
I'm OK with enabling it for all of the esp32 and esp32s2 boards. Hopefully it will get more use/testing/fixes that way. Oh that reminds me, these patches are on top of the 9.2.1 tag because I was unable to get a build from main (without my patches) to work on my board.
There was a problem hiding this comment.
If you are testing with the tip of main, make sure all the submodules are up to date including esp-idf. Is this a custom board or something we could try building. Does a tip-of-main build work for your board without your commits (e.g., just a clean clone of adafruit/circuitpython checked out to main). If that works try your fork brought up to main as well.
There was a problem hiding this comment.
A clean clone did not work when I tried it a few weeks ago, but I'll give it another go. The board I have is the stock feather esp32 v2.
There was a problem hiding this comment.
As a marker: I just did a clean build for ESP32 V2 with the current tip of main, erased the flash, and loaded it, and it worked.
There was a problem hiding this comment.
It also works for me now. Would you rather me rebase this branch on top of main or do a merge?
| self->channel_mode = DAC_CHANNEL_MODE_ALTER; | ||
| } else if ((left_channel_pin == &pin_CHANNEL_0 || | ||
| left_channel_pin == &pin_CHANNEL_1) && | ||
| right_channel_pin == left_channel_pin) { |
There was a problem hiding this comment.
can this case actually happen? I'm surprised, but maybe it can.
There was a problem hiding this comment.
are you asking if someone can someone pass the same pin to the left and right channel arguments? if so, the answer is yes.
There was a problem hiding this comment.
if you are asking if it makes sense to support the dual mono mode by supplying the same pin (or at all) i'm not entirely sure. i couldn't think of a better way to support it and i'm not even sure how useful this mode would be. i know it isn't exactly intuitive.
There was a problem hiding this comment.
@tannewt is passing the same pin as left & right channel supposed to be allowed? or is this something we just forgot to check in shared-bindings?
There was a problem hiding this comment.
it doesn't appear to be checked because i tested it and it worked 🤷
Added audioio support for the one esp32 board that I have (adafruit_feather_esp32_v2) but I believe it will work with other espressif boards. This is my first contribution to CircuitPython so I'm sure there will be lots of little things that I did wrong. Resolves #3898.