[openthread] rewrite openthread esp.cpp to use new esp functions (#16334)

Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: J. Nick Koston <nick@home-assistant.io>
Co-authored-by: J. Nick Koston <nick@koston.org>

esphome/components/openthread/openthread.cpp

@@ -230,23 +230,27 @@ void *OpenThreadSrpComponent::pool_alloc_(size_t size) {
 bool OpenThreadComponent::teardown() {
   switch (this->teardown_stage_) {
     case TeardownStage::TEARDOWN_STAGE_NOT_STARTED: {
-      auto lock = InstanceLock::try_acquire(100);
-      if (!lock) {
-        // Try again on next teardown loop
-        ESP_LOGV(TAG, "Failed to acquire OpenThread lock during teardown");
-        return false;
-      }
-      // Start tearing down
-      this->teardown_stage_ = TeardownStage::TEARDOWN_STAGE_STOP_IN_PROCESS;
-      ESP_LOGV(TAG, "Clear SRP");
-      otInstance *instance = lock.get_instance();
-      otSrpClientClearHostAndServices(instance);
-      otSrpClientBuffersFreeAllServices(instance);
-      if (otThreadSetEnabled(instance, false) != OT_ERROR_NONE) {
-        ESP_LOGW(TAG, "Failed to disable Thread during teardown");
-      }
-      if (otIp6SetEnabled(instance, false) != OT_ERROR_NONE) {
-        ESP_LOGW(TAG, "Failed to disable IPv6 during teardown");
+      {
+        auto lock = InstanceLock::try_acquire(100);
+        // The OT task may still be starting up; stay pending and retry on
+        // the next call rather than giving up after a single failed attempt.
+        if (!lock) {
+          ESP_LOGV(TAG, "Failed to acquire OpenThread lock during teardown");
+          return false;
+        }
+        this->teardown_stage_ = TeardownStage::TEARDOWN_STAGE_STOP_IN_PROCESS;
+        ESP_LOGV(TAG, "Clear SRP");
+        otInstance *instance = lock.get_instance();
+        otSrpClientClearHostAndServices(instance);
+        otSrpClientBuffersFreeAllServices(instance);
+        if (otThreadSetEnabled(instance, false) != OT_ERROR_NONE) {
+          ESP_LOGW(TAG, "Failed to disable Thread during teardown");
+        }
+        if (otIp6SetEnabled(instance, false) != OT_ERROR_NONE) {
+          ESP_LOGW(TAG, "Failed to disable IPv6 during teardown");
+        }
+        // Release the lock before stopping -- openthread_stop_() (esp_openthread_stop() on
+        // ESP32) acquires it internally, and the lock is not recursive.
       }
       // Stop OpenThread
       global_openthread_component = nullptr;
@@ -254,11 +258,11 @@ bool OpenThreadComponent::teardown() {
       int error = this->openthread_stop_();
       if (error != 0) {
         ESP_LOGW(TAG, "Failed attempt to stop OpenThread %d", error);
-        this->teardown_stage_ = TeardownStage::TEARDOWN_STAGE_COMPLETED;
       }
     } break;
     case TeardownStage::TEARDOWN_STAGE_STOP_IN_PROCESS:
-      // Waiting on OpenThread stop
+      // Unreachable today; teardown is synchronous on both platforms. Kept for a future
+      // graceful-exit path, or a platform whose teardown() cannot be made synchronous.
       break;
     case TeardownStage::TEARDOWN_STAGE_COMPLETED:
       ESP_LOGV(TAG, "OpenThreadComponent Teardown Complete");
@@ -286,7 +290,7 @@ void OpenThreadComponent::apply_poll_period(uint32_t poll_period) {
 #if CONFIG_OPENTHREAD_MTD
   this->set_poll_period(poll_period);
   if (!this->is_lock_initialized()) {
-    // The action may run before the stack is up, e.g. from a restore mode; ot_main applies the stored value.
+    // The action may run before the stack is up, e.g. from a restore mode; setup() applies the stored value.
     ESP_LOGD(TAG, "Not (yet) ready to apply");
     return;
   }

esphome/components/openthread/openthread.h

@@ -35,11 +35,10 @@ class OpenThreadComponent final : public Component {
   float get_setup_priority() const override { return setup_priority::WIFI; }
 
   bool is_connected() const { return this->connected_; }
-  /// Returns true once esp_openthread_init() has completed and the OT lock is usable.
+  /// Returns true once esp_openthread_start() has completed and the OT lock is usable.
   bool is_lock_initialized() const { return this->lock_initialized_; }
   network::IPAddresses get_ip_addresses();
   std::optional<otIp6Address> get_omr_address();
-  void ot_main();
   void on_factory_reset(std::function<void()> callback);
   void defer_factory_reset_external_callback();
 
@@ -60,7 +59,6 @@ class OpenThreadComponent final : public Component {
  protected:
   /** Apply Link Mode settings (incl poll period).
    * Callers running outside the OpenThread task must hold InstanceLock.
-   * ot_main() runs on the OpenThread task itself and must not acquire the lock.
    */
   void apply_linkmode_(otInstance *instance);
 
@@ -73,7 +71,8 @@ class OpenThreadComponent final : public Component {
 #endif
   std::optional<int8_t> output_power_{};
   std::atomic<bool> lock_initialized_{false};
-  std::atomic<TeardownStage> teardown_stage_{TeardownStage::TEARDOWN_STAGE_NOT_STARTED};
+  // Only ever written from teardown(), on the main task -- no atomic needed.
+  TeardownStage teardown_stage_{TeardownStage::TEARDOWN_STAGE_NOT_STARTED};
   std::atomic<bool> connected_{false};
 
  private:

esphome/components/openthread/openthread_esp.cpp

@@ -1,6 +1,5 @@
 #include "esphome/core/defines.h"
 #if defined(USE_OPENTHREAD) && defined(USE_ESP32)
-#include <openthread/logging.h>
 #include "openthread.h"
 
 #include "esp_log.h"
@@ -13,14 +12,9 @@
 #include "esphome/core/log.h"
 
 #include "esp_err.h"
-#include "esp_event.h"
 #include "esp_netif.h"
-#include "esp_netif_types.h"
-#include "esp_openthread_cli.h"
 #include "esp_openthread_netif_glue.h"
 #include "esp_vfs_eventfd.h"
-#include "freertos/FreeRTOS.h"
-#include "freertos/task.h"
 #include "nvs_flash.h"
 
 static const char *const TAG = "openthread";
@@ -39,140 +33,94 @@ void OpenThreadComponent::setup() {
   ESP_ERROR_CHECK(nvs_flash_init());
   ESP_ERROR_CHECK(esp_vfs_eventfd_register(&eventfd_config));
 
-  xTaskCreate(
-      [](void *arg) {
-        static_cast<OpenThreadComponent *>(arg)->ot_main();
-        vTaskDelete(nullptr);
-      },
-      "ot_main", 10240, this, 5, nullptr);
-}
+  esp_openthread_config_t config = {.netif_config = ESP_NETIF_DEFAULT_OPENTHREAD(),
+                                    .platform_config = {
+                                        .radio_config =
+                                            {
+                                                .radio_mode = RADIO_MODE_NATIVE,
+                                                .radio_uart_config = {},
+                                            },
+                                        .host_config =
+                                            {
+                                                // There is a conflict between esphome's logger which also
+                                                // claims the usb serial jtag device.
+                                                // .host_connection_mode = HOST_CONNECTION_MODE_CLI_USB,
+                                                // .host_usb_config = USB_SERIAL_JTAG_DRIVER_CONFIG_DEFAULT(),
+                                            },
+                                        .port_config =
+                                            {
+                                                .storage_partition_name = "nvs",
+                                                .netif_queue_size = 10,
+                                                .task_queue_size = 10,
+                                            },
+                                    }};
 
-static esp_netif_t *init_openthread_netif(const esp_openthread_platform_config_t *config) {
-  esp_netif_config_t cfg = ESP_NETIF_DEFAULT_OPENTHREAD();
-  esp_netif_t *netif = esp_netif_new(&cfg);
-  assert(netif != nullptr);
-  ESP_ERROR_CHECK(esp_netif_attach(netif, esp_openthread_netif_glue_init(config)));
-
-  return netif;
-}
-
-void OpenThreadComponent::ot_main() {
-  esp_openthread_platform_config_t config = {
-      .radio_config =
-          {
-              .radio_mode = RADIO_MODE_NATIVE,
-              .radio_uart_config = {},
-          },
-      .host_config =
-          {
-              // There is a conflict between esphome's logger which also
-              // claims the usb serial jtag device.
-              // .host_connection_mode = HOST_CONNECTION_MODE_CLI_USB,
-              // .host_usb_config = USB_SERIAL_JTAG_DRIVER_CONFIG_DEFAULT(),
-          },
-      .port_config =
-          {
-              .storage_partition_name = "nvs",
-              .netif_queue_size = 10,
-              .task_queue_size = 10,
-          },
-  };
-
-  // Initialize the OpenThread stack
-  // otLoggingSetLevel(OT_LOG_LEVEL_DEBG);
-  ESP_ERROR_CHECK(esp_openthread_init(&config));
+  ESP_ERROR_CHECK(esp_openthread_start(&config));
   // Mark lock as initialized so InstanceLock callers know it's safe to acquire.
-  // Must be set after esp_openthread_init() which creates the internal semaphore.
+  // Must be set after esp_openthread_start() which creates the internal semaphore.
   this->lock_initialized_ = true;
   // Fetch OT instance once to avoid repeated call into OT stack
   otInstance *instance = esp_openthread_get_instance();
+  {
+    InstanceLock lock = InstanceLock::acquire();
 
-#if CONFIG_OPENTHREAD_STATE_INDICATOR_ENABLE
-  ESP_ERROR_CHECK(esp_openthread_state_indicator_init(instance));
-#endif
+    this->apply_linkmode_(instance);
 
-#if CONFIG_OPENTHREAD_LOG_LEVEL_DYNAMIC
-  // The OpenThread log level directly matches ESP log level
-  (void) otLoggingSetLevel(CONFIG_LOG_DEFAULT_LEVEL);
-#endif
-  // Initialize the OpenThread cli
-#if CONFIG_OPENTHREAD_CLI
-  esp_openthread_cli_init();
-#endif
-
-  esp_netif_t *openthread_netif;
-  // Initialize the esp_netif bindings
-  openthread_netif = init_openthread_netif(&config);
-  esp_netif_set_default_netif(openthread_netif);
-
-#if CONFIG_OPENTHREAD_CLI_ESP_EXTENSION
-  esp_cli_custom_command_init();
-#endif  // CONFIG_OPENTHREAD_CLI_ESP_EXTENSION
-
-  ESP_LOGD(TAG, "Thread Version: %" PRIu16, otThreadGetVersion());
-
-  this->apply_linkmode_(instance);
-
-  if (this->output_power_.has_value()) {
-    if (const auto err = otPlatRadioSetTransmitPower(instance, *this->output_power_); err != OT_ERROR_NONE) {
-      ESP_LOGE(TAG, "Failed to set power: %s", otThreadErrorToString(err));
+    if (this->output_power_.has_value()) {
+      if (const auto err = otPlatRadioSetTransmitPower(instance, *this->output_power_); err != OT_ERROR_NONE) {
+        ESP_LOGE(TAG, "Failed to set power: %s", otThreadErrorToString(err));
+      }
     }
-  }
-
-  // Run the main loop
-#if CONFIG_OPENTHREAD_CLI
-  esp_openthread_cli_create_task();
-#endif
-  ESP_LOGI(TAG, "Activating dataset...");
-  otOperationalDatasetTlvs dataset = {};
+    ESP_LOGI(TAG, "Activating dataset...");
+    otOperationalDatasetTlvs dataset = {};
 
 #ifndef USE_OPENTHREAD_FORCE_DATASET
-  // Check if openthread has a valid dataset from a previous execution
-  otError error = otDatasetGetActiveTlvs(instance, &dataset);
-  if (error != OT_ERROR_NONE) {
-    // Make sure the length is 0 so we fallback to the configuration
-    dataset.mLength = 0;
-  } else {
-    ESP_LOGI(TAG, "Found existing dataset, ignoring config (force_dataset: true to override)");
-  }
+    // Check if openthread has a valid dataset from a previous execution
+    otError error = otDatasetGetActiveTlvs(instance, &dataset);
+    if (error != OT_ERROR_NONE) {
+      // Make sure the length is 0 so we fallback to the configuration
+      dataset.mLength = 0;
+    } else {
+      ESP_LOGI(TAG, "Found existing dataset, ignoring config (force_dataset: true to override)");
+    }
 #endif
 
 #ifdef USE_OPENTHREAD_TLVS
-  if (dataset.mLength == 0) {
-    // If we didn't have an active dataset, and we have tlvs, parse it and pass it to esp_openthread_auto_start
-    size_t len = (sizeof(USE_OPENTHREAD_TLVS) - 1) / 2;
-    if (len > sizeof(dataset.mTlvs)) {
-      ESP_LOGW(TAG, "TLV buffer too small, truncating");
-      len = sizeof(dataset.mTlvs);
+    if (dataset.mLength == 0) {
+      // If we didn't have an active dataset, and we have tlvs, parse it and pass it to esp_openthread_auto_start
+      size_t len = (sizeof(USE_OPENTHREAD_TLVS) - 1) / 2;
+      if (len > sizeof(dataset.mTlvs)) {
+        ESP_LOGW(TAG, "TLV buffer too small, truncating");
+        len = sizeof(dataset.mTlvs);
+      }
+      parse_hex(USE_OPENTHREAD_TLVS, sizeof(USE_OPENTHREAD_TLVS) - 1, dataset.mTlvs, len);
+      dataset.mLength = len;
     }
-    parse_hex(USE_OPENTHREAD_TLVS, sizeof(USE_OPENTHREAD_TLVS) - 1, dataset.mTlvs, len);
-    dataset.mLength = len;
-  }
 #endif
 
-  // Pass the existing dataset, or NULL which will use the preprocessor definitions
-  ESP_ERROR_CHECK(esp_openthread_auto_start(dataset.mLength > 0 ? &dataset : nullptr));
+    // Pass the existing dataset, or NULL which will use the preprocessor definitions
+    ESP_ERROR_CHECK(esp_openthread_auto_start(dataset.mLength > 0 ? &dataset : nullptr));
 
-  // Register state change callback to update connected_ reactively instead of polling
-  otError ot_err = otSetStateChangedCallback(instance, OpenThreadComponent::on_state_changed, this);
-  if (ot_err != OT_ERROR_NONE) {
-    ESP_LOGW(TAG, "Failed to register state change callback: %d", ot_err);
+    // Register state change callback to update connected_ reactively instead of polling
+    otError ot_err = otSetStateChangedCallback(instance, OpenThreadComponent::on_state_changed, this);
+    if (ot_err != OT_ERROR_NONE) {
+      ESP_LOGW(TAG, "Failed to register state change callback: %d", ot_err);
+    }
   }
 
-  esp_openthread_launch_mainloop();
-
-  // Clean up - reset lock flag before deinit destroys the semaphore
-  this->lock_initialized_ = false;
-  esp_openthread_deinit();
-  esp_openthread_netif_glue_deinit();
-  esp_netif_destroy(openthread_netif);
-
-  esp_vfs_eventfd_unregister();
-  this->teardown_stage_ = TeardownStage::TEARDOWN_STAGE_COMPLETED;
-  vTaskDelete(NULL);
+  ESP_LOGD(TAG, "Thread Version: %" PRIu16, otThreadGetVersion());
 }
 
-int OpenThreadComponent::openthread_stop_() { return esp_openthread_mainloop_exit(); }
+int OpenThreadComponent::openthread_stop_() {
+  // Clean up - reset lock flag before deinit destroys the semaphore
+  this->lock_initialized_ = false;
+  int error = esp_openthread_stop();
+  // Mark complete even on failure: we're already mid-shutdown/reboot, so there's no
+  // recovery path to retry into -- leaving the stage stuck would only burn the full
+  // teardown timeout for no benefit.
+  this->teardown_stage_ = TeardownStage::TEARDOWN_STAGE_COMPLETED;
+  return error;
+}
 
 network::IPAddresses OpenThreadComponent::get_ip_addresses() {
   network::IPAddresses addresses;
@@ -199,8 +147,14 @@ InstanceLock InstanceLock::try_acquire(int delay) {
 }
 
 InstanceLock InstanceLock::acquire() {
-  // Wait for the lock to be created by ot_main() before attempting to acquire it.
-  // esp_openthread_lock_acquire() will assert-crash if called before esp_openthread_init().
+  // teardown() clears global_openthread_component before the stack fully stops; a caller
+  // racing teardown would otherwise dereference a null pointer below.
+  if (global_openthread_component == nullptr) {
+    ESP_LOGE(TAG, "OpenThread component torn down, cannot acquire instance lock");
+    abort();
+  }
+  // Wait for the lock to be created before attempting to acquire it.
+  // esp_openthread_lock_acquire() will assert-crash if called before esp_openthread_start().
   constexpr uint32_t lock_init_timeout_ms = 10000;
   uint32_t start = millis();
   while (!global_openthread_component->is_lock_initialized()) {

esphome/components/openthread/openthread_zephyr.cpp

@@ -83,9 +83,10 @@ void OpenThreadComponent::setup() {
   }
   openthread_state_changed_cb_register(context, &ot_state_changed_cb);
   openthread_start(context);
-}
 
-void OpenThreadComponent::ot_main() {}
+  InstanceLock lock = InstanceLock::acquire();
+  this->apply_linkmode_(lock.get_instance());
+}
 
 otInstance *OpenThreadComponent::get_openthread_instance_() { return openthread_get_default_instance(); }