Audio System + Drivers (#562)

Platforms/platform-esp32/CMakeLists.txt

@@ -9,4 +9,12 @@ idf_component_register(
     REQUIRES TactilityKernel driver esp_adc esp_driver_i2c vfs fatfs esp_wifi esp_netif esp_event
 )
 
+if (DEFINED ENV{ESP_IDF_VERSION})
+    GET_PROPERTY_FILE_CONTENT("${CMAKE_CURRENT_LIST_DIR}/../../sdkconfig" sdkconfig_text)
+    GET_PROPERTY_VALUE(sdkconfig_text "CONFIG_ESP_HOSTED_ENABLED" esp_host_enabled "n")
+    if (esp_host_enabled STREQUAL "y")
+        idf_component_optional_requires(PRIVATE espressif__esp_hosted)
+    endif ()
+endif ()
+
 idf_component_optional_requires(PRIVATE bt usb espressif__usb_host_hid espressif__usb_host_msc)

Platforms/platform-esp32/bindings/espressif,esp32-i2s.yaml

@@ -13,12 +13,12 @@ properties:
       Depending on the hardware, these values are available: I2S_NUM_0, I2S_NUM_1
   pin-bclk:
     type: phandles
-    required: true
-    description: Bit clock pin
+    default: GPIO_PIN_SPEC_NONE
+    description: Bit clock pin. Not used in PDM RX mode (e.g. a PDM microphone), where pin-ws carries the PDM clock instead.
   pin-ws:
     type: phandles
     required: true
-    description: Word (slot) select pin
+    description: Word (slot) select pin. Doubles as the PDM clock pin in PDM RX mode.
   pin-data-out:
     type: phandles
     default: GPIO_PIN_SPEC_NONE

Platforms/platform-esp32/private/bluetooth/esp32_ble_internal.h

@@ -68,6 +68,26 @@ struct BleCtx {
     // One-shot timer used to dispatch dispatchDisable off the NimBLE host task.
     // nimble_port_stop() must not be called from the NimBLE host task itself.
     esp_timer_handle_t disable_timer;
+    // Given by host_task() right after nimble_port_run() returns (i.e. the host
+    // task has fully drained g_eventq_dflt and will process no further NimBLE
+    // events). dispatch_disable() must wait on this before nimble_port_deinit(),
+    // otherwise a reset event still executing on the host task can dereference
+    // timer/callout state that deinit just freed. See on_reset()/dispatch_disable().
+    SemaphoreHandle_t host_task_done_sem;
+    // Posted to NimBLE's own default event queue (nimble_port_get_dflt_eventq())
+    // from on_reset() instead of kicking disable_timer directly. ble_hs_reset()
+    // calls reset_cb (on_reset) synchronously, then unconditionally falls through
+    // to ble_hs_sync() -> ble_hs_timer_sched(), which touches the global static
+    // ble_hs_timer with no locking. Firing disable_timer (and thus
+    // nimble_port_stop() -> ble_hs_stop() -> ble_npl_callout_deinit(&ble_hs_timer))
+    // from on_reset() races that in-flight ble_hs_sync() call on the SAME host
+    // task from a DIFFERENT task (esp_timer task), nulling out ble_hs_timer's
+    // callout mid-use -> null deref crash in ble_npl_callout_is_active(). Posting
+    // this event instead defers the giveup until the host task's event loop is
+    // idle again (FIFO-after the in-flight reset), so by the time disable_timer
+    // fires, ble_hs_sync() for this reset has already returned and there is no
+    // concurrent user of ble_hs_timer left on the host task.
+    struct ble_npl_event giveup_event;
 
     // BLE device name (set before or after radio enable; applied in dispatch_enable)
     char device_name[BLE_DEVICE_NAME_MAX + 1];

Platforms/platform-esp32/source/drivers/bluetooth/esp32_ble.cpp

@@ -29,6 +29,16 @@ constexpr auto* TAG = "esp32_ble";
 #include <tactility/log.h>
 #include <esp_timer.h>
 
+#if defined(CONFIG_ESP_HOSTED_ENABLED)
+#include <esp_hosted.h>
+// esp_hosted_misc.h lacks its own extern "C" guard, so its declarations get C++
+// name-mangled when included from a .cpp file, causing "undefined reference" at
+// link time against the library's plain-C symbols.
+extern "C" {
+#include <esp_hosted_misc.h>
+}
+#endif
+
 // ble_store_config_init() is not declared in the public header in some IDF versions.
 extern "C" void ble_store_config_init(void);
 
@@ -446,7 +456,24 @@ static void on_sync() {
 
 static void dispatch_disable_timer_cb(void* arg) {
     BleCtx* ctx = (BleCtx*)arg;
+    // Matches api_set_radio_enabled()'s locking so the give-up teardown can't run
+    // concurrently with a user-initiated dispatch_enable()/dispatch_disable() on
+    // the same ctx (both would otherwise race nimble_port_stop()/deinit()).
+    xSemaphoreTake(ctx->radio_mutex, portMAX_DELAY);
     dispatch_disable(ctx);
+    xSemaphoreGive(ctx->radio_mutex);
+}
+
+// Runs on the NimBLE host task, but only once the event loop is idle again —
+// i.e. strictly after the ble_hs_reset()/ble_hs_sync() call that triggered the
+// giveup has fully returned (this event was queued behind it on g_eventq_dflt).
+// Safe to kick disable_timer here: no in-flight NimBLE call is touching
+// ble_hs_timer on this task anymore. See giveup_event's comment in BleCtx.
+static void giveup_event_cb(struct ble_npl_event* ev) {
+    BleCtx* ctx = (BleCtx*)ble_npl_event_get_arg(ev);
+    if (ctx != nullptr && ctx->disable_timer != nullptr) {
+        esp_timer_start_once(ctx->disable_timer, 0);
+    }
 }
 
 static void on_reset(int reason) {
@@ -458,11 +485,12 @@ static void on_reset(int reason) {
         int count = ctx->pending_reset_count.fetch_add(1) + 1;
         if (count == 3) {
             LOG_E(TAG, "BT controller unresponsive after 3 resets — giving up");
-            // Can't call nimble_port_stop() from the NimBLE host task.
-            // Fire a one-shot esp_timer (delay=0 → fires on esp_timer task immediately).
-            if (ctx->disable_timer != nullptr) {
-                esp_timer_start_once(ctx->disable_timer, 0);
-            }
+            // Do NOT fire disable_timer directly from here: ble_hs_reset() (our
+            // caller) unconditionally falls through to ble_hs_sync() right after
+            // reset_cb returns, which touches the unlocked global ble_hs_timer.
+            // Queue the giveup behind that on NimBLE's own event queue instead —
+            // see giveup_event_cb().
+            ble_npl_eventq_put(nimble_port_get_dflt_eventq(), &ctx->giveup_event);
         }
     }
 }
@@ -470,6 +498,16 @@ static void on_reset(int reason) {
 static void host_task(void* param) {
     LOG_I(TAG, "BLE host task started");
     nimble_port_run();
+    // Signal that this task will not process any further NimBLE events (g_eventq_dflt
+    // is fully drained past the stop sentinel) before dispatch_disable() is allowed to
+    // proceed to nimble_port_deinit(). nimble_port_stop()'s own semaphore only confirms
+    // the stop event was serviced, not that the host task has stopped dequeuing events —
+    // a fresh HCI-ack-wait timeout can still queue and run ble_hs_ev_reset in that gap,
+    // racing against deinit freeing ble_hs_timer's callout (crash: ble_hs_timer_sched ->
+    // ble_npl_callout_is_active on a freed/zeroed callout).
+    if (s_ctx != nullptr && s_ctx->host_task_done_sem != nullptr) {
+        xSemaphoreGive(s_ctx->host_task_done_sem);
+    }
     // nimble_port_deinit() is called by dispatch_disable() after nimble_port_stop() returns.
     nimble_port_freertos_deinit();
 }
@@ -637,6 +675,31 @@ static void dispatch_enable(BleCtx* ctx) {
         ble_publish_event(ctx->device, e);
     }
 
+#if defined(CONFIG_ESP_HOSTED_ENABLED)
+    // Over esp_hosted (SDIO to a co-processor, e.g. C6), the slave's on-chip BT
+    // controller is never auto-started at slave boot — it only comes up in
+    // response to these RPCs. Without them, HCI Reset (the first command NimBLE
+    // ever sends) is handed to esp_vhci_host_send_packet() on the slave with no
+    // controller underneath to consume it, so it's silently dropped and NimBLE
+    // times out (BLE_HS_ETIMEOUT_HCI) forever. Must run before nimble_port_init().
+    // esp_hosted_bt_controller_init/enable() both bail out immediately (no retry,
+    // no blocking) if the SDIO/SPI transport to the co-processor isn't marked up
+    // yet. On boot, BT can auto-enable before Wi-Fi has driven that bring-up, so
+    // explicitly (re)connect here and let it block until the slave handshake
+    // (the "Attempt connection with slave" / "Card init success" sequence)
+    // completes — esp_hosted_connect_to_slave() is a thin wrapper that's safe to
+    // call again if the transport is already up.
+    if (esp_hosted_connect_to_slave() != ESP_OK) {
+        LOG_W(TAG, "esp_hosted_connect_to_slave failed");
+    }
+    if (esp_hosted_bt_controller_init() != ESP_OK) {
+        LOG_W(TAG, "esp_hosted_bt_controller_init failed");
+    }
+    if (esp_hosted_bt_controller_enable() != ESP_OK) {
+        LOG_W(TAG, "esp_hosted_bt_controller_enable failed");
+    }
+#endif
+
     int rc = nimble_port_init();
     if (rc != 0) {
         LOG_E(TAG, "nimble_port_init failed (rc=%d)", rc);
@@ -648,6 +711,10 @@ static void dispatch_enable(BleCtx* ctx) {
         return;
     }
 
+    // npl_funcs only becomes valid after nimble_port_init() above; must (re-)init
+    // the giveup event each enable cycle, not once at device-start time.
+    ble_npl_event_init(&ctx->giveup_event, giveup_event_cb, ctx);
+
     ble_hs_cfg.sync_cb         = on_sync;
     ble_hs_cfg.reset_cb        = on_reset;
     ble_hs_cfg.store_status_cb = ble_store_util_status_rr;
@@ -703,6 +770,13 @@ static void dispatch_enable(BleCtx* ctx) {
 #if defined(CONFIG_ESP_HOSTED_ENABLED)
     ble_hci_gate_set_active(true);  // Open gate: NimBLE transport pool is ready
 #endif
+    // Drain any stale "done" signal left over from a previous cycle (e.g. if the
+    // last dispatch_disable() timed out waiting on it — see xSemaphoreTake below
+    // with pdMS_TO_TICKS(2000)) so dispatch_disable() only ever consumes the
+    // completion signal for THIS host task instance.
+    if (ctx->host_task_done_sem != nullptr) {
+        xSemaphoreTake(ctx->host_task_done_sem, 0);
+    }
     // Start NimBLE host task (on_sync will fire when ready)
     nimble_port_freertos_init(host_task);
 }
@@ -731,6 +805,18 @@ static void dispatch_disable(BleCtx* ctx) {
     // controller. Closing the gate before this point starves NimBLE of those events,
     // causing HCI timeouts and a cascade of resets that crash in ble_hs_timer_sched.
     nimble_port_stop();
+    // nimble_port_stop()'s internal semaphore only confirms the stop sentinel event was
+    // serviced — not that host_task()'s call to nimble_port_run() has returned. A reset
+    // event (e.g. from an independent HCI-ack-wait timeout) can still be running on the
+    // host task in that gap. Wait for host_task() to explicitly signal completion before
+    // freeing anything NimBLE still references (ble_hs_timer et al in nimble_port_deinit()),
+    // otherwise ble_hs_timer_sched() on the host task can dereference a freed callout —
+    // see host_task() for the crash this fixes.
+    if (ctx->host_task_done_sem != nullptr) {
+        if (xSemaphoreTake(ctx->host_task_done_sem, pdMS_TO_TICKS(2000)) != pdTRUE) {
+            LOG_W(TAG, "host task did not signal completion in time");
+        }
+    }
 #if defined(CONFIG_ESP_HOSTED_ENABLED)
     // Close the gate NOW — after nimble_port_stop() returns the NimBLE host task has
     // exited and nimble_port_deinit() is about to zero npl_funcs. Any HCI packet
@@ -742,6 +828,16 @@ static void dispatch_disable(BleCtx* ctx) {
 #endif
     nimble_port_deinit();
 
+#if defined(CONFIG_ESP_HOSTED_ENABLED)
+    // Symmetric with the enable-side esp_hosted_bt_controller_init/enable() calls.
+    if (esp_hosted_bt_controller_disable() != ESP_OK) {
+        LOG_W(TAG, "esp_hosted_bt_controller_disable failed");
+    }
+    if (esp_hosted_bt_controller_deinit(true) != ESP_OK) {
+        LOG_W(TAG, "esp_hosted_bt_controller_deinit failed");
+    }
+#endif
+
     ctx->spp_conn_handle.store(BLE_HS_CONN_HANDLE_NONE);
     ctx->spp_active.store(false);
     ctx->midi_conn_handle.store(BLE_HS_CONN_HANDLE_NONE);
@@ -1048,6 +1144,10 @@ static error_t esp32_ble_start_device(struct Device* device) {
     ctx->scan_mutex           = xSemaphoreCreateMutex();
     ctx->scan_count           = 0;
     memset(ctx->scan_results, 0, sizeof(ctx->scan_results));
+    ctx->host_task_done_sem   = xSemaphoreCreateBinary();
+    if (ctx->host_task_done_sem == nullptr) {
+        LOG_E(TAG, "start_device: host_task_done_sem create failed");
+    }
 
     // Create the disable timer used to dispatch dispatchDisable off the NimBLE host task.
     esp_timer_create_args_t disable_args = {};
@@ -1085,7 +1185,9 @@ static error_t esp32_ble_stop_device(struct Device* device) {
     destroy_child_device(ctx->serial_child);
 
     if (ctx->radio_state.load() != BT_RADIO_STATE_OFF) {
+        xSemaphoreTake(ctx->radio_mutex, portMAX_DELAY);
         dispatch_disable(ctx);
+        xSemaphoreGive(ctx->radio_mutex);
     }
 
     if (ctx->scan_mutex != nullptr) {
@@ -1093,6 +1195,11 @@ static error_t esp32_ble_stop_device(struct Device* device) {
         ctx->scan_mutex = nullptr;
     }
 
+    if (ctx->host_task_done_sem != nullptr) {
+        vSemaphoreDelete(ctx->host_task_done_sem);
+        ctx->host_task_done_sem = nullptr;
+    }
+
     if (ctx->disable_timer != nullptr) {
         esp_timer_stop(ctx->disable_timer);
         esp_timer_delete(ctx->disable_timer);

Platforms/platform-esp32/source/drivers/esp32_i2s.cpp

@@ -5,6 +5,9 @@
 #ifdef SOC_I2S_SUPPORTS_TDM
 #include <driver/i2s_tdm.h>
 #endif
+#ifdef SOC_I2S_SUPPORTS_PDM_RX
+#include <driver/i2s_pdm.h>
+#endif
 
 #include <tactility/driver.h>
 #include <tactility/drivers/gpio_controller.h>
@@ -191,6 +194,16 @@ static error_t set_config(Device* device, const struct I2sConfig* config) {
 
     auto* internal = GET_DATA(device);
     auto* dts_config = GET_CONFIG(device);
+
+    // Standard mode always drives BCLK -- unlike PDM RX (which uses pin-ws as its clock
+    // line), there's no mode where BCLK is legitimately absent here. pin-bclk is optional
+    // in the devicetree binding only to support PDM-only controllers; reject a missing
+    // pin here rather than silently building an std_cfg with BCLK=-1.
+    if (internal->bclk_descriptor == nullptr) {
+        LOG_E(TAG, "Standard I2S mode requires pin-bclk");
+        return ERROR_INVALID_ARGUMENT;
+    }
+
     lock(internal);
 
     cleanup_channel_handles(internal);
@@ -290,6 +303,29 @@ static error_t reset(Device* device) {
     return ERROR_NONE;
 }
 
+static error_t disable_direction(Device* device, bool is_input) {
+    if (xPortInIsrContext()) return ERROR_ISR_STATUS;
+
+    LOG_I(TAG, "disable_direction %s (%s)", device->name, is_input ? "rx" : "tx");
+    auto* driver_data = GET_DATA(device);
+    lock(driver_data);
+    if (is_input) {
+        if (driver_data->rx_handle) {
+            i2s_channel_disable(driver_data->rx_handle);
+            i2s_del_channel(driver_data->rx_handle);
+            driver_data->rx_handle = nullptr;
+        }
+    } else {
+        if (driver_data->tx_handle) {
+            i2s_channel_disable(driver_data->tx_handle);
+            i2s_del_channel(driver_data->tx_handle);
+            driver_data->tx_handle = nullptr;
+        }
+    }
+    unlock(driver_data);
+    return ERROR_NONE;
+}
+
 #ifdef SOC_I2S_SUPPORTS_TDM
 static error_t set_rx_tdm_config(Device* device, const struct I2sTdmRxConfig* config) {
     if (xPortInIsrContext()) return ERROR_ISR_STATUS;
@@ -330,6 +366,14 @@ static error_t set_rx_tdm_config(Device* device, const struct I2sTdmRxConfig* co
 
     auto* internal = GET_DATA(device);
     auto* dts_config = GET_CONFIG(device);
+
+    // TDM, like standard mode, always drives BCLK -- reject rather than build a tdm_cfg
+    // with BCLK=-1 (see set_config() for the matching check/rationale).
+    if (internal->bclk_descriptor == nullptr) {
+        LOG_E(TAG, "TDM mode requires pin-bclk");
+        return ERROR_INVALID_ARGUMENT;
+    }
+
     lock(internal);
 
     // Tear down only the RX channel; TX stays in standard mode for playback.
@@ -414,6 +458,73 @@ static error_t set_rx_tdm_config(Device* device, const struct I2sTdmRxConfig* co
 }
 #endif // SOC_I2S_SUPPORTS_TDM
 
+#ifdef SOC_I2S_SUPPORTS_PDM_RX
+static error_t set_rx_pdm_config(Device* device, const struct I2sPdmRxConfig* config) {
+    if (xPortInIsrContext()) return ERROR_ISR_STATUS;
+
+    if (config->sample_rate_hz == 0) {
+        return ERROR_INVALID_ARGUMENT;
+    }
+
+    auto* internal = GET_DATA(device);
+    auto* dts_config = GET_CONFIG(device);
+
+    // PDM RX is hardware-restricted to I2S controller 0 on ESP32 targets.
+    if (dts_config->port != I2S_NUM_0) {
+        return ERROR_NOT_SUPPORTED;
+    }
+
+    lock(internal);
+
+    // Tear down only the RX channel; TX (if any) stays in its own mode for playback.
+    if (internal->rx_handle) {
+        i2s_channel_disable(internal->rx_handle);
+        i2s_del_channel(internal->rx_handle);
+        internal->rx_handle = nullptr;
+    }
+
+    i2s_chan_config_t chan_cfg = I2S_CHANNEL_DEFAULT_CONFIG(dts_config->port, I2S_ROLE_MASTER);
+    i2s_chan_handle_t new_rx = nullptr;
+    esp_err_t esp_error = i2s_new_channel(&chan_cfg, nullptr, &new_rx);
+    if (esp_error != ESP_OK) {
+        LOG_E(TAG, "PDM: failed to create RX channel: %s", esp_err_to_name(esp_error));
+        unlock(internal);
+        return ERROR_RESOURCE;
+    }
+
+    gpio_num_t clk_pin    = get_native_pin(internal->ws_descriptor);
+    gpio_num_t data_in_pin = get_native_pin(internal->data_in_descriptor);
+
+    i2s_slot_mode_t slot_mode = config->stereo ? I2S_SLOT_MODE_STEREO : I2S_SLOT_MODE_MONO;
+
+    i2s_pdm_rx_config_t pdm_cfg = {
+        .clk_cfg = I2S_PDM_RX_CLK_DEFAULT_CONFIG(config->sample_rate_hz),
+        .slot_cfg = I2S_PDM_RX_SLOT_DEFAULT_CONFIG(I2S_DATA_BIT_WIDTH_16BIT, slot_mode),
+        .gpio_cfg = {
+            .clk = clk_pin,
+            .din = data_in_pin,
+            .invert_flags = {
+                .clk_inv = is_pin_inverted(internal->ws_descriptor),
+            },
+        },
+    };
+
+    esp_error = i2s_channel_init_pdm_rx_mode(new_rx, &pdm_cfg);
+    if (esp_error == ESP_OK) esp_error = i2s_channel_enable(new_rx);
+
+    if (esp_error != ESP_OK) {
+        LOG_E(TAG, "PDM: failed to init/enable RX channel: %s", esp_err_to_name(esp_error));
+        i2s_del_channel(new_rx);
+        unlock(internal);
+        return esp_err_to_error(esp_error);
+    }
+
+    internal->rx_handle = new_rx;
+    unlock(internal);
+    return ERROR_NONE;
+}
+#endif // SOC_I2S_SUPPORTS_PDM_RX
+
 const static I2sControllerApi esp32_i2s_api = {
     .read = read,
     .write = write,
@@ -425,6 +536,12 @@ const static I2sControllerApi esp32_i2s_api = {
 #else
     .set_rx_tdm_config = nullptr,
 #endif
+#ifdef SOC_I2S_SUPPORTS_PDM_RX
+    .set_rx_pdm_config = set_rx_pdm_config,
+#else
+    .set_rx_pdm_config = nullptr,
+#endif
+    .disable_direction = disable_direction,
 };
 
 extern struct Module platform_esp32_module;