Custom Cluster Custom Attribute Manufacturer Specific Read results in Unsupported Attribute (TZ-688)

qcrist commented 7 months ago

Answers checklist.

[X] I have read the documentation ESP Zigbee SDK Programming Guide and tried the debugging tips, the issue is not addressed there.
[X] I have updated ESP Zigbee libs (esp-zboss-lib and esp-zigbee-lib) to the latest version, with corresponding IDF version, and checked that the issue is present there.
[X] I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v5.2

esp-zigbee-lib version.

1.2.0

esp-zboss-lib version.

1.2.0

Espressif SoC revision.

ESP32-C6

What is the expected behavior?

Successful read

What is the actual behavior?

Unsupported attribute is the response

Steps to reproduce.

setup:

    esp_zb_cluster_list_t* c_list = esp_zb_zcl_cluster_list_create();
    const uint8_t attr_mra = ESP_ZB_ZCL_ATTR_ACCESS_READ_WRITE | ESP_ZB_ZCL_ATTR_ACCESS_REPORTING| ESP_ZB_ZCL_ATTR_MANUF_SPEC;
    esp_zb_attribute_list_t* esp_zb_custom_cluster_output = esp_zb_zcl_attr_list_create(0xFCCE);
    ESP_ERROR_CHECK(esp_zb_custom_cluster_add_custom_attr(esp_zb_custom_cluster_output, 0, ESP_ZB_ZCL_ATTR_TYPE_48BIT, attr_mra, &current_state));
    ESP_ERROR_CHECK(esp_zb_cluster_list_add_custom_cluster(c_list, esp_zb_custom_cluster_output, ESP_ZB_ZCL_CLUSTER_SERVER_ROLE));

    ESP_ERROR_CHECK(esp_zb_ep_list_add_ep(ep_list, c_list, {
            2,
            ESP_ZB_AF_HA_PROFILE_ID,
            ESP_ZB_HA_CUSTOM_ATTR_DEVICE_ID,
            0}));

read request:

response:

If I remove ESP_ZB_ZCL_ATTR_MANUF_SPEC, and use manufacturer code 0xFFFF the request will be successful.

More Information.

I can provide a pcap if necessary

xieqinan commented 7 months ago

@qcrist ，

The issue is likely triggered by the fixed manufacturer code in the esp-zigbee-sdk.

Here is a workaround that may help resolve your issue. This workaround involves changing the fixed manufacturer code in the esp_zb_custom_cluster_output.

Add ESP_ZB_ZCL_ATTR_MANUF_SPEC to attribute access.
Iterate through the esp_zb_custom_cluster_output attribute list to find the attribute with the cluster ID 0xFCCE and attribute ID 0. Finally, set the manuf_code to 0x1234. After that, execute ESP_ERROR_CHECK(esp_zb_custom_cluster_add_custom_attr(esp_zb_custom_cluster_output, 0, ESP_ZB_ZCL_ATTR_TYPE_48BIT, attr_mra, &current_state));.

Suxsem commented 7 months ago

@xieqinan I had the same issue and your workaround (adding ESP_ZB_ZCL_ATTR_MANUF_SPEC and manuf_code) fixed the issue.

But now I cannot set attribute value anymore with method esp_zb_zcl_set_attribute_val, it returns ESP_ZB_ZCL_STATUS_FAIL.

Without adding ESP_ZB_ZCL_ATTR_MANUF_SPEC the esp_zb_zcl_set_attribute_val works, but the manuf_code is not considered.

Any help?

Futhermore, I would be great to add a parameter to esp_zb_custom_cluster_add_custom_attr to allow setting the manuf_code without looping the attribute list.

Suxsem commented 7 months ago

Here it is the debug trace:

