From 435340057f69d914863a3008822dc821b6403bad Mon Sep 17 00:00:00 2001 From: yunhanw-google Date: Mon, 31 Jan 2022 17:32:30 -0800 Subject: [PATCH] Add initial cluster data version support (#13400) --- .../commands/clusters/ReportCommand.h | 2 +- src/app/AttributeCache.cpp | 5 ++- src/app/AttributeCache.h | 3 +- src/app/BufferedReadCallback.cpp | 8 ++-- src/app/BufferedReadCallback.h | 5 ++- src/app/ReadClient.cpp | 8 ++-- src/app/ReadClient.h | 5 ++- src/app/reporting/Engine.cpp | 17 +++++--- src/app/tests/TestAttributeCache.cpp | 6 +-- src/app/tests/TestBufferedReadCallback.cpp | 20 +++++---- src/app/tests/TestMessageDef.cpp | 2 +- src/app/tests/TestReadInteraction.cpp | 4 +- .../tests/integration/chip_im_initiator.cpp | 4 +- .../util/ember-compatibility-functions.cpp | 41 +++++++++++++++++-- src/controller/TypedReadCallback.h | 2 +- .../python/chip/clusters/Attribute.py | 16 ++++---- .../python/chip/clusters/attribute.cpp | 9 ++-- .../test/test_scripts/mobile-device-test.py | 6 +++ src/controller/tests/TestReadChunking.cpp | 4 +- src/darwin/Framework/CHIP/CHIPDevice.mm | 5 ++- src/lib/core/DataModelTypes.h | 2 +- 21 files changed, 116 insertions(+), 58 deletions(-) diff --git a/examples/chip-tool/commands/clusters/ReportCommand.h b/examples/chip-tool/commands/clusters/ReportCommand.h index 38a6cb101fe069..1f5822dc4d08b1 100644 --- a/examples/chip-tool/commands/clusters/ReportCommand.h +++ b/examples/chip-tool/commands/clusters/ReportCommand.h @@ -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(); diff --git a/src/app/AttributeCache.cpp b/src/app/AttributeCache.cpp index 349a8563636144..35a9b70bdba1ef 100644 --- a/src/app/AttributeCache.cpp +++ b/src/app/AttributeCache.cpp @@ -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 @@ -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) diff --git a/src/app/AttributeCache.h b/src/app/AttributeCache.h index 4590885cc0ef81..97e3bd63166408 100644 --- a/src/app/AttributeCache.h +++ b/src/app/AttributeCache.h @@ -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 diff --git a/src/app/BufferedReadCallback.cpp b/src/app/BufferedReadCallback.cpp index 3b44a9e4401b81..b2686910972058 100644 --- a/src/app/BufferedReadCallback.cpp +++ b/src/app/BufferedReadCallback.cpp @@ -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; @@ -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) diff --git a/src/app/BufferedReadCallback.h b/src/app/BufferedReadCallback.h index 4c96cc46290c90..ce9d959e9fdbfb 100644 --- a/src/app/BufferedReadCallback.h +++ b/src/app/BufferedReadCallback.h @@ -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 { @@ -93,7 +94,7 @@ class BufferedReadCallback : public ReadClient::Callback * */ CHIP_ERROR BufferListItem(TLV::TLVReader & reader); - + DataVersion mDataVersion; ConcreteDataAttributePath mBufferedPath; std::vector mBufferedList; Callback & mCallback; diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 67ebc62f55f645..126910fde78da5 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -569,7 +569,8 @@ 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; @@ -577,13 +578,14 @@ CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeRepo 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 @@ -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); } } diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h index 1e2deffb45f4e9..f1f98e46e07358 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -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. diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index 402d8e6a1edc43..244e896aef229f 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -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. @@ -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); + } } } } diff --git a/src/app/tests/TestAttributeCache.cpp b/src/app/tests/TestAttributeCache.cpp index 1c0c960c087fb5..f5d6048a5e43b5 100644 --- a/src/app/tests/TestAttributeCache.cpp +++ b/src/app/tests/TestAttributeCache.cpp @@ -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; @@ -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++; diff --git a/src/app/tests/TestBufferedReadCallback.cpp b/src/app/tests/TestBufferedReadCallback.cpp index 82f45c6ca7bad9..d53e36f38d8771 100644 --- a/src/app/tests/TestBufferedReadCallback.cpp +++ b/src/app/tests/TestBufferedReadCallback.cpp @@ -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 mInstructionList; @@ -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; @@ -301,6 +302,7 @@ void DataSeriesGenerator::Generate() ReadClient::Callback * callback = &mReadCallback; StatusIB status; bool hasData; + DataVersion version = kUndefinedDataVersion; callback->OnReportBegin(); @@ -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; } @@ -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; } @@ -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"); @@ -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; @@ -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"); @@ -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; @@ -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++; diff --git a/src/app/tests/TestMessageDef.cpp b/src/app/tests/TestMessageDef.cpp index 95c3cd52f075e2..37dead643545b4 100644 --- a/src/app/tests/TestMessageDef.cpp +++ b/src/app/tests/TestMessageDef.cpp @@ -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); diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 056ce6aad126f4..941f1801a28afb 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -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) { diff --git a/src/app/tests/integration/chip_im_initiator.cpp b/src/app/tests/integration/chip_im_initiator.cpp index 3068e83a4a9739..8f21aa48398ca3 100644 --- a/src/app/tests/integration/chip_im_initiator.cpp +++ b/src/app/tests/integration/chip_im_initiator.cpp @@ -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()); } diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp index da09fbe0984b5e..69de7983936a3e 100644 --- a/src/app/util/ember-compatibility-functions.cpp +++ b/src/app/util/ember-compatibility-functions.cpp @@ -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); @@ -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()); @@ -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) @@ -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(); @@ -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 diff --git a/src/controller/TypedReadCallback.h b/src/controller/TypedReadCallback.h index fcdec491eccdbf..16c4b9d41eec37 100644 --- a/src/controller/TypedReadCallback.h +++ b/src/controller/TypedReadCallback.h @@ -67,7 +67,7 @@ class TypedReadAttributeCallback final : public app::ReadClient::Callback void AdoptReadClient(Platform::UniquePtr aReadClient) { mReadClient = std::move(aReadClient); } private: - void OnAttributeData(const app::ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, + void OnAttributeData(const app::ConcreteDataAttributePath & aPath, DataVersion aVersion, TLV::TLVReader * apData, const app::StatusIB & aStatus) override { CHIP_ERROR err = CHIP_NO_ERROR; diff --git a/src/controller/python/chip/clusters/Attribute.py b/src/controller/python/chip/clusters/Attribute.py index b3901dcdf9b308..9620f1ba6af532 100644 --- a/src/controller/python/chip/clusters/Attribute.py +++ b/src/controller/python/chip/clusters/Attribute.py @@ -318,7 +318,7 @@ class AttributeCache: attributeCache: Dict[int, List[Cluster]] = field( default_factory=lambda: {}) - def UpdateTLV(self, path: AttributePath, data: Union[bytes, ValueDecodeFailure]): + def UpdateTLV(self, path: AttributePath, dataVersion: int, data: Union[bytes, ValueDecodeFailure]): ''' Store data in TLV since that makes it easiest to eventually convert to either the cluster or attribute view representations (see below in UpdateCachedData). ''' @@ -539,7 +539,7 @@ def SetClientObjPointers(self, pReadClient, pReadCallback): def GetAllEventValues(self): return self._events - def _handleAttributeData(self, path: AttributePathWithListIndex, status: int, data: bytes): + def _handleAttributeData(self, path: AttributePathWithListIndex, dataVersion: int, status: int, data: bytes): try: imStatus = status try: @@ -554,14 +554,14 @@ def _handleAttributeData(self, path: AttributePathWithListIndex, status: int, da tlvData = chip.tlv.TLVReader(data).get().get("Any", {}) attributeValue = tlvData - self._cache.UpdateTLV(path, attributeValue) + self._cache.UpdateTLV(path, dataVersion, attributeValue) self._changedPathSet.add(path) except Exception as ex: logging.exception(ex) - def handleAttributeData(self, path: AttributePath, status: int, data: bytes): - self._handleAttributeData(path, status, data) + def handleAttributeData(self, path: AttributePath, dataVersion: int, status: int, data: bytes): + self._handleAttributeData(path, dataVersion, status, data) def _handleEventData(self, header: EventHeader, path: EventPath, data: bytes): try: @@ -687,7 +687,7 @@ def handleDone(self): _OnReadAttributeDataCallbackFunct = CFUNCTYPE( - None, py_object, c_uint16, c_uint32, c_uint32, c_uint32, c_void_p, c_size_t) + None, py_object, c_uint32, c_uint16, c_uint32, c_uint32, c_uint32, c_void_p, c_size_t) _OnSubscriptionEstablishedCallbackFunct = CFUNCTYPE(None, py_object, c_uint64) _OnReadEventDataCallbackFunct = CFUNCTYPE( None, py_object, c_uint16, c_uint32, c_uint32, c_uint32, c_uint8, c_uint64, c_uint8, c_void_p, c_size_t) @@ -702,10 +702,10 @@ def handleDone(self): @_OnReadAttributeDataCallbackFunct -def _OnReadAttributeDataCallback(closure, endpoint: int, cluster: int, attribute: int, status, data, len): +def _OnReadAttributeDataCallback(closure, dataVersion: int, endpoint: int, cluster: int, attribute: int, status, data, len): dataBytes = ctypes.string_at(data, len) closure.handleAttributeData(AttributePath( - EndpointId=endpoint, ClusterId=cluster, AttributeId=attribute), status, dataBytes[:]) + EndpointId=endpoint, ClusterId=cluster, AttributeId=attribute), dataVersion, status, dataBytes[:]) @_OnReadEventDataCallbackFunct diff --git a/src/controller/python/chip/clusters/attribute.cpp b/src/controller/python/chip/clusters/attribute.cpp index a493783bcd52b5..f1e11d2f804afe 100644 --- a/src/controller/python/chip/clusters/attribute.cpp +++ b/src/controller/python/chip/clusters/attribute.cpp @@ -50,8 +50,8 @@ struct __attribute__((packed)) EventPath chip::EventId eventId; }; -using OnReadAttributeDataCallback = void (*)(PyObject * appContext, chip::EndpointId endpointId, chip::ClusterId clusterId, - chip::AttributeId attributeId, +using OnReadAttributeDataCallback = void (*)(PyObject * appContext, chip::DataVersion version, chip::EndpointId endpointId, + chip::ClusterId clusterId, chip::AttributeId attributeId, std::underlying_type_t imstatus, uint8_t * data, uint32_t dataLen); using OnReadEventDataCallback = void (*)(PyObject * appContext, chip::EndpointId endpointId, chip::ClusterId clusterId, @@ -78,7 +78,8 @@ class ReadClientCallback : public ReadClient::Callback app::BufferedReadCallback * GetBufferedReadCallback() { return &mBufferedReadCallback; } - 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 { // // We shouldn't be getting list item operations in the provided path since that should be handled by the buffered read @@ -106,7 +107,7 @@ class ReadClientCallback : public ReadClient::Callback size = writer.GetLengthWritten(); } - gOnReadAttributeDataCallback(mAppContext, aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId, + gOnReadAttributeDataCallback(mAppContext, aVersion, aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId, to_underlying(aStatus.mStatus), buffer.get(), size); } diff --git a/src/controller/python/test/test_scripts/mobile-device-test.py b/src/controller/python/test/test_scripts/mobile-device-test.py index f30f2a2f12a52d..fa196c92842071 100755 --- a/src/controller/python/test/test_scripts/mobile-device-test.py +++ b/src/controller/python/test/test_scripts/mobile-device-test.py @@ -138,6 +138,12 @@ def main(): group=GROUP_ID), "Failed to test Write Basic Attributes") + logger.info("Testing attribute reading basic again") + FailIfNot(test.TestReadBasicAttributes(nodeid=1, + endpoint=ENDPOINT_ID, + group=GROUP_ID), + "Failed to test Read Basic Attributes") + logger.info("Testing subscription") FailIfNot(test.TestSubscription(nodeid=1, endpoint=LIGHTING_ENDPOINT_ID), "Failed to subscribe attributes.") diff --git a/src/controller/tests/TestReadChunking.cpp b/src/controller/tests/TestReadChunking.cpp index ba3e0d2687c69c..9304e24c8039c8 100644 --- a/src/controller/tests/TestReadChunking.cpp +++ b/src/controller/tests/TestReadChunking.cpp @@ -96,7 +96,7 @@ class TestReadCallback : public app::ReadClient::Callback { public: TestReadCallback() : mBufferedCallback(*this) {} - void OnAttributeData(const app::ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, + void OnAttributeData(const app::ConcreteDataAttributePath & aPath, DataVersion aVersion, TLV::TLVReader * apData, const app::StatusIB & aStatus) override; void OnDone() override; @@ -108,7 +108,7 @@ class TestReadCallback : public app::ReadClient::Callback app::BufferedReadCallback mBufferedCallback; }; -void TestReadCallback::OnAttributeData(const app::ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, +void TestReadCallback::OnAttributeData(const app::ConcreteDataAttributePath & aPath, DataVersion aVersion, TLV::TLVReader * apData, const app::StatusIB & aStatus) { if (aPath.mAttributeId != kTestListAttribute) diff --git a/src/darwin/Framework/CHIP/CHIPDevice.mm b/src/darwin/Framework/CHIP/CHIPDevice.mm index 7bec9771a852cd..c37e96228463e5 100644 --- a/src/darwin/Framework/CHIP/CHIPDevice.mm +++ b/src/darwin/Framework/CHIP/CHIPDevice.mm @@ -96,7 +96,8 @@ - (instancetype)initWithDevice:(chip::DeviceProxy *)device 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; @@ -214,7 +215,7 @@ - (instancetype)initWithPath:(const ConcreteDataAttributePath &)path value:(null } void SubscriptionCallback::OnAttributeData( - const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) + const ConcreteDataAttributePath & aPath, DataVersion aVersion, TLV::TLVReader * apData, const StatusIB & aStatus) { if (aPath.IsListItemOperation()) { ReportError(CHIP_ERROR_INCORRECT_STATE); diff --git a/src/lib/core/DataModelTypes.h b/src/lib/core/DataModelTypes.h index 77989bd52c8845..9aba0ef466798f 100644 --- a/src/lib/core/DataModelTypes.h +++ b/src/lib/core/DataModelTypes.h @@ -48,7 +48,7 @@ constexpr FabricIndex kUndefinedFabricIndex = 0; constexpr EndpointId kInvalidEndpointId = 0xFFFF; constexpr EndpointId kRootEndpointId = 0; constexpr ListIndex kInvalidListIndex = 0xFFFF; // List index is a uint16 thus 0xFFFF is a invalid list index. - +constexpr DataVersion kUndefinedDataVersion = 0; // These are MEIs, 0xFFFF is not a valid manufacturer code, // thus 0xFFFF'FFFF is not a valid MEI. static constexpr ClusterId kInvalidClusterId = 0xFFFF'FFFF;