Skip to content

Commit

Permalink
Add initial cluster data version support (#13400)
Browse files Browse the repository at this point in the history
  • Loading branch information
yunhanw-google authored Feb 1, 2022
1 parent 723671f commit f09c5fc
Show file tree
Hide file tree
Showing 21 changed files with 116 additions and 58 deletions.
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/clusters/ReportCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class ReportCommand : public ModelCommand, public chip::app::ReadClient::Callbac
virtual void OnEventSubscription(){};

/////////// ReadClient Callback Interface /////////
void OnAttributeData(const chip::app::ConcreteDataAttributePath & path, chip::TLV::TLVReader * data,
void OnAttributeData(const chip::app::ConcreteDataAttributePath & path, chip::DataVersion aVersion, chip::TLV::TLVReader * data,
const chip::app::StatusIB & status) override
{
CHIP_ERROR error = status.ToChipError();
Expand Down
5 changes: 3 additions & 2 deletions src/app/AttributeCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ void AttributeCache::OnReportEnd()
mCallback.OnReportEnd();
}

void AttributeCache::OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus)
void AttributeCache::OnAttributeData(const ConcreteDataAttributePath & aPath, DataVersion aVersion, TLV::TLVReader * apData,
const StatusIB & aStatus)
{
//
// Since the cache itself is a ReadClient::Callback, it may be incorrectly passed in directly when registering with the
Expand All @@ -120,7 +121,7 @@ void AttributeCache::OnAttributeData(const ConcreteDataAttributePath & aPath, TL
//
// Forward the call through.
//
mCallback.OnAttributeData(aPath, apData, aStatus);
mCallback.OnAttributeData(aPath, aVersion, apData, aStatus);
}

CHIP_ERROR AttributeCache::Get(const ConcreteAttributePath & path, TLV::TLVReader & reader)
Expand Down
3 changes: 2 additions & 1 deletion src/app/AttributeCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,8 @@ class AttributeCache : protected ReadClient::Callback
//
void OnReportBegin() override;
void OnReportEnd() override;
void OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) override;
void OnAttributeData(const ConcreteDataAttributePath & aPath, DataVersion aVersion, TLV::TLVReader * apData,
const StatusIB & aStatus) override;
void OnError(CHIP_ERROR aError) override { return mCallback.OnError(aError); }

void OnEventData(const EventHeader & aEventHeader, TLV::TLVReader * apData, const StatusIB * apStatus) override
Expand Down
8 changes: 4 additions & 4 deletions src/app/BufferedReadCallback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,18 +215,17 @@ CHIP_ERROR BufferedReadCallback::DispatchBufferedData(const ConcreteAttributePat
//
ReturnErrorOnFailure(reader.Next());

mCallback.OnAttributeData(mBufferedPath, &reader, statusIB);
mCallback.OnAttributeData(mBufferedPath, mDataVersion, &reader, statusIB);

//
// Clear out our buffered contents to free up allocated buffers, and reset the buffered path.
//
mBufferedList.clear();
mBufferedPath = ConcreteDataAttributePath();

return CHIP_NO_ERROR;
}

void BufferedReadCallback::OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData,
void BufferedReadCallback::OnAttributeData(const ConcreteDataAttributePath & aPath, DataVersion aVersion, TLV::TLVReader * apData,
const StatusIB & aStatus)
{
CHIP_ERROR err;
Expand All @@ -247,13 +246,14 @@ void BufferedReadCallback::OnAttributeData(const ConcreteDataAttributePath & aPa
}
else
{
mCallback.OnAttributeData(aPath, apData, aStatus);
mCallback.OnAttributeData(aPath, aVersion, apData, aStatus);
}

//
// Update our latched buffered path.
//
mBufferedPath = aPath;
mDataVersion = aVersion;

exit:
if (err != CHIP_NO_ERROR)
Expand Down
5 changes: 3 additions & 2 deletions src/app/BufferedReadCallback.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ class BufferedReadCallback : public ReadClient::Callback
//
void OnReportBegin() override;
void OnReportEnd() override;
void OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) override;
void OnAttributeData(const ConcreteDataAttributePath & aPath, DataVersion aVersion, TLV::TLVReader * apData,
const StatusIB & aStatus) override;
void OnError(CHIP_ERROR aError) override { return mCallback.OnError(aError); }
void OnEventData(const EventHeader & aEventHeader, TLV::TLVReader * apData, const StatusIB * apStatus) override
{
Expand All @@ -93,7 +94,7 @@ class BufferedReadCallback : public ReadClient::Callback
*
*/
CHIP_ERROR BufferListItem(TLV::TLVReader & reader);

DataVersion mDataVersion;
ConcreteDataAttributePath mBufferedPath;
std::vector<System::PacketBufferHandle> mBufferedList;
Callback & mCallback;
Expand Down
8 changes: 5 additions & 3 deletions src/app/ReadClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -569,21 +569,23 @@ CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeRepo
TLV::TLVReader reader = aAttributeReportIBsReader;
ReturnErrorOnFailure(report.Init(reader));

err = report.GetAttributeStatus(&status);
DataVersion version = kUndefinedDataVersion;
err = report.GetAttributeStatus(&status);
if (CHIP_NO_ERROR == err)
{
StatusIB::Parser errorStatus;
ReturnErrorOnFailure(status.GetPath(&path));
ReturnErrorOnFailure(ProcessAttributePath(path, attributePath));
ReturnErrorOnFailure(status.GetErrorStatus(&errorStatus));
ReturnErrorOnFailure(errorStatus.DecodeStatusIB(statusIB));
mpCallback.OnAttributeData(attributePath, nullptr, statusIB);
mpCallback.OnAttributeData(attributePath, version, nullptr, statusIB);
}
else if (CHIP_END_OF_TLV == err)
{
ReturnErrorOnFailure(report.GetAttributeData(&data));
ReturnErrorOnFailure(data.GetPath(&path));
ReturnErrorOnFailure(ProcessAttributePath(path, attributePath));
ReturnErrorOnFailure(data.GetDataVersion(&version));
ReturnErrorOnFailure(data.GetData(&dataReader));

// The element in an array may be another array -- so we should only set the list operation when we are handling the
Expand All @@ -593,7 +595,7 @@ CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeRepo
attributePath.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll;
}

mpCallback.OnAttributeData(attributePath, &dataReader, statusIB);
mpCallback.OnAttributeData(attributePath, version, &dataReader, statusIB);
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/app/ReadClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,14 @@ class ReadClient : public Messaging::ExchangeDelegate
* receives an OnDone call to destroy the object.
*
* @param[in] aPath The attribute path field in report response.
* @param[in] aVersion The data version for cluster in report response.
* @param[in] apData The attribute data of the given path, will be a nullptr if status is not Success.
* @param[in] aStatus Attribute-specific status, containing an InteractionModel::Status code as well as an
* optional cluster-specific status code.
*/
virtual void OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) {}
virtual void OnAttributeData(const ConcreteDataAttributePath & aPath, DataVersion aVersion, TLV::TLVReader * apData,
const StatusIB & aStatus)
{}

/**
* OnSubscriptionEstablished will be called when a subscription is established for the given subscription transaction.
Expand Down
17 changes: 12 additions & 5 deletions src/app/reporting/Engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ CHIP_ERROR Engine::BuildSingleReportDataAttributeReportIBs(ReportDataMessage::Bu
"Error retrieving data from clusterId: " ChipLogFormatMEI ", err = %" CHIP_ERROR_FORMAT,
ChipLogValueMEI(pathForRetrieval.mClusterId), err.Format());

// If error is not CHIP_ERROR_BUFFER_TOO_SMALL and is not CHIP_ERROR_NO_MEMORY, rollback and encode status.
// Otherwise, if partial data allowed, save the encode state.
// Otherwise roll back. If we have already encoded some chunks, we are done; otherwise encode status.

if (encodeState.AllowPartialData() && ((err == CHIP_ERROR_BUFFER_TOO_SMALL) || (err == CHIP_ERROR_NO_MEMORY)))
{
// Encoding is aborted but partial data is allowed, then we don't rollback and save the state for next chunk.
Expand All @@ -138,12 +142,15 @@ CHIP_ERROR Engine::BuildSingleReportDataAttributeReportIBs(ReportDataMessage::Bu
attributeReportIBs.Rollback(attributeBackup);
apReadHandler->SetAttributeEncodeState(AttributeValueEncoder::AttributeEncodeState());

// Try to encode our error as a status response.
err = attributeReportIBs.EncodeAttributeStatus(pathForRetrieval, StatusIB(err));
if (err != CHIP_NO_ERROR)
if (err != CHIP_ERROR_NO_MEMORY && err != CHIP_ERROR_BUFFER_TOO_SMALL)
{
// OK, just roll back again and give up.
attributeReportIBs.Rollback(attributeBackup);
// Try to encode our error as a status response.
err = attributeReportIBs.EncodeAttributeStatus(pathForRetrieval, StatusIB(err));
if (err != CHIP_NO_ERROR)
{
// OK, just roll back again and give up.
attributeReportIBs.Rollback(attributeBackup);
}
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/app/tests/TestAttributeCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ void DataSeriesGenerator::Generate()
System::PacketBufferTLVReader reader;
ReadClient::Callback * callback = mReadCallback;
StatusIB status;

DataVersion version = kUndefinedDataVersion;
callback->OnReportBegin();

uint8_t index = 0;
Expand Down Expand Up @@ -198,13 +198,13 @@ void DataSeriesGenerator::Generate()
writer.Finalize(&handle);
reader.Init(std::move(handle));
NL_TEST_ASSERT(gSuite, reader.Next() == CHIP_NO_ERROR);
callback->OnAttributeData(path, &reader, status);
callback->OnAttributeData(path, version, &reader, status);
}
else
{
ChipLogProgress(DataManagement, "\t -- Generating Status");
status.mStatus = Protocols::InteractionModel::Status::Failure;
callback->OnAttributeData(path, nullptr, status);
callback->OnAttributeData(path, version, nullptr, status);
}

index++;
Expand Down
20 changes: 11 additions & 9 deletions src/app/tests/TestBufferedReadCallback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ class DataSeriesValidator : public BufferedReadCallback::Callback

void OnReportBegin() override;
void OnReportEnd() override;
void OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) override;
void OnAttributeData(const ConcreteDataAttributePath & aPath, DataVersion aVersion, TLV::TLVReader * apData,
const StatusIB & aStatus) override;
void OnDone() override {}

std::vector<ValidationInstruction> mInstructionList;
Expand All @@ -94,7 +95,7 @@ void DataSeriesValidator::OnReportBegin()

void DataSeriesValidator::OnReportEnd() {}

void DataSeriesValidator::OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData,
void DataSeriesValidator::OnAttributeData(const ConcreteDataAttributePath & aPath, DataVersion aVersion, TLV::TLVReader * apData,
const StatusIB & aStatus)
{
uint32_t expectedListLength;
Expand Down Expand Up @@ -301,6 +302,7 @@ void DataSeriesGenerator::Generate()
ReadClient::Callback * callback = &mReadCallback;
StatusIB status;
bool hasData;
DataVersion version = kUndefinedDataVersion;

callback->OnReportBegin();

Expand Down Expand Up @@ -401,7 +403,7 @@ void DataSeriesGenerator::Generate()
path.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll;
status.mStatus = Protocols::InteractionModel::Status::Failure;
hasData = false;
callback->OnAttributeData(path, &reader, status);
callback->OnAttributeData(path, version, &reader, status);
break;
}

Expand All @@ -412,7 +414,7 @@ void DataSeriesGenerator::Generate()
path.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll;
status.mStatus = Protocols::InteractionModel::Status::Failure;
hasData = false;
callback->OnAttributeData(path, &reader, status);
callback->OnAttributeData(path, version, &reader, status);
break;
}

Expand All @@ -430,7 +432,7 @@ void DataSeriesGenerator::Generate()
writer.Finalize(&handle);
reader.Init(std::move(handle));
NL_TEST_ASSERT(gSuite, reader.Next() == CHIP_NO_ERROR);
callback->OnAttributeData(path, &reader, status);
callback->OnAttributeData(path, version, &reader, status);
}

ChipLogProgress(DataManagement, "\t -- Generating C0..C512");
Expand All @@ -453,7 +455,7 @@ void DataSeriesGenerator::Generate()
writer.Finalize(&handle);
reader.Init(std::move(handle));
NL_TEST_ASSERT(gSuite, reader.Next() == CHIP_NO_ERROR);
callback->OnAttributeData(path, &reader, status);
callback->OnAttributeData(path, version, &reader, status);
}

break;
Expand All @@ -473,7 +475,7 @@ void DataSeriesGenerator::Generate()
writer.Finalize(&handle);
reader.Init(std::move(handle));
NL_TEST_ASSERT(gSuite, reader.Next() == CHIP_NO_ERROR);
callback->OnAttributeData(path, &reader, status);
callback->OnAttributeData(path, version, &reader, status);
}

ChipLogProgress(DataManagement, "\t -- Generating D0..D512");
Expand All @@ -492,7 +494,7 @@ void DataSeriesGenerator::Generate()
writer.Finalize(&handle);
reader.Init(std::move(handle));
NL_TEST_ASSERT(gSuite, reader.Next() == CHIP_NO_ERROR);
callback->OnAttributeData(path, &reader, status);
callback->OnAttributeData(path, version, &reader, status);
}

break;
Expand All @@ -507,7 +509,7 @@ void DataSeriesGenerator::Generate()
writer.Finalize(&handle);
reader.Init(std::move(handle));
NL_TEST_ASSERT(gSuite, reader.Next() == CHIP_NO_ERROR);
callback->OnAttributeData(path, &reader, status);
callback->OnAttributeData(path, version, &reader, status);
}

index++;
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/TestMessageDef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ void ParseAttributeDataIB(nlTestSuite * apSuite, AttributeDataIB::Parser & aAttr
{
CHIP_ERROR err = CHIP_NO_ERROR;
AttributePathIB::Parser attributePathParser;
chip::DataVersion version = 0;
chip::DataVersion version = chip::kUndefinedDataVersion;
#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
err = aAttributeDataIBParser.CheckSchemaValidity();
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
Expand Down
4 changes: 2 additions & 2 deletions src/app/tests/TestReadInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ class MockInteractionModelApp : public chip::app::ReadClient::Callback
mGotEventResponse = true;
}

void OnAttributeData(const chip::app::ConcreteDataAttributePath & aPath, chip::TLV::TLVReader * apData,
const chip::app::StatusIB & status) override
void OnAttributeData(const chip::app::ConcreteDataAttributePath & aPath, chip::DataVersion aVersion,
chip::TLV::TLVReader * apData, const chip::app::StatusIB & status) override
{
if (status.mStatus == chip::Protocols::InteractionModel::Status::Success)
{
Expand Down
4 changes: 2 additions & 2 deletions src/app/tests/integration/chip_im_initiator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate,
}
}
}
void OnAttributeData(const chip::app::ConcreteDataAttributePath & aPath, chip::TLV::TLVReader * aData,
const chip::app::StatusIB & status) override
void OnAttributeData(const chip::app::ConcreteDataAttributePath & aPath, chip::DataVersion aVersion,
chip::TLV::TLVReader * aData, const chip::app::StatusIB & status) override
{}

void OnError(CHIP_ERROR aError) override { printf("ReadError with err %" CHIP_ERROR_FORMAT, aError.Format()); }
Expand Down
41 changes: 37 additions & 4 deletions src/app/util/ember-compatibility-functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ namespace chip {
namespace app {
namespace Compatibility {
namespace {
constexpr uint32_t kTemporaryDataVersion = 0;
// On some apps, ATTRIBUTE_LARGEST can as small as 3, making compiler unhappy since data[kAttributeReadBufferSize] cannot hold
// uint64_t. Make kAttributeReadBufferSize at least 8 so it can fit all basic types.
constexpr size_t kAttributeReadBufferSize = (ATTRIBUTE_LARGEST >= 8 ? ATTRIBUTE_LARGEST : 8);
Expand Down Expand Up @@ -257,6 +256,36 @@ bool ServerClusterCommandExists(const ConcreteCommandPath & aCommandPath)

namespace {

CHIP_ERROR ReadClusterDataVersion(const EndpointId & aEndpointId, const ClusterId & aClusterId, DataVersion & aDataVersion)
{
DataVersion * version = emberAfDataVersionStorage(aEndpointId, aClusterId);
if (version == nullptr)
{
ChipLogError(DataManagement, "Endpoint %" PRIx16 ", Cluster " ChipLogFormatMEI " not found in ReadClusterDataVersion!",
aEndpointId, ChipLogValueMEI(aClusterId));
return CHIP_ERROR_NOT_FOUND;
}
aDataVersion = *version;
return CHIP_NO_ERROR;
}

void IncreaseClusterDataVersion(const EndpointId & aEndpointId, const ClusterId & aClusterId)
{
DataVersion * version = emberAfDataVersionStorage(aEndpointId, aClusterId);
if (version == nullptr)
{
ChipLogError(DataManagement, "Endpoint %" PRIx16 ", Cluster " ChipLogFormatMEI " not found in IncreaseClusterDataVersion!",
aEndpointId, ChipLogValueMEI(aClusterId));
}
else
{
(*(version))++;
ChipLogDetail(DataManagement, "Endpoint %" PRIx16 ", Cluster " ChipLogFormatMEI " update version to %" PRIx32, aEndpointId,
ChipLogValueMEI(aClusterId), *(version));
}
return;
}

CHIP_ERROR SendSuccessStatus(AttributeReportIB::Builder & aAttributeReport, AttributeDataIB::Builder & aAttributeDataIBBuilder)
{
ReturnErrorOnFailure(aAttributeDataIBBuilder.EndOfAttributeDataIB().GetError());
Expand Down Expand Up @@ -331,8 +360,9 @@ CHIP_ERROR ReadViaAccessInterface(FabricIndex aAccessingFabricIndex, bool aIsFab
{
AttributeValueEncoder::AttributeEncodeState state =
(aEncoderState == nullptr ? AttributeValueEncoder::AttributeEncodeState() : *aEncoderState);
AttributeValueEncoder valueEncoder(aAttributeReports, aAccessingFabricIndex, aPath, kTemporaryDataVersion, aIsFabricFiltered,
state);
DataVersion version = kUndefinedDataVersion;
ReturnErrorOnFailure(ReadClusterDataVersion(aPath.mEndpointId, aPath.mClusterId, version));
AttributeValueEncoder valueEncoder(aAttributeReports, aAccessingFabricIndex, aPath, version, aIsFabricFiltered, state);
CHIP_ERROR err = aAccessInterface->Read(aPath, valueEncoder);

if (err != CHIP_NO_ERROR)
Expand Down Expand Up @@ -435,7 +465,9 @@ CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, b
AttributeDataIB::Builder & attributeDataIBBuilder = attributeReport.CreateAttributeData();
ReturnErrorOnFailure(attributeDataIBBuilder.GetError());

attributeDataIBBuilder.DataVersion(kTemporaryDataVersion);
DataVersion version = kUndefinedDataVersion;
ReturnErrorOnFailure(ReadClusterDataVersion(aPath.mEndpointId, aPath.mClusterId, version));
attributeDataIBBuilder.DataVersion(version);
ReturnErrorOnFailure(attributeDataIBBuilder.GetError());

AttributePathIB::Builder & attributePathIBBuilder = attributeDataIBBuilder.CreatePath();
Expand Down Expand Up @@ -897,6 +929,7 @@ void MatterReportingAttributeChangeCallback(EndpointId endpoint, ClusterId clust
info.mAttributeId = attributeId;
info.mEndpointId = endpoint;

IncreaseClusterDataVersion(endpoint, clusterId);
InteractionModelEngine::GetInstance()->GetReportingEngine().SetDirty(info);

// Schedule work to run asynchronously on the CHIP thread. The scheduled work won't execute until the current execution context
Expand Down
Loading

0 comments on commit f09c5fc

Please sign in to comment.