ZBOSS: zcl/zcl_common.c:1016   >> zb_zcl_set_attr_val check_access 0, ep 1, cluster role 1, cluster 0xfc00, attr 1
ZBOSS: zcl/zcl_general_commands.c:603   >> zb_zcl_get_attr_desc_manuf_a ep 1, cluster_id 0xfc00, attr_id 0x1, manuf_code 0xffff
ZBOSS: zcl/zcl_common.c:589   >>get_endpoint_desc 0x40818614 0x40821b78 ep_id 1
ZBOSS: zcl/zcl_common.c:594   device_ctx 0x40821b78, ep_count 1
ZBOSS: zcl/zcl_common.c:596   ep_id 1
ZBOSS: zcl/zcl_common.c:606   <<get_endpoint_desc desc 0x40821cb8
ZBOSS: zcl/zcl_common.c:1678   >>get_cluster_desc ep_desc 0x40821cb8, cluster_id 0xfc00 cluster_role 1
ZBOSS: zcl/zcl_common.c:1693   <<get_cluster_desc cluster_desc 0x40821d11
ZBOSS: zcl/zcl_general_commands.c:559   >> zb_zcl_get_attr_desc_manuf cluster_desc 0x40821d11, attr_id 0x1, manuf_code 0xffff, cluster id: 0xfc00
ZBOSS: zcl/zcl_general_commands.c:570   Checking attribute with id 0xfffd: is_attr_ids_match 0, is_manuf_codes_match 0, is_ms_bit_set 0, is_ms_bit_set_correctly 1
ZBOSS: zcl/zcl_general_commands.c:570   Checking attribute with id 0x0: is_attr_ids_match 0, is_manuf_codes_match 0, is_ms_bit_set 1, is_ms_bit_set_correctly 0
ZBOSS: zcl/zcl_general_commands.c:570   Checking attribute with id 0x1: is_attr_ids_match 1, is_manuf_codes_match 0, is_ms_bit_set 1, is_ms_bit_set_correctly 0
ZBOSS: zcl/zcl_general_commands.c:570   Checking attribute with id 0x2: is_attr_ids_match 0, is_manuf_codes_match 0, is_ms_bit_set 1, is_ms_bit_set_correctly 0
ZBOSS: zcl/zcl_general_commands.c:570   Checking attribute with id 0xffff: is_attr_ids_match 0, is_manuf_codes_match 1, is_ms_bit_set 0, is_ms_bit_set_correctly 1
ZBOSS: zcl/zcl_general_commands.c:587   << zb_zcl_get_attr_desc_manuf, attr_desc 0x0
ZBOSS: zcl/zcl_general_commands.c:614   << zb_zcl_get_attr_desc_manuf_a ret 0x0
ZBOSS: zcl/zcl_common.c:1019   attr_desc 0x0, value 0x4081ddef
ZBOSS: zcl/zcl_common.c:1046   << zb_zcl_set_attr_val, cluster_id 0xfc00, attr_id 0x1 status 1

grebenyuk51 commented 7 months ago

Futhermore, I would be great to add a parameter to esp_zb_custom_cluster_add_custom_attrto allow setting the manuf_codewithout looping the attribute list.

I have the same idea :) #294

xieqinan commented 7 months ago

@Suxsem @grebenyuk51,

Futhermore, I would be great to add a parameter to esp_zb_custom_cluster_add_custom_attr to allow setting the manuf_code without looping the attribute list.

The request may involve some breaking changes, which we will consider in the future. In the meantime, we can use the following workaround, which may be more convenient:

After the esp_zb_device_register() function call, we can use zb_zcl_get_attr_desc_manuf() to retrieve the attribute object, and then modify it according to your requirements. However, please note that it is not advisable to make changes after calling esp_zb_start().

Suxsem commented 7 months ago

@xieqinan how this would solve the esp_zb_zcl_set_attribute_val fail?

xieqinan commented 6 months ago

@Suxsem ，

After the esp_zb_device_register() function call, we can use zb_zcl_get_attr_desc_manuf() to retrieve the attribute object, and then modify it according to your requirements. However, please note that it is not advisable to make changes after calling esp_zb_start().

Do you have attempt to use the above workaround?

Suxsem commented 6 months ago

@xieqinan not really, can you please provide an example on how to use it? Specifically, i don't know how to construct the first argument. thank you

xieqinan commented 6 months ago

@Suxsem

how this would solve the esp_zb_zcl_set_attribute_val fail?

Please utilize the replace esp_zb_zcl_set_attribute_val() with zb_zcl_get_attr_desc_manuf() temporarily.

Suxsem commented 6 months ago

@xieqinan but can you provide an example please?

xieqinan commented 6 months ago

@Suxsem I don't think an example is necessary. In response to your query about the esp_zb_zcl_set_attribute_val() returning failed in https://github.com/espressif/esp-zigbee-sdk/issues/278#issuecomment-2016438365, I suggest using zb_zcl_get_attr_desc_manuf() as a replacement. You just need to add "zb_zcl_common.h" to link the function to your project .

Suxsem commented 6 months ago

@xieqinan first argument of zb_zcl_get_attr_desc_manuf is of type zb_zcl_cluster_desc_t* so I don't know how to construct this type of argument. In example is indeed much appreciated

xieqinan commented 6 months ago

@Suxsem，

@xieqinan first argument of zb_zcl_get_attr_desc_manuf is of type zb_zcl_cluster_desc_t* so I don't know how to construct this > type of argument. In example is indeed much appreciated

The ESP-Zigbee-SDK v1.2.3 now provides the function to set and get the manufacturer attribute.

Suxsem commented 6 months ago

