Skip to content

Commit

Permalink
Add data version support for IM write (#15101)
Browse files Browse the repository at this point in the history
  • Loading branch information
@yunhanw-google @pull
yunhanw-google authored and pull[bot] committed Oct 13, 2023
1 parent c73c0c8 commit 3882327
Show file tree
Hide file tree
Showing 18 changed files with 255 additions and 62 deletions.
8 changes: 6 additions & 2 deletions examples/chip-tool/commands/clusters/WriteAttributeCommand.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand All @@ -41,13 +42,15 @@ 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();
}

WriteAttribute(const char * attributeName, CredentialIssuerCommands * credsIssuerConfig) :
ModelCommand("write", credsIssuerConfig)
{
AddArgument("timedInteractionTimeoutMs", 0, UINT16_MAX, &mTimedInteractionTimeoutMs);
AddArgument("data-version", 0, UINT32_MAX, &mDataVersion);
}

~WriteAttribute() {}
Expand Down Expand Up @@ -98,15 +101,16 @@ class WriteAttribute : public ModelCommand, public chip::app::WriteClient::Callb

mWriteClient = std::make_unique<chip::app::WriteClient>(device->GetExchangeManager(), this, mTimedInteractionTimeoutMs);

ReturnErrorOnFailure(mWriteClient->EncodeAttribute(attributePathParams, value));
ReturnErrorOnFailure(mWriteClient->EncodeAttribute(attributePathParams, value, mDataVersion));

return mWriteClient->SendWriteRequest(device->GetSecureSession().Value());
}

private:
chip::ClusterId mClusterId;
chip::AttributeId mAttributeId;
chip::Optional<uint16_t> mTimedInteractionTimeoutMs;

chip::Optional<chip::DataVersion> mDataVersion = chip::NullOptional;
CustomArgument mAttributeValue;
std::unique_ptr<chip::app::WriteClient> mWriteClient;
};
2 changes: 1 addition & 1 deletion examples/chip-tool/templates/logging/DataModelLogger-src.zapt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
12 changes: 7 additions & 5 deletions src/app/ConcreteAttributePath.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<DataVersion> & aDataVersion) :
ConcreteAttributePath(aEndpointId, aClusterId, aAttributeId),
mDataVersion(aDataVersion)
{}

ConcreteDataAttributePath(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId, ListOperation aListOp,
Expand All @@ -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<DataVersion> mDataVersion = NullOptional;
};

} // namespace app
Expand Down
6 changes: 4 additions & 2 deletions src/app/ReadClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}
}
Expand Down
10 changes: 7 additions & 3 deletions src/app/WriteClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -153,6 +156,7 @@ CHIP_ERROR WriteClient::PrepareAttributeIB(const ConcreteDataAttributePath & aPa
}
}
ReturnErrorOnFailure(path.EndOfAttributePathIB().GetError());

return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -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())
Expand Down
16 changes: 10 additions & 6 deletions src/app/WriteClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,27 +138,29 @@ 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 <class T>
CHIP_ERROR EncodeAttribute(const AttributePathParams & attributePath, const T & value)
CHIP_ERROR EncodeAttribute(const AttributePathParams & attributePath, const T & value,
const Optional<DataVersion> & 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);
}

/**
* Encode a possibly-chunked list attribute value. Will create a new chunk when necessary.
*/
template <class T>
CHIP_ERROR EncodeAttribute(const AttributePathParams & attributePath, const DataModel::List<T> & value)
CHIP_ERROR EncodeAttribute(const AttributePathParams & attributePath, const DataModel::List<T> & value,
const Optional<DataVersion> & 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());

