openthread: cleanup

esphome/components/openthread/openthread.cpp

@@ -1,4 +1,5 @@
 #include "esphome/core/defines.h"
+
 #ifdef USE_OPENTHREAD
 #include "openthread.h"
 
@@ -27,7 +28,7 @@ OpenThreadComponent *global_openthread_component = nullptr;
 OpenThreadComponent::OpenThreadComponent() { global_openthread_component = this; }
 
 OpenThreadComponent::~OpenThreadComponent() {
-  auto lock = OpenThreadLockGuard::try_acquire(100);
+  auto lock = InstanceLock::try_acquire(100);
   if (!lock) {
     ESP_LOGW(TAG, "Failed to acquire OpenThread lock in destructor, leaking memory");
     return;
@@ -35,12 +36,11 @@ OpenThreadComponent::~OpenThreadComponent() {
   otInstance *instance = lock->get_instance();
   otSrpClientClearHostAndServices(instance);
   otSrpClientBuffersFreeAllServices(instance);
-
   global_openthread_component = nullptr;
 }
 
 bool OpenThreadComponent::is_connected() {
-  auto lock = OpenThreadLockGuard::try_acquire(100);
+  auto lock = InstanceLock::try_acquire(100);
   if (!lock) {
     ESP_LOGW(TAG, "Failed to acquire OpenThread lock in is_connected");
     return false;
@@ -59,26 +59,21 @@ bool OpenThreadComponent::is_connected() {
 
 // Gets the off-mesh routable address
 std::optional<otIp6Address> OpenThreadComponent::get_omr_address() {
-  auto lock = OpenThreadLockGuard::acquire();
+  auto lock = InstanceLock::acquire();
   return this->get_omr_address_(lock);
 }
 
-std::optional<otIp6Address> OpenThreadComponent::get_omr_address_(std::optional<OpenThreadLockGuard> &lock) {
+std::optional<otIp6Address> OpenThreadComponent::get_omr_address_(std::optional<InstanceLock> &lock) {
   otNetworkDataIterator iterator = OT_NETWORK_DATA_ITERATOR_INIT;
   otInstance *instance = nullptr;
 
   instance = lock->get_instance();
 
   otBorderRouterConfig aConfig;
-  while (otNetDataGetNextOnMeshPrefix(instance, &iterator, &aConfig) != OT_ERROR_NONE) {
-    lock.reset();
-    vTaskDelay(100);
-    lock = OpenThreadLockGuard::try_acquire(portMAX_DELAY);
-    if (!lock) {
-      ESP_LOGW(TAG, "Could not re-acquire lock");
-      return {};
+  if (otNetDataGetNextOnMeshPrefix(instance, &iterator, &aConfig) != OT_ERROR_NONE) {
+    return std::nullopt;
   }
-  };
+
   const otIp6Prefix *omrPrefix = &aConfig.mPrefix;
   const otNetifAddress *unicastAddrs = otIp6GetUnicastAddresses(instance);
   for (const otNetifAddress *addr = unicastAddrs; addr; addr = addr->mNext) {
@@ -87,15 +82,26 @@ std::optional<otIp6Address> OpenThreadComponent::get_omr_address_(std::optional<
       return *localIp;
     }
   }
-  ESP_LOGW(TAG, "Could not find the OMR address");
   return {};
 }
 
+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,
+           aServices, aRemovedServices);
+}
+
+void srpStartCallback(const otSockAddr *aServerSockAddr, void *aContext) {
+  ESP_LOGW(TAG, "*** SRP start callback ***");
+}
+
 void OpenThreadComponent::srp_setup_() {
   otError error;
-  auto lock = OpenThreadLockGuard::acquire();
+  auto lock = InstanceLock::acquire();
   otInstance *instance = lock->get_instance();
 
+  otSrpClientSetCallback(instance, srpCallback, nullptr);
+
   // set the host name
   uint16_t size;
   char *existing_host_name = otSrpClientBuffersGetHostNameString(instance, &size);
@@ -168,7 +174,8 @@ void OpenThreadComponent::srp_setup_() {
     }
   }
 
-  otSrpClientEnableAutoStartMode(instance, nullptr, nullptr);
+  otSrpClientEnableAutoStartMode(instance, srpStartCallback, nullptr);
+  ESP_LOGW(TAG, "Finished SRP setup **** ");
 }
 
 void *OpenThreadComponent::pool_alloc_(size_t size) {

esphome/components/openthread/openthread.h

@@ -14,7 +14,7 @@
 namespace esphome {
 namespace openthread {
 
-class OpenThreadLockGuard;
+class InstanceLock;
 
 class OpenThreadComponent : public Component {
  public:
@@ -32,13 +32,11 @@ class OpenThreadComponent : public Component {
 
  protected:
   void srp_setup_();
-  std::optional<otIp6Address> get_omr_address_(std::optional<OpenThreadLockGuard> &lock);
+  std::optional<otIp6Address> get_omr_address_(std::optional<InstanceLock> &lock);
   std::string host_name_;
   void *pool_alloc_(size_t size);
 
  private:
-  // void platform_init();
-
   esphome::mdns::MDNSComponent *mdns_{nullptr};
   std::vector<esphome::mdns::MDNSService> mdns_services_;
   std::vector<std::unique_ptr<uint8_t[]>> memory_pool_;
@@ -46,11 +44,11 @@ class OpenThreadComponent : public Component {
 
 extern OpenThreadComponent *global_openthread_component;
 
-class OpenThreadLockGuard {
+class InstanceLock {
  public:
-  static std::optional<OpenThreadLockGuard> try_acquire(int delay);
-  static std::optional<OpenThreadLockGuard> acquire();
-  ~OpenThreadLockGuard();
+  static std::optional<InstanceLock> try_acquire(int delay);
+  static std::optional<InstanceLock> acquire();
+  ~InstanceLock();
 
   // Returns the global openthread instance guarded by this lock
   otInstance *get_instance();
@@ -58,7 +56,7 @@ class OpenThreadLockGuard {
  private:
   // Use a private constructor in order to force thehandling
   // of acquisition failure
-  OpenThreadLockGuard() {}
+  InstanceLock() {}
 };
 
 }  // namespace openthread

esphome/components/openthread/openthread_esp.cpp

@@ -48,12 +48,12 @@ 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);
+  // xTaskCreate(
+  //     [](void *arg) {
+  //       static_cast<OpenThreadComponent *>(arg)->srp_setup_();
+  //       vTaskDelete(nullptr);
+  //     },
+  //     "ot_srp_setup", 10240, this, 5, nullptr);
 
   ESP_LOGI(TAG, "OpenThread started");
 }
@@ -92,6 +92,8 @@ void OpenThreadComponent::ot_main() {
   // Initialize the OpenThread stack
   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
@@ -132,9 +134,6 @@ void OpenThreadComponent::ot_main() {
   esp_vfs_eventfd_unregister();
 }
 
-// TODO: This gets used by mqtt in order to register the device's IP. Likely it doesn't
-// make sense to return thread-local addresses, since they can't be reached from outside the thread network.
-// It could make more sense to return the off-mesh-routable address instead.
 network::IPAddresses OpenThreadComponent::get_ip_addresses() {
   network::IPAddresses addresses;
   struct esp_ip6_addr if_ip6s[CONFIG_LWIP_IPV6_NUM_ADDRESSES];
@@ -148,23 +147,23 @@ network::IPAddresses OpenThreadComponent::get_ip_addresses() {
   return addresses;
 }
 
-std::optional<OpenThreadLockGuard> OpenThreadLockGuard::try_acquire(int delay) {
+std::optional<InstanceLock> InstanceLock::try_acquire(int delay) {
   if (esp_openthread_lock_acquire(delay)) {
-    return OpenThreadLockGuard();
+    return InstanceLock();
   }
   return {};
 }
 
-std::optional<OpenThreadLockGuard> OpenThreadLockGuard::acquire() {
+std::optional<InstanceLock> InstanceLock::acquire() {
   while (!esp_openthread_lock_acquire(100)) {
     esp_task_wdt_reset();
   }
-  return OpenThreadLockGuard();
+  return InstanceLock();
 }
 
-otInstance *OpenThreadLockGuard::get_instance() { return esp_openthread_get_instance(); }
+otInstance *InstanceLock::get_instance() { return esp_openthread_get_instance(); }
 
-OpenThreadLockGuard::~OpenThreadLockGuard() { esp_openthread_lock_release(); }
+InstanceLock::~InstanceLock() { esp_openthread_lock_release(); }
 
 }  // namespace openthread
 }  // namespace esphome

esphome/components/openthread_info/openthread_info_text_sensor.h

@@ -8,12 +8,12 @@
 namespace esphome {
 namespace openthread_info {
 
-using esphome::openthread::OpenThreadLockGuard;
+using esphome::openthread::InstanceLock;
 
 class OpenThreadInstancePollingComponent : public PollingComponent {
  public:
   void update() override {
-    auto lock = OpenThreadLockGuard::try_acquire(100);
+    auto lock = InstanceLock::try_acquire(10);
     if (!lock) {
       return;
     }
@@ -93,7 +93,7 @@ class ExtAddrOpenThreadInfo : public OpenThreadInstancePollingComponent, public
     auto extaddr = otLinkGetExtendedAddress(instance);
     if (!std::equal(this->last_extaddr_.begin(), this->last_extaddr_.end(), extaddr->m8)) {
       std::copy(extaddr->m8, extaddr->m8 + 8, this->last_extaddr_.begin());
-      this->publish_state(format_hex_pretty(extaddr->m8, 8));
+      this->publish_state(format_hex(extaddr->m8, 8));
     }
   }
   float get_setup_priority() const override { return setup_priority::AFTER_WIFI; }
@@ -112,7 +112,7 @@ class Eui64OpenThreadInfo : public OpenThreadInstancePollingComponent, public te
 
     if (!std::equal(this->last_eui64_.begin(), this->last_eui64_.end(), addr.m8)) {
       std::copy(addr.m8, addr.m8 + 8, this->last_eui64_.begin());
-      this->publish_state(format_hex_pretty(this->last_eui64_.begin(), 8));
+      this->publish_state(format_hex(this->last_eui64_.begin(), 8));
     }
   }
   float get_setup_priority() const override { return setup_priority::AFTER_WIFI; }
@@ -176,7 +176,7 @@ class NetworkKeyOpenThreadInfo : public DatasetOpenThreadInfo, public text_senso
   void update_dataset_(otOperationalDataset *dataset) override {
     if (!std::equal(this->last_key_.begin(), this->last_key_.end(), dataset->mNetworkKey.m8)) {
       std::copy(dataset->mNetworkKey.m8, dataset->mNetworkKey.m8 + 16, this->last_key_.begin());
-      this->publish_state(format_hex_pretty(dataset->mNetworkKey.m8, 16));
+      this->publish_state(format_hex(dataset->mNetworkKey.m8, 16));
     }
   }
   float get_setup_priority() const override { return setup_priority::AFTER_WIFI; }
@@ -211,7 +211,7 @@ class ExtPanIdOpenThreadInfo : public DatasetOpenThreadInfo, public text_sensor:
   void update_dataset_(otOperationalDataset *dataset) override {
     if (!std::equal(this->last_extpanid_.begin(), this->last_extpanid_.end(), dataset->mExtendedPanId.m8)) {
       std::copy(dataset->mExtendedPanId.m8, dataset->mExtendedPanId.m8 + 8, this->last_extpanid_.begin());
-      this->publish_state(format_hex_pretty(this->last_extpanid_.begin(), 8));
+      this->publish_state(format_hex(this->last_extpanid_.begin(), 8));
     }
   }