@xieqinan it's working now, thanks!

Next issue is with esp_zb_zcl_report_attr_cmd_req.

ZBOSS: zcl/zcl_common.c:589   >>get_endpoint_desc 0x40813d14 0x40824058 ep_id 1
ZBOSS: zcl/zcl_common.c:594   device_ctx 0x40824058, ep_count 1
ZBOSS: zcl/zcl_common.c:596   ep_id 1
ZBOSS: zcl/zcl_common.c:606   <<get_endpoint_desc desc 0x40824078
ZBOSS: zcl/zcl_general_commands.c:603   >> zb_zcl_get_attr_desc_manuf_a ep 1, cluster_id 0xfeb2, attr_id 0x3, manuf_code 0xffff
ZBOSS: zcl/zcl_common.c:589   >>get_endpoint_desc 0x40813d14 0x40824058 ep_id 1
ZBOSS: zcl/zcl_common.c:594   device_ctx 0x40824058, ep_count 1
ZBOSS: zcl/zcl_common.c:596   ep_id 1
ZBOSS: zcl/zcl_common.c:606   <<get_endpoint_desc desc 0x40824078
ZBOSS: zcl/zcl_common.c:1678   >>get_cluster_desc ep_desc 0x40824078, cluster_id 0xfeb2 cluster_role 1
ZBOSS: zcl/zcl_common.c:1693   <<get_cluster_desc cluster_desc 0x40823d4d
ZBOSS: zcl/zcl_general_commands.c:559   >> zb_zcl_get_attr_desc_manuf cluster_desc 0x40823d4d, attr_id 0x3, manuf_code 0xffff, cluster id: 0xfeb2
ZBOSS: zcl/zcl_general_commands.c:570   Checking attribute with id 0xfffd: is_attr_ids_match 0, is_manuf_codes_match 0, is_ms_bit_set 0, is_ms_bit_set_correctly 1
ZBOSS: zcl/zcl_general_commands.c:570   Checking attribute with id 0x0: is_attr_ids_match 0, is_manuf_codes_match 0, is_ms_bit_set 0, is_ms_bit_set_correctly 1
ZBOSS: zcl/zcl_general_commands.c:570   Checking attribute with id 0x3: is_attr_ids_match 1, is_manuf_codes_match 0, is_ms_bit_set 1, is_ms_bit_set_correctly 0
ZBOSS: zcl/zcl_general_commands.c:570   Checking attribute with id 0x1: is_attr_ids_match 0, is_manuf_codes_match 0, is_ms_bit_set 0, is_ms_bit_set_correctly 1
ZBOSS: zcl/zcl_general_commands.c:570   Checking attribute with id 0x2: is_attr_ids_match 0, is_manuf_codes_match 0, is_ms_bit_set 0, is_ms_bit_set_correctly 1
ZBOSS: zcl/zcl_general_commands.c:570   Checking attribute with id 0x4: is_attr_ids_match 0, is_manuf_codes_match 0, is_ms_bit_set 0, is_ms_bit_set_correctly 1
ZBOSS: zcl/zcl_general_commands.c:570   Checking attribute with id 0xffff: is_attr_ids_match 0, is_manuf_codes_match 1, is_ms_bit_set 0, is_ms_bit_set_correctly 1
ZBOSS: zcl/zcl_general_commands.c:587   << zb_zcl_get_attr_desc_manuf, attr_desc 0x0
ZBOSS: zcl/zcl_general_commands.c:614   << zb_zcl_get_attr_desc_manuf_a ret 0x0
E (7213) ESP_ZIGBEE_COMMAND: esp_zb_zcl_report_attr_cmd_req(293): This attribute: 0x3 is not existed!

I think esp_zb_zcl_report_attr_cmd_t should provide a manufacturer_code parameter

Thank you

xieqinan commented 6 months ago

@Suxsem

I think esp_zb_zcl_report_attr_cmd_t should provide a manufacturer_code parameter

We will consider adding a manufacturer_code parameter in a future version. However, since this change would require a breaking change, we have decided not to implement it temporarily.

Suxsem commented 6 months ago

@xieqinan why not adding a manuf_code member in the structure with default value 0xffff? In this way it would not be a breaking change, what do you think?

xieqinan commented 3 months ago

@Suxsem,

The C language does not support setting default values in structures. Therefore, we can only set the default manf_code to 0xffff in esp_zb_zcl_report_attr_cmd_req(). If we do this, other functions for the ZCL will also need this member. This is not fully backward compatible with older versions. Please use the solution provided in https://github.com/espressif/esp-zigbee-sdk/issues/278#issuecomment-2055547492 for now.

espressif / esp-zigbee-sdk