From a9ed39abd0f162db2407a135bc03c5e8a654e647 Mon Sep 17 00:00:00 2001 From: yunhanw Date: Fri, 28 Jan 2022 21:12:12 -0800 Subject: [PATCH] Integreate data version starge with IM --- .../commands/clusters/ReportCommand.h | 4 +- src/app/AttributeCache.cpp | 5 ++- src/app/AttributeCache.h | 3 +- src/app/BufferedReadCallback.cpp | 11 ++--- src/app/BufferedReadCallback.h | 5 ++- src/app/ReadClient.cpp | 16 +++++++- src/app/ReadClient.h | 6 ++- src/app/reporting/Engine.cpp | 17 +++++--- src/app/tests/TestAttributeCache.cpp | 6 +-- src/app/tests/TestBufferedReadCallback.cpp | 22 +++++----- src/app/tests/TestReadInteraction.cpp | 4 +- .../tests/integration/chip_im_initiator.cpp | 4 +- .../util/ember-compatibility-functions.cpp | 40 +++++++++++++++++-- src/controller/TypedReadCallback.h | 2 +- .../python/chip/clusters/Attribute.py | 16 ++++---- .../python/chip/clusters/attribute.cpp | 14 +++++-- .../test/test_scripts/mobile-device-test.py | 6 +++ src/controller/tests/TestReadChunking.cpp | 15 +++++-- 18 files changed, 140 insertions(+), 56 deletions(-) diff --git a/examples/chip-tool/commands/clusters/ReportCommand.h b/examples/chip-tool/commands/clusters/ReportCommand.h index 590bdd8b431a0a..35b1e63d01970d 100644 --- a/examples/chip-tool/commands/clusters/ReportCommand.h +++ b/examples/chip-tool/commands/clusters/ReportCommand.h @@ -32,8 +32,8 @@ 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, - const chip::app::StatusIB & status) override + void OnAttributeData(const chip::app::ConcreteDataAttributePath & path, chip::Optional & aVersion, + chip::TLV::TLVReader * data, const chip::app::StatusIB & status) override { CHIP_ERROR error = status.ToChipError(); if (CHIP_NO_ERROR != error) diff --git a/src/app/AttributeCache.cpp b/src/app/AttributeCache.cpp index 349a8563636144..a159eaaf172850 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, Optional & 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 37fb41ba759e04..98746ca7cd6451 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, Optional & 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..3cec4498550a63 100644 --- a/src/app/BufferedReadCallback.cpp +++ b/src/app/BufferedReadCallback.cpp @@ -215,19 +215,19 @@ 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(); - + mDataVersion.ClearValue(); return CHIP_NO_ERROR; } -void BufferedReadCallback::OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, - const StatusIB & aStatus) +void BufferedReadCallback::OnAttributeData(const ConcreteDataAttributePath & aPath, Optional & aVersion, + TLV::TLVReader * apData, const StatusIB & aStatus) { CHIP_ERROR err; @@ -247,13 +247,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 039316e06f0650..93264bb46dc5cd 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, Optional & 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 { @@ -88,7 +89,7 @@ class BufferedReadCallback : public ReadClient::Callback * */ CHIP_ERROR BufferListItem(TLV::TLVReader & reader); - + Optional mDataVersion; ConcreteDataAttributePath mBufferedPath; std::vector mBufferedList; Callback & mCallback; diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 1da2997055d9d0..9dc1c74fbd6821 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -502,6 +502,7 @@ CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeRepo TLV::TLVReader reader = aAttributeReportIBsReader; ReturnErrorOnFailure(report.Init(reader)); + Optional dataVersion; err = report.GetAttributeStatus(&status); if (CHIP_NO_ERROR == err) { @@ -510,13 +511,24 @@ 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, dataVersion, nullptr, statusIB); } else if (CHIP_END_OF_TLV == err) { ReturnErrorOnFailure(report.GetAttributeData(&data)); ReturnErrorOnFailure(data.GetPath(&path)); ReturnErrorOnFailure(ProcessAttributePath(path, attributePath)); + DataVersion version = 0; + err = data.GetDataVersion(&version); + if (err == CHIP_NO_ERROR) + { + dataVersion.SetValue(version); + } + else if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + ReturnErrorOnFailure(err); 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 @@ -526,7 +538,7 @@ CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeRepo attributePath.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; } - mpCallback.OnAttributeData(attributePath, &dataReader, statusIB); + mpCallback.OnAttributeData(attributePath, dataVersion, &dataReader, statusIB); } } diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h index 653a9cb1d0b716..4d91c16b558490 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -110,11 +110,15 @@ 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. The version could be omited when + * EnableTagCompression in the Path field is true or the current report response don't have AttributeDataIB. * @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, Optional & 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..72482d4010d57f 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..fa71dd68e75f30 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; - + Optional version; 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..1457d6965d9a0e 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, Optional & aVersion, TLV::TLVReader * apData, + const StatusIB & aStatus) override; void OnDone() override {} std::vector mInstructionList; @@ -94,8 +95,8 @@ void DataSeriesValidator::OnReportBegin() void DataSeriesValidator::OnReportEnd() {} -void DataSeriesValidator::OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, - const StatusIB & aStatus) +void DataSeriesValidator::OnAttributeData(const ConcreteDataAttributePath & aPath, Optional & 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; + Optional version; 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/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index ab6f49bc225597..00010d987e4972 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::Optional & 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 8d13596e5a24a4..87e54f76d8c645 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::Optional & 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 106bd9b58a102c..6c926d13dff7a2 100644 --- a/src/app/util/ember-compatibility-functions.cpp +++ b/src/app/util/ember-compatibility-functions.cpp @@ -257,6 +257,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 +361,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 = kTemporaryDataVersion; + 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) @@ -430,7 +461,9 @@ CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, b AttributeDataIB::Builder & attributeDataIBBuilder = attributeReport.CreateAttributeData(); ReturnErrorOnFailure(attributeDataIBBuilder.GetError()); - attributeDataIBBuilder.DataVersion(kTemporaryDataVersion); + DataVersion version = kTemporaryDataVersion; + ReturnErrorOnFailure(ReadClusterDataVersion(aPath.mEndpointId, aPath.mClusterId, version)); + attributeDataIBBuilder.DataVersion(version); ReturnErrorOnFailure(attributeDataIBBuilder.GetError()); AttributePathIB::Builder & attributePathIBBuilder = attributeDataIBBuilder.CreatePath(); @@ -887,6 +920,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 2f82215681e96d..11eb5a406d5ab2 100644 --- a/src/controller/TypedReadCallback.h +++ b/src/controller/TypedReadCallback.h @@ -66,7 +66,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, Optional & 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..342fc4237e6461 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, version: 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, version: 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, version, 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, version: int, status: int, data: bytes): + self._handleAttributeData(path, version, 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, version: 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), version, status, dataBytes[:]) @_OnReadEventDataCallbackFunct diff --git a/src/controller/python/chip/clusters/attribute.cpp b/src/controller/python/chip/clusters/attribute.cpp index 93ca755dbc3da3..b2cf6911bda5f5 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, Optional & 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 @@ -88,6 +89,7 @@ class ReadClientCallback : public ReadClient::Callback size_t bufferLen = (apData == nullptr ? 0 : apData->GetRemainingLength() + apData->GetLengthRead()); std::unique_ptr buffer = std::unique_ptr(apData == nullptr ? nullptr : new uint8_t[bufferLen]); uint32_t size = 0; + DataVersion version = 0; // When the apData is nullptr, means we did not receive a valid attribute data from server, status will be some error // status. if (apData != nullptr) @@ -104,9 +106,13 @@ class ReadClientCallback : public ReadClient::Callback return; } size = writer.GetLengthWritten(); + if (aVersion.HasValue()) + { + version = aVersion.Value(); + } } - gOnReadAttributeDataCallback(mAppContext, aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId, + gOnReadAttributeDataCallback(mAppContext, version, 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..8bfa3648983074 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, Optional & aVersion, TLV::TLVReader * apData, const app::StatusIB & aStatus) override; void OnDone() override; @@ -108,8 +108,8 @@ class TestReadCallback : public app::ReadClient::Callback app::BufferedReadCallback mBufferedCallback; }; -void TestReadCallback::OnAttributeData(const app::ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, - const app::StatusIB & aStatus) +void TestReadCallback::OnAttributeData(const app::ConcreteDataAttributePath & aPath, Optional & aVersion, + TLV::TLVReader * apData, const app::StatusIB & aStatus) { if (aPath.mAttributeId != kTestListAttribute) { @@ -209,6 +209,9 @@ void TestCommandInteraction::TestChunking(nlTestSuite * apSuite, void * apContex DataVersion dataVersionStorage[ArraySize(testEndpointClusters)]; emberAfSetDynamicEndpoint(0, kTestEndpointId, &testEndpoint, 0, 0, Span(dataVersionStorage)); + // Register our fake attribute access interface. + registerAttributeAccessOverride(&testServer); + app::AttributePathParams attributePath(kTestEndpointId, app::Clusters::TestCluster::Id); app::ReadPrepareParams readParams(sessionHandle); @@ -281,6 +284,9 @@ void TestCommandInteraction::TestListChunking(nlTestSuite * apSuite, void * apCo DataVersion dataVersionStorage[ArraySize(testEndpoint3Clusters)]; emberAfSetDynamicEndpoint(0, kTestEndpointId3, &testEndpoint3, 0, 0, Span(dataVersionStorage)); + // Register our fake attribute access interface. + registerAttributeAccessOverride(&testServer); + app::AttributePathParams attributePath(kTestEndpointId3, app::Clusters::TestCluster::Id, kTestListAttribute); app::ReadPrepareParams readParams(sessionHandle); @@ -354,6 +360,9 @@ void TestCommandInteraction::TestBadChunking(nlTestSuite * apSuite, void * apCon DataVersion dataVersionStorage[ArraySize(testEndpoint3Clusters)]; emberAfSetDynamicEndpoint(0, kTestEndpointId3, &testEndpoint3, 0, 0, Span(dataVersionStorage)); + // Register our fake attribute access interface. + registerAttributeAccessOverride(&testServer); + app::AttributePathParams attributePath(kTestEndpointId3, app::Clusters::TestCluster::Id, kTestBadAttribute); app::ReadPrepareParams readParams(sessionHandle);