[i2s-audio] ensure mic task isn't pinned to a core (#8879)

esphome/components/i2s_audio/microphone/i2s_audio_microphone.cpp

@@ -30,11 +30,11 @@ static const int32_t DC_OFFSET_MOVING_AVERAGE_COEFFICIENT_DENOMINATOR = 1000;
 static const char *const TAG = "i2s_audio.microphone";
 
 enum MicrophoneEventGroupBits : uint32_t {
-  COMMAND_STOP = (1 << 0),  // stops the microphone task
+  COMMAND_STOP = (1 << 0),  // stops the microphone task, set and cleared by ``loop``
-  TASK_STARTING = (1 << 10),
+
-  TASK_RUNNING = (1 << 11),
+  TASK_STARTING = (1 << 10),  // set by mic task, cleared by ``loop``
-  TASK_STOPPING = (1 << 12),
+  TASK_RUNNING = (1 << 11),   // set by mic task, cleared by ``loop``
-  TASK_STOPPED = (1 << 13),
+  TASK_STOPPED = (1 << 13),   // set by mic task, cleared by ``loop``
 
   ALL_BITS = 0x00FFFFFF,  // All valid FreeRTOS event group bits
 };
@@ -151,24 +151,21 @@ bool I2SAudioMicrophone::start_driver_() {
     config.mode = (i2s_mode_t) (config.mode | I2S_MODE_ADC_BUILT_IN);
     err = i2s_driver_install(this->parent_->get_port(), &config, 0, nullptr);
     if (err != ESP_OK) {
-      ESP_LOGW(TAG, "Error installing I2S driver: %s", esp_err_to_name(err));
+      ESP_LOGE(TAG, "Error installing I2S driver: %s", esp_err_to_name(err));
-      this->status_set_error();
       return false;
     }
 
     err = i2s_set_adc_mode(ADC_UNIT_1, this->adc_channel_);
     if (err != ESP_OK) {
-      ESP_LOGW(TAG, "Error setting ADC mode: %s", esp_err_to_name(err));
+      ESP_LOGE(TAG, "Error setting ADC mode: %s", esp_err_to_name(err));
-      this->status_set_error();
-      return false;
-    }
-    err = i2s_adc_enable(this->parent_->get_port());
-    if (err != ESP_OK) {
-      ESP_LOGW(TAG, "Error enabling ADC: %s", esp_err_to_name(err));
-      this->status_set_error();
       return false;
     }
 
+    err = i2s_adc_enable(this->parent_->get_port());
+    if (err != ESP_OK) {
+      ESP_LOGE(TAG, "Error enabling ADC: %s", esp_err_to_name(err));
+      return false;
+    }
   } else
 #endif
   {
@@ -177,8 +174,7 @@ bool I2SAudioMicrophone::start_driver_() {
 
     err = i2s_driver_install(this->parent_->get_port(), &config, 0, nullptr);
     if (err != ESP_OK) {
-      ESP_LOGW(TAG, "Error installing I2S driver: %s", esp_err_to_name(err));
+      ESP_LOGE(TAG, "Error installing I2S driver: %s", esp_err_to_name(err));
-      this->status_set_error();
       return false;
     }
 
@@ -187,8 +183,7 @@ bool I2SAudioMicrophone::start_driver_() {
 
     err = i2s_set_pin(this->parent_->get_port(), &pin_config);
     if (err != ESP_OK) {
-      ESP_LOGW(TAG, "Error setting I2S pin: %s", esp_err_to_name(err));
+      ESP_LOGE(TAG, "Error setting I2S pin: %s", esp_err_to_name(err));
-      this->status_set_error();
       return false;
     }
   }
@@ -203,8 +198,7 @@ bool I2SAudioMicrophone::start_driver_() {
   /* Allocate a new RX channel and get the handle of this channel */
   err = i2s_new_channel(&chan_cfg, NULL, &this->rx_handle_);
   if (err != ESP_OK) {
-    ESP_LOGW(TAG, "Error creating new I2S channel: %s", esp_err_to_name(err));
+    ESP_LOGE(TAG, "Error creating new I2S channel: %s", esp_err_to_name(err));
-    this->status_set_error();
     return false;
   }
 
@@ -276,22 +270,20 @@ bool I2SAudioMicrophone::start_driver_() {
     err = i2s_channel_init_std_mode(this->rx_handle_, &std_cfg);
   }
   if (err != ESP_OK) {
-    ESP_LOGW(TAG, "Error initializing I2S channel: %s", esp_err_to_name(err));
+    ESP_LOGE(TAG, "Error initializing I2S channel: %s", esp_err_to_name(err));
-    this->status_set_error();
     return false;
   }
 
   /* Before reading data, start the RX channel first */
   i2s_channel_enable(this->rx_handle_);
   if (err != ESP_OK) {
-    ESP_LOGW(TAG, "Error enabling I2S Microphone: %s", esp_err_to_name(err));
+    ESP_LOGE(TAG, "Error enabling I2S Microphone: %s", esp_err_to_name(err));
-    this->status_set_error();
     return false;
   }
 #endif
 
-  this->status_clear_error();
   this->configure_stream_settings_();  // redetermine the settings in case some settings were changed after compilation
+
   return true;
 }
 
@@ -303,71 +295,55 @@ void I2SAudioMicrophone::stop() {
 }
 
 void I2SAudioMicrophone::stop_driver_() {
+  // There is no harm continuing to unload the driver if an error is ever returned by the various functions. This
+  // ensures that we stop/unload the driver when it only partially starts.
+
   esp_err_t err;
 #ifdef USE_I2S_LEGACY
 #if SOC_I2S_SUPPORTS_ADC
   if (this->adc_) {
     err = i2s_adc_disable(this->parent_->get_port());
     if (err != ESP_OK) {
-      ESP_LOGW(TAG, "Error disabling ADC: %s", esp_err_to_name(err));
+      ESP_LOGW(TAG, "Error disabling ADC - it may not have started: %s", esp_err_to_name(err));
-      this->status_set_error();
-      return;
     }
   }
 #endif
   err = i2s_stop(this->parent_->get_port());
   if (err != ESP_OK) {
-    ESP_LOGW(TAG, "Error stopping I2S microphone: %s", esp_err_to_name(err));
+    ESP_LOGW(TAG, "Error stopping I2S microphone - it may not have started: %s", esp_err_to_name(err));
-    this->status_set_error();
-    return;
   }
   err = i2s_driver_uninstall(this->parent_->get_port());
   if (err != ESP_OK) {
-    ESP_LOGW(TAG, "Error uninstalling I2S driver: %s", esp_err_to_name(err));
+    ESP_LOGW(TAG, "Error uninstalling I2S driver - it may not have started: %s", esp_err_to_name(err));
-    this->status_set_error();
-    return;
   }
 #else
   /* Have to stop the channel before deleting it */
   err = i2s_channel_disable(this->rx_handle_);
   if (err != ESP_OK) {
-    ESP_LOGW(TAG, "Error stopping I2S microphone: %s", esp_err_to_name(err));
+    ESP_LOGW(TAG, "Error stopping I2S microphone - it may not have started: %s", esp_err_to_name(err));
-    this->status_set_error();
-    return;
   }
   /* If the handle is not needed any more, delete it to release the channel resources */
   err = i2s_del_channel(this->rx_handle_);
   if (err != ESP_OK) {
-    ESP_LOGW(TAG, "Error deleting I2S channel: %s", esp_err_to_name(err));
+    ESP_LOGW(TAG, "Error deleting I2S channel - it may not have started: %s", esp_err_to_name(err));
-    this->status_set_error();
-    return;
   }
 #endif
   this->parent_->unlock();
-  this->status_clear_error();
 }
 
 void I2SAudioMicrophone::mic_task(void *params) {
   I2SAudioMicrophone *this_microphone = (I2SAudioMicrophone *) params;
-
   xEventGroupSetBits(this_microphone->event_group_, MicrophoneEventGroupBits::TASK_STARTING);
 
-  uint8_t start_counter = 0;
+  {  // Ensures the samples vector is freed when the task stops
-  bool started = this_microphone->start_driver_();
-  while (!started && start_counter < 10) {
-    // Attempt to load the driver again in 100 ms. Doesn't slow down main loop since its in a task.
-    vTaskDelay(pdMS_TO_TICKS(100));
-    ++start_counter;
-    started = this_microphone->start_driver_();
-  }
 
-  if (started) {
-    xEventGroupSetBits(this_microphone->event_group_, MicrophoneEventGroupBits::TASK_RUNNING);
     const size_t bytes_to_read = this_microphone->audio_stream_info_.ms_to_bytes(READ_DURATION_MS);
     std::vector<uint8_t> samples;
     samples.reserve(bytes_to_read);
 
-    while (!(xEventGroupGetBits(this_microphone->event_group_) & COMMAND_STOP)) {
+    xEventGroupSetBits(this_microphone->event_group_, MicrophoneEventGroupBits::TASK_RUNNING);
+
+    while (!(xEventGroupGetBits(this_microphone->event_group_) & MicrophoneEventGroupBits::COMMAND_STOP)) {
       if (this_microphone->data_callbacks_.size() > 0) {
         samples.resize(bytes_to_read);
         size_t bytes_read = this_microphone->read_(samples.data(), bytes_to_read, 2 * pdMS_TO_TICKS(READ_DURATION_MS));
@@ -382,9 +358,6 @@ void I2SAudioMicrophone::mic_task(void *params) {
     }
   }
 
-  xEventGroupSetBits(this_microphone->event_group_, MicrophoneEventGroupBits::TASK_STOPPING);
-  this_microphone->stop_driver_();
-
   xEventGroupSetBits(this_microphone->event_group_, MicrophoneEventGroupBits::TASK_STOPPED);
   while (true) {
     // Continuously delay until the loop method deletes the task
@@ -425,7 +398,10 @@ size_t I2SAudioMicrophone::read_(uint8_t *buf, size_t len, TickType_t ticks_to_w
 #endif
   if ((err != ESP_OK) && ((err != ESP_ERR_TIMEOUT) || (ticks_to_wait != 0))) {
     // Ignore ESP_ERR_TIMEOUT if ticks_to_wait = 0, as it will read the data on the next call
-    ESP_LOGW(TAG, "Error reading from I2S microphone: %s", esp_err_to_name(err));
+    if (!this->status_has_warning()) {
+      // Avoid spamming the logs with the error message if its repeated
+      ESP_LOGW(TAG, "Error reading from I2S microphone: %s", esp_err_to_name(err));
+    }
     this->status_set_warning();
     return 0;
   }
@@ -452,7 +428,7 @@ void I2SAudioMicrophone::loop() {
   uint32_t event_group_bits = xEventGroupGetBits(this->event_group_);
 
   if (event_group_bits & MicrophoneEventGroupBits::TASK_STARTING) {
-    ESP_LOGD(TAG, "Task has started, attempting to setup I2S audio driver");
+    ESP_LOGD(TAG, "Task started, attempting to allocate buffer");
     xEventGroupClearBits(this->event_group_, MicrophoneEventGroupBits::TASK_STARTING);
   }
 
@@ -463,23 +439,25 @@ void I2SAudioMicrophone::loop() {
     this->state_ = microphone::STATE_RUNNING;
   }
 
-  if (event_group_bits & MicrophoneEventGroupBits::TASK_STOPPING) {
-    ESP_LOGD(TAG, "Task is stopping, attempting to unload the I2S audio driver");
-    xEventGroupClearBits(this->event_group_, MicrophoneEventGroupBits::TASK_STOPPING);
-  }
-
   if ((event_group_bits & MicrophoneEventGroupBits::TASK_STOPPED)) {
-    ESP_LOGD(TAG, "Task is finished, freeing resources");
+    ESP_LOGD(TAG, "Task finished, freeing resources and uninstalling I2S driver");
+
     vTaskDelete(this->task_handle_);
     this->task_handle_ = nullptr;
+    this->stop_driver_();
     xEventGroupClearBits(this->event_group_, ALL_BITS);
+    this->status_clear_error();
+
     this->state_ = microphone::STATE_STOPPED;
   }
 
+  // Start the microphone if any semaphores are taken
   if ((uxSemaphoreGetCount(this->active_listeners_semaphore_) < MAX_LISTENERS) &&
       (this->state_ == microphone::STATE_STOPPED)) {
     this->state_ = microphone::STATE_STARTING;
   }
+
+  // Stop the microphone if all semaphores are returned
   if ((uxSemaphoreGetCount(this->active_listeners_semaphore_) == MAX_LISTENERS) &&
       (this->state_ == microphone::STATE_RUNNING)) {
     this->state_ = microphone::STATE_STOPPING;
@@ -487,14 +465,26 @@ void I2SAudioMicrophone::loop() {
 
   switch (this->state_) {
     case microphone::STATE_STARTING:
-      if ((this->task_handle_ == nullptr) && !this->status_has_error()) {
+      if (this->status_has_error()) {
+        break;
+      }
+
+      if (!this->start_driver_()) {
+        this->status_momentary_error("I2S driver failed to start, unloading it and attempting again in 1 second", 1000);
+        this->stop_driver_();  // Stop/frees whatever possibly started
+        break;
+      }
+
+      if (this->task_handle_ == nullptr) {
         xTaskCreate(I2SAudioMicrophone::mic_task, "mic_task", TASK_STACK_SIZE, (void *) this, TASK_PRIORITY,
                     &this->task_handle_);
 
         if (this->task_handle_ == nullptr) {
           this->status_momentary_error("Task failed to start, attempting again in 1 second", 1000);
+          this->stop_driver_();  // Stops the driver to return the lock; will be reloaded in next attempt
         }
       }
+
       break;
     case microphone::STATE_RUNNING:
       break;

esphome/components/i2s_audio/microphone/i2s_audio_microphone.h

@@ -43,7 +43,11 @@ class I2SAudioMicrophone : public I2SAudioIn, public microphone::Microphone, pub
 #endif
 
  protected:
+  /// @brief Starts the I2S driver. Updates the ``audio_stream_info_`` member variable with the current setttings.
+  /// @return True if succesful, false otherwise
   bool start_driver_();
+
+  /// @brief Stops the I2S driver.
   void stop_driver_();
 
   /// @brief Attempts to correct a microphone DC offset; e.g., a microphones silent level is offset from 0. Applies a