[zigbee] cleanup, docstrings, refactor connected state on esp32 (1/3) (#18007)

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

esphome/components/zigbee/zigbee_attribute_esp32.cpp

@@ -9,16 +9,18 @@ namespace esphome::zigbee {
 static const char *const TAG = "zigbee.attribute";
 
 void ZigbeeAttribute::set_attr_() {
-  if (!this->zb_->is_connected()) {
+  if (!this->zb_->is_started()) {
     return;
   }
   if (esp_zigbee_lock_acquire(10 / portTICK_PERIOD_MS)) {
     ezb_zcl_status_t state = ezb_zcl_set_attr_value(this->endpoint_id_, this->cluster_id_, this->role_, this->attr_id_,
                                                     EZB_ZCL_STD_MANUF_CODE, this->value_p_, false);
+    // cleared before report_() so it can disable the loop
+    // when the report has to wait for join
+    this->set_attr_requested_ = false;
     if (this->force_report_) {
       this->report_(true);
     }
-    this->set_attr_requested_ = false;
     // Check for error
     if (state != EZB_ZCL_STATUS_SUCCESS) {
       ESP_LOGE(TAG, "Setting attribute failed, ZCL status: %u", static_cast<unsigned>(state));
@@ -28,7 +30,14 @@ void ZigbeeAttribute::set_attr_() {
 }
 
 void ZigbeeAttribute::report_(bool has_lock) {
-  if (!this->zb_->is_connected() || !this->report_enabled) {
+  if (!this->report_enabled) {
+    return;
+  }
+  if (!this->zb_->is_joined()) {
+    this->report_requested_ = true;
+    if (!this->set_attr_requested_) {
+      this->disable_loop();
+    }
     return;
   }
   if (has_lock or esp_zigbee_lock_acquire(10 / portTICK_PERIOD_MS)) {
@@ -44,6 +53,7 @@ void ZigbeeAttribute::report_(bool has_lock) {
     cmd.payload.attr_id = this->attr_id_;
 
     ezb_zcl_report_attr_cmd_req(&cmd);
+    this->report_requested_ = false;
     if (!has_lock) {
       esp_zigbee_lock_release();
     }
@@ -55,6 +65,11 @@ void ZigbeeAttribute::set_report(ZigbeeReportT report) {
   if (report == ZigbeeReportT::ZIGBEE_REPORT_FORCE) {
     this->force_report_ = true;
   }
+  this->zb_->add_on_join_callback([this](bool) {
+    if (this->report_requested_) {
+      this->enable_loop();
+    }
+  });
 }
 
 void ZigbeeAttribute::loop() {
@@ -62,7 +77,11 @@ void ZigbeeAttribute::loop() {
     this->set_attr_();
   }
 
-  if (!this->set_attr_requested_) {
+  if (this->report_requested_) {
+    this->report_(false);
+  }
+
+  if (!this->report_requested_ && !this->set_attr_requested_) {
     this->disable_loop();
   }
 }

esphome/components/zigbee/zigbee_attribute_esp32.h

@@ -66,6 +66,7 @@ class ZigbeeAttribute final : public Component {
   float scale_;
   void *value_p_{nullptr};
   bool set_attr_requested_{false};
+  bool report_requested_{false};
   bool force_report_{false};
 };
 

esphome/components/zigbee/zigbee_ep_esp32.py

@@ -83,7 +83,7 @@ ep_configs: dict[str, dict[str, Any]] = {
 }
 
 
-def get_next_ep_num(eps: list[int]) -> int:
+def _get_next_ep_num(eps: list[int]) -> int:
     try:
         ep_num = [i for i in range(1, CONF_MAX_EP_NUMBER + 1) if i not in eps][0]
         eps.append(ep_num)
@@ -94,7 +94,7 @@ def get_next_ep_num(eps: list[int]) -> int:
     return ep_num
 
 
-def compare_clusters(
+def _compare_clusters(
     existing_ep: dict[str, Any],
     ep: dict[str, Any],
 ) -> tuple[str | int, str] | None:
@@ -105,12 +105,12 @@ def compare_clusters(
     return None
 
 
-def merge_endpoints(
+def _merge_endpoints(
     existing_ep: dict[str, Any],
     ep: dict[str, Any],
     use_type: bool | None,
 ) -> bool:
-    if compare_clusters(existing_ep, ep):
+    if _compare_clusters(existing_ep, ep):
         return False
     if (
         ep.get(DEVICE_TYPE)
@@ -134,7 +134,12 @@ def merge_endpoints(
     return True
 
 
-def validate_endpoints(ep_dict: dict[int, dict]) -> None:
+def _validate_endpoints(ep_dict: dict[int, dict]) -> None:
+    """Validate endpoint device type selection before endpoint creation.
+
+    This resolves any deferred device type selections stored in CONF_USE_DEVICE_TYPE,
+    ensuring each endpoint has at most one active device type.
+    """
     for num, ep in ep_dict.items():
         types_dict = ep.get(CONF_USE_DEVICE_TYPE)
         if not types_dict:
@@ -157,10 +162,18 @@ def validate_endpoints(ep_dict: dict[int, dict]) -> None:
 
 
 def create_ep(router: bool) -> None:
+    """Finalize Zigbee endpoint creation and normalize endpoint storage.
+
+    Validate endpoints, merge endpoints, and assign numbers to endpoints without an explicit number.
+    This is called from final_validate.
+
+    Args:
+        router: Whether the device is acting as a Zigbee router.
+    """
     zb_data = CORE.data.setdefault(KEY_ZIGBEE, {})
     ep_dict: dict[int, dict] = zb_data.setdefault(KEY_ZIGBEE_EP, {})
     ep_list: list[dict] = zb_data.setdefault(KEY_ZIGBEE_EP_NO_NUM, [])
-    validate_endpoints(ep_dict)
+    _validate_endpoints(ep_dict)
     # create dummy endpoint if list is empty
     if not ep_dict and not ep_list:
         ep_type = "CUSTOM_ATTR"
@@ -173,7 +186,7 @@ def create_ep(router: bool) -> None:
         for ep in ep_list:
             added = False
             for existing_ep in ep_list_new:
-                if merge_endpoints(existing_ep, ep, ep.get(CONF_USE_DEVICE_TYPE)):
+                if _merge_endpoints(existing_ep, ep, ep.get(CONF_USE_DEVICE_TYPE)):
                     added = True
                     break
             if not added:
@@ -182,7 +195,7 @@ def create_ep(router: bool) -> None:
         # Add endpoints with no number to the endpoint dict with a new number
         eps = list(ep_dict.keys())
         for ep in ep_list_new:
-            ep_num = get_next_ep_num(eps)
+            ep_num = _get_next_ep_num(eps)
             ep_dict[ep_num] = ep
 
         # clear list so that it is not processed again
@@ -195,6 +208,14 @@ def create_ep(router: bool) -> None:
 
 
 def add_ep(ep: dict[str, Any], ep_num: int | None, use_type: bool | None) -> None:
+    """Add a Zigbee endpoint configuration to CORE.data.
+
+    Args:
+        ep: Endpoint configuration dictionary.
+        ep_num: Optional explicit endpoint number.
+        use_type: Optional boolean indicating whether this component's device type should be
+        used for the endpoint (True claims it, False drops it, None leaves it as a candidate).
+    """
     zb_data = CORE.data.setdefault(KEY_ZIGBEE, {})
     if use_type is False:
         ep.pop(DEVICE_TYPE, None)
@@ -208,7 +229,7 @@ def add_ep(ep: dict[str, Any], ep_num: int | None, use_type: bool | None) -> Non
         if ep_num in ep_dict:
             # check if the existing endpoint has same clusters
             existing_ep = ep_dict[ep_num]
-            if cl := compare_clusters(
+            if cl := _compare_clusters(
                 existing_ep,
                 ep,
             ):

esphome/components/zigbee/zigbee_esp32.cpp

@@ -53,6 +53,7 @@ bool ZigbeeComponent::app_signal_handler(const ezb_app_signal_t *app_signal) {
   switch (signal_type) {
     case EZB_ZDO_SIGNAL_SKIP_STARTUP:
       ESP_LOGD(TAG, "Zigbee stack initialized");
+      global_zigbee->started = true;
       ezb_bdb_start_top_level_commissioning(EZB_BDB_MODE_INITIALIZATION);
       break;
     case EZB_BDB_SIGNAL_DEVICE_FIRST_START:
@@ -60,7 +61,6 @@ bool ZigbeeComponent::app_signal_handler(const ezb_app_signal_t *app_signal) {
       ezb_bdb_comm_status_t status = *((ezb_bdb_comm_status_t *) ezb_app_signal_get_params(app_signal));
       if (status == EZB_BDB_STATUS_SUCCESS) {
         ESP_LOGD(TAG, "Device started up in %sfactory-reset mode", ezb_bdb_is_factory_new() ? "" : "non ");
-        global_zigbee->started = true;
         if (ezb_bdb_is_factory_new()) {
           global_zigbee->factory_new = true;
           ESP_LOGD(TAG, "Start network steering");
@@ -303,9 +303,10 @@ void ZigbeeComponent::setup() {
 }
 
 void ZigbeeComponent::loop() {
-  if (this->joined.exchange(false)) {
-    this->connected_ = true;
+  if (!this->join_reported_ && this->joined) {
+    this->join_reported_ = true;
     this->join_cb_.call(this->factory_new);
+    this->factory_new = false;
   }
   this->disable_loop();
 }

esphome/components/zigbee/zigbee_esp32.h

@@ -63,8 +63,13 @@ class ZigbeeComponent final : public Component {
   template<typename F> void add_on_join_callback(F &&cb) { this->join_cb_.add(std::forward<F>(cb)); }
 
   bool is_battery_powered() { return this->basic_cluster_data_.power_source == EZB_ZCL_BASIC_POWER_SOURCE_BATTERY; }
+
+  // True after the Zigbee stack has been initialized and the device has started up. Is set before the stack started
+  // network commissioning or has joined a network and won't be reset until the device is rebooted.
   bool is_started() { return this->started; }
-  bool is_connected() { return this->connected_; }
+
+  // True if the device has joined a network and is ready to send and receive messages.
+  bool is_joined() { return this->joined; }
   std::atomic<bool> started = false;
   std::atomic<bool> joined = false;
   std::atomic<bool> factory_new = false;
@@ -76,7 +81,6 @@ class ZigbeeComponent final : public Component {
     uint8_t *date;
     uint8_t power_source;
   } basic_cluster_data_;
-  bool connected_ = false;
 #ifdef CONFIG_ZB_ZED
   ezb_nwk_device_type_t device_role_ = EZB_NWK_DEVICE_TYPE_END_DEVICE;
 #else
@@ -91,6 +95,7 @@ class ZigbeeComponent final : public Component {
   // key tuple could be replaced by single 64 (48) bit int with bit fields for endpoint, cluster, role and attr_id
   std::map<std::tuple<uint8_t, uint16_t, uint8_t, uint16_t>, ZigbeeAttribute *> attributes_;
   ezb_af_device_desc_t dev_desc_;
+  bool join_reported_{false};
   CallbackManager<void(bool)> join_cb_{};
 };