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.

Sign up for GitHub

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

Add initial cluster data version support #13400

Merged
merged 4 commits into from
Feb 1, 2022
Merged

Add initial cluster data version support #13400

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
55b3f98
Integreate data version starge with IM
yunhanw-google Jan 29, 2022
419c0b8
address comment
yunhanw-google Jan 30, 2022
cb1ab20
use DataVersion instead of optional<DataVersion>
yunhanw-google Jan 31, 2022
f7fa122
address comments
yunhanw-google Jan 31, 2022
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
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/clusters/ReportCommand.h
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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)
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
{
// 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));
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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)
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
{
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)
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
{
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));
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
AttributeValueEncoder valueEncoder(aAttributeReports, aAccessingFabricIndex, aPath, version, aIsFabricFiltered, state);
CHIP_ERROR err = aAccessInterface->Read(aPath, valueEncoder);

if (err != CHIP_NO_ERROR)
Expand Down Expand Up @@ -430,7 +460,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 @@ -887,6 +919,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