From 4211b57a84b5e2d3bed6c39c3136e61957b631e9 Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Fri, 28 Mar 2025 10:35:33 -0300 Subject: [PATCH 01/11] fix(zigbee): memory leak issue with malloc --- libraries/Zigbee/src/ZigbeeEP.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/libraries/Zigbee/src/ZigbeeEP.cpp b/libraries/Zigbee/src/ZigbeeEP.cpp index e7d507dc441..66951bb718d 100644 --- a/libraries/Zigbee/src/ZigbeeEP.cpp +++ b/libraries/Zigbee/src/ZigbeeEP.cpp @@ -40,9 +40,9 @@ bool ZigbeeEP::setManufacturerAndModel(const char *name, const char *model) { log_e("Manufacturer or model name is too long"); return false; } - // Allocate a new array of size length + 2 (1 for the length, 1 for null terminator) - char *zb_name = new char[name_length + 2]; - char *zb_model = new char[model_length + 2]; + // Allocate an array of size length + 2 (1 for the length, 1 for null terminator) + char zb_name[name_length + 2]; + char zb_model[model_length + 2]; // Store the length as the first element zb_name[0] = static_cast(name_length); // Cast size_t to char zb_model[0] = static_cast(model_length); @@ -63,8 +63,6 @@ bool ZigbeeEP::setManufacturerAndModel(const char *name, const char *model) { if (ret_model != ESP_OK) { log_e("Failed to set model: 0x%x: %s", ret_model, esp_err_to_name(ret_model)); } - delete[] zb_name; - delete[] zb_model; return ret_name == ESP_OK && ret_model == ESP_OK; } @@ -245,7 +243,7 @@ void ZigbeeEP::zbReadBasicCluster(const esp_zb_zcl_attribute_t *attribute) { /* Basic cluster attributes */ if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID && attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && attribute->data.value) { zbstring_t *zbstr = (zbstring_t *)attribute->data.value; - char *string = (char *)malloc(zbstr->len + 1); + char string[zbstr->len + 1]; memcpy(string, zbstr->data, zbstr->len); string[zbstr->len] = '\0'; log_i("Peer Manufacturer is \"%s\"", string); @@ -254,7 +252,7 @@ void ZigbeeEP::zbReadBasicCluster(const esp_zb_zcl_attribute_t *attribute) { } if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID && attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && attribute->data.value) { zbstring_t *zbstr = (zbstring_t *)attribute->data.value; - char *string = (char *)malloc(zbstr->len + 1); + char string[zbstr->len + 1]; memcpy(string, zbstr->data, zbstr->len); string[zbstr->len] = '\0'; log_i("Peer Model is \"%s\"", string); From cb581f3824875b6c6a39346d71ffa820032332c1 Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Fri, 28 Mar 2025 11:03:09 -0300 Subject: [PATCH 02/11] fix(zigbee): test basic_cluster value for null to avoid crash --- libraries/Zigbee/src/ZigbeeEP.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libraries/Zigbee/src/ZigbeeEP.cpp b/libraries/Zigbee/src/ZigbeeEP.cpp index 66951bb718d..ff91034a90f 100644 --- a/libraries/Zigbee/src/ZigbeeEP.cpp +++ b/libraries/Zigbee/src/ZigbeeEP.cpp @@ -55,6 +55,10 @@ bool ZigbeeEP::setManufacturerAndModel(const char *name, const char *model) { // Get the basic cluster and update the manufacturer and model attributes esp_zb_attribute_list_t *basic_cluster = esp_zb_cluster_list_get_cluster(_cluster_list, ESP_ZB_ZCL_CLUSTER_ID_BASIC, ESP_ZB_ZCL_CLUSTER_SERVER_ROLE); + if (basic_cluster == nullptr) { + log_e("Failed to get basic cluster"); + return false; + } esp_err_t ret_name = esp_zb_basic_cluster_add_attr(basic_cluster, ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID, (void *)zb_name); if (ret_name != ESP_OK) { log_e("Failed to set manufacturer: 0x%x: %s", ret_name, esp_err_to_name(ret_name)); From 031f1176c6d5d06875eb40b059924d307333986f Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Fri, 28 Mar 2025 12:04:19 -0300 Subject: [PATCH 03/11] feat(zigbee): use stack memory for name and model --- libraries/Zigbee/src/ZigbeeEP.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/libraries/Zigbee/src/ZigbeeEP.cpp b/libraries/Zigbee/src/ZigbeeEP.cpp index ff91034a90f..36f106b0504 100644 --- a/libraries/Zigbee/src/ZigbeeEP.cpp +++ b/libraries/Zigbee/src/ZigbeeEP.cpp @@ -33,16 +33,18 @@ void ZigbeeEP::setVersion(uint8_t version) { } bool ZigbeeEP::setManufacturerAndModel(const char *name, const char *model) { + constexpr size_t ZB_MAX_NAME_LENGTH = 32; + // Convert manufacturer to ZCL string size_t name_length = strlen(name); size_t model_length = strlen(model); - if (name_length > 32 || model_length > 32) { + if (name_length > ZB_MAX_NAME_LENGTH || model_length > ZB_MAX_NAME_LENGTH) { log_e("Manufacturer or model name is too long"); return false; } // Allocate an array of size length + 2 (1 for the length, 1 for null terminator) - char zb_name[name_length + 2]; - char zb_model[model_length + 2]; + char zb_name[ZB_MAX_NAME_LENGTH + 2]; + char zb_model[ZB_MAX_NAME_LENGTH + 2]; // Store the length as the first element zb_name[0] = static_cast(name_length); // Cast size_t to char zb_model[0] = static_cast(model_length); From f921e2a18c6c73632ea3fae0f34bd4e027889f1f Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Fri, 28 Mar 2025 12:22:40 -0300 Subject: [PATCH 04/11] feat(zigbee): use std::vector for memory allocation and explicit initalization --- libraries/Zigbee/src/ZigbeeEP.cpp | 254 +++++++++++++++++------------- 1 file changed, 141 insertions(+), 113 deletions(-) diff --git a/libraries/Zigbee/src/ZigbeeEP.cpp b/libraries/Zigbee/src/ZigbeeEP.cpp index 36f106b0504..dbec77e60ef 100644 --- a/libraries/Zigbee/src/ZigbeeEP.cpp +++ b/libraries/Zigbee/src/ZigbeeEP.cpp @@ -19,6 +19,9 @@ ZigbeeEP::ZigbeeEP(uint8_t endpoint) { _ep_config.endpoint = 0; _cluster_list = nullptr; _on_identify = nullptr; + _read_model = nullptr; // Explicitly initialize here + _read_manufacturer = nullptr; // Explicitly initialize here + _time_status = 0; if (!lock) { lock = xSemaphoreCreateBinary(); @@ -33,43 +36,51 @@ void ZigbeeEP::setVersion(uint8_t version) { } bool ZigbeeEP::setManufacturerAndModel(const char *name, const char *model) { - constexpr size_t ZB_MAX_NAME_LENGTH = 32; + constexpr size_t ZB_MAX_NAME_LENGTH = 32; + + // Validate input lengths + size_t name_length = strlen(name); + size_t model_length = strlen(model); + if (name_length > ZB_MAX_NAME_LENGTH || model_length > ZB_MAX_NAME_LENGTH) { + log_e("Manufacturer or model name is too long"); + return false; + } - // Convert manufacturer to ZCL string - size_t name_length = strlen(name); - size_t model_length = strlen(model); - if (name_length > ZB_MAX_NAME_LENGTH || model_length > ZB_MAX_NAME_LENGTH) { - log_e("Manufacturer or model name is too long"); - return false; - } - // Allocate an array of size length + 2 (1 for the length, 1 for null terminator) - char zb_name[ZB_MAX_NAME_LENGTH + 2]; - char zb_model[ZB_MAX_NAME_LENGTH + 2]; - // Store the length as the first element - zb_name[0] = static_cast(name_length); // Cast size_t to char - zb_model[0] = static_cast(model_length); - // Use memcpy to copy the characters to the result array - memcpy(zb_name + 1, name, name_length); - memcpy(zb_model + 1, model, model_length); - // Null-terminate the array - zb_name[name_length + 1] = '\0'; - zb_model[model_length + 1] = '\0'; - - // Get the basic cluster and update the manufacturer and model attributes - esp_zb_attribute_list_t *basic_cluster = esp_zb_cluster_list_get_cluster(_cluster_list, ESP_ZB_ZCL_CLUSTER_ID_BASIC, ESP_ZB_ZCL_CLUSTER_SERVER_ROLE); - if (basic_cluster == nullptr) { - log_e("Failed to get basic cluster"); - return false; - } - esp_err_t ret_name = esp_zb_basic_cluster_add_attr(basic_cluster, ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID, (void *)zb_name); - if (ret_name != ESP_OK) { - log_e("Failed to set manufacturer: 0x%x: %s", ret_name, esp_err_to_name(ret_name)); - } - esp_err_t ret_model = esp_zb_basic_cluster_add_attr(basic_cluster, ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID, (void *)zb_model); - if (ret_model != ESP_OK) { - log_e("Failed to set model: 0x%x: %s", ret_model, esp_err_to_name(ret_model)); - } - return ret_name == ESP_OK && ret_model == ESP_OK; + // Use std::vector for dynamic memory management - free() is for free when out of scope + std::vector zb_name(name_length + 2); // +2 for length byte and null terminator + std::vector zb_model(model_length + 2); + + // Convert manufacturer to ZCL string + zb_name[0] = static_cast(name_length); // Store the length as the first element + memcpy(zb_name.data() + 1, name, name_length); + zb_name[name_length + 1] = '\0'; // Null-terminate the array + + // Convert model to ZCL string + zb_model[0] = static_cast(model_length); + memcpy(zb_model.data() + 1, model, model_length); + zb_model[model_length + 1] = '\0'; + + // Get the basic cluster and update the manufacturer and model attributes + esp_zb_attribute_list_t *basic_cluster = esp_zb_cluster_list_get_cluster( + _cluster_list, ESP_ZB_ZCL_CLUSTER_ID_BASIC, ESP_ZB_ZCL_CLUSTER_SERVER_ROLE); + if (!basic_cluster) { + log_e("Failed to get basic cluster"); + return false; + } + + esp_err_t ret_name = esp_zb_basic_cluster_add_attr( + basic_cluster, ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID, zb_name.data()); + if (ret_name != ESP_OK) { + log_e("Failed to set manufacturer: 0x%x: %s", ret_name, esp_err_to_name(ret_name)); + } + + esp_err_t ret_model = esp_zb_basic_cluster_add_attr( + basic_cluster, ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID, zb_model.data()); + if (ret_model != ESP_OK) { + log_e("Failed to set model: 0x%x: %s", ret_model, esp_err_to_name(ret_model)); + } + + return ret_name == ESP_OK && ret_model == ESP_OK; } bool ZigbeeEP::setPowerSource(zb_power_source_t power_source, uint8_t battery_percentage) { @@ -146,79 +157,77 @@ bool ZigbeeEP::reportBatteryPercentage() { } char *ZigbeeEP::readManufacturer(uint8_t endpoint, uint16_t short_addr, esp_zb_ieee_addr_t ieee_addr) { - /* Read peer Manufacture Name & Model Identifier */ - esp_zb_zcl_read_attr_cmd_t read_req; - - if (short_addr != 0) { - read_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT; - read_req.zcl_basic_cmd.dst_addr_u.addr_short = short_addr; - } else { - read_req.address_mode = ESP_ZB_APS_ADDR_MODE_64_ENDP_PRESENT; - memcpy(read_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t)); - } + /* Read peer Manufacturer Name */ + esp_zb_zcl_read_attr_cmd_t read_req; + + if (short_addr != 0) { + read_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT; + read_req.zcl_basic_cmd.dst_addr_u.addr_short = short_addr; + } else { + read_req.address_mode = ESP_ZB_APS_ADDR_MODE_64_ENDP_PRESENT; + memcpy(read_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t)); + } - read_req.zcl_basic_cmd.src_endpoint = _endpoint; - read_req.zcl_basic_cmd.dst_endpoint = endpoint; - read_req.clusterID = ESP_ZB_ZCL_CLUSTER_ID_BASIC; + read_req.zcl_basic_cmd.src_endpoint = _endpoint; + read_req.zcl_basic_cmd.dst_endpoint = endpoint; + read_req.clusterID = ESP_ZB_ZCL_CLUSTER_ID_BASIC; - uint16_t attributes[] = { - ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID, - }; - read_req.attr_number = ZB_ARRAY_LENTH(attributes); - read_req.attr_field = attributes; + uint16_t attributes[] = { + ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID, + }; + read_req.attr_number = ZB_ARRAY_LENTH(attributes); + read_req.attr_field = attributes; - if (_read_manufacturer != nullptr) { + // Free previously allocated memory for _read_manufacturer free(_read_manufacturer); - } - _read_manufacturer = nullptr; + _read_manufacturer = nullptr; - esp_zb_lock_acquire(portMAX_DELAY); - esp_zb_zcl_read_attr_cmd_req(&read_req); - esp_zb_lock_release(); + esp_zb_lock_acquire(portMAX_DELAY); + esp_zb_zcl_read_attr_cmd_req(&read_req); + esp_zb_lock_release(); - //Wait for response or timeout - if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) { - log_e("Error while reading manufacturer"); - } - return _read_manufacturer; + // Wait for response or timeout + if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) { + log_e("Error while reading manufacturer"); + } + return _read_manufacturer; } char *ZigbeeEP::readModel(uint8_t endpoint, uint16_t short_addr, esp_zb_ieee_addr_t ieee_addr) { - /* Read peer Manufacture Name & Model Identifier */ - esp_zb_zcl_read_attr_cmd_t read_req; + /* Read peer Model Identifier */ + esp_zb_zcl_read_attr_cmd_t read_req; + + if (short_addr != 0) { + read_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT; + read_req.zcl_basic_cmd.dst_addr_u.addr_short = short_addr; + } else { + read_req.address_mode = ESP_ZB_APS_ADDR_MODE_64_ENDP_PRESENT; + memcpy(read_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t)); + } - if (short_addr != 0) { - read_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT; - read_req.zcl_basic_cmd.dst_addr_u.addr_short = short_addr; - } else { - read_req.address_mode = ESP_ZB_APS_ADDR_MODE_64_ENDP_PRESENT; - memcpy(read_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t)); - } + read_req.zcl_basic_cmd.src_endpoint = _endpoint; + read_req.zcl_basic_cmd.dst_endpoint = endpoint; + read_req.clusterID = ESP_ZB_ZCL_CLUSTER_ID_BASIC; - read_req.zcl_basic_cmd.src_endpoint = _endpoint; - read_req.zcl_basic_cmd.dst_endpoint = endpoint; - read_req.clusterID = ESP_ZB_ZCL_CLUSTER_ID_BASIC; + uint16_t attributes[] = { + ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID, + }; + read_req.attr_number = ZB_ARRAY_LENTH(attributes); + read_req.attr_field = attributes; - uint16_t attributes[] = { - ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID, - }; - read_req.attr_number = ZB_ARRAY_LENTH(attributes); - read_req.attr_field = attributes; - - if (_read_model != nullptr) { + // Free previously allocated memory for _read_model free(_read_model); - } - _read_model = nullptr; + _read_model = nullptr; - esp_zb_lock_acquire(portMAX_DELAY); - esp_zb_zcl_read_attr_cmd_req(&read_req); - esp_zb_lock_release(); + esp_zb_lock_acquire(portMAX_DELAY); + esp_zb_zcl_read_attr_cmd_req(&read_req); + esp_zb_lock_release(); - //Wait for response or timeout - if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) { - log_e("Error while reading model"); - } - return _read_model; + // Wait for response or timeout + if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) { + log_e("Error while reading model"); + } + return _read_model; } void ZigbeeEP::printBoundDevices() { @@ -246,25 +255,44 @@ void ZigbeeEP::printBoundDevices(Print &print) { } void ZigbeeEP::zbReadBasicCluster(const esp_zb_zcl_attribute_t *attribute) { - /* Basic cluster attributes */ - if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID && attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && attribute->data.value) { - zbstring_t *zbstr = (zbstring_t *)attribute->data.value; - char string[zbstr->len + 1]; - memcpy(string, zbstr->data, zbstr->len); - string[zbstr->len] = '\0'; - log_i("Peer Manufacturer is \"%s\"", string); - _read_manufacturer = string; - xSemaphoreGive(lock); - } - if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID && attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && attribute->data.value) { - zbstring_t *zbstr = (zbstring_t *)attribute->data.value; - char string[zbstr->len + 1]; - memcpy(string, zbstr->data, zbstr->len); - string[zbstr->len] = '\0'; - log_i("Peer Model is \"%s\"", string); - _read_model = string; - xSemaphoreGive(lock); - } + /* Basic cluster attributes */ + if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID && + attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && + attribute->data.value) { + zbstring_t *zbstr = (zbstring_t *)attribute->data.value; + + // Use std::vector for automatic memory management + std::vector string(zbstr->len + 1); // Allocate space for the string and null terminator + memcpy(string.data(), zbstr->data, zbstr->len); + string[zbstr->len] = '\0'; // Null-terminate the string + + log_i("Peer Manufacturer is \"%s\"", string.data()); + + // Update _read_manufacturer with a dynamically allocated copy + free(_read_manufacturer); // Free any previously allocated memory + _read_manufacturer = strdup(string.data()); // Duplicate the string for persistent storage + + xSemaphoreGive(lock); + } + + if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID && + attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && + attribute->data.value) { + zbstring_t *zbstr = (zbstring_t *)attribute->data.value; + + // Use std::vector for automatic memory management + std::vector string(zbstr->len + 1); // Allocate space for the string and null terminator + memcpy(string.data(), zbstr->data, zbstr->len); + string[zbstr->len] = '\0'; // Null-terminate the string + + log_i("Peer Model is \"%s\"", string.data()); + + // Update _read_model with a dynamically allocated copy + free(_read_model); // Free any previously allocated memory + _read_model = strdup(string.data()); // Duplicate the string for persistent storage + + xSemaphoreGive(lock); + } } void ZigbeeEP::zbIdentify(const esp_zb_zcl_set_attr_value_message_t *message) { From 7d8cb69b17d1679a992fd2245fa20f0f00b64fda Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Fri, 28 Mar 2025 12:49:32 -0300 Subject: [PATCH 05/11] fix(zigbee): using std::vector and fix memory leak --- libraries/Zigbee/src/ZigbeeEP.cpp | 254 +++++++++++++----------------- 1 file changed, 113 insertions(+), 141 deletions(-) diff --git a/libraries/Zigbee/src/ZigbeeEP.cpp b/libraries/Zigbee/src/ZigbeeEP.cpp index dbec77e60ef..167fcbd6f6d 100644 --- a/libraries/Zigbee/src/ZigbeeEP.cpp +++ b/libraries/Zigbee/src/ZigbeeEP.cpp @@ -36,51 +36,42 @@ void ZigbeeEP::setVersion(uint8_t version) { } bool ZigbeeEP::setManufacturerAndModel(const char *name, const char *model) { - constexpr size_t ZB_MAX_NAME_LENGTH = 32; - - // Validate input lengths - size_t name_length = strlen(name); - size_t model_length = strlen(model); - if (name_length > ZB_MAX_NAME_LENGTH || model_length > ZB_MAX_NAME_LENGTH) { - log_e("Manufacturer or model name is too long"); - return false; - } - - // Use std::vector for dynamic memory management - free() is for free when out of scope - std::vector zb_name(name_length + 2); // +2 for length byte and null terminator - std::vector zb_model(model_length + 2); - - // Convert manufacturer to ZCL string - zb_name[0] = static_cast(name_length); // Store the length as the first element - memcpy(zb_name.data() + 1, name, name_length); - zb_name[name_length + 1] = '\0'; // Null-terminate the array - - // Convert model to ZCL string - zb_model[0] = static_cast(model_length); - memcpy(zb_model.data() + 1, model, model_length); - zb_model[model_length + 1] = '\0'; - - // Get the basic cluster and update the manufacturer and model attributes - esp_zb_attribute_list_t *basic_cluster = esp_zb_cluster_list_get_cluster( - _cluster_list, ESP_ZB_ZCL_CLUSTER_ID_BASIC, ESP_ZB_ZCL_CLUSTER_SERVER_ROLE); - if (!basic_cluster) { - log_e("Failed to get basic cluster"); - return false; - } - - esp_err_t ret_name = esp_zb_basic_cluster_add_attr( - basic_cluster, ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID, zb_name.data()); - if (ret_name != ESP_OK) { - log_e("Failed to set manufacturer: 0x%x: %s", ret_name, esp_err_to_name(ret_name)); - } - - esp_err_t ret_model = esp_zb_basic_cluster_add_attr( - basic_cluster, ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID, zb_model.data()); - if (ret_model != ESP_OK) { - log_e("Failed to set model: 0x%x: %s", ret_model, esp_err_to_name(ret_model)); - } - - return ret_name == ESP_OK && ret_model == ESP_OK; + constexpr size_t ZB_MAX_NAME_LENGTH = 32; + // Convert manufacturer to ZCL string + size_t name_length = strlen(name); + size_t model_length = strlen(model); + if (name_length > ZB_MAX_NAME_LENGTH || model_length > ZB_MAX_NAME_LENGTH) { + log_e("Manufacturer or model name is too long"); + return false; + } + // Allocate a new array of size length + 2 (1 for the length, 1 for null terminator) + std::vector zb_name(name_length + 2); // +2 for length byte and null terminator + std::vector zb_model(model_length + 2); + // Store the length as the first element + zb_name[0] = static_cast(name_length); // Cast size_t to char + zb_model[0] = static_cast(model_length); + // Use memcpy to copy the characters to the result array + memcpy(zb_name.data() + 1, name, name_length); + memcpy(zb_model.data() + 1, model, model_length); + // Null-terminate the array + zb_name[name_length + 1] = '\0'; + zb_model[model_length + 1] = '\0'; + + // Get the basic cluster and update the manufacturer and model attributes + esp_zb_attribute_list_t *basic_cluster = esp_zb_cluster_list_get_cluster(_cluster_list, ESP_ZB_ZCL_CLUSTER_ID_BASIC, ESP_ZB_ZCL_CLUSTER_SERVER_ROLE); + if (!basic_cluster) { + log_e("Failed to get basic cluster"); + return false; + } + esp_err_t ret_name = esp_zb_basic_cluster_add_attr(basic_cluster, ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID, (void *)zb_name.data()); + if (ret_name != ESP_OK) { + log_e("Failed to set manufacturer: 0x%x: %s", ret_name, esp_err_to_name(ret_name)); + } + esp_err_t ret_model = esp_zb_basic_cluster_add_attr(basic_cluster, ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID, (void *)zb_model.data()); + if (ret_model != ESP_OK) { + log_e("Failed to set model: 0x%x: %s", ret_model, esp_err_to_name(ret_model)); + } + return ret_name == ESP_OK && ret_model == ESP_OK; } bool ZigbeeEP::setPowerSource(zb_power_source_t power_source, uint8_t battery_percentage) { @@ -157,77 +148,75 @@ bool ZigbeeEP::reportBatteryPercentage() { } char *ZigbeeEP::readManufacturer(uint8_t endpoint, uint16_t short_addr, esp_zb_ieee_addr_t ieee_addr) { - /* Read peer Manufacturer Name */ - esp_zb_zcl_read_attr_cmd_t read_req; - - if (short_addr != 0) { - read_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT; - read_req.zcl_basic_cmd.dst_addr_u.addr_short = short_addr; - } else { - read_req.address_mode = ESP_ZB_APS_ADDR_MODE_64_ENDP_PRESENT; - memcpy(read_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t)); - } + /* Read peer Manufacture Name & Model Identifier */ + esp_zb_zcl_read_attr_cmd_t read_req; + + if (short_addr != 0) { + read_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT; + read_req.zcl_basic_cmd.dst_addr_u.addr_short = short_addr; + } else { + read_req.address_mode = ESP_ZB_APS_ADDR_MODE_64_ENDP_PRESENT; + memcpy(read_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t)); + } - read_req.zcl_basic_cmd.src_endpoint = _endpoint; - read_req.zcl_basic_cmd.dst_endpoint = endpoint; - read_req.clusterID = ESP_ZB_ZCL_CLUSTER_ID_BASIC; + read_req.zcl_basic_cmd.src_endpoint = _endpoint; + read_req.zcl_basic_cmd.dst_endpoint = endpoint; + read_req.clusterID = ESP_ZB_ZCL_CLUSTER_ID_BASIC; - uint16_t attributes[] = { - ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID, - }; - read_req.attr_number = ZB_ARRAY_LENTH(attributes); - read_req.attr_field = attributes; + uint16_t attributes[] = { + ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID, + }; + read_req.attr_number = ZB_ARRAY_LENTH(attributes); + read_req.attr_field = attributes; - // Free previously allocated memory for _read_manufacturer - free(_read_manufacturer); - _read_manufacturer = nullptr; + free(_read_manufacturer); // CPP tests it for nullptr + _read_manufacturer = nullptr; - esp_zb_lock_acquire(portMAX_DELAY); - esp_zb_zcl_read_attr_cmd_req(&read_req); - esp_zb_lock_release(); + esp_zb_lock_acquire(portMAX_DELAY); + esp_zb_zcl_read_attr_cmd_req(&read_req); + esp_zb_lock_release(); - // Wait for response or timeout - if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) { - log_e("Error while reading manufacturer"); - } - return _read_manufacturer; + //Wait for response or timeout + if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) { + log_e("Error while reading manufacturer"); + } + return _read_manufacturer; } char *ZigbeeEP::readModel(uint8_t endpoint, uint16_t short_addr, esp_zb_ieee_addr_t ieee_addr) { - /* Read peer Model Identifier */ - esp_zb_zcl_read_attr_cmd_t read_req; - - if (short_addr != 0) { - read_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT; - read_req.zcl_basic_cmd.dst_addr_u.addr_short = short_addr; - } else { - read_req.address_mode = ESP_ZB_APS_ADDR_MODE_64_ENDP_PRESENT; - memcpy(read_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t)); - } + /* Read peer Manufacture Name & Model Identifier */ + esp_zb_zcl_read_attr_cmd_t read_req; - read_req.zcl_basic_cmd.src_endpoint = _endpoint; - read_req.zcl_basic_cmd.dst_endpoint = endpoint; - read_req.clusterID = ESP_ZB_ZCL_CLUSTER_ID_BASIC; + if (short_addr != 0) { + read_req.address_mode = ESP_ZB_APS_ADDR_MODE_16_ENDP_PRESENT; + read_req.zcl_basic_cmd.dst_addr_u.addr_short = short_addr; + } else { + read_req.address_mode = ESP_ZB_APS_ADDR_MODE_64_ENDP_PRESENT; + memcpy(read_req.zcl_basic_cmd.dst_addr_u.addr_long, ieee_addr, sizeof(esp_zb_ieee_addr_t)); + } + + read_req.zcl_basic_cmd.src_endpoint = _endpoint; + read_req.zcl_basic_cmd.dst_endpoint = endpoint; + read_req.clusterID = ESP_ZB_ZCL_CLUSTER_ID_BASIC; - uint16_t attributes[] = { - ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID, - }; - read_req.attr_number = ZB_ARRAY_LENTH(attributes); - read_req.attr_field = attributes; + uint16_t attributes[] = { + ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID, + }; + read_req.attr_number = ZB_ARRAY_LENTH(attributes); + read_req.attr_field = attributes; - // Free previously allocated memory for _read_model - free(_read_model); - _read_model = nullptr; + free(_read_model); // CPP tests it for nullptr + _read_model = nullptr; - esp_zb_lock_acquire(portMAX_DELAY); - esp_zb_zcl_read_attr_cmd_req(&read_req); - esp_zb_lock_release(); + esp_zb_lock_acquire(portMAX_DELAY); + esp_zb_zcl_read_attr_cmd_req(&read_req); + esp_zb_lock_release(); - // Wait for response or timeout - if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) { - log_e("Error while reading model"); - } - return _read_model; + //Wait for response or timeout + if (xSemaphoreTake(lock, ZB_CMD_TIMEOUT) != pdTRUE) { + log_e("Error while reading model"); + } + return _read_model; } void ZigbeeEP::printBoundDevices() { @@ -255,44 +244,27 @@ void ZigbeeEP::printBoundDevices(Print &print) { } void ZigbeeEP::zbReadBasicCluster(const esp_zb_zcl_attribute_t *attribute) { - /* Basic cluster attributes */ - if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID && - attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && - attribute->data.value) { - zbstring_t *zbstr = (zbstring_t *)attribute->data.value; - - // Use std::vector for automatic memory management - std::vector string(zbstr->len + 1); // Allocate space for the string and null terminator - memcpy(string.data(), zbstr->data, zbstr->len); - string[zbstr->len] = '\0'; // Null-terminate the string - - log_i("Peer Manufacturer is \"%s\"", string.data()); - - // Update _read_manufacturer with a dynamically allocated copy - free(_read_manufacturer); // Free any previously allocated memory - _read_manufacturer = strdup(string.data()); // Duplicate the string for persistent storage - - xSemaphoreGive(lock); - } - - if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID && - attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && - attribute->data.value) { - zbstring_t *zbstr = (zbstring_t *)attribute->data.value; - - // Use std::vector for automatic memory management - std::vector string(zbstr->len + 1); // Allocate space for the string and null terminator - memcpy(string.data(), zbstr->data, zbstr->len); - string[zbstr->len] = '\0'; // Null-terminate the string - - log_i("Peer Model is \"%s\"", string.data()); - - // Update _read_model with a dynamically allocated copy - free(_read_model); // Free any previously allocated memory - _read_model = strdup(string.data()); // Duplicate the string for persistent storage - - xSemaphoreGive(lock); - } + /* Basic cluster attributes */ + if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID && attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && attribute->data.value) { + zbstring_t *zbstr = (zbstring_t *)attribute->data.value; + std::vector string(zbstr->len + 1); + memcpy(string.data(), zbstr->data, zbstr->len); + string[zbstr->len] = '\0'; + log_i("Peer Manufacturer is \"%s\"", string); + free(_read_manufacturer); // Free any previously allocated memory + _read_manufacturer = strdup(string.data()); // Duplicate the string for persistent storage + xSemaphoreGive(lock); + } + if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID && attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && attribute->data.value) { + zbstring_t *zbstr = (zbstring_t *)attribute->data.value; + std::vector string(zbstr->len + 1); + memcpy(string.data(), zbstr->data, zbstr->len); + string[zbstr->len] = '\0'; + log_i("Peer Model is \"%s\"", string); + free(_read_model); // Free any previously allocated memory + _read_model = strdup(string.data()); // Duplicate the string for persistent storage + xSemaphoreGive(lock); + } } void ZigbeeEP::zbIdentify(const esp_zb_zcl_set_attr_value_message_t *message) { From d3a1c5ce0efaf7b599de08951867378e31ba822a Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Fri, 28 Mar 2025 12:50:14 -0300 Subject: [PATCH 06/11] fix(zigbe): removing unecessary empty line --- libraries/Zigbee/src/ZigbeeEP.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/Zigbee/src/ZigbeeEP.cpp b/libraries/Zigbee/src/ZigbeeEP.cpp index 167fcbd6f6d..31e43509bcd 100644 --- a/libraries/Zigbee/src/ZigbeeEP.cpp +++ b/libraries/Zigbee/src/ZigbeeEP.cpp @@ -21,7 +21,6 @@ ZigbeeEP::ZigbeeEP(uint8_t endpoint) { _on_identify = nullptr; _read_model = nullptr; // Explicitly initialize here _read_manufacturer = nullptr; // Explicitly initialize here - _time_status = 0; if (!lock) { lock = xSemaphoreCreateBinary(); From 2a21a138fa1dc9e5b4698f91a4a164ac1be6cb44 Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Fri, 28 Mar 2025 12:51:40 -0300 Subject: [PATCH 07/11] fix(zigbee): fixing TAB identation --- libraries/Zigbee/src/ZigbeeEP.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/Zigbee/src/ZigbeeEP.cpp b/libraries/Zigbee/src/ZigbeeEP.cpp index 31e43509bcd..af9090666e4 100644 --- a/libraries/Zigbee/src/ZigbeeEP.cpp +++ b/libraries/Zigbee/src/ZigbeeEP.cpp @@ -40,8 +40,8 @@ bool ZigbeeEP::setManufacturerAndModel(const char *name, const char *model) { size_t name_length = strlen(name); size_t model_length = strlen(model); if (name_length > ZB_MAX_NAME_LENGTH || model_length > ZB_MAX_NAME_LENGTH) { - log_e("Manufacturer or model name is too long"); - return false; + log_e("Manufacturer or model name is too long"); + return false; } // Allocate a new array of size length + 2 (1 for the length, 1 for null terminator) std::vector zb_name(name_length + 2); // +2 for length byte and null terminator From c7e73879156af8bf49cdaff9805b5907b05758e3 Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Fri, 28 Mar 2025 12:53:42 -0300 Subject: [PATCH 08/11] fix(zigbee): unecessary duplicated comment --- libraries/Zigbee/src/ZigbeeEP.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/Zigbee/src/ZigbeeEP.cpp b/libraries/Zigbee/src/ZigbeeEP.cpp index af9090666e4..af09952d261 100644 --- a/libraries/Zigbee/src/ZigbeeEP.cpp +++ b/libraries/Zigbee/src/ZigbeeEP.cpp @@ -44,7 +44,7 @@ bool ZigbeeEP::setManufacturerAndModel(const char *name, const char *model) { return false; } // Allocate a new array of size length + 2 (1 for the length, 1 for null terminator) - std::vector zb_name(name_length + 2); // +2 for length byte and null terminator + std::vector zb_name(name_length + 2); std::vector zb_model(model_length + 2); // Store the length as the first element zb_name[0] = static_cast(name_length); // Cast size_t to char From 558747d23b4e44295f3bf99546b7725f89731ddc Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Fri, 28 Mar 2025 12:58:22 -0300 Subject: [PATCH 09/11] fix(zigbee): not using reserved word string --- libraries/Zigbee/src/ZigbeeEP.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/libraries/Zigbee/src/ZigbeeEP.cpp b/libraries/Zigbee/src/ZigbeeEP.cpp index af09952d261..e8bc1578b30 100644 --- a/libraries/Zigbee/src/ZigbeeEP.cpp +++ b/libraries/Zigbee/src/ZigbeeEP.cpp @@ -246,22 +246,22 @@ void ZigbeeEP::zbReadBasicCluster(const esp_zb_zcl_attribute_t *attribute) { /* Basic cluster attributes */ if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID && attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && attribute->data.value) { zbstring_t *zbstr = (zbstring_t *)attribute->data.value; - std::vector string(zbstr->len + 1); - memcpy(string.data(), zbstr->data, zbstr->len); - string[zbstr->len] = '\0'; - log_i("Peer Manufacturer is \"%s\"", string); + std::vector m_manufacturer(zbstr->len + 1); + memcpy(m_manufacturer.data(), zbstr->data, zbstr->len); + m_manufacturer[zbstr->len] = '\0'; + log_i("Peer Manufacturer is \"%s\"", m_manufacturer); free(_read_manufacturer); // Free any previously allocated memory - _read_manufacturer = strdup(string.data()); // Duplicate the string for persistent storage + _read_manufacturer = strdup(m_manufacturer.data()); // Duplicate the information for persistent storage xSemaphoreGive(lock); } if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID && attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && attribute->data.value) { zbstring_t *zbstr = (zbstring_t *)attribute->data.value; - std::vector string(zbstr->len + 1); - memcpy(string.data(), zbstr->data, zbstr->len); - string[zbstr->len] = '\0'; - log_i("Peer Model is \"%s\"", string); + std::vector m_model(zbstr->len + 1); + memcpy(m_model.data(), zbstr->data, zbstr->len); + m_model[zbstr->len] = '\0'; + log_i("Peer Model is \"%s\"", m_model); free(_read_model); // Free any previously allocated memory - _read_model = strdup(string.data()); // Duplicate the string for persistent storage + _read_model = strdup(m_model.data()); // Duplicate the information for persistent storage xSemaphoreGive(lock); } } From 0540823131fd5e327b41d8564d79d2eeaa3176f2 Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Fri, 28 Mar 2025 13:05:57 -0300 Subject: [PATCH 10/11] fix(zigbee): keeps the same prefix style along the code --- libraries/Zigbee/src/ZigbeeEP.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/libraries/Zigbee/src/ZigbeeEP.cpp b/libraries/Zigbee/src/ZigbeeEP.cpp index e8bc1578b30..0fdbb9332ff 100644 --- a/libraries/Zigbee/src/ZigbeeEP.cpp +++ b/libraries/Zigbee/src/ZigbeeEP.cpp @@ -246,22 +246,22 @@ void ZigbeeEP::zbReadBasicCluster(const esp_zb_zcl_attribute_t *attribute) { /* Basic cluster attributes */ if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MANUFACTURER_NAME_ID && attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && attribute->data.value) { zbstring_t *zbstr = (zbstring_t *)attribute->data.value; - std::vector m_manufacturer(zbstr->len + 1); - memcpy(m_manufacturer.data(), zbstr->data, zbstr->len); - m_manufacturer[zbstr->len] = '\0'; - log_i("Peer Manufacturer is \"%s\"", m_manufacturer); + std::vector zb_manufacturer(zbstr->len + 1); + memcpy(zb_manufacturer.data(), zbstr->data, zbstr->len); + zb_manufacturer[zbstr->len] = '\0'; + log_i("Peer Manufacturer is \"%s\"", zb_manufacturer); free(_read_manufacturer); // Free any previously allocated memory - _read_manufacturer = strdup(m_manufacturer.data()); // Duplicate the information for persistent storage + _read_manufacturer = strdup(zb_manufacturer.data()); // Duplicate the information for persistent storage xSemaphoreGive(lock); } if (attribute->id == ESP_ZB_ZCL_ATTR_BASIC_MODEL_IDENTIFIER_ID && attribute->data.type == ESP_ZB_ZCL_ATTR_TYPE_CHAR_STRING && attribute->data.value) { zbstring_t *zbstr = (zbstring_t *)attribute->data.value; - std::vector m_model(zbstr->len + 1); - memcpy(m_model.data(), zbstr->data, zbstr->len); - m_model[zbstr->len] = '\0'; - log_i("Peer Model is \"%s\"", m_model); + std::vector zb_model(zbstr->len + 1); + memcpy(zb_model.data(), zbstr->data, zbstr->len); + zb_model[zbstr->len] = '\0'; + log_i("Peer Model is \"%s\"", zb_model); free(_read_model); // Free any previously allocated memory - _read_model = strdup(m_model.data()); // Duplicate the information for persistent storage + _read_model = strdup(zb_model.data()); // Duplicate the information for persistent storage xSemaphoreGive(lock); } } From d09c3ee07ccd053a9c7315be17136fed4ebe7e24 Mon Sep 17 00:00:00 2001 From: Sugar Glider Date: Fri, 28 Mar 2025 13:13:56 -0300 Subject: [PATCH 11/11] fix(zigbee): missing char * data() for logging --- libraries/Zigbee/src/ZigbeeEP.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/Zigbee/src/ZigbeeEP.cpp b/libraries/Zigbee/src/ZigbeeEP.cpp index 0fdbb9332ff..a1ca2a4f4ad 100644 --- a/libraries/Zigbee/src/ZigbeeEP.cpp +++ b/libraries/Zigbee/src/ZigbeeEP.cpp @@ -249,7 +249,7 @@ void ZigbeeEP::zbReadBasicCluster(const esp_zb_zcl_attribute_t *attribute) { std::vector zb_manufacturer(zbstr->len + 1); memcpy(zb_manufacturer.data(), zbstr->data, zbstr->len); zb_manufacturer[zbstr->len] = '\0'; - log_i("Peer Manufacturer is \"%s\"", zb_manufacturer); + log_i("Peer Manufacturer is \"%s\"", zb_manufacturer.data()); free(_read_manufacturer); // Free any previously allocated memory _read_manufacturer = strdup(zb_manufacturer.data()); // Duplicate the information for persistent storage xSemaphoreGive(lock); @@ -259,7 +259,7 @@ void ZigbeeEP::zbReadBasicCluster(const esp_zb_zcl_attribute_t *attribute) { std::vector zb_model(zbstr->len + 1); memcpy(zb_model.data(), zbstr->data, zbstr->len); zb_model[zbstr->len] = '\0'; - log_i("Peer Model is \"%s\"", zb_model); + log_i("Peer Model is \"%s\"", zb_model.data()); free(_read_model); // Free any previously allocated memory _read_model = strdup(zb_model.data()); // Duplicate the information for persistent storage xSemaphoreGive(lock);