Skip to content

Commit be4547d

Browse files
committed
Refactor attribute write helpers per code review from Cecille:
- Add parameter names to AttributeWriteRequest calls in ChipDeviceCtrl.py for clarity - Update type hints to show union of 2-tuple and 3-tuple formats - Refactor ProcessWriteAttributesData in attribute.cpp to accept forceLegacyListEncoding parameter, eliminating code duplication between legacy and normal encoding paths
1 parent 200bd11 commit be4547d

File tree

2 files changed

+32
-38
lines changed

2 files changed

+32
-38
lines changed

src/controller/python/matter/ChipDeviceCtrl.py

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,16 +1607,30 @@ async def TestOnlySendCommandTimedRequestFlagWithNoTimedInvoke(self, nodeId: int
16071607
), payload).raise_on_error()
16081608
return await future
16091609

1610-
def _prepare_write_attribute_requests(self, attributes: typing.List[typing.Tuple[int, ClusterObjects.ClusterAttributeDescriptor]]) -> typing.List[ClusterAttribute.AttributeWriteRequest]:
1610+
def _prepare_write_attribute_requests(
1611+
self,
1612+
attributes: typing.List[typing.Union[
1613+
typing.Tuple[int, ClusterObjects.ClusterAttributeDescriptor],
1614+
typing.Tuple[int, ClusterObjects.ClusterAttributeDescriptor, int]
1615+
]]
1616+
) -> typing.List[ClusterAttribute.AttributeWriteRequest]:
16111617
"""Helper method to prepare attribute write requests."""
16121618
attrs = []
16131619
for v in attributes:
16141620
if len(v) == 2:
16151621
attrs.append(ClusterAttribute.AttributeWriteRequest(
1616-
v[0], v[1], 0, 0, v[1].value)) # type: ignore[attr-defined]
1622+
EndpointId=v[0],
1623+
Attribute=v[1],
1624+
DataVersion=0,
1625+
HasDataVersion=0,
1626+
Data=v[1].value)) # type: ignore[attr-defined]
16171627
else:
16181628
attrs.append(ClusterAttribute.AttributeWriteRequest(
1619-
v[0], v[1], v[2], 1, v[1].value))
1629+
EndpointId=v[0],
1630+
Attribute=v[1],
1631+
DataVersion=v[2],
1632+
HasDataVersion=1,
1633+
Data=v[1].value))
16201634
return attrs
16211635

16221636
async def TestOnlyWriteAttributeWithMismatchedTimedRequestField(self, nodeid: int,

src/controller/python/matter/clusters/attribute.cpp

Lines changed: 15 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ using namespace chip::python;
325325
namespace {
326326
// Helper function to process write attributes data - reduces code duplication
327327
CHIP_ERROR ProcessWriteAttributesData(WriteClient * client, python::PyWriteAttributeData * writeAttributesData,
328-
size_t attributeDataLength)
328+
size_t attributeDataLength, bool forceLegacyListEncoding = false)
329329
{
330330
CHIP_ERROR err = CHIP_NO_ERROR;
331331

@@ -346,9 +346,19 @@ CHIP_ERROR ProcessWriteAttributesData(WriteClient * client, python::PyWriteAttri
346346
dataVersion.SetValue(path.dataVersion);
347347
}
348348

349-
SuccessOrExit(
350-
err = client->PutPreencodedAttribute(
351-
chip::app::ConcreteDataAttributePath(path.endpointId, path.clusterId, path.attributeId, dataVersion), reader));
349+
if (forceLegacyListEncoding)
350+
{
351+
auto listEncodingOverride = WriteClient::TestListEncodingOverride::kForceLegacyEncoding;
352+
SuccessOrExit(err = client->PutPreencodedAttribute(
353+
chip::app::ConcreteDataAttributePath(path.endpointId, path.clusterId, path.attributeId, dataVersion),
354+
reader, listEncodingOverride));
355+
}
356+
else
357+
{
358+
SuccessOrExit(
359+
err = client->PutPreencodedAttribute(
360+
chip::app::ConcreteDataAttributePath(path.endpointId, path.clusterId, path.attributeId, dataVersion), reader));
361+
}
352362
}
353363

354364
exit:
@@ -402,37 +412,7 @@ PyChipError pychip_WriteClient_WriteAttributes(void * appContext, DeviceProxy *
402412

403413
VerifyOrExit(device != nullptr && device->GetSecureSession().HasValue(), err = CHIP_ERROR_MISSING_SECURE_SESSION);
404414

405-
// Handle legacy list encoding override if needed
406-
if (forceLegacyListEncoding)
407-
{
408-
for (size_t i = 0; i < attributeDataLength; i++)
409-
{
410-
python::PyAttributePath path = writeAttributesData[i].attributePath;
411-
void * tlv = writeAttributesData[i].tlvData;
412-
size_t length = writeAttributesData[i].tlvLength;
413-
414-
uint8_t * tlvBuffer = reinterpret_cast<uint8_t *>(tlv);
415-
416-
TLV::TLVReader reader;
417-
reader.Init(tlvBuffer, static_cast<uint32_t>(length));
418-
reader.Next();
419-
Optional<DataVersion> dataVersion;
420-
if (path.hasDataVersion == 1)
421-
{
422-
dataVersion.SetValue(path.dataVersion);
423-
}
424-
425-
auto listEncodingOverride = WriteClient::TestListEncodingOverride::kForceLegacyEncoding;
426-
427-
SuccessOrExit(err = client->PutPreencodedAttribute(
428-
chip::app::ConcreteDataAttributePath(path.endpointId, path.clusterId, path.attributeId, dataVersion),
429-
reader, listEncodingOverride));
430-
}
431-
}
432-
else
433-
{
434-
SuccessOrExit(err = ProcessWriteAttributesData(client.get(), writeAttributesData, attributeDataLength));
435-
}
415+
SuccessOrExit(err = ProcessWriteAttributesData(client.get(), writeAttributesData, attributeDataLength, forceLegacyListEncoding));
436416

437417
SuccessOrExit(err = client->SendWriteRequest(device->GetSecureSession().Value(),
438418
interactionTimeoutMs != 0 ? System::Clock::Milliseconds32(interactionTimeoutMs)

0 commit comments

Comments
 (0)