Expand All @@ -179,7 +181,8 @@ class WriteClient : public Messaging::ExchangeDelegate
* EncodeAttribute when writing a nullable list.
*/
template <class T>
CHIP_ERROR EncodeAttribute(const AttributePathParams & attributePath, const DataModel::Nullable<T> & value)
CHIP_ERROR EncodeAttribute(const AttributePathParams & attributePath, const DataModel::Nullable<T> & value,
const Optional<DataVersion> & aDataVersion = NullOptional)
{
ReturnErrorOnFailure(EnsureMessage());

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions src/app/WriteHandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
8 changes: 8 additions & 0 deletions src/app/util/ember-compatibility-functions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
27 changes: 16 additions & 11 deletions src/controller/CHIPCluster.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ class DLL_EXPORT ClusterBase
template <typename AttrType>
CHIP_ERROR WriteAttribute(const AttrType & requestData, void * context, ClusterId clusterId, AttributeId attributeId,
WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb,
const Optional<uint16_t> & aTimedWriteTimeoutMs, WriteResponseDoneCallback doneCb = nullptr)
const Optional<uint16_t> & aTimedWriteTimeoutMs, WriteResponseDoneCallback doneCb = nullptr,
const Optional<DataVersion> & aDataVersion = NullOptional)
{
CHIP_ERROR err = CHIP_NO_ERROR;
VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE);
Expand Down Expand Up @@ -143,14 +144,16 @@ class DLL_EXPORT ClusterBase

if (mGroupSession)
{
err = chip::Controller::WriteAttribute<AttrType>(mGroupSession.Get(), mEndpoint, clusterId, attributeId, requestData,
onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb);
err =
chip::Controller::WriteAttribute<AttrType>(mGroupSession.Get(), mEndpoint, clusterId, attributeId, requestData,
onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb, aDataVersion);
mDevice->GetExchangeManager()->GetSessionManager()->RemoveGroupSession(mGroupSession->AsGroupSession());
}
else
{
err = chip::Controller::WriteAttribute<AttrType>(mDevice->GetSecureSession().Value(), mEndpoint, clusterId, attributeId,
requestData, onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb);
requestData, onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb,
aDataVersion);
}

return err;
Expand All @@ -159,27 +162,29 @@ class DLL_EXPORT ClusterBase
template <typename AttributeInfo>
CHIP_ERROR WriteAttribute(const typename AttributeInfo::Type & requestData, void * context,
WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb,
const Optional<uint16_t> & aTimedWriteTimeoutMs, WriteResponseDoneCallback doneCb = nullptr)
const Optional<uint16_t> & aTimedWriteTimeoutMs, WriteResponseDoneCallback doneCb = nullptr,
const Optional<DataVersion> & aDataVersion = NullOptional)
{
return WriteAttribute(requestData, context, AttributeInfo::GetClusterId(), AttributeInfo::GetAttributeId(), successCb,
failureCb, aTimedWriteTimeoutMs, doneCb);
failureCb, aTimedWriteTimeoutMs, doneCb, aDataVersion);
}

template <typename AttributeInfo>
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<DataVersion> & aDataVersion = NullOptional)
{
return WriteAttribute<AttributeInfo>(requestData, context, successCb, failureCb, MakeOptional(aTimedWriteTimeoutMs),
doneCb);
return WriteAttribute<AttributeInfo>(requestData, context, successCb, failureCb, MakeOptional(aTimedWriteTimeoutMs), doneCb,
aDataVersion);
}

template <typename AttributeInfo, typename std::enable_if_t<!AttributeInfo::MustUseTimedWrite(), int> = 0>
CHIP_ERROR WriteAttribute(const typename AttributeInfo::Type & requestData, void * context,
WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb,
WriteResponseDoneCallback doneCb = nullptr)
WriteResponseDoneCallback doneCb = nullptr, const Optional<DataVersion> & aDataVersion = NullOptional)
{
return WriteAttribute<AttributeInfo>(requestData, context, successCb, failureCb, NullOptional, doneCb);
return WriteAttribute<AttributeInfo>(requestData, context, successCb, failureCb, NullOptional, doneCb, aDataVersion);
}

/**
Expand Down
Loading

0 comments on commit 3882327

Please sign in to comment.