From e56dea1ccdecf328a590deefaf9e7c0fc83526ce Mon Sep 17 00:00:00 2001 From: ainyan03 <205502311+ainyan03@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:38:59 +0000 Subject: [PATCH 1/3] Mic_Class: add a post-start callback and serialize begin()/end() Some codecs accept part of their setup only while the I2S bus clock is running (ES8311), but the enable callback runs before the capture task starts the clock. Add an optional post-start callback that begin() invokes in the caller's context once the capture task has enabled the I2S channel, waiting up to one second for the clock. A missing clock or a false return from the callback tears the port back down and fails begin(). Publishing the codec state safely also requires the lifecycle to be serialized: end() now takes _begin_lock (internal callers use the new _end_locked()), and the already-running early return in begin() moved under the lock, so a concurrent end() can no longer tear the port down while begin() is still bringing it up, or right after its early check. The capture task publishes the clock state (_i2s_active) only when the I2S start actually succeeds, and a failed enable callback or setup now runs the disable callback and driver uninstall before begin() reports failure, instead of leaving a half-built port behind. --- src/utility/Mic_Class.cpp | 65 ++++++++++++++++++++++++++++++++------- src/utility/Mic_Class.hpp | 26 ++++++++++++++-- 2 files changed, 78 insertions(+), 13 deletions(-) diff --git a/src/utility/Mic_Class.cpp b/src/utility/Mic_Class.cpp index 05e7038..5c07abb 100644 --- a/src/utility/Mic_Class.cpp +++ b/src/utility/Mic_Class.cpp @@ -591,7 +591,10 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) { #endif - _i2s_start(self->_cfg.i2s_port); + if (ESP_OK == _i2s_start(self->_cfg.i2s_port)) + { + self->_i2s_active.store(true, std::memory_order_release); + } int32_t gain = self->_cfg.magnification; const float f_gain = (float)gain / (oversampling << 1); @@ -764,6 +767,7 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) { } } _i2s_stop(self->_cfg.i2s_port); + self->_i2s_active.store(false, std::memory_order_release); self->_task_handle = nullptr; vTaskDelete(nullptr); @@ -771,20 +775,22 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) { bool Mic_Class::begin(void) { - // _rec_sample_rate was written before _begun was released, so the - // acquire load makes this pair of reads safe without the lock. - if (_begun.load(std::memory_order_acquire) && _rec_sample_rate == _calc_rec_rate()) { return true; } - // record() calls begin() lazily from whichever task gets there first, // and both the setup and the sample-rate change tear the port down: two // of these racing rip the live channel out from under the running task. // One caller goes through at a time; the others wait for its outcome. + // The already-running check also lives under the lock (vs. end()). bool zero = false; while (!_begin_lock.compare_exchange_strong(zero, true)) { zero = false; vTaskDelay(1); } + if (_begun.load(std::memory_order_acquire) && _rec_sample_rate == _calc_rec_rate()) + { + _begin_lock.store(false); + return true; + } bool res = true; if (_task_running) @@ -793,7 +799,7 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) { if (_rec_sample_rate != rate) { do { vTaskDelay(1); } while (isRecording()); - end(); + _end_locked(); _rec_sample_rate = rate; } } @@ -816,7 +822,14 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) { bool res = true; if (_cb_set_enabled) { res = _cb_set_enabled(_cb_set_enabled_args, true); } - res = (ESP_OK == _setup_i2s()) && res; + if (res) { res = (ESP_OK == _setup_i2s()); } + if (!res) + { // no task to tear down yet, but whatever the enable callback powered + // up still has to come back down + if (_cb_set_enabled) { _cb_set_enabled(_cb_set_enabled_args, false); } + _i2s_driver_uninstall(_cfg.i2s_port); + return false; + } if (res) { size_t stack_size = 2048 + (_cfg.dma_buf_len * sizeof(uint32_t)); @@ -831,16 +844,46 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) { { res = (pdPASS == xTaskCreate(mic_task, "mic_task", stack_size, this, _cfg.task_priority, &_task_handle)); } - // end() takes the driver and the callback back down; it still sees the - // class as running, which is what lets it do that. - if (!res) { end(); } - else { _begun.store(true, std::memory_order_release); } + // _end_locked() takes the driver and the callback back down; it still + // sees the class as running, which is what lets it do that. + if (!res) { _end_locked(); } + else + { + if (_cb_post_start) + { // the callback needs the bus clock running: wait for the task to + // enable the channel, and fail the begin when that never happens + const uint32_t start_tick = xTaskGetTickCount(); + while (!_i2s_active.load(std::memory_order_acquire) + && (uint32_t)(xTaskGetTickCount() - start_tick) < pdMS_TO_TICKS(1000)) { vTaskDelay(1); } + if (!_i2s_active.load(std::memory_order_acquire) + || !_cb_post_start(_cb_post_start_args)) + { + _end_locked(); + res = false; + } + } + if (res) { _begun.store(true, std::memory_order_release); } + } } return res; } void Mic_Class::end(void) + { + // taking _begin_lock keeps end() from tearing the port down while a + // begin() is still bringing it up + bool zero = false; + while (!_begin_lock.compare_exchange_strong(zero, true)) + { + zero = false; + vTaskDelay(1); + } + _end_locked(); + _begin_lock.store(false); + } + + void Mic_Class::_end_locked(void) { _begun.store(false, std::memory_order_release); if (!_task_running) { return; } diff --git a/src/utility/Mic_Class.hpp b/src/utility/Mic_Class.hpp index dde8d93..5fb7e3b 100644 --- a/src/utility/Mic_Class.hpp +++ b/src/utility/Mic_Class.hpp @@ -105,8 +105,13 @@ namespace m5 mic_config_t config(void) const { return _cfg; } void config(const mic_config_t& cfg) { _cfg = cfg; } + /// start the capture port. serialized with end(). + /// Success means the port runs and the codec is configured; some codecs + /// (ES8311) additionally warm up for about a second after power-up, + /// during which captured samples can be all zero. bool begin(void); + /// stop the capture port. serialized with begin(). void end(void); bool isRunning(void) const { return _task_running; } @@ -159,6 +164,8 @@ namespace m5 protected: + /// The callbacks run under the begin()/end() lock and must not call + /// begin() or end() themselves. void setCallback(void* args, bool(*func)(void*, bool)) { _cb_set_enabled = func; _cb_set_enabled_args = args; } struct recording_info_t @@ -200,8 +207,7 @@ namespace m5 /// begin() runs from whichever task records first, and setup starts by /// tearing the port down - so only one call may go through. std::atomic _begin_lock { false }; - /// True only once begin() has fully finished; the lock-free early return - /// keys on this, so a caller can never see a half-built port as ready. + /// true only once begin() has fully finished. std::atomic _begun { false }; #if defined (SDL_h_) SDL_Thread* _task_handle = nullptr; @@ -209,6 +215,22 @@ namespace m5 TaskHandle_t _task_handle = nullptr; volatile SemaphoreHandle_t _task_semaphore = nullptr; #endif + + private: + + /// set a callback that begin() invokes once the capture task has brought + /// the I2S clock up, for codecs that accept part of their setup only + /// while the bus clock runs (ES8311). A false return (or no clock within + /// one second) fails begin() and tears the port back down. + void setPostStartCallback(void* args, bool(*func)(void*)) { _cb_post_start = func; _cb_post_start_args = args; } + + void _end_locked(void); + + bool (*_cb_post_start)(void* args) = nullptr; + void* _cb_post_start_args = nullptr; + /// set by the task once the I2S channel is enabled; begin() waits on it + /// before invoking the post-start callback. + std::atomic _i2s_active { false }; }; } From 5215debad27bebf7d09f098196120223c21f5729 Mon Sep 17 00:00:00 2001 From: ainyan03 <205502311+ainyan03@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:39:15 +0000 Subject: [PATCH 2/3] StopWatch: recover ES8311 capture after power-on reset (fixes #347) After a power-on reset the ES8311 runs an internal sequence once the CSM is enabled and the bus clock arrives. A SYSTEM(0x0D) analog power-up write issued before the I2S clock starts is absorbed without effect even though the register reads back the written value, so a capture path initialized by the enable callback (which runs before the clock starts) records only zeros, forever. Measured on StopWatch hardware with both arduino-esp32 2.x and 3.x: - a 0x0D write arms the capture path only when issued while the bus clock is running (earliest observed working point ~25 ms after the clock start); pre-clock writes never arm it - once armed, real samples appear when the analog warm-up finishes, roughly a second after the codec powered up - a fixed physical time that neither blocking nor extra writes shorten - the arming is needed once per codec power-on reset; warm re-inits (including Mic end/begin cycles) keep it - cutting the codec power rail at speaker disable forces the next capture through another reset and warm-up, and a quickly cycled rail produces marginal resets whose recovery misfires Register the new Mic post-start callback for the StopWatch: it writes 0x0D=0x01 at roughly +30/+60/+90 ms after the clock start and returns, so the first Mic.begin() after a codec reset blocks for about 100 ms; later ones are unaffected. The armed state is latched only when the latest (most conservative) write was acknowledged. The analog warm-up then completes in the background: begin() success means the port runs and the codec is configured, and for about the first second after a codec reset captured samples can still be all zero - documented as part of the begin() contract. A reset performed outside this library (rail cycled by the application, brown-out) is detected through register 0x17, which keeps the value a previous capture setup wrote only while no reset has occurred. The speaker disable path keeps the audio power rail on and powers down only the DAC over I2C; the deep I2C power-down still happens on mic disable. This changes the idle power draw after Speaker.end() and is deliberate - see the comments for the measured failure modes behind it. in_i2c_bulk_write() now reports whether every write was acknowledged, and the StopWatch mic enable callback fails the enable when its setup bulk did not fully land. ChainCaptain shares the same callback structure and codec and likely needs the same treatment; it is left unchanged here because it could not be verified on hardware. --- src/M5Unified.cpp | 65 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/src/M5Unified.cpp b/src/M5Unified.cpp index e44dd99..8467a60 100644 --- a/src/M5Unified.cpp +++ b/src/M5Unified.cpp @@ -493,7 +493,8 @@ static constexpr const uint8_t _pin_table_mbus[][31] = { } #endif - static void in_i2c_bulk_write(const uint8_t i2c_addr, const uint8_t* bulk_data, const uint32_t i2c_freq = 100000u, const uint8_t retry = 0) + /// @return true when every write in the table was acknowledged. + static bool in_i2c_bulk_write(const uint8_t i2c_addr, const uint8_t* bulk_data, const uint32_t i2c_freq = 100000u, const uint8_t retry = 0) { // bulk_data example.. // const uint8_t bulk_data[] = { @@ -501,17 +502,52 @@ static constexpr const uint8_t _pin_table_mbus[][31] = { // 3, 0x01, 0x00, 0x02, // <- datalen = 3, reg = 0x01, data = 0x00, 0x02 // 0 }; // <- datalen 0 is end of data. + bool all_ok = true; while (*bulk_data) { uint8_t len = *bulk_data++; uint8_t r = retry + 1; while (!M5.In_I2C.writeRegister(i2c_addr, bulk_data[0], &bulk_data[1], len - 1, i2c_freq) && --r) { m5gfx::delay(1); } + all_ok &= (r != 0); bulk_data += len; } + return all_ok; } static constexpr uint8_t es7210_i2c_addr = 0x40; static constexpr uint8_t es8311_i2c_addr0 = 0x18; static constexpr uint8_t es8311_i2c_addr1 = 0x19; + +#if defined (CONFIG_IDF_TARGET_ESP32S3) + /// The ES8311 arms its capture path only through a SYSTEM(0x0D) write done + /// while the I2S clock is running; a pre-clock write is absorbed silently + /// (it even reads back). The arming is needed once per codec power-on + /// reset and survives I2C power-down/up cycles. + static std::atomic es8311_capture_armed { false }; + + /// Post-start callback: arms the capture path once the I2S clock runs. + /// The analog stage then warms up on its own (~1 s after power-up); that + /// is chip physics, so begin() pays only for the arming here. + static bool _microphone_post_start_cb_stopwatch(void* args) + { + (void)args; + if (es8311_capture_armed.load(std::memory_order_acquire)) { return true; } + m5gfx::i2c::i2c_temporary_switcher_t backup_i2c_setting(1, GPIO_NUM_47, GPIO_NUM_48); + bool ok = false; + bool last_ok = false; + for (int i = 0; i < 3; ++i) + { // writes at ~+30/+60/+90 ms after the clock start; the earliest arming + // observed on hardware is ~+25 ms, the later points are the margin + m5gfx::delay(30); + last_ok = M5.In_I2C.writeRegister8(es8311_i2c_addr0, 0x0D, 0x01, 100000); + ok |= last_ok; + } + backup_i2c_setting.restore(); + /// an acknowledge proves only the transport, so the latch requires the + /// latest (most conservative) write to have been acknowledged + if (last_ok) { es8311_capture_armed.store(true, std::memory_order_release); } + return ok; + } +#endif static constexpr uint8_t es8388_i2c_addr = 0x10; static constexpr uint8_t pi4io1_i2c_addr = 0x43; static constexpr uint8_t m5pm1_i2c_addr = 0x6E; @@ -692,10 +728,17 @@ static constexpr const uint8_t _pin_table_mbus[][31] = { ioe1.digitalWrite(M5IOE1_Class::gpio10, true); // Enable PA (M5IOE1_G10) } else - { + { /// Keep Audio Power (M5IOE1_G3) on: cutting it forces another codec + /// reset, re-arming and re-warm-up on the next capture (and a quickly + /// cycled rail misfires). Only the DAC is powered down; Mic disable + /// does the deeper I2C power-down. auto& ioe1 = self->getIOExpander(0); ioe1.digitalWrite(M5IOE1_Class::gpio10, false); // Disable PA (M5IOE1_G10) - ioe1.digitalWrite(M5IOE1_Class::gpio3, false); // Disable Audio Power (M5IOE1_G3) + static constexpr const uint8_t disabled_bulk_data[] = { + 2, 0x12, 0x02, // 0x12 SYSTEM/ power-down DAC + 0 + }; + in_i2c_bulk_write(es8311_i2c_addr0, disabled_bulk_data, 100000, 3); } #endif return true; @@ -1263,8 +1306,19 @@ static constexpr const uint8_t _pin_table_mbus[][31] = { self->delay(5); } m5gfx::i2c::i2c_temporary_switcher_t backup_i2c_setting(1, GPIO_NUM_47, GPIO_NUM_48); - in_i2c_bulk_write(es8311_i2c_addr0, enabled ? enabled_bulk_data : disabled_bulk_data, 100000, 3); + if (enabled) + { /// 0x17 loses the value a previous setup wrote on any codec reset, + /// so it witnesses a reset done outside this library: re-arm then. + uint8_t v = 0; + if (!M5.In_I2C.readRegister(es8311_i2c_addr0, 0x17, &v, 1, 100000) || v != 0xFF) + { + es8311_capture_armed.store(false, std::memory_order_release); + } + } + bool setup_ok = in_i2c_bulk_write(es8311_i2c_addr0, enabled ? enabled_bulk_data : disabled_bulk_data, 100000, 3); backup_i2c_setting.restore(); + /// a codec with an incomplete setup must not be published as working + if (enabled && !setup_ok) { return false; } #endif return true; } @@ -2531,6 +2585,7 @@ static constexpr const uint8_t _pin_table_mbus[][31] = { void M5Unified::_begin_audio(config_t& cfg) { bool(*mic_enable_cb)(void*, bool) = nullptr; + bool(*mic_post_start_cb)(void*) = nullptr; auto mic_cfg = Mic.config(); bool(*spk_enable_cb)(void*, bool) = nullptr; @@ -2637,6 +2692,7 @@ static constexpr const uint8_t _pin_table_mbus[][31] = { mic_cfg.pin_data_in = GPIO_NUM_16; mic_cfg.i2s_port = I2S_NUM_1; mic_enable_cb = _microphone_enabled_cb_stopwatch; + mic_post_start_cb = _microphone_post_start_cb_stopwatch; } break; @@ -3226,6 +3282,7 @@ static constexpr const uint8_t _pin_table_mbus[][31] = { if (mic_cfg.pin_data_in >= 0) { Mic.setCallback(this, mic_enable_cb); + Mic.setPostStartCallback(this, mic_post_start_cb); Mic.config(mic_cfg); } if (spk_cfg.pin_data_out >= 0) From 3b6fd1b4970dbf15d825bc408281aa34cf9ba93b Mon Sep 17 00:00:00 2001 From: ainyan03 <205502311+ainyan03@users.noreply.github.com> Date: Tue, 1 Sep 2026 01:44:34 +0000 Subject: [PATCH 3/3] Mic_Class: do not uninstall the I2S driver from the pre-setup failure path The rollback for a failed enable callback installed nothing itself: _setup_i2s cleans up its own failures, and on the legacy driver an unconditional i2s_driver_uninstall(port) could take down a driver some other code installed on the same port. --- src/utility/Mic_Class.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/utility/Mic_Class.cpp b/src/utility/Mic_Class.cpp index 5c07abb..729267b 100644 --- a/src/utility/Mic_Class.cpp +++ b/src/utility/Mic_Class.cpp @@ -825,9 +825,10 @@ if (_cfg.pin_bck < 0 || _cfg.pin_ws < 0) { if (res) { res = (ESP_OK == _setup_i2s()); } if (!res) { // no task to tear down yet, but whatever the enable callback powered - // up still has to come back down + // up still has to come back down. No driver uninstall here: this + // attempt installed nothing (_setup_i2s cleans up its own failures), + // and on the legacy driver the port could belong to someone else. if (_cb_set_enabled) { _cb_set_enabled(_cb_set_enabled_args, false); } - _i2s_driver_uninstall(_cfg.i2s_port); return false; } if (res)