Skip to content

Commit

Permalink
[Python] Improve attribute cache code quality (#31074)
Browse files Browse the repository at this point in the history
* Improve/fix exception handling in attribute update callbacks

Commit 79cebcf ("[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.
  • Loading branch information
agners authored and pull[bot] committed Jan 10, 2024
1 parent ce124f9 commit 1176731
Showing 1 changed file with 30 additions and 52 deletions.
82 changes: 30 additions & 52 deletions src/controller/python/chip/clusters/Attribute.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
#
Expand All @@ -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:
#
Expand All @@ -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:
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 1176731

Please sign in to comment.