diff --git a/esphome/components/i2s_audio/speaker/i2s_audio_speaker.cpp b/esphome/components/i2s_audio/speaker/i2s_audio_speaker.cpp index 1382a870465..9feaf39ffff 100644 --- a/esphome/components/i2s_audio/speaker/i2s_audio_speaker.cpp +++ b/esphome/components/i2s_audio/speaker/i2s_audio_speaker.cpp @@ -53,6 +53,13 @@ void I2SAudioSpeakerBase::dump_config() { void I2SAudioSpeakerBase::loop() { uint32_t event_group_bits = xEventGroupGetBits(this->event_group_); + // A stop that arrives while stopped cancels any start that has not been processed yet + constexpr uint32_t stop_bits = SpeakerEventGroupBits::COMMAND_STOP | SpeakerEventGroupBits::COMMAND_STOP_GRACEFULLY; + if ((event_group_bits & stop_bits) && (this->state_ == speaker::STATE_STOPPED)) { + xEventGroupClearBits(this->event_group_, stop_bits | SpeakerEventGroupBits::COMMAND_START); + event_group_bits &= ~(stop_bits | SpeakerEventGroupBits::COMMAND_START); + } + if ((event_group_bits & SpeakerEventGroupBits::COMMAND_START) && (this->state_ == speaker::STATE_STOPPED)) { this->state_ = speaker::STATE_STARTING; xEventGroupClearBits(this->event_group_, SpeakerEventGroupBits::COMMAND_START); @@ -239,8 +246,6 @@ void I2SAudioSpeakerBase::start() { if ((this->state_ == speaker::STATE_STARTING) || (this->state_ == speaker::STATE_RUNNING)) return; - // Mark STARTING immediately to avoid transient STOPPED observations before loop() processes COMMAND_START. - this->state_ = speaker::STATE_STARTING; xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::COMMAND_START); } @@ -249,11 +254,10 @@ void I2SAudioSpeakerBase::stop() { this->stop_(false); } void I2SAudioSpeakerBase::finish() { this->stop_(true); } void I2SAudioSpeakerBase::stop_(bool wait_on_empty) { - if (this->is_failed()) - return; - if (this->state_ == speaker::STATE_STOPPED) + if (!this->is_ready() || this->is_failed()) return; + // Always set the bit, even when stopped, so loop() can cancel a start that is still pending if (wait_on_empty) { xEventGroupSetBits(this->event_group_, SpeakerEventGroupBits::COMMAND_STOP_GRACEFULLY); } else { diff --git a/esphome/components/router/speaker/router_speaker.cpp b/esphome/components/router/speaker/router_speaker.cpp index f4bf7420ab0..dd2428e4df4 100644 --- a/esphome/components/router/speaker/router_speaker.cpp +++ b/esphome/components/router/speaker/router_speaker.cpp @@ -2,6 +2,8 @@ #ifdef USE_ESP32 +#include "esphome/core/application.h" +#include "esphome/core/hal.h" #include "esphome/core/log.h" #include "esp_timer.h" @@ -12,6 +14,9 @@ namespace esphome::router { static const char *const TAG = "router.speaker"; +// Maximum time to wait for the active output to report running after start() before giving up +static const uint32_t STATE_TRANSITION_TIMEOUT_MS = 5000; + static inline uint32_t atomic_subtract_clamped(std::atomic &var, uint32_t amount) { uint32_t current = var.load(std::memory_order_acquire); uint32_t subtracted = 0; @@ -72,6 +77,7 @@ void Router::loop() { this->apply_cached_state_to_active_(); this->state_ = speaker::STATE_STARTING; + this->state_start_ms_ = App.get_loop_component_start_time(); active->start(); } return; @@ -86,10 +92,17 @@ void Router::loop() { // set_audio_stream_info() and never reaches the output on its own; if the format // changed while stopped, only start()'s apply_cached_state_to_active_() pushes it // down before the output's play()-side auto-start locks in the stale format. - if (active->is_stopped()) { + // While STARTING, ignore a transient stopped report as speaker running state + // is set asynchronously from start(). Timeout if the speaker never transitions. + if (this->state_ == speaker::STATE_STARTING) { + if (active->is_running()) { + this->state_ = speaker::STATE_RUNNING; + } else if ((App.get_loop_component_start_time() - this->state_start_ms_) > STATE_TRANSITION_TIMEOUT_MS) { + ESP_LOGW(TAG, "Active output did not start; giving up"); + this->state_ = speaker::STATE_STOPPED; + } + } else if (active->is_stopped()) { this->state_ = speaker::STATE_STOPPED; - } else if (this->state_ == speaker::STATE_STARTING && active->is_running()) { - this->state_ = speaker::STATE_RUNNING; } } @@ -133,6 +146,8 @@ void Router::start() { this->frames_in_pipeline_.store(0, std::memory_order_release); this->apply_cached_state_to_active_(); this->state_ = speaker::STATE_STARTING; + // May run on a producer task, so the cached loop timestamp is not usable here + this->state_start_ms_ = millis(); this->get_active_output()->start(); } diff --git a/esphome/components/router/speaker/router_speaker.h b/esphome/components/router/speaker/router_speaker.h index 801d0906cee..31f3f906301 100644 --- a/esphome/components/router/speaker/router_speaker.h +++ b/esphome/components/router/speaker/router_speaker.h @@ -59,6 +59,9 @@ class Router final : public Component, public speaker::Speaker { // frames_in_pipeline_. std::atomic frames_in_pipeline_{0}; + // Set when entering STATE_STARTING; used to time out a start the output never acts on + uint32_t state_start_ms_{0}; + bool cached_pause_{false}; void apply_cached_state_to_active_();