From 5cce2db020e8a90e7b27b97cf79ac2b417b5ee79 Mon Sep 17 00:00:00 2001 From: Tomas Bedrich Date: Mon, 16 Oct 2023 23:32:40 +0200 Subject: [PATCH 1/9] Fix read-before-write --- custom_components/zha_toolkit/zcl_attr.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/custom_components/zha_toolkit/zcl_attr.py b/custom_components/zha_toolkit/zcl_attr.py index 64621ce..4f218e1 100644 --- a/custom_components/zha_toolkit/zcl_attr.py +++ b/custom_components/zha_toolkit/zcl_attr.py @@ -348,8 +348,7 @@ async def attr_write( # noqa: C901 result_read = None if ( params[p.READ_BEFORE_WRITE] - or (len(attr_write_list) == 0) - or (cmd != S.ATTR_WRITE) + or (attr_read_list and cmd == S.ATTR_READ) ): if use_cache > 0: # Try to get value from cache From 33d6086981a187d26378a8544791dd1e4ed5b7e8 Mon Sep 17 00:00:00 2001 From: mdeweerd Date: Tue, 17 Oct 2023 01:40:18 +0200 Subject: [PATCH 2/9] Fix to repair 'access' field text represention (because of zigpy change) --- custom_components/zha_toolkit/scan_device.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/custom_components/zha_toolkit/scan_device.py b/custom_components/zha_toolkit/scan_device.py index 12f2fb6..3ced0e0 100644 --- a/custom_components/zha_toolkit/scan_device.py +++ b/custom_components/zha_toolkit/scan_device.py @@ -2,7 +2,6 @@ import asyncio import logging -import re from zigpy import types as t from zigpy.exceptions import ControllerException, DeliveryError @@ -14,6 +13,9 @@ LOGGER = logging.getLogger(__name__) +ACCESS_CONTROL_MAP = {0x01: "READ", 0x02: "WRITE", 0x04: "REPORT"} + + @u.retryable( ( DeliveryError, @@ -231,13 +233,16 @@ async def discover_attributes_extended(cluster, manufacturer=None, tries=3): ] else: attr_type = attr_type_hex - try: - access = re.sub( - "^.*\\.", - "", - str(foundation.AttributeAccessControl(access_acl)), + + if access_acl != 0: + access = "|".join( + [ + s + for x, s in ACCESS_CONTROL_MAP.items() + if x & access_acl != 0 + ] ) - except ValueError: + else: access = "undefined" result[attr_id] = { From e13f3a7209ff59e5e4abea56a2fc6753935cad1c Mon Sep 17 00:00:00 2001 From: Tomas Bedrich Date: Mon, 16 Oct 2023 23:56:06 +0200 Subject: [PATCH 3/9] Enable Array writes --- README.md | 18 ++++++++++++++++++ custom_components/zha_toolkit/utils.py | 6 ++++++ 2 files changed, 24 insertions(+) diff --git a/README.md b/README.md index 0254c43..778ef09 100644 --- a/README.md +++ b/README.md @@ -793,6 +793,24 @@ data: write_if_equal: false ``` +In case ZCL Array type needs to be written, `attr_val` needs to be provided as a raw sequence of bytes, +i.e. user is responsible to generate a sequence which complies to the ZCL spec. +The following examples illustrates configuration of Ubisys C4 (see [the device manual](https://www.ubisys.de/wp-content/uploads/ubisys-c4-technical-reference.pdf) - section 7.8.5.2. InputActions Attribute - example): + +```yaml +service: zha_toolkit.attr_write +data: + ieee: 00:1f:ee:00:00:aa:aa:aa + endpoint: 232 + cluster: 64512 + attribute: 1 + attr_type: 0x48 + attr_val: [65, 4, 0, 6, 0, 13, 1, 6, 0, 2, 6, 1, 13, 2, 6, 0, 2, 6, 2, 13, 3, 6, 0, 2, 6, 3, 13, 4, 6, 0, 2] + read_before_write: false + read_after_write: false + use_cache: false +``` + Using the symbolic name of the attribute, and automatic endpoint selection. ```yaml diff --git a/custom_components/zha_toolkit/utils.py b/custom_components/zha_toolkit/utils.py index c9cea6f..8842b1e 100644 --- a/custom_components/zha_toolkit/utils.py +++ b/custom_components/zha_toolkit/utils.py @@ -603,6 +603,12 @@ def attr_encode(attr_val_in, attr_type): # noqa C901 ) attr_obj = f.TypeValue(attr_type, t.LVBytes(attr_val_in)) + elif attr_type == 0x48: # Array + # let user construct the Array and pass it as raw bytes + array_type = attr_val_in[0] + array_body = t.SerializableBytes(bytes(attr_val_in[1:])) + attr_obj = f.TypeValue(attr_type, f.Array(array_type, array_body)) + compare_val = None # not implemented because of raw approach elif attr_type == 0xFF or attr_type is None: compare_val = str2int(attr_val_in) # This should not happen ideally From b0cd170b9dcb97c7d49d54ee32bf250f5ce203f5 Mon Sep 17 00:00:00 2001 From: mdeweerd Date: Tue, 17 Oct 2023 14:54:23 +0200 Subject: [PATCH 4/9] Improve handling of unknown types, works for arrays --- custom_components/zha_toolkit/utils.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/custom_components/zha_toolkit/utils.py b/custom_components/zha_toolkit/utils.py index 8842b1e..9d7d5be 100644 --- a/custom_components/zha_toolkit/utils.py +++ b/custom_components/zha_toolkit/utils.py @@ -603,12 +603,6 @@ def attr_encode(attr_val_in, attr_type): # noqa C901 ) attr_obj = f.TypeValue(attr_type, t.LVBytes(attr_val_in)) - elif attr_type == 0x48: # Array - # let user construct the Array and pass it as raw bytes - array_type = attr_val_in[0] - array_body = t.SerializableBytes(bytes(attr_val_in[1:])) - attr_obj = f.TypeValue(attr_type, f.Array(array_type, array_body)) - compare_val = None # not implemented because of raw approach elif attr_type == 0xFF or attr_type is None: compare_val = str2int(attr_val_in) # This should not happen ideally @@ -617,8 +611,14 @@ def attr_encode(attr_val_in, attr_type): # noqa C901 # Try to apply conversion using foundation DATA_TYPES table data_type = f.DATA_TYPES[attr_type][1] LOGGER.debug(f"Data type '{data_type}' for attr type {attr_type}") - compare_val = data_type(str2int(attr_val_in)) - attr_obj = f.TypeValue(attr_type, data_type(compare_val)) + if isinstance(attr_val_in, list): + # Without length byte after serialisation: + compare_val = t.LVList[t.uint8_t](attr_val_in) + # With length byte after serialisation: + # compare_val = t.LVBytes(attr_val_in) + else: + compare_val = data_type(str2int(attr_val_in)) + attr_obj = data_type(attr_type, compare_val) LOGGER.debug( "Converted %s to %s - will compare to %s - Type: 0x%02X", attr_val_in, From 39abaac472faff5eb41bebef478f2dda98142f5e Mon Sep 17 00:00:00 2001 From: mdeweerd Date: Tue, 17 Oct 2023 15:42:10 +0200 Subject: [PATCH 5/9] Use generic method, avoid adding type in attr_val --- README.md | 41 +++++++++++++++++++++++--- custom_components/zha_toolkit/utils.py | 6 ++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 778ef09..03533ee 100644 --- a/README.md +++ b/README.md @@ -793,9 +793,13 @@ data: write_if_equal: false ``` -In case ZCL Array type needs to be written, `attr_val` needs to be provided as a raw sequence of bytes, -i.e. user is responsible to generate a sequence which complies to the ZCL spec. -The following examples illustrates configuration of Ubisys C4 (see [the device manual](https://www.ubisys.de/wp-content/uploads/ubisys-c4-technical-reference.pdf) - section 7.8.5.2. InputActions Attribute - example): +In case ZCL Array type needs to be written, `attr_val` needs to be provided +as a raw sequence of bytes, i.e. user is responsible to generate a sequence +which complies to the ZCL spec.\ +The following examples illustrates +configuration of Ubisys C4 (see +[the device manual](https://www.ubisys.de/wp-content/uploads/ubisys-c4-technical-reference.pdf) +\- section 7.8.5.2. InputActions Attribute - example): ```yaml service: zha_toolkit.attr_write @@ -805,12 +809,41 @@ data: cluster: 64512 attribute: 1 attr_type: 0x48 - attr_val: [65, 4, 0, 6, 0, 13, 1, 6, 0, 2, 6, 1, 13, 2, 6, 0, 2, 6, 2, 13, 3, 6, 0, 2, 6, 3, 13, 4, 6, 0, 2] + # For the array type, the first to bytes compose the length (little endian) + # So here: `4, 0` is 0x0004, so two elements in the array, each of 56 bytes. + attr_val: [4, 0, 6, 0, 13, 1, 6, 0, 2, 6, 1, 13, 2, 6, 0, 2, 6, 2, 13, 3, 6, 0, + 2, 6, 3, 13, 4, 6, 0, 2] read_before_write: false read_after_write: false use_cache: false ``` +Such a packet decoded using tshark/wireshark, the above results in: + +```plaintext +ZigBee Cluster Library Frame, Command: Write Attributes, Seq: 102 + Frame Control Field: Profile-wide (0x00) + .... ..00 = Frame Type: Profile-wide (0x0) + .... .0.. = Manufacturer Specific: False + .... 0... = Direction: Client to Server + ...0 .... = Disable Default Response: False + Sequence Number: 102 + Command: Write Attributes (0x02) + Attribute Field + Attribute: Unknown (0xfde8) + Data Type: Array (0x48) + Elements Type: 56-Bit Bitmap (0x1e) + Elements Number: 4 + Element #1, Bitmap: 020006010d0006 + Bitmap56: 0x00020006010d0006 + Element #2, Bitmap: 020006020d0106 + Bitmap56: 0x00020006020d0106 + Element #3, Bitmap: 020006030d0206 + Bitmap56: 0x00020006030d0206 + Element #4, Bitmap: 020006040d0306 + Bitmap56: 0x00020006040d0306 +``` + Using the symbolic name of the attribute, and automatic endpoint selection. ```yaml diff --git a/custom_components/zha_toolkit/utils.py b/custom_components/zha_toolkit/utils.py index 9d7d5be..e1018c9 100644 --- a/custom_components/zha_toolkit/utils.py +++ b/custom_components/zha_toolkit/utils.py @@ -603,6 +603,12 @@ def attr_encode(attr_val_in, attr_type): # noqa C901 ) attr_obj = f.TypeValue(attr_type, t.LVBytes(attr_val_in)) + elif attr_type == 0xFF48: # Array (disabled, see 'else:') + # let user construct the Array and pass it as raw bytes + array_type = attr_val_in[0] + array_body = t.SerializableBytes(bytes(attr_val_in[1:])) + attr_obj = f.TypeValue(attr_type, f.Array(array_type, array_body)) + compare_val = None # not implemented because of raw approach elif attr_type == 0xFF or attr_type is None: compare_val = str2int(attr_val_in) # This should not happen ideally From 2e92c2f82e573a718d8372a342449786da9bb079 Mon Sep 17 00:00:00 2001 From: mdeweerd Date: Tue, 17 Oct 2023 17:47:41 +0200 Subject: [PATCH 6/9] Specific implementation for Array (,Set, Bag) is needed --- custom_components/zha_toolkit/utils.py | 29 +++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/custom_components/zha_toolkit/utils.py b/custom_components/zha_toolkit/utils.py index e1018c9..eb98d94 100644 --- a/custom_components/zha_toolkit/utils.py +++ b/custom_components/zha_toolkit/utils.py @@ -20,6 +20,8 @@ LOGGER = logging.getLogger(__name__) +# pylint: disable=too-many-lines + HA_VERSION = get_distribution("homeassistant").version ZIGPY_VERSION = get_distribution("zigpy").version @@ -603,28 +605,41 @@ def attr_encode(attr_val_in, attr_type): # noqa C901 ) attr_obj = f.TypeValue(attr_type, t.LVBytes(attr_val_in)) - elif attr_type == 0xFF48: # Array (disabled, see 'else:') - # let user construct the Array and pass it as raw bytes - array_type = attr_val_in[0] + elif attr_type == 0x48: # Array, (+Bag?, Set?) + # TODO: apply to Bag and Set ? + # + # Array List of bytes currently is: + # First byte: type of array items + # Next bytes: bytes for array items + # + # Maybe in future accept: + # Specifying array item type in 'attr_items_type:' + # (/detect items type from read). + compare_val = t.List[t.uint8_t](attr_val_in) + array_item_type = attr_val_in[0] array_body = t.SerializableBytes(bytes(attr_val_in[1:])) - attr_obj = f.TypeValue(attr_type, f.Array(array_type, array_body)) - compare_val = None # not implemented because of raw approach + attr_obj = f.TypeValue(attr_type, f.Array(array_item_type, array_body)) elif attr_type == 0xFF or attr_type is None: compare_val = str2int(attr_val_in) # This should not happen ideally attr_obj = f.TypeValue(attr_type, t.LVBytes(compare_val)) else: # Try to apply conversion using foundation DATA_TYPES table + # Note: this is not perfect and specific conversions may be needed. data_type = f.DATA_TYPES[attr_type][1] LOGGER.debug(f"Data type '{data_type}' for attr type {attr_type}") if isinstance(attr_val_in, list): # Without length byte after serialisation: - compare_val = t.LVList[t.uint8_t](attr_val_in) + compare_val = t.List[t.uint8_t](attr_val_in) # With length byte after serialisation: # compare_val = t.LVBytes(attr_val_in) + + attr_obj = data_type(compare_val) + # Not using : attr_obj = data_type(attr_type, compare_val) + # which may add extra bytes else: compare_val = data_type(str2int(attr_val_in)) - attr_obj = data_type(attr_type, compare_val) + attr_obj = data_type(attr_type, compare_val) LOGGER.debug( "Converted %s to %s - will compare to %s - Type: 0x%02X", attr_val_in, From 88c64af2c700c01f63a565d8d2198ef6389ebb73 Mon Sep 17 00:00:00 2001 From: mdeweerd Date: Tue, 17 Oct 2023 17:56:27 +0200 Subject: [PATCH 7/9] Update documentation --- README.md | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 03533ee..5910e02 100644 --- a/README.md +++ b/README.md @@ -809,10 +809,13 @@ data: cluster: 64512 attribute: 1 attr_type: 0x48 - # For the array type, the first to bytes compose the length (little endian) - # So here: `4, 0` is 0x0004, so two elements in the array, each of 56 bytes. - attr_val: [4, 0, 6, 0, 13, 1, 6, 0, 2, 6, 1, 13, 2, 6, 0, 2, 6, 2, 13, 3, 6, 0, - 2, 6, 3, 13, 4, 6, 0, 2] + # For the array type (type 0x48): + # - The first byte is the type of items. here 65 or 0x41: octet str. + # - The second and third byte compose the length (little endian) + # So here: `4, 0` is 0x0004, so two octet strings the array. + # - All the octet strings in this example have a length of 6. + attr_val: [65, 4, 0, 6, 0, 13, 1, 6, 0, 2, 6, 1, 13, 2, 6, 0, 2, 6, 2, 13, 3, 6, + 0, 2, 6, 3, 13, 4, 6, 0, 2] read_before_write: false read_after_write: false use_cache: false @@ -821,27 +824,32 @@ data: Such a packet decoded using tshark/wireshark, the above results in: ```plaintext -ZigBee Cluster Library Frame, Command: Write Attributes, Seq: 102 +ZigBee Cluster Library Frame, Command: Write Attributes, Seq: 40 Frame Control Field: Profile-wide (0x00) .... ..00 = Frame Type: Profile-wide (0x0) .... .0.. = Manufacturer Specific: False .... 0... = Direction: Client to Server ...0 .... = Disable Default Response: False - Sequence Number: 102 + Sequence Number: 40 Command: Write Attributes (0x02) Attribute Field Attribute: Unknown (0xfde8) Data Type: Array (0x48) - Elements Type: 56-Bit Bitmap (0x1e) + Elements Type: Octet String (0x41) Elements Number: 4 - Element #1, Bitmap: 020006010d0006 - Bitmap56: 0x00020006010d0006 - Element #2, Bitmap: 020006020d0106 - Bitmap56: 0x00020006020d0106 - Element #3, Bitmap: 020006030d0206 - Bitmap56: 0x00020006030d0206 - Element #4, Bitmap: 020006040d0306 - Bitmap56: 0x00020006040d0306 + Element #1, Octets: 00:0d:01:06:00:02 + Octet String: 00:0d:01:06:00:02 + Element #2, Octets: 01:0d:02:06:00:02 + Octet String: 01:0d:02:06:00:02 + Element #3, Octets: 02:0d:03:06:00:02 + Octet String: 02:0d:03:06:00:02 + Element #4, Octets: 03:0d:04:06:00:02 + Octet String: 03:0d:04:06:00:02 + +Decrypted ZigBee Payload (45 bytes) - only Array related data is shown: +0000 48 41 04 @........(...HA. +0010 00 06 00 0d 01 06 00 02 06 01 0d 02 06 00 02 06 ................ +0020 02 0d 03 06 00 02 06 03 0d 04 06 00 02 ............. ``` Using the symbolic name of the attribute, and automatic endpoint selection. From 7e34ac1bad0dbcd528597900eee2ae270b52a404 Mon Sep 17 00:00:00 2001 From: mdeweerd Date: Tue, 17 Oct 2023 18:23:24 +0200 Subject: [PATCH 8/9] Fix typo two->four --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5910e02..69cdca6 100644 --- a/README.md +++ b/README.md @@ -812,7 +812,7 @@ data: # For the array type (type 0x48): # - The first byte is the type of items. here 65 or 0x41: octet str. # - The second and third byte compose the length (little endian) - # So here: `4, 0` is 0x0004, so two octet strings the array. + # So here: `4, 0` is 0x0004, so four octet strings the array. # - All the octet strings in this example have a length of 6. attr_val: [65, 4, 0, 6, 0, 13, 1, 6, 0, 2, 6, 1, 13, 2, 6, 0, 2, 6, 2, 13, 3, 6, 0, 2, 6, 3, 13, 4, 6, 0, 2] From 994a91f910df6dda3e204beb0d74ae22673827a8 Mon Sep 17 00:00:00 2001 From: mdeweerd Date: Wed, 18 Oct 2023 12:00:13 +0200 Subject: [PATCH 9/9] Add debug info regarding false read_after_write --- custom_components/zha_toolkit/zcl_attr.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/custom_components/zha_toolkit/zcl_attr.py b/custom_components/zha_toolkit/zcl_attr.py index 4f218e1..3aada55 100644 --- a/custom_components/zha_toolkit/zcl_attr.py +++ b/custom_components/zha_toolkit/zcl_attr.py @@ -491,11 +491,20 @@ async def attr_write( # noqa: C901 f"Reading attr result (attrs, status): {result_read!r}" ) # read_is_equal = (result_read[0][attr_id] == compare_val) - success = ( - success - and (len(result_read[1]) == 0 and len(result_read[0]) == 1) - and (result_read[0][attr_id] == compare_val) + success = success and ( + len(result_read[1]) == 0 and len(result_read[0]) == 1 ) + if success and compare_val is not None: + if result_read[0][attr_id] != compare_val: + success = False + msg = "Read does not match expected: {!r} <> {!r}".format( + result_read[0][attr_id].serialize(), + compare_val.serialize(), + ) + LOGGER.warning(msg) + if "warnings" not in event_data: + event_data["warnings"] = [] + event_data["warnings"].append(msg) if result_read is not None: event_data["result_read"] = result_read