Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restyle Add data version support for IM write #15108

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions examples/chip-tool/commands/clusters/WriteAttributeCommand.h
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;
};
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
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
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
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
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
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
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
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