Fix problems with string type attributes

- Use the defined consts when creating attributes
- Pass in the strlen and the max size of attribute when creating
  attribute
- Allocate max-str-size plus the size for storing the string length when
  creating the attribute metadata
This commit is contained in:
Shubham Patil
2024-01-29 16:03:54 +05:30
parent f511d22642
commit 3d9d6d1fa6
8 changed files with 46 additions and 15 deletions
+6
View File
@@ -1,3 +1,9 @@
# 14-February-2024
- An optional argument, `max_val_size`, has been introduced to the `esp_matter::attribute::create()` API.
This argument is utilized specifically when creating attributes of the char string and long char string data types
to specify the maximum supported value size of an attribute.
# 29-August-2023 # 29-August-2023
- `ot_storage` partition is no longer required for Thread devices as the mechanism for storing data related to the Thread network has been changed in the `openthread` component in ESP-IDF. - `ot_storage` partition is no longer required for Thread devices as the mechanism for storing data related to the Thread network has been changed in the `openthread` component in ESP-IDF.
@@ -14,6 +14,8 @@
#include <esp_log.h> #include <esp_log.h>
#include <esp_matter_attribute.h> #include <esp_matter_attribute.h>
#include <esp_matter.h>
#include <esp_matter_core.h>
static const char *TAG = "esp_matter_attribute"; static const char *TAG = "esp_matter_attribute";
@@ -173,7 +175,7 @@ attribute_t *create_node_label(cluster_t *cluster, char *value, uint16_t length)
{ {
return esp_matter::attribute::create(cluster, BasicInformation::Attributes::NodeLabel::Id, return esp_matter::attribute::create(cluster, BasicInformation::Attributes::NodeLabel::Id,
ATTRIBUTE_FLAG_WRITABLE | ATTRIBUTE_FLAG_NONVOLATILE, ATTRIBUTE_FLAG_WRITABLE | ATTRIBUTE_FLAG_NONVOLATILE,
esp_matter_char_str(value, length)); esp_matter_char_str(value, length), k_max_node_label_length);
} }
attribute_t *create_location(cluster_t *cluster, char *value, uint16_t length) attribute_t *create_location(cluster_t *cluster, char *value, uint16_t length)
@@ -719,7 +721,7 @@ attribute_t *create_node_label(cluster_t *cluster, char *value, uint16_t length)
{ {
return esp_matter::attribute::create(cluster, BridgedDeviceBasicInformation::Attributes::NodeLabel::Id, return esp_matter::attribute::create(cluster, BridgedDeviceBasicInformation::Attributes::NodeLabel::Id,
ATTRIBUTE_FLAG_NONVOLATILE | ATTRIBUTE_FLAG_WRITABLE, ATTRIBUTE_FLAG_NONVOLATILE | ATTRIBUTE_FLAG_WRITABLE,
esp_matter_char_str(value, length)); esp_matter_char_str(value, length), k_max_node_label_length);
} }
attribute_t *create_reachable(cluster_t *cluster, bool value) attribute_t *create_reachable(cluster_t *cluster, bool value)
+8 -1
View File
@@ -14,7 +14,6 @@
#pragma once #pragma once
#include <esp_matter.h>
#include <esp_matter_core.h> #include <esp_matter_core.h>
namespace esp_matter { namespace esp_matter {
@@ -62,6 +61,8 @@ attribute_t *create_access_control_entries_per_fabric(cluster_t *cluster, uint16
} /* access_control */ } /* access_control */
namespace basic_information { namespace basic_information {
constexpr uint8_t k_max_node_label_length = 32;
namespace attribute { namespace attribute {
attribute_t *create_data_model_revision(cluster_t *cluster, uint16_t value); attribute_t *create_data_model_revision(cluster_t *cluster, uint16_t value);
attribute_t *create_vendor_name(cluster_t *cluster, char *value, uint16_t length); attribute_t *create_vendor_name(cluster_t *cluster, char *value, uint16_t length);
@@ -210,6 +211,9 @@ attribute_t *create_active_network_faults(cluster_t *cluster, uint8_t *value, ui
namespace bridged_device_basic_information { namespace bridged_device_basic_information {
namespace attribute { namespace attribute {
constexpr uint8_t k_max_node_label_length = 32;
attribute_t *create_node_label(cluster_t *cluster, char *value, uint16_t length); attribute_t *create_node_label(cluster_t *cluster, char *value, uint16_t length);
attribute_t *create_reachable(cluster_t *cluster, bool value); attribute_t *create_reachable(cluster_t *cluster, bool value);
} /* attribute */ } /* attribute */
@@ -449,6 +453,9 @@ attribute_t *state_value(cluster_t *cluster, bool value);
} /* boolean_state */ } /* boolean_state */
namespace localization_configuration { namespace localization_configuration {
constexpr uint8_t k_max_active_locale_length = 35;
namespace attribute { namespace attribute {
attribute_t *create_active_locale(cluster_t *cluster, char *value, uint16_t length); attribute_t *create_active_locale(cluster_t *cluster, char *value, uint16_t length);
attribute_t *create_supported_locales(cluster_t *cluster, uint8_t *value, uint16_t length, uint16_t count); attribute_t *create_supported_locales(cluster_t *cluster, uint8_t *value, uint16_t length, uint16_t count);
+2 -2
View File
@@ -214,7 +214,7 @@ cluster_t *create(endpoint_t *endpoint, config_t *config, uint8_t flags)
/* Attributes not managed internally */ /* Attributes not managed internally */
if (config) { if (config) {
global::attribute::create_cluster_revision(cluster, config->cluster_revision); global::attribute::create_cluster_revision(cluster, config->cluster_revision);
attribute::create_node_label(cluster, config->node_label, sizeof(config->node_label)); attribute::create_node_label(cluster, config->node_label, strlen(config->node_label));
} else { } else {
ESP_LOGE(TAG, "Config is NULL. Cannot add some attributes."); ESP_LOGE(TAG, "Config is NULL. Cannot add some attributes.");
} }
@@ -1672,7 +1672,7 @@ cluster_t *create(endpoint_t *endpoint, config_t *config, uint8_t flags)
if (config) { if (config) {
/* Attributes not managed internally */ /* Attributes not managed internally */
global::attribute::create_cluster_revision(cluster, config->cluster_revision); global::attribute::create_cluster_revision(cluster, config->cluster_revision);
attribute::create_active_locale(cluster, config->active_locale, sizeof(config->active_locale)); attribute::create_active_locale(cluster, config->active_locale, strlen(config->active_locale));
/* Attributes managed internally */ /* Attributes managed internally */
attribute::create_supported_locales(cluster, NULL, 0, 0); attribute::create_supported_locales(cluster, NULL, 0, 0);
+3 -2
View File
@@ -15,6 +15,7 @@
#pragma once #pragma once
#include <esp_matter_core.h> #include <esp_matter_core.h>
#include <esp_matter_attribute.h>
#include <esp_matter_feature.h> #include <esp_matter_feature.h>
#include <stdint.h> #include <stdint.h>
@@ -55,7 +56,7 @@ cluster_t *create(endpoint_t *endpoint, uint8_t flags);
namespace basic_information { namespace basic_information {
typedef struct config { typedef struct config {
uint16_t cluster_revision; uint16_t cluster_revision;
char node_label[32]; char node_label[k_max_node_label_length + 1];
config() : cluster_revision(1), node_label{0} {} config() : cluster_revision(1), node_label{0} {}
} config_t; } config_t;
@@ -392,7 +393,7 @@ cluster_t *create(endpoint_t *endpoint, config_t *config, uint8_t flags);
namespace localization_configuration { namespace localization_configuration {
typedef struct config { typedef struct config {
uint16_t cluster_revision; uint16_t cluster_revision;
char active_locale[35]; char active_locale[k_max_active_locale_length + 1];
config() : cluster_revision(4), active_locale{0} {} config() : cluster_revision(4), active_locale{0} {}
} config_t; } config_t;
+13 -2
View File
@@ -147,6 +147,9 @@ typedef struct _attribute {
esp_matter_attr_bounds_t *bounds; esp_matter_attr_bounds_t *bounds;
EmberAfDefaultOrMinMaxAttributeValue default_value; EmberAfDefaultOrMinMaxAttributeValue default_value;
uint16_t default_value_size; uint16_t default_value_size;
// This is required when creating metadata for char string and long char string types of attributes.
// The size in the attribute metadata remains constant and is verified during write operations.
uint16_t max_val_size;
attribute::callback_t override_callback; attribute::callback_t override_callback;
struct _attribute *next; struct _attribute *next;
} _attribute_t; } _attribute_t;
@@ -606,7 +609,13 @@ esp_err_t enable(endpoint_t *endpoint)
* when writing a longer string. * when writing a longer string.
*/ */
if (attribute->val.type == ESP_MATTER_VAL_TYPE_CHAR_STRING) { if (attribute->val.type == ESP_MATTER_VAL_TYPE_CHAR_STRING) {
matter_attributes[attribute_index].size = attribute->val.val.a.s; // Once the metadata is created, the attribute size becomes fixed and cannot be modified thereafter.
// For string and long string types, the size should be the maximum size defined in the specification
// plus the size_for_storing_str_len. The length byte is 1 for char string and 2 for long char string.
// For example, the maximum size of the Node-Label in the basic information cluster is 32 bytes,
// and it is a char string. Therefore, the size should be (32 + 1).
uint16_t size_for_storing_str_len = attribute->val.val.a.t - attribute->val.val.a.s;
matter_attributes[attribute_index].size = attribute->max_val_size + size_for_storing_str_len;
} }
matter_clusters[cluster_index].clusterSize += matter_attributes[attribute_index].size; matter_clusters[cluster_index].clusterSize += matter_attributes[attribute_index].size;
@@ -1071,7 +1080,8 @@ esp_err_t factory_reset()
} }
namespace attribute { namespace attribute {
attribute_t *create(cluster_t *cluster, uint32_t attribute_id, uint8_t flags, esp_matter_attr_val_t val) attribute_t *create(cluster_t *cluster, uint32_t attribute_id, uint8_t flags, esp_matter_attr_val_t val,
uint16_t max_val_size)
{ {
/* Find */ /* Find */
if (!cluster) { if (!cluster) {
@@ -1099,6 +1109,7 @@ attribute_t *create(cluster_t *cluster, uint32_t attribute_id, uint8_t flags, es
attribute->endpoint_id = current_cluster->endpoint_id; attribute->endpoint_id = current_cluster->endpoint_id;
attribute->flags = flags; attribute->flags = flags;
attribute->flags |= ATTRIBUTE_FLAG_EXTERNAL_STORAGE; attribute->flags |= ATTRIBUTE_FLAG_EXTERNAL_STORAGE;
attribute->max_val_size = max_val_size;
// After reboot, string and array are treated as Invalid. So need to store val.type and size of attribute value. // After reboot, string and array are treated as Invalid. So need to store val.type and size of attribute value.
attribute->val.type = val.type; attribute->val.type = val.type;
+9 -5
View File
@@ -413,15 +413,19 @@ namespace attribute {
* *
* This will create a new attribute and add it to the cluster. * This will create a new attribute and add it to the cluster.
* *
* @param[in] cluster Cluster handle. * @param[in] cluster Cluster handle.
* @param[in] attribute_id Attribute ID for the attribute. * @param[in] attribute_id Attribute ID for the attribute.
* @param[in] flags Bitmap of `attribute_flags_t`. * @param[in] flags Bitmap of `attribute_flags_t`.
* @param[in] val Default type and value of the attribute. Use appropriate elements as per the value type. * @param[in] val Default type and value of the attribute. Use appropriate elements as per the value type.
* @param[in] max_val_size For attributes of type char string and long char string, the size should correspond to the
* maximum size defined in the specification. However, for other types of attributes, this
* parameter remains unused, and therefore the default value is set to 0
* *
* @return Attribute handle on success. * @return Attribute handle on success.
* @return NULL in case of failure. * @return NULL in case of failure.
*/ */
attribute_t *create(cluster_t *cluster, uint32_t attribute_id, uint8_t flags, esp_matter_attr_val_t val); attribute_t *create(cluster_t *cluster, uint32_t attribute_id, uint8_t flags, esp_matter_attr_val_t val,
uint16_t max_val_size = 0);
/** Get attribute /** Get attribute
* *
+1 -1
View File
@@ -14,8 +14,8 @@
#pragma once #pragma once
#include <esp_matter_core.h>
#include <stdint.h> #include <stdint.h>
#include <esp_matter_attribute.h>
#define ESP_MATTER_NONE_FEATURE_ID 0x0000 #define ESP_MATTER_NONE_FEATURE_ID 0x0000