openthread: split SRP into its own component because mdns has a different setup priority than networking

esphome/components/openthread/__init__.py

@@ -20,6 +20,7 @@ CONF_PSKC = "pskc"
 CONF_PANID = "panid"
 CONF_EXTPANID = "extpanid"
 CONF_MDNS_ID = "mdns_id"
+CONF_SRP_ID = "srp_id"
 
 
 def set_sdkconfig_options(config):
@@ -64,11 +65,13 @@ def set_sdkconfig_options(config):
 
 openthread_ns = cg.esphome_ns.namespace("openthread")
 OpenThreadComponent = openthread_ns.class_("OpenThreadComponent", cg.Component)
+OpenThreadSrpComponent = openthread_ns.class_("OpenThreadSrpComponent", cg.Component)
 
 CONFIG_SCHEMA = cv.All(
     cv.Schema(
         {
             cv.GenerateID(): cv.declare_id(OpenThreadComponent),
+            cv.GenerateID(CONF_SRP_ID): cv.declare_id(OpenThreadSrpComponent),
             cv.GenerateID(CONF_MDNS_ID): cv.use_id(MDNSComponent),
             cv.Required(CONF_PANID): cv.int_,
             cv.Required(CONF_CHANNEL): cv.int_,
@@ -84,10 +87,13 @@ CONFIG_SCHEMA = cv.All(
 async def to_code(config):
     cg.add_define("USE_OPENTHREAD")
 
-    var = cg.new_Pvariable(config[CONF_ID])
+    ot = cg.new_Pvariable(config[CONF_ID])
-    cg.add(var.set_host_name(cg.RawExpression(f'"{CORE.name}"')))
+    await cg.register_component(ot, config)
+
+    srp = cg.new_Pvariable(config[CONF_SRP_ID])
+    cg.add(srp.set_host_name(cg.RawExpression(f'"{CORE.name}"')))
     mdns_component = await cg.get_variable(config[CONF_MDNS_ID])
-    cg.add(var.set_mdns(mdns_component))
+    cg.add(srp.set_mdns(mdns_component))
-    await cg.register_component(var, config)
+    await cg.register_component(srp, config)
 
     set_sdkconfig_options(config)

esphome/components/openthread/openthread.cpp

@@ -1,5 +1,5 @@
 #include "esphome/core/defines.h"
-
+#define USE_OPENTHREAD
 #ifdef USE_OPENTHREAD
 #include "openthread.h"
 
@@ -87,15 +87,20 @@ std::optional<otIp6Address> OpenThreadComponent::get_omr_address_(std::optional<
 
 void srpCallback(otError aError, const otSrpClientHostInfo *aHostInfo, const otSrpClientService *aServices,
                  const otSrpClientService *aRemovedServices, void *aContext) {
-  ESP_LOGW(TAG, "*** SRP callback *** error=%d hostInfo=%p services=%p removedServices=%p", aError, aHostInfo,
+  if (aError != 0) {
-           aServices, aRemovedServices);
+    ESP_LOGW(TAG, "SRP client reported an error: %s", otThreadErrorToString(aError));
+    for (const otSrpClientHostInfo *host = aHostInfo; host; host = nullptr) {
+      ESP_LOGW(TAG, "  Host: %s", host->mName);
+    }
+    for (const otSrpClientService *service = aServices; service; service = service->mNext) {
+      ESP_LOGW(TAG, "  Service: %s", service->mName);
+    }
+  }
 }
 
-void srpStartCallback(const otSockAddr *aServerSockAddr, void *aContext) {
+void srpStartCallback(const otSockAddr *aServerSockAddr, void *aContext) { ESP_LOGI(TAG, "SRP client has started"); }
-  ESP_LOGW(TAG, "*** SRP start callback ***");
-}
 
-void OpenThreadComponent::srp_setup_() {
+void OpenThreadSrpComponent::setup() {
   otError error;
   auto lock = InstanceLock::acquire();
   otInstance *instance = lock->get_instance();
@@ -114,7 +119,7 @@ void OpenThreadComponent::srp_setup_() {
 
   error = otSrpClientSetHostName(instance, existing_host_name);
   if (error != 0) {
-    ESP_LOGW(TAG, "Could not set host name with srp server");
+    ESP_LOGW(TAG, "Could not set host name");
     return;
   }
 
@@ -127,6 +132,7 @@ void OpenThreadComponent::srp_setup_() {
   // Copy the mdns services to our local instance so that the c_str pointers remain valid for the lifetime of this
   // component
   this->mdns_services_ = this->mdns_->get_services();
+  ESP_LOGW(TAG, "Setting up SRP services. count = %d\n", this->mdns_services_.size());
   for (const auto &service : this->mdns_services_) {
     otSrpClientBuffersServiceEntry *entry = otSrpClientBuffersAllocateService(instance);
     if (!entry) {
@@ -172,23 +178,22 @@ void OpenThreadComponent::srp_setup_() {
     if (error != OT_ERROR_NONE) {
       ESP_LOGW(TAG, "Failed to add service: %s", otThreadErrorToString(error));
     }
+    ESP_LOGW(TAG, "Added service: %s", full_service.c_str());
   }
 
   otSrpClientEnableAutoStartMode(instance, srpStartCallback, nullptr);
   ESP_LOGW(TAG, "Finished SRP setup **** ");
 }
 
-void *OpenThreadComponent::pool_alloc_(size_t size) {
+void *OpenThreadSrpComponent::pool_alloc_(size_t size) {
   uint8_t *ptr = new uint8_t[size];
-  if (ptr) {
+  this->memory_pool_.emplace_back(std::unique_ptr<uint8_t[]>(ptr));
-    this->memory_pool_.emplace_back(std::unique_ptr<uint8_t[]>(ptr));
-  }
   return ptr;
 }
 
-void OpenThreadComponent::set_host_name(std::string host_name) { this->host_name_ = host_name; }
+void OpenThreadSrpComponent::set_host_name(std::string host_name) { this->host_name_ = host_name; }
 
-void OpenThreadComponent::set_mdns(esphome::mdns::MDNSComponent *mdns) { this->mdns_ = mdns; }
+void OpenThreadSrpComponent::set_mdns(esphome::mdns::MDNSComponent *mdns) { this->mdns_ = mdns; }
 
 }  // namespace openthread
 }  // namespace esphome

esphome/components/openthread/openthread.h

@@ -23,27 +23,35 @@ class OpenThreadComponent : public Component {
   void setup() override;
   float get_setup_priority() const override { return setup_priority::WIFI; }
 
-  void set_host_name(std::string host_name);
-  void set_mdns(esphome::mdns::MDNSComponent *mdns);
   bool is_connected();
   network::IPAddresses get_ip_addresses();
   std::optional<otIp6Address> get_omr_address();
   void ot_main();
 
  protected:
-  void srp_setup_();
   std::optional<otIp6Address> get_omr_address_(std::optional<InstanceLock> &lock);
-  std::string host_name_;
-  void *pool_alloc_(size_t size);
-
- private:
-  esphome::mdns::MDNSComponent *mdns_{nullptr};
-  std::vector<esphome::mdns::MDNSService> mdns_services_;
-  std::vector<std::unique_ptr<uint8_t[]>> memory_pool_;
 };
 
 extern OpenThreadComponent *global_openthread_component;
 
+class OpenThreadSrpComponent : public Component {
+ public:
+  void set_mdns(esphome::mdns::MDNSComponent *mdns);
+  void set_host_name(std::string host_name);
+  // This has to run after the mdns component or else no services are available to advertise
+  float get_setup_priority() const override { return this->mdns_->get_setup_priority() - 1.0; }
+
+ protected:
+  void setup() override;
+
+ private:
+  std::string host_name_;
+  esphome::mdns::MDNSComponent *mdns_{nullptr};
+  std::vector<esphome::mdns::MDNSService> mdns_services_;
+  std::vector<std::unique_ptr<uint8_t[]>> memory_pool_;
+  void *pool_alloc_(size_t size);
+};
+
 class InstanceLock {
  public:
   static std::optional<InstanceLock> try_acquire(int delay);

esphome/components/openthread/openthread_esp.cpp

@@ -48,13 +48,6 @@ void OpenThreadComponent::setup() {
       },
       "ot_main", 10240, this, 5, nullptr);
 
-  // xTaskCreate(
-  //     [](void *arg) {
-  //       static_cast<OpenThreadComponent *>(arg)->srp_setup_();
-  //       vTaskDelete(nullptr);
-  //     },
-  //     "ot_srp_setup", 10240, this, 5, nullptr);
-
   ESP_LOGI(TAG, "OpenThread started");
 }
 
@@ -90,10 +83,9 @@ void OpenThreadComponent::ot_main() {
   };
 
   // Initialize the OpenThread stack
+  // otLoggingSetLevel(OT_LOG_LEVEL_DEBG);
   ESP_ERROR_CHECK(esp_openthread_init(&config));
 
-  this->srp_setup_();
-
 #if CONFIG_OPENTHREAD_STATE_INDICATOR_ENABLE
   ESP_ERROR_CHECK(esp_openthread_state_indicator_init(esp_openthread_get_instance()));
 #endif