diff --git a/examples/chip-tool/commands/clusters/WriteAttributeCommand.h b/examples/chip-tool/commands/clusters/WriteAttributeCommand.h index 120d44cd775d6c..5b1fed4f287f35 100644 --- a/examples/chip-tool/commands/clusters/WriteAttributeCommand.h +++ b/examples/chip-tool/commands/clusters/WriteAttributeCommand.h @@ -32,6 +32,7 @@ class WriteAttribute : public ModelCommand, public chip::app::WriteClient::Callb AddArgument("attribute-id", 0, UINT32_MAX, &mAttributeId); AddArgument("attribute-value", &mAttributeValue); AddArgument("timedInteractionTimeoutMs", 0, UINT16_MAX, &mTimedInteractionTimeoutMs); + AddArgument("data-version", 0, UINT32_MAX, &mDataVersion); ModelCommand::AddArguments(); } @@ -41,6 +42,7 @@ class WriteAttribute : public ModelCommand, public chip::app::WriteClient::Callb AddArgument("attribute-id", 0, UINT32_MAX, &mAttributeId); AddArgument("attribute-value", &mAttributeValue); AddArgument("timedInteractionTimeoutMs", 0, UINT16_MAX, &mTimedInteractionTimeoutMs); + AddArgument("data-version", 0, UINT32_MAX, &mDataVersion); ModelCommand::AddArguments(); } @@ -48,6 +50,7 @@ class WriteAttribute : public ModelCommand, public chip::app::WriteClient::Callb ModelCommand("write", credsIssuerConfig) { AddArgument("timedInteractionTimeoutMs", 0, UINT16_MAX, &mTimedInteractionTimeoutMs); + AddArgument("data-version", 0, UINT32_MAX, &mDataVersion); } ~WriteAttribute() {} @@ -98,7 +101,8 @@ class WriteAttribute : public ModelCommand, public chip::app::WriteClient::Callb mWriteClient = std::make_unique(device->GetExchangeManager(), this, mTimedInteractionTimeoutMs); - ReturnErrorOnFailure(mWriteClient->EncodeAttribute(attributePathParams, value)); + ReturnErrorOnFailure(mWriteClient->EncodeAttribute(attributePathParams, value, mDataVersion)); + return mWriteClient->SendWriteRequest(device->GetSecureSession().Value()); } @@ -106,7 +110,7 @@ class WriteAttribute : public ModelCommand, public chip::app::WriteClient::Callb chip::ClusterId mClusterId; chip::AttributeId mAttributeId; chip::Optional mTimedInteractionTimeoutMs; - + chip::Optional mDataVersion = chip::NullOptional; CustomArgument mAttributeValue; std::unique_ptr mWriteClient; }; diff --git a/examples/chip-tool/templates/logging/DataModelLogger-src.zapt b/examples/chip-tool/templates/logging/DataModelLogger-src.zapt index c91a6aadad3a1e..62b5f708652404 100644 --- a/examples/chip-tool/templates/logging/DataModelLogger-src.zapt +++ b/examples/chip-tool/templates/logging/DataModelLogger-src.zapt @@ -63,7 +63,7 @@ CHIP_ERROR DataModelLogger::LogValue(const char * label, size_t indent, const {{ CHIP_ERROR DataModelLogger::LogAttribute(const chip::app::ConcreteDataAttributePath & path, chip::TLV::TLVReader * data) { ChipLogProgress(chipTool, "Endpoint: %" PRIu16 " Cluster: " ChipLogFormatMEI " Attribute " ChipLogFormatMEI "DataVersion: %" PRIu32, path.mEndpointId, - ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId), path.mDataVersion); + ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId), path.mDataVersion.ValueOr(0)); switch (path.mClusterId) { diff --git a/src/app/ConcreteAttributePath.h b/src/app/ConcreteAttributePath.h index 486c5dcb8a0188..ac2c7afbc3fb30 100644 --- a/src/app/ConcreteAttributePath.h +++ b/src/app/ConcreteAttributePath.h @@ -111,8 +111,10 @@ struct ConcreteDataAttributePath : public ConcreteAttributePath ConcreteAttributePath(aEndpointId, aClusterId, aAttributeId) {} - ConcreteDataAttributePath(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId, DataVersion aDataVersion) : - ConcreteAttributePath(aEndpointId, aClusterId, aAttributeId), mDataVersion(aDataVersion) + ConcreteDataAttributePath(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId, + const Optional & aDataVersion) : + ConcreteAttributePath(aEndpointId, aClusterId, aAttributeId), + mDataVersion(aDataVersion) {} ConcreteDataAttributePath(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId, ListOperation aListOp, @@ -130,9 +132,9 @@ struct ConcreteDataAttributePath : public ConcreteAttributePath // This index is only valid if `mListOp` is set to a list item operation, i.e // ReplaceItem, DeleteItem or AppendItem. Otherwise, it is to be ignored. // - uint16_t mListIndex = 0; - ListOperation mListOp = ListOperation::NotList; - DataVersion mDataVersion = 0; + uint16_t mListIndex = 0; + ListOperation mListOp = ListOperation::NotList; + Optional mDataVersion = NullOptional; }; } // namespace app diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index e88aa7c810a0f3..264d44a9832458 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -599,7 +599,9 @@ CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeRepo ReturnErrorOnFailure(report.GetAttributeData(&data)); ReturnErrorOnFailure(data.GetPath(&path)); ReturnErrorOnFailure(ProcessAttributePath(path, attributePath)); - ReturnErrorOnFailure(data.GetDataVersion(&attributePath.mDataVersion)); + DataVersion version = 0; + ReturnErrorOnFailure(data.GetDataVersion(&version)); + attributePath.mDataVersion.SetValue(version); if (mReadPrepareParams.mResubscribePolicy != nullptr) { UpdateDataVersionFilters(attributePath); @@ -880,7 +882,7 @@ void ReadClient::UpdateDataVersionFilters(const ConcreteDataAttributePath & aPat mReadPrepareParams.mpDataVersionFilterList[index].mClusterId == aPath.mClusterId) { // Now we know the current version for this cluster is aPath.mDataVersion. - mReadPrepareParams.mpDataVersionFilterList[index].mDataVersion.SetValue(aPath.mDataVersion); + mReadPrepareParams.mpDataVersionFilterList[index].mDataVersion = aPath.mDataVersion; } } } diff --git a/src/app/WriteClient.cpp b/src/app/WriteClient.cpp index 2ecc1ec2c3bf5e..ea688bfb5b1fd5 100644 --- a/src/app/WriteClient.cpp +++ b/src/app/WriteClient.cpp @@ -127,8 +127,11 @@ CHIP_ERROR WriteClient::PrepareAttributeIB(const ConcreteDataAttributePath & aPa AttributeDataIBs::Builder & writeRequests = mWriteRequestBuilder.GetWriteRequests(); AttributeDataIB::Builder & attributeDataIB = writeRequests.CreateAttributeDataIBBuilder(); ReturnErrorOnFailure(writeRequests.GetError()); - // TODO: Add attribute version support - attributeDataIB.DataVersion(0); + if (aPath.mDataVersion.HasValue()) + { + attributeDataIB.DataVersion(aPath.mDataVersion.Value()); + mHasDataVersion = true; + } ReturnErrorOnFailure(attributeDataIB.GetError()); AttributePathIB::Builder & path = attributeDataIB.CreatePath(); @@ -153,6 +156,7 @@ CHIP_ERROR WriteClient::PrepareAttributeIB(const ConcreteDataAttributePath & aPa } } ReturnErrorOnFailure(path.EndOfAttributePathIB().GetError()); + return CHIP_NO_ERROR; } @@ -368,7 +372,7 @@ CHIP_ERROR WriteClient::SendWriteRequest(const SessionHandle & session, System:: // Create a new exchange context. mpExchangeCtx = mpExchangeMgr->NewContext(session, this); VerifyOrExit(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY); - + VerifyOrReturnError(!(mpExchangeCtx->IsGroupExchangeContext() && mHasDataVersion), CHIP_ERROR_INVALID_MESSAGE_TYPE); mpExchangeCtx->SetResponseTimeout(timeout); if (mTimedWriteTimeoutMs.HasValue()) diff --git a/src/app/WriteClient.h b/src/app/WriteClient.h index 19839c877e633f..ef4c698a9b2c63 100644 --- a/src/app/WriteClient.h +++ b/src/app/WriteClient.h @@ -138,14 +138,15 @@ class WriteClient : public Messaging::ExchangeDelegate * Encode an attribute value that can be directly encoded using DataModel::Encode. Will create a new chunk when necessary. */ template - CHIP_ERROR EncodeAttribute(const AttributePathParams & attributePath, const T & value) + CHIP_ERROR EncodeAttribute(const AttributePathParams & attributePath, const T & value, + const Optional & aDataVersion = NullOptional) { ReturnErrorOnFailure(EnsureMessage()); // Here, we are using kInvalidEndpointId for missing endpoint id, which is used when sending group write requests. return EncodeSingleAttributeDataIB( ConcreteDataAttributePath(attributePath.HasWildcardEndpointId() ? kInvalidEndpointId : attributePath.mEndpointId, - attributePath.mClusterId, attributePath.mAttributeId), + attributePath.mClusterId, attributePath.mAttributeId, aDataVersion), value); } @@ -153,12 +154,13 @@ class WriteClient : public Messaging::ExchangeDelegate * Encode a possibly-chunked list attribute value. Will create a new chunk when necessary. */ template - CHIP_ERROR EncodeAttribute(const AttributePathParams & attributePath, const DataModel::List & value) + CHIP_ERROR EncodeAttribute(const AttributePathParams & attributePath, const DataModel::List & value, + const Optional & aDataVersion = NullOptional) { // Here, we are using kInvalidEndpointId for missing endpoint id, which is used when sending group write requests. ConcreteDataAttributePath path = ConcreteDataAttributePath(attributePath.HasWildcardEndpointId() ? kInvalidEndpointId : attributePath.mEndpointId, - attributePath.mClusterId, attributePath.mAttributeId); + attributePath.mClusterId, attributePath.mAttributeId, aDataVersion); ReturnErrorOnFailure(EnsureMessage()); @@ -179,7 +181,8 @@ class WriteClient : public Messaging::ExchangeDelegate * EncodeAttribute when writing a nullable list. */ template - CHIP_ERROR EncodeAttribute(const AttributePathParams & attributePath, const DataModel::Nullable & value) + CHIP_ERROR EncodeAttribute(const AttributePathParams & attributePath, const DataModel::Nullable & value, + const Optional & aDataVersion = NullOptional) { ReturnErrorOnFailure(EnsureMessage()); @@ -188,7 +191,7 @@ class WriteClient : public Messaging::ExchangeDelegate // Here, we are using kInvalidEndpointId to for missing endpoint id, which is used when sending group write requests. return EncodeSingleAttributeDataIB( ConcreteDataAttributePath(attributePath.HasWildcardEndpointId() ? kInvalidEndpointId : attributePath.mEndpointId, - attributePath.mClusterId, attributePath.mAttributeId), + attributePath.mClusterId, attributePath.mAttributeId, aDataVersion), value); } else @@ -421,6 +424,7 @@ class WriteClient : public Messaging::ExchangeDelegate // of WriteRequestMessage (another end of container)). static constexpr uint16_t kReservedSizeForTLVEncodingOverhead = kReservedSizeForIMRevision + kReservedSizeForMoreChunksFlag + kReservedSizeForEndOfContainer + kReservedSizeForEndOfContainer; + bool mHasDataVersion = false; }; } // namespace app diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index 5aa9908b08aeff..47426526e9882e 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -247,7 +247,18 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData { MatterPreAttributeWriteCallback(dataAttributePath); TLV::TLVWriter backup; + DataVersion version = 0; mWriteResponseBuilder.Checkpoint(backup); + err = element.GetDataVersion(&version); + if (CHIP_NO_ERROR == err) + { + dataAttributePath.mDataVersion.SetValue(version); + } + else if (CHIP_END_OF_TLV == err) + { + err = CHIP_NO_ERROR; + } + SuccessOrExit(err); err = WriteSingleClusterData(subjectDescriptor, dataAttributePath, dataReader, this); if (err != CHIP_NO_ERROR) { diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp index a62e03cadcfe87..833c057eb40ae9 100644 --- a/src/app/util/ember-compatibility-functions.cpp +++ b/src/app/util/ember-compatibility-functions.cpp @@ -948,6 +948,14 @@ CHIP_ERROR WriteSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, return apWriteHandler->AddStatus(aPath, Protocols::InteractionModel::Status::NeedsTimedInteraction); } + if (aPath.mDataVersion.HasValue() && + !IsClusterDataVersionEqual(aPath.mEndpointId, aPath.mClusterId, aPath.mDataVersion.Value())) + { + ChipLogError(DataManagement, "Write Version mismatch for Endpoint %" PRIx16 ", Cluster " ChipLogFormatMEI, + aPath.mEndpointId, ChipLogValueMEI(aPath.mClusterId)); + return apWriteHandler->AddStatus(aPath, Protocols::InteractionModel::Status::DataVersionMismatch); + } + if (auto * attrOverride = findAttributeAccessOverride(aPath.mEndpointId, aPath.mClusterId)) { AttributeValueDecoder valueDecoder(aReader, aSubjectDescriptor); diff --git a/src/controller/CHIPCluster.h b/src/controller/CHIPCluster.h index 942f2870b9b121..c414e0d947eb4a 100644 --- a/src/controller/CHIPCluster.h +++ b/src/controller/CHIPCluster.h @@ -115,7 +115,8 @@ class DLL_EXPORT ClusterBase template CHIP_ERROR WriteAttribute(const AttrType & requestData, void * context, ClusterId clusterId, AttributeId attributeId, WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb, - const Optional & aTimedWriteTimeoutMs, WriteResponseDoneCallback doneCb = nullptr) + const Optional & aTimedWriteTimeoutMs, WriteResponseDoneCallback doneCb = nullptr, + const Optional & aDataVersion = NullOptional) { CHIP_ERROR err = CHIP_NO_ERROR; VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE); @@ -143,14 +144,16 @@ class DLL_EXPORT ClusterBase if (mGroupSession) { - err = chip::Controller::WriteAttribute(mGroupSession.Get(), mEndpoint, clusterId, attributeId, requestData, - onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb); + err = + chip::Controller::WriteAttribute(mGroupSession.Get(), mEndpoint, clusterId, attributeId, requestData, + onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb, aDataVersion); mDevice->GetExchangeManager()->GetSessionManager()->RemoveGroupSession(mGroupSession->AsGroupSession()); } else { err = chip::Controller::WriteAttribute(mDevice->GetSecureSession().Value(), mEndpoint, clusterId, attributeId, - requestData, onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb); + requestData, onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb, + aDataVersion); } return err; @@ -159,27 +162,29 @@ class DLL_EXPORT ClusterBase template CHIP_ERROR WriteAttribute(const typename AttributeInfo::Type & requestData, void * context, WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb, - const Optional & aTimedWriteTimeoutMs, WriteResponseDoneCallback doneCb = nullptr) + const Optional & aTimedWriteTimeoutMs, WriteResponseDoneCallback doneCb = nullptr, + const Optional & aDataVersion = NullOptional) { return WriteAttribute(requestData, context, AttributeInfo::GetClusterId(), AttributeInfo::GetAttributeId(), successCb, - failureCb, aTimedWriteTimeoutMs, doneCb); + failureCb, aTimedWriteTimeoutMs, doneCb, aDataVersion); } template CHIP_ERROR WriteAttribute(const typename AttributeInfo::Type & requestData, void * context, WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb, - uint16_t aTimedWriteTimeoutMs, WriteResponseDoneCallback doneCb = nullptr) + uint16_t aTimedWriteTimeoutMs, WriteResponseDoneCallback doneCb = nullptr, + const Optional & aDataVersion = NullOptional) { - return WriteAttribute(requestData, context, successCb, failureCb, MakeOptional(aTimedWriteTimeoutMs), - doneCb); + return WriteAttribute(requestData, context, successCb, failureCb, MakeOptional(aTimedWriteTimeoutMs), doneCb, + aDataVersion); } template = 0> CHIP_ERROR WriteAttribute(const typename AttributeInfo::Type & requestData, void * context, WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb, - WriteResponseDoneCallback doneCb = nullptr) + WriteResponseDoneCallback doneCb = nullptr, const Optional & aDataVersion = NullOptional) { - return WriteAttribute(requestData, context, successCb, failureCb, NullOptional, doneCb); + return WriteAttribute(requestData, context, successCb, failureCb, NullOptional, doneCb, aDataVersion); } /** diff --git a/src/controller/WriteInteraction.h b/src/controller/WriteInteraction.h index 24ae88322a9abd..96c6965bf8bd23 100644 --- a/src/controller/WriteInteraction.h +++ b/src/controller/WriteInteraction.h @@ -101,7 +101,8 @@ template CHIP_ERROR WriteAttribute(const SessionHandle & sessionHandle, chip::EndpointId endpointId, ClusterId clusterId, AttributeId attributeId, const AttrType & requestData, WriteCallback::OnSuccessCallbackType onSuccessCb, WriteCallback::OnErrorCallbackType onErrorCb, const Optional & aTimedWriteTimeoutMs, - WriteCallback::OnDoneCallbackType onDoneCb = nullptr) + WriteCallback::OnDoneCallbackType onDoneCb = nullptr, + const Optional & aDataVersion = NullOptional) { auto callback = Platform::MakeUnique(onSuccessCb, onErrorCb, onDoneCb); VerifyOrReturnError(callback != nullptr, CHIP_ERROR_NO_MEMORY); @@ -117,7 +118,7 @@ CHIP_ERROR WriteAttribute(const SessionHandle & sessionHandle, chip::EndpointId else { ReturnErrorOnFailure( - client->EncodeAttribute(chip::app::AttributePathParams(endpointId, clusterId, attributeId), requestData)); + client->EncodeAttribute(chip::app::AttributePathParams(endpointId, clusterId, attributeId), requestData, aDataVersion)); } ReturnErrorOnFailure(client->SendWriteRequest(sessionHandle)); @@ -134,28 +135,32 @@ template CHIP_ERROR WriteAttribute(const SessionHandle & sessionHandle, chip::EndpointId endpointId, const typename AttributeInfo::Type & requestData, WriteCallback::OnSuccessCallbackType onSuccessCb, WriteCallback::OnErrorCallbackType onErrorCb, const Optional & aTimedWriteTimeoutMs, - WriteCallback::OnDoneCallbackType onDoneCb = nullptr) + WriteCallback::OnDoneCallbackType onDoneCb = nullptr, + const Optional & aDataVersion = NullOptional) { return WriteAttribute(sessionHandle, endpointId, AttributeInfo::GetClusterId(), AttributeInfo::GetAttributeId(), requestData, - onSuccessCb, onErrorCb, aTimedWriteTimeoutMs, onDoneCb); + onSuccessCb, onErrorCb, aTimedWriteTimeoutMs, onDoneCb, aDataVersion); } template CHIP_ERROR WriteAttribute(const SessionHandle & sessionHandle, chip::EndpointId endpointId, const typename AttributeInfo::Type & requestData, WriteCallback::OnSuccessCallbackType onSuccessCb, WriteCallback::OnErrorCallbackType onErrorCb, uint16_t aTimedWriteTimeoutMs, - WriteCallback::OnDoneCallbackType onDoneCb = nullptr) + WriteCallback::OnDoneCallbackType onDoneCb = nullptr, + const Optional & aDataVersion = NullOptional) { return WriteAttribute(sessionHandle, endpointId, requestData, onSuccessCb, onErrorCb, onDoneCb, - MakeOptional(aTimedWriteTimeoutMs), onDoneCb); + MakeOptional(aTimedWriteTimeoutMs), onDoneCb, aDataVersion); } template = 0> CHIP_ERROR WriteAttribute(const SessionHandle & sessionHandle, chip::EndpointId endpointId, const typename AttributeInfo::Type & requestData, WriteCallback::OnSuccessCallbackType onSuccessCb, - WriteCallback::OnErrorCallbackType onErrorCb, WriteCallback::OnDoneCallbackType onDoneCb = nullptr) + WriteCallback::OnErrorCallbackType onErrorCb, WriteCallback::OnDoneCallbackType onDoneCb = nullptr, + const Optional & aDataVersion = NullOptional) { - return WriteAttribute(sessionHandle, endpointId, requestData, onSuccessCb, onErrorCb, NullOptional, onDoneCb); + return WriteAttribute(sessionHandle, endpointId, requestData, onSuccessCb, onErrorCb, NullOptional, onDoneCb, + aDataVersion); } } // namespace Controller diff --git a/src/controller/python/chip/ChipDeviceCtrl.py b/src/controller/python/chip/ChipDeviceCtrl.py index 152b76dcbae3d8..b0a0498618ca0f 100644 --- a/src/controller/python/chip/ChipDeviceCtrl.py +++ b/src/controller/python/chip/ChipDeviceCtrl.py @@ -539,7 +539,7 @@ async def SendCommand(self, nodeid: int, endpoint: int, payload: ClusterObjects. future.set_exception(self._ChipStack.ErrorToException(res)) return await future - async def WriteAttribute(self, nodeid: int, attributes: typing.List[typing.Tuple[int, ClusterObjects.ClusterAttributeDescriptor]], timedRequestTimeoutMs: int = None): + async def WriteAttribute(self, nodeid: int, attributes: typing.List[typing.Tuple[int, ClusterObjects.ClusterAttributeDescriptor, int]], timedRequestTimeoutMs: int = None): ''' Write a list of attributes on a target node. @@ -559,8 +559,12 @@ async def WriteAttribute(self, nodeid: int, attributes: typing.List[typing.Tuple attrs = [] for v in attributes: - attrs.append(ClusterAttribute.AttributeWriteRequest( - v[0], v[1], v[1].value)) + if len(v) == 2: + attrs.append(ClusterAttribute.AttributeWriteRequest( + v[0], v[1], 0, 0, v[1].value)) + else: + attrs.append(ClusterAttribute.AttributeWriteRequest( + v[0], v[1], v[2], 1, v[1].value)) res = self._ChipStack.Call( lambda: ClusterAttribute.WriteAttributes( @@ -774,7 +778,7 @@ def ZCLReadAttribute(self, cluster, attribute, nodeid, endpoint, groupid, blocki EndpointId=endpoint, Attribute=attributeType) return im.AttributeReadResult(path=im.AttributePath(nodeId=nodeid, endpointId=path.EndpointId, clusterId=path.ClusterId, attributeId=path.AttributeId), status=0, value=result[0][endpoint][clusterType][attributeType]) - def ZCLWriteAttribute(self, cluster: str, attribute: str, nodeid, endpoint, groupid, value, blocking=True): + def ZCLWriteAttribute(self, cluster: str, attribute: str, nodeid, endpoint, groupid, value, dataVersion=0, blocking=True): req = None try: req = eval( @@ -782,7 +786,7 @@ def ZCLWriteAttribute(self, cluster: str, attribute: str, nodeid, endpoint, grou except: raise UnknownAttribute(cluster, attribute) - return asyncio.run(self.WriteAttribute(nodeid, [(endpoint, req)])) + return asyncio.run(self.WriteAttribute(nodeid, [(endpoint, req, dataVersion)])) def ZCLSubscribeAttribute(self, cluster, attribute, nodeid, endpoint, minInterval, maxInterval, blocking=True): self.CheckIsActive() diff --git a/src/controller/python/chip/clusters/Attribute.py b/src/controller/python/chip/clusters/Attribute.py index f7f0aceecbbf10..4214a45364791f 100644 --- a/src/controller/python/chip/clusters/Attribute.py +++ b/src/controller/python/chip/clusters/Attribute.py @@ -239,6 +239,8 @@ class EventStatus: class AttributeDescriptorWithEndpoint: EndpointId: int Attribute: ClusterAttributeDescriptor + DataVersion: int + HasDataVersion: int @dataclass @@ -821,6 +823,8 @@ def WriteAttributes(future: Future, eventLoop, device, attributes: List[Attribut path.EndpointId = attr.EndpointId path.ClusterId = attr.Attribute.cluster_id path.AttributeId = attr.Attribute.attribute_id + path.DataVersion = attr.DataVersion + path.HasDataVersion = attr.HasDataVersion path = chip.interaction_model.AttributePathIBstruct.build(path) tlv = attr.Attribute.ToTLV(None, attr.Data) writeargs.append(ctypes.c_char_p(path)) diff --git a/src/controller/python/chip/clusters/attribute.cpp b/src/controller/python/chip/clusters/attribute.cpp index 02bdba67ad600a..3156b077f6137b 100644 --- a/src/controller/python/chip/clusters/attribute.cpp +++ b/src/controller/python/chip/clusters/attribute.cpp @@ -43,6 +43,7 @@ struct __attribute__((packed)) AttributePath chip::ClusterId clusterId; chip::AttributeId attributeId; chip::DataVersion dataVersion; + uint8_t hasDataVersion; }; struct __attribute__((packed)) EventPath @@ -115,7 +116,16 @@ class ReadClientCallback : public ReadClient::Callback size = writer.GetLengthWritten(); } - gOnReadAttributeDataCallback(mAppContext, aPath.mDataVersion, aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId, + DataVersion version = 0; + if (aPath.mDataVersion.HasValue()) + { + version = aPath.mDataVersion.Value(); + } + else + { + ChipLogError(DataManagement, "expect aPath has valid mDataVersion"); + } + gOnReadAttributeDataCallback(mAppContext, version, aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId, to_underlying(aStatus.mStatus), buffer.get(), size); } @@ -313,10 +323,15 @@ chip::ChipError::StorageType pychip_WriteClient_WriteAttributes(void * appContex TLV::TLVReader reader; reader.Init(tlvBuffer, static_cast(length)); reader.Next(); - + Optional dataVersion; + if (pathObj.hasDataVersion == 1) + { + dataVersion.SetValue(pathObj.dataVersion); + } SuccessOrExit( err = client->PutPreencodedAttribute( - chip::app::ConcreteDataAttributePath(pathObj.endpointId, pathObj.clusterId, pathObj.attributeId), reader)); + chip::app::ConcreteDataAttributePath(pathObj.endpointId, pathObj.clusterId, pathObj.attributeId, dataVersion), + reader)); } } diff --git a/src/controller/python/chip/interaction_model/delegate.py b/src/controller/python/chip/interaction_model/delegate.py index dbbd818f98fd4b..620ed58dbdf5d6 100644 --- a/src/controller/python/chip/interaction_model/delegate.py +++ b/src/controller/python/chip/interaction_model/delegate.py @@ -52,6 +52,7 @@ "ClusterId" / Int32ul, "AttributeId" / Int32ul, "DataVersion" / Int32ul, + "HasDataVersion" / Int8ul, ) # EventPath should not contain padding diff --git a/src/controller/python/test/test_scripts/cluster_objects.py b/src/controller/python/test/test_scripts/cluster_objects.py index 32ee18cf588eaa..c83ccaf7c8f71d 100644 --- a/src/controller/python/test/test_scripts/cluster_objects.py +++ b/src/controller/python/test/test_scripts/cluster_objects.py @@ -121,9 +121,9 @@ async def SendWriteRequest(cls, devCtrl): res = await devCtrl.WriteAttribute(nodeid=NODE_ID, attributes=[ (0, Clusters.Basic.Attributes.NodeLabel( - "Test"), 0), + "Test")), (0, Clusters.Basic.Attributes.Location( - "A loooong string"), 0) + "A loooong string")) ]) expectedRes = [ AttributeStatus(Path=AttributePath(EndpointId=0, ClusterId=40, @@ -321,7 +321,7 @@ async def TestTimedRequest(cls, devCtrl): await devCtrl.WriteAttribute(nodeid=NODE_ID, attributes=[ (1, Clusters.TestCluster.Attributes.TimedWriteBoolean( - True), 0), + True)), ], timedRequestTimeoutMs=1000) @@ -339,7 +339,7 @@ async def TestTimedRequest(cls, devCtrl): await devCtrl.WriteAttribute(nodeid=NODE_ID, attributes=[ (1, Clusters.TestCluster.Attributes.TimedWriteBoolean( - True), 0), + True)), ], timedRequestTimeoutMs=10) raise AssertionError("Timeout expected!") @@ -361,7 +361,7 @@ async def TestTimedRequest(cls, devCtrl): await devCtrl.WriteAttribute(nodeid=NODE_ID, attributes=[ (1, Clusters.TestCluster.Attributes.TimedWriteBoolean( - True), 0), + True)), ]) raise AssertionError("The write request should be rejected.") except ValueError: @@ -380,7 +380,7 @@ async def TestReadWriteAttributeRequestsWithVersion(cls, devCtrl): res = await devCtrl.WriteAttribute(nodeid=NODE_ID, attributes=[ (0, Clusters.Basic.Attributes.NodeLabel( - "Test"), 0) + "Test")) ]) expectedRes = [ AttributeStatus(Path=AttributePath(EndpointId=0, ClusterId=40, @@ -406,6 +406,42 @@ async def TestReadWriteAttributeRequestsWithVersion(cls, devCtrl): res = await devCtrl.ReadAttribute(nodeid=NODE_ID, attributes=req, dataVersionFilters=[(0, Clusters.Basic, new_data_version)]) VerifyDecodeSuccess(res) + res = await devCtrl.WriteAttribute(nodeid=NODE_ID, + attributes=[ + (0, Clusters.Basic.Attributes.NodeLabel( + "Test"), new_data_version) + ]) + + expectedRes = [ + AttributeStatus(Path=AttributePath(EndpointId=0, ClusterId=40, + AttributeId=5), Status=chip.interaction_model.Status.Success), + ] + + if res != expectedRes: + for i in range(len(res)): + if res[i] != expectedRes[i]: + logger.error( + f"Item {i} is not expected, expect {expectedRes[i]} got {res[i]}") + raise AssertionError("Write returned unexpected result.") + + res = await devCtrl.WriteAttribute(nodeid=NODE_ID, + attributes=[ + (0, Clusters.Basic.Attributes.NodeLabel( + "Test"), new_data_version) + ]) + + expectedRes = [ + AttributeStatus(Path=AttributePath(EndpointId=0, ClusterId=40, + AttributeId=5), Status=chip.interaction_model.Status.DataVersionMismatch), + ] + + if res != expectedRes: + for i in range(len(res)): + if res[i] != expectedRes[i]: + logger.error( + f"Item {i} is not expected, expect {expectedRes[i]} got {res[i]}") + raise AssertionError("Write returned unexpected result.") + @classmethod async def RunTest(cls, devCtrl): try: diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 2451eda01410cb..27a71c5909a46e 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -175,7 +175,7 @@ void TestReadInteraction::TestReadAttributeResponse(nlTestSuite * apSuite, void auto onSuccessCb = [apSuite, &onSuccessCbInvoked](const app::ConcreteDataAttributePath & attributePath, const auto & dataResponse) { uint8_t i = 0; - NL_TEST_ASSERT(apSuite, attributePath.mDataVersion == kDataVersion); + NL_TEST_ASSERT(apSuite, attributePath.mDataVersion.HasValue() && attributePath.mDataVersion.Value() == kDataVersion); auto iter = dataResponse.begin(); while (iter.Next()) { @@ -453,7 +453,7 @@ void TestReadInteraction::TestReadHandler_MultipleSubscriptionsWithDataVersionFi // not safe to do so. auto onSuccessCb = [apSuite, &numSuccessCalls](const app::ConcreteDataAttributePath & attributePath, const auto & dataResponse) { - NL_TEST_ASSERT(apSuite, attributePath.mDataVersion == kDataVersion); + NL_TEST_ASSERT(apSuite, attributePath.mDataVersion.HasValue() && attributePath.mDataVersion.Value() == kDataVersion); numSuccessCalls++; }; diff --git a/src/controller/tests/data_model/TestWrite.cpp b/src/controller/tests/data_model/TestWrite.cpp index c13f5473a8438c..780bf183d51ed8 100644 --- a/src/controller/tests/data_model/TestWrite.cpp +++ b/src/controller/tests/data_model/TestWrite.cpp @@ -38,7 +38,9 @@ using namespace chip::Protocols; namespace { -constexpr EndpointId kTestEndpointId = 1; +constexpr EndpointId kTestEndpointId = 1; +constexpr DataVersion kRejectedDataVersion = 1; +constexpr DataVersion kAcceptedDataVersion = 5; enum ResponseDirective { @@ -52,11 +54,15 @@ ResponseDirective responseDirective; namespace chip { namespace app { - CHIP_ERROR WriteSingleClusterData(const Access::SubjectDescriptor & aSubjectDescriptor, const ConcreteDataAttributePath & aPath, TLV::TLVReader & aReader, WriteHandler * aWriteHandler) { static ListIndex listStructOctetStringElementCount = 0; + if (aPath.mDataVersion.HasValue() && aPath.mDataVersion.Value() == kRejectedDataVersion) + { + return aWriteHandler->AddStatus(aPath, Protocols::InteractionModel::Status::DataVersionMismatch); + } + if (aPath.mClusterId == TestCluster::Id && aPath.mAttributeId == Attributes::ListStructOctetString::TypeInfo::GetAttributeId()) { if (responseDirective == kSendAttributeSuccess) @@ -104,7 +110,6 @@ CHIP_ERROR WriteSingleClusterData(const Access::SubjectDescriptor & aSubjectDesc return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; } - } // namespace app } // namespace chip @@ -116,10 +121,10 @@ class TestWriteInteraction TestWriteInteraction() {} static void TestDataResponse(nlTestSuite * apSuite, void * apContext); + static void TestDataResponseWithAcceptedDataVersion(nlTestSuite * apSuite, void * apContext); + static void TestDataResponseWithRejectedDataVersion(nlTestSuite * apSuite, void * apContext); static void TestAttributeError(nlTestSuite * apSuite, void * apContext); static void TestWriteTimeout(nlTestSuite * apSuite, void * apContext); - -private: }; void TestWriteInteraction::TestDataResponse(nlTestSuite * apSuite, void * apContext) @@ -161,6 +166,87 @@ void TestWriteInteraction::TestDataResponse(nlTestSuite * apSuite, void * apCont NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); } +void TestWriteInteraction::TestDataResponseWithAcceptedDataVersion(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + bool onSuccessCbInvoked = false, onFailureCbInvoked = false; + TestCluster::Structs::TestListStructOctet::Type valueBuf[4]; + TestCluster::Attributes::ListStructOctetString::TypeInfo::Type value; + + value = valueBuf; + + uint8_t i = 0; + for (auto & item : valueBuf) + { + item.fabricIndex = i; + i++; + } + + responseDirective = kSendAttributeSuccess; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [&onSuccessCbInvoked](const app::ConcreteAttributePath & attributePath) { onSuccessCbInvoked = true; }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [&onFailureCbInvoked](const app::ConcreteAttributePath * attributePath, CHIP_ERROR aError) { + onFailureCbInvoked = true; + }; + + chip::Optional dataVersion; + dataVersion.SetValue(kAcceptedDataVersion); + chip::Controller::WriteAttribute( + sessionHandle, kTestEndpointId, value, onSuccessCb, onFailureCb, nullptr, dataVersion); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, onSuccessCbInvoked && !onFailureCbInvoked); + NL_TEST_ASSERT(apSuite, chip::app::InteractionModelEngine::GetInstance()->GetNumActiveWriteHandlers() == 0); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); +} + +void TestWriteInteraction::TestDataResponseWithRejectedDataVersion(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + bool onSuccessCbInvoked = false, onFailureCbInvoked = false; + TestCluster::Structs::TestListStructOctet::Type valueBuf[4]; + TestCluster::Attributes::ListStructOctetString::TypeInfo::Type value; + + value = valueBuf; + + uint8_t i = 0; + for (auto & item : valueBuf) + { + item.fabricIndex = i; + i++; + } + + responseDirective = kSendAttributeSuccess; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [&onSuccessCbInvoked](const app::ConcreteAttributePath & attributePath) { onSuccessCbInvoked = true; }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [&onFailureCbInvoked](const app::ConcreteAttributePath * attributePath, CHIP_ERROR aError) { + onFailureCbInvoked = true; + }; + + chip::Optional dataVersion(kRejectedDataVersion); + chip::Controller::WriteAttribute( + sessionHandle, kTestEndpointId, value, onSuccessCb, onFailureCb, nullptr, dataVersion); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, !onSuccessCbInvoked && onFailureCbInvoked); + NL_TEST_ASSERT(apSuite, chip::app::InteractionModelEngine::GetInstance()->GetNumActiveWriteHandlers() == 0); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); +} + void TestWriteInteraction::TestAttributeError(nlTestSuite * apSuite, void * apContext) { TestContext & ctx = *static_cast(apContext); @@ -205,6 +291,8 @@ void TestWriteInteraction::TestAttributeError(nlTestSuite * apSuite, void * apCo const nlTest sTests[] = { NL_TEST_DEF("TestDataResponse", TestWriteInteraction::TestDataResponse), + NL_TEST_DEF("TestDataResponseWithAcceptedDataVersion", TestWriteInteraction::TestDataResponseWithAcceptedDataVersion), + NL_TEST_DEF("TestDataResponseWithRejectedDataVersion", TestWriteInteraction::TestDataResponseWithRejectedDataVersion), NL_TEST_DEF("TestAttributeError", TestWriteInteraction::TestAttributeError), NL_TEST_SENTINEL() }; diff --git a/zzz_generated/chip-tool/zap-generated/cluster/logging/DataModelLogger.cpp b/zzz_generated/chip-tool/zap-generated/cluster/logging/DataModelLogger.cpp index d2203d405eaa26..a698c4dd14a524 100644 --- a/zzz_generated/chip-tool/zap-generated/cluster/logging/DataModelLogger.cpp +++ b/zzz_generated/chip-tool/zap-generated/cluster/logging/DataModelLogger.cpp @@ -4120,9 +4120,9 @@ CHIP_ERROR DataModelLogger::LogValue(const char * label, size_t indent, CHIP_ERROR DataModelLogger::LogAttribute(const chip::app::ConcreteDataAttributePath & path, chip::TLV::TLVReader * data) { - ChipLogProgress(chipTool, - "Endpoint: %" PRIu16 " Cluster: " ChipLogFormatMEI " Attribute " ChipLogFormatMEI "DataVersion: %" PRIu32, - path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId), path.mDataVersion); + ChipLogProgress( + chipTool, "Endpoint: %" PRIu16 " Cluster: " ChipLogFormatMEI " Attribute " ChipLogFormatMEI "DataVersion: %" PRIu32, + path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId), path.mDataVersion.ValueOr(0)); switch (path.mClusterId) {