From 2d5d34d33f40a05bf4d97d4fb21793f44490f02b Mon Sep 17 00:00:00 2001 From: luar123 <49960470+luar123@users.noreply.github.com> Date: Wed, 5 Aug 2026 16:19:57 +0200 Subject: [PATCH] [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> --- .../zigbee/zigbee_attribute_esp32.cpp | 27 +++++++++++-- .../zigbee/zigbee_attribute_esp32.h | 1 + esphome/components/zigbee/zigbee_ep_esp32.py | 39 ++++++++++++++----- esphome/components/zigbee/zigbee_esp32.cpp | 7 ++-- esphome/components/zigbee/zigbee_esp32.h | 9 ++++- 5 files changed, 65 insertions(+), 18 deletions(-) diff --git a/esphome/components/zigbee/zigbee_attribute_esp32.cpp b/esphome/components/zigbee/zigbee_attribute_esp32.cpp index d7176e6ca5..1fb8d1abe4 100644 --- a/esphome/components/zigbee/zigbee_attribute_esp32.cpp +++ b/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(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(); } } diff --git a/esphome/components/zigbee/zigbee_attribute_esp32.h b/esphome/components/zigbee/zigbee_attribute_esp32.h index e5f8c8b1cf..fc229b4e95 100644 --- a/esphome/components/zigbee/zigbee_attribute_esp32.h +++ b/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}; }; diff --git a/esphome/components/zigbee/zigbee_ep_esp32.py b/esphome/components/zigbee/zigbee_ep_esp32.py index 2ed3dddb67..c2001c66d6 100644 --- a/esphome/components/zigbee/zigbee_ep_esp32.py +++ b/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, ): diff --git a/esphome/components/zigbee/zigbee_esp32.cpp b/esphome/components/zigbee/zigbee_esp32.cpp index 3e0f6cd745..dcfdf2f3e1 100644 --- a/esphome/components/zigbee/zigbee_esp32.cpp +++ b/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(); } diff --git a/esphome/components/zigbee/zigbee_esp32.h b/esphome/components/zigbee/zigbee_esp32.h index f4bafac294..986ffea449 100644 --- a/esphome/components/zigbee/zigbee_esp32.h +++ b/esphome/components/zigbee/zigbee_esp32.h @@ -63,8 +63,13 @@ class ZigbeeComponent final : public Component { template void add_on_join_callback(F &&cb) { this->join_cb_.add(std::forward(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 started = false; std::atomic joined = false; std::atomic 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, ZigbeeAttribute *> attributes_; ezb_af_device_desc_t dev_desc_; + bool join_reported_{false}; CallbackManager join_cb_{}; };