From 11767311351c3d0243799feeec5905be6bc42c38 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Thu, 4 Jan 2024 16:57:20 +0100 Subject: [PATCH] [Python] Improve attribute cache code quality (#31074) * Improve/fix exception handling in attribute update callbacks Commit 79cebcfce374 ("[controller] Fix flake8 warnings (#25311)") changed the exceptions catched from a global catch to a specific catch for chip.exceptions.ChipStackException exceptions. However, in this two cases the error is just converted to a Python enum, there is no case where chip.exceptions.ChipStackException would be raised. Since those callbacks are called from the C++ SDK, to the best of my knowledge, the intention was that exception would not bubble up into SDK code. Handle the Python specific exceptions and make sure they get logged verbosley. * [Python] Simplify UpdateCachedData for better readability Extract cache type implementation in functions. Also make the code a bit more Pythonic. Note: This no longer initializes endpointCache in the Cluster-View. This shouldn't matter in practice as the dictionary wasn't used. --- .../python/chip/clusters/Attribute.py | 82 +++++++------------ 1 file changed, 30 insertions(+), 52 deletions(-) diff --git a/src/controller/python/chip/clusters/Attribute.py b/src/controller/python/chip/clusters/Attribute.py index 308ffe9d8a9349..25691bb826cf55 100644 --- a/src/controller/python/chip/clusters/Attribute.py +++ b/src/controller/python/chip/clusters/Attribute.py @@ -408,18 +408,30 @@ def UpdateCachedData(self, changedPathSet: set[AttributePath]): instead of a cluster object value, a ValueDecodeFailure shall be present. ''' - tlvCache = self.attributeTLVCache - attributeCache = self.attributeCache + def handle_cluster_view(endpointId, clusterId, clusterType): + try: + decodedData = clusterType.FromDict( + data=clusterType.descriptor.TagDictToLabelDict([], self.attributeTLVCache[endpointId][clusterId])) + decodedData.SetDataVersion(self.versionList.get(endpointId, {}).get(clusterId)) + return decodedData + except Exception as ex: + return ValueDecodeFailure(self.attributeTLVCache[endpointId][clusterId], ex) + + def handle_attribute_view(endpointId, clusterId, attributeId, attributeType): + value = self.attributeTLVCache[endpointId][clusterId][attributeId] + if isinstance(value, ValueDecodeFailure): + return value + try: + return attributeType.FromTagDictOrRawValue(value) + except Exception as ex: + return ValueDecodeFailure(value, ex) for attributePath in changedPathSet: - endpointId = attributePath.EndpointId - - if endpointId not in attributeCache: - attributeCache[endpointId] = {} + endpointId, clusterId, attributeId = attributePath.EndpointId, attributePath.ClusterId, attributePath.AttributeId - endpointCache = attributeCache[endpointId] - - clusterId = attributePath.ClusterId + if endpointId not in self.attributeCache: + self.attributeCache[endpointId] = {} + endpointCache = self.attributeCache[endpointId] if clusterId not in _ClusterIndex: # @@ -430,30 +442,13 @@ def UpdateCachedData(self, changedPathSet: set[AttributePath]): clusterType = _ClusterIndex[clusterId] - if clusterType not in endpointCache: - endpointCache[clusterType] = {} - - clusterCache = endpointCache[clusterType] - clusterDataVersion = self.versionList.get( - endpointId, {}).get(clusterId, None) - if self.returnClusterObject: - try: - # Since the TLV data is already organized by attribute tags, we can trivially convert to a cluster object representation. - endpointCache[clusterType] = clusterType.FromDict( - data=clusterType.descriptor.TagDictToLabelDict([], tlvCache[endpointId][clusterId])) - endpointCache[clusterType].SetDataVersion( - clusterDataVersion) - except Exception as ex: - decodedValue = ValueDecodeFailure( - tlvCache[endpointId][clusterId], ex) - endpointCache[clusterType] = decodedValue + endpointCache[clusterType] = handle_cluster_view(endpointId, clusterId, clusterType) else: - clusterCache[DataVersion] = clusterDataVersion - - attributeId = attributePath.AttributeId - - value = tlvCache[endpointId][clusterId][attributeId] + if clusterType not in endpointCache: + endpointCache[clusterType] = {} + clusterCache = endpointCache[clusterType] + clusterCache[DataVersion] = self.versionList.get(endpointId, {}).get(clusterId) if (clusterId, attributeId) not in _AttributeIndex: # @@ -463,20 +458,7 @@ def UpdateCachedData(self, changedPathSet: set[AttributePath]): continue attributeType = _AttributeIndex[(clusterId, attributeId)][0] - - if attributeType not in clusterCache: - clusterCache[attributeType] = {} - - if isinstance(value, ValueDecodeFailure): - clusterCache[attributeType] = value - else: - try: - decodedValue = attributeType.FromTagDictOrRawValue( - tlvCache[endpointId][clusterId][attributeId]) - except Exception as ex: - decodedValue = ValueDecodeFailure(value, ex) - - clusterCache[attributeType] = decodedValue + clusterCache[attributeType] = handle_attribute_view(endpointId, clusterId, attributeId, attributeType) class SubscriptionTransaction: @@ -692,11 +674,7 @@ def GetAllEventValues(self): def handleAttributeData(self, path: AttributePathWithListIndex, dataVersion: int, status: int, data: bytes): try: - imStatus = status - try: - imStatus = chip.interaction_model.Status(status) - except chip.exceptions.ChipStackException: - pass + imStatus = chip.interaction_model.Status(status) if (imStatus != chip.interaction_model.Status.Success): attributeValue = ValueDecodeFailure( @@ -849,8 +827,8 @@ def handleResponse(self, path: AttributePath, status: int): try: imStatus = chip.interaction_model.Status(status) self._resultData.append(AttributeWriteResult(Path=path, Status=imStatus)) - except chip.exceptions.ChipStackException: - self._resultData.append(AttributeWriteResult(Path=path, Status=status)) + except ValueError as ex: + logging.exception(ex) def handleError(self, chipError: PyChipError): self._resultError = chipError