diff --git a/src/app/AttributeCache.cpp b/src/app/AttributeCache.cpp index 64fc1d57ce9e58..6e2bd671c767f7 100644 --- a/src/app/AttributeCache.cpp +++ b/src/app/AttributeCache.cpp @@ -100,8 +100,8 @@ void AttributeCache::OnReportEnd(const ReadClient * apReadClient) mCallback.OnReportEnd(apReadClient); } -void AttributeCache::OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, - TLV::TLVReader * apData, const StatusIB & aStatus) +void AttributeCache::OnAttributeData(const ReadClient * apReadClient, DataVersion * apVersion, + const ConcreteDataAttributePath & aPath, 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 @@ -121,7 +121,7 @@ void AttributeCache::OnAttributeData(const ReadClient * apReadClient, const Conc // // Forward the call through. // - mCallback.OnAttributeData(apReadClient, aPath, apData, aStatus); + mCallback.OnAttributeData(apReadClient, apVersion, aPath, 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 3abb288582a2b8..71e0e17fb11306 100644 --- a/src/app/AttributeCache.h +++ b/src/app/AttributeCache.h @@ -341,8 +341,8 @@ class AttributeCache : protected ReadClient::Callback // void OnReportBegin(const ReadClient * apReadClient) override; void OnReportEnd(const ReadClient * apReadClient) override; - void OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, - const StatusIB & aStatus) override; + void OnAttributeData(const ReadClient * apReadClient, DataVersion * apVersion, const ConcreteDataAttributePath & aPath, + TLV::TLVReader * apData, const StatusIB & aStatus) override; void OnError(const ReadClient * apReadClient, CHIP_ERROR aError) override { return mCallback.OnError(apReadClient, aError); } void OnEventData(const ReadClient * apReadClient, const EventHeader & aEventHeader, TLV::TLVReader * apData, const StatusIB * apStatus) override diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index 01506f2c38036e..8f43ad533f1ec3 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -48,9 +48,12 @@ static_library("app") { "CASEClientPool.h", "CASESessionManager.cpp", "CASESessionManager.h", + "ClusterDataVersionPersistenceProvider.h", "CommandHandler.cpp", "CommandSender.cpp", "DefaultAttributePersistenceProvider.cpp", + "DefaultClusterDataVersionPersistenceProvider.cpp", + "DefaultClusterDataVersionPersistenceProvider.h", "DeviceProxy.cpp", "DeviceProxy.h", "EventManagement.cpp", diff --git a/src/app/BufferedReadCallback.cpp b/src/app/BufferedReadCallback.cpp index f69bb972e2b772..d1b1b3ab4b3126 100644 --- a/src/app/BufferedReadCallback.cpp +++ b/src/app/BufferedReadCallback.cpp @@ -215,19 +215,21 @@ CHIP_ERROR BufferedReadCallback::DispatchBufferedData(const ReadClient * apReadC // ReturnErrorOnFailure(reader.Next()); - mCallback.OnAttributeData(apReadClient, mBufferedPath, &reader, statusIB); + mCallback.OnAttributeData(apReadClient, mpDataVersion, mBufferedPath, &reader, statusIB); // // Clear out our buffered contents to free up allocated buffers, and reset the buffered path. // mBufferedList.clear(); mBufferedPath = ConcreteDataAttributePath(); + mpDataVersion = nullptr; return CHIP_NO_ERROR; } -void BufferedReadCallback::OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, - TLV::TLVReader * apData, const StatusIB & aStatus) +void BufferedReadCallback::OnAttributeData(const ReadClient * apReadClient, DataVersion * apVersion, + const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, + const StatusIB & aStatus) { CHIP_ERROR err; @@ -247,13 +249,14 @@ void BufferedReadCallback::OnAttributeData(const ReadClient * apReadClient, cons } else { - mCallback.OnAttributeData(apReadClient, aPath, apData, aStatus); + mCallback.OnAttributeData(apReadClient, apVersion, aPath, apData, aStatus); } // // Update our latched buffered path. // mBufferedPath = aPath; + mpDataVersion = apVersion; exit: if (err != CHIP_NO_ERROR) diff --git a/src/app/BufferedReadCallback.h b/src/app/BufferedReadCallback.h index f177d8358b67ba..3a2ec45068917f 100644 --- a/src/app/BufferedReadCallback.h +++ b/src/app/BufferedReadCallback.h @@ -70,8 +70,8 @@ class BufferedReadCallback : public ReadClient::Callback // void OnReportBegin(const ReadClient * apReadClient) override; void OnReportEnd(const ReadClient * apReadClient) override; - void OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, - const StatusIB & aStatus) override; + void OnAttributeData(const ReadClient * apReadClient, DataVersion * apVersion, const ConcreteDataAttributePath & aPath, + TLV::TLVReader * apData, const StatusIB & aStatus) override; void OnError(const ReadClient * apReadClient, CHIP_ERROR aError) override { return mCallback.OnError(apReadClient, aError); } void OnEventData(const ReadClient * apReadClient, const EventHeader & aEventHeader, TLV::TLVReader * apData, const StatusIB * apStatus) override @@ -93,6 +93,7 @@ class BufferedReadCallback : public ReadClient::Callback CHIP_ERROR BufferListItem(TLV::TLVReader & reader); ConcreteDataAttributePath mBufferedPath; + DataVersion * mpDataVersion = nullptr; std::vector mBufferedList; Callback & mCallback; }; diff --git a/src/app/ClusterDataVersionPersistenceProvider.h b/src/app/ClusterDataVersionPersistenceProvider.h new file mode 100644 index 00000000000000..d9ca2946508adc --- /dev/null +++ b/src/app/ClusterDataVersionPersistenceProvider.h @@ -0,0 +1,76 @@ +/* + * Copyright (c) 2021 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once +#include +#include +#include + +namespace chip { +namespace app { + +/** + * Interface for persisting cluster data version value. + */ + +class ClusterDataVersionPersistenceProvider +{ +public: + virtual ~ClusterDataVersionPersistenceProvider() = default; + ClusterDataVersionPersistenceProvider() = default; + + /** + * Write a version value from the attribute store to non-volatile memory. + * + * @param [in] aPath the cluster path for the version being written. + * @param [in] aValue the data to write. Integer is + * represented in native endianness. The length is + * stored as little-endian. + */ + virtual CHIP_ERROR WriteValue(const ConcreteClusterPath & aPath, const chip::DataVersion & aValue) = 0; + + /** + * Read an attribute value from non-volatile memory. + * + * @param [in] aPath the cluster path for the version being persisted. + * @param [inout] aValue where to place the data. + * + * The data is expected to be in native endianness for integer. + */ + virtual CHIP_ERROR ReadValue(const ConcreteClusterPath & aPath, chip::DataVersion & aValue) = 0; +}; + +/** + * Instance getter for the global ClusterDataVersionPersistenceProvider. + * + * Callers have to externally synchronize usage of this function. + * + * @return The global ClusterDataVersionPersistenceProvider. This must never be null. + */ +ClusterDataVersionPersistenceProvider * GetClusterDataVersionPersistenceProvider(); + +/** + * Instance setter for the global ClusterDataVersionPersistenceProvider. + * + * Callers have to externally synchronize usage of this function. + * + * If the `provider` is nullptr, the value is not changed. + * + * @param[in] provider the ClusterDataVersionPersistenceProvider implementation to use. + */ +void SetClusterDataVersionPersistenceProvider(ClusterDataVersionPersistenceProvider * aProvider); + +} // namespace app +} // namespace chip diff --git a/src/app/ConcreteClusterPath.h b/src/app/ConcreteClusterPath.h new file mode 100644 index 00000000000000..ca075c9e858bf5 --- /dev/null +++ b/src/app/ConcreteClusterPath.h @@ -0,0 +1,35 @@ +/* + * + * Copyright (c) 2021 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +namespace chip { +namespace app { +struct ConcreteClusterPath +{ + ConcreteClusterPath(EndpointId aEndpointId, ClusterId aClusterId) : mEndpointId(aEndpointId), mClusterId(aClusterId) {} + + ConcreteClusterPath() {} + + EndpointId mEndpointId = 0; + ClusterId mClusterId = 0; +}; +} // namespace app +} // namespace chip diff --git a/src/app/DefaultClusterDataVersionPersistenceProvider.cpp b/src/app/DefaultClusterDataVersionPersistenceProvider.cpp new file mode 100644 index 00000000000000..43f5b04f634de1 --- /dev/null +++ b/src/app/DefaultClusterDataVersionPersistenceProvider.cpp @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2021 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include + +namespace chip { +namespace app { + +CHIP_ERROR DefaultClusterDataVersionPersistenceProvider::WriteValue(const ConcreteClusterPath & aPath, + const chip::DataVersion & aValue) +{ + // TODO: we may want to have a small cache for values that change a lot, so + // we only write them once a bunch of changes happen or on timer or + // shutdown. + DefaultStorageKeyAllocator key; + return mStorage.SyncSetKeyValue(key.ClusterValue(aPath), &aValue, sizeof(aValue)); +} + +CHIP_ERROR DefaultClusterDataVersionPersistenceProvider::ReadValue(const ConcreteClusterPath & aPath, chip::DataVersion & aValue) +{ + DefaultStorageKeyAllocator key; + uint16_t size = sizeof(aValue); + ReturnErrorOnFailure(mStorage.SyncGetKeyValue(key.ClusterValue(aPath), &aValue, size)); + return CHIP_NO_ERROR; +} + +namespace { + +ClusterDataVersionPersistenceProvider * gDataVersionSaver = nullptr; + +} // anonymous namespace + +ClusterDataVersionPersistenceProvider * GetClusterDataVersionPersistenceProvider() +{ + return gDataVersionSaver; +} + +void SetClusterDataVersionPersistenceProvider(ClusterDataVersionPersistenceProvider * aProvider) +{ + if (aProvider != nullptr) + { + gDataVersionSaver = aProvider; + } +} + +} // namespace app +} // namespace chip diff --git a/src/app/DefaultClusterDataVersionPersistenceProvider.h b/src/app/DefaultClusterDataVersionPersistenceProvider.h new file mode 100644 index 00000000000000..25a168d8488a4c --- /dev/null +++ b/src/app/DefaultClusterDataVersionPersistenceProvider.h @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2021 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +#include +#include +#include + +namespace chip { +namespace app { + +/** + * Default implementation of ClusterDataVersionPersistenceProvider. This uses + * PersistentStorageDelegate to store the version. + * + * NOTE: SetAttributePersistenceProvider must still be called with an instance + * of this class, since it can't be constructed automatically without knowing + * what PersistentStorageDelegate is to be used. + */ +class DefaultClusterDataVersionPersistenceProvider : public ClusterDataVersionPersistenceProvider +{ +public: + // aStorage must outlive this object. + DefaultClusterDataVersionPersistenceProvider(PersistentStorageDelegate & aStorage) : mStorage(aStorage) {} + + // AttributePersistenceProvider implementation. + CHIP_ERROR WriteValue(const ConcreteClusterPath & aPath, const chip::DataVersion & aValue) override; + CHIP_ERROR ReadValue(const ConcreteClusterPath & aPath, chip::DataVersion & aValue) override; + +private: + PersistentStorageDelegate & mStorage; +}; + +} // namespace app +} // namespace chip diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index e46654b6a77359..462543fb012586 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -499,6 +499,7 @@ CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeRepo AttributePathIB::Parser path; ConcreteDataAttributePath attributePath; StatusIB statusIB; + DataVersion version = 0; TLV::TLVReader reader = aAttributeReportIBsReader; ReturnErrorOnFailure(report.Init(reader)); @@ -511,13 +512,14 @@ CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeRepo ReturnErrorOnFailure(ProcessAttributePath(path, attributePath)); ReturnErrorOnFailure(status.GetErrorStatus(&errorStatus)); ReturnErrorOnFailure(errorStatus.DecodeStatusIB(statusIB)); - mpCallback.OnAttributeData(this, attributePath, nullptr, statusIB); + mpCallback.OnAttributeData(this, nullptr, attributePath, 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 @@ -527,7 +529,7 @@ CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeRepo attributePath.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; } - mpCallback.OnAttributeData(this, attributePath, &dataReader, statusIB); + mpCallback.OnAttributeData(this, &version, attributePath, &dataReader, statusIB); } } diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h index 511e46b83163f2..6d9b8c83f0b30e 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -113,13 +113,14 @@ class ReadClient : public Messaging::ExchangeDelegate * receives an OnDone call to destroy the object. * * @param[in] apReadClient The read client object that initiated the read or subscribe transaction. + * @param[in] apVersion The cluster version in report response. * @param[in] aPath The attribute path field 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 ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, - TLV::TLVReader * apData, const StatusIB & aStatus) + virtual void OnAttributeData(const ReadClient * apReadClient, DataVersion * apVersion, + const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) {} /** diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index e432af81147aff..0f80fc339bd0d3 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -24,6 +24,8 @@ */ #include +#include +#include #include #include #include diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 1364d11ecd956c..579dd2c55f2cfc 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -93,7 +93,7 @@ Server::Server() : .devicePool = &mDevicePool, .dnsResolver = nullptr, }), mCommissioningWindowManager(this), mGroupsProvider(mGroupsStorage), - mAttributePersister(mServerStorage) + mAttributePersister(mServerStorage), mClusterDataVersionProvider(mServerStorage) {} CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint16_t unsecureServicePort) @@ -108,12 +108,6 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint mCommissioningWindowManager.SetAppDelegate(delegate); mCommissioningWindowManager.SetSessionIDAllocator(&mSessionIDAllocator); - // Set up attribute persistence before we try to bring up the data model - // handler. - SetAttributePersistenceProvider(&mAttributePersister); - - InitDataModelHandler(&mExchangeMgr); - #if CHIP_DEVICE_LAYER_TARGET_DARWIN err = DeviceLayer::PersistedStorage::KeyValueStoreMgrImpl().Init("chip.store"); SuccessOrExit(err); @@ -121,6 +115,14 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint DeviceLayer::PersistedStorage::KeyValueStoreMgrImpl().Init(CHIP_CONFIG_KVS_PATH); #endif + // Set up attribute persistence before we try to bring up the data model + // handler. + + SetAttributePersistenceProvider(&mAttributePersister); + SetClusterDataVersionPersistenceProvider(&mClusterDataVersionProvider); + + InitDataModelHandler(&mExchangeMgr); + err = mFabrics.Init(&mServerStorage); SuccessOrExit(err); diff --git a/src/app/server/Server.h b/src/app/server/Server.h index 33c56610cded88..137959c443728f 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -169,6 +170,7 @@ class Server TestPersistentStorageDelegate mGroupsStorage; Credentials::GroupDataProviderImpl mGroupsProvider; app::DefaultAttributePersistenceProvider mAttributePersister; + app::DefaultClusterDataVersionPersistenceProvider mClusterDataVersionProvider; // TODO @ceille: Maybe use OperationalServicePort and CommissionableServicePort uint16_t mSecuredServicePort; diff --git a/src/app/tests/TestAttributeCache.cpp b/src/app/tests/TestAttributeCache.cpp index d448c3337066f1..7137da031aade3 100644 --- a/src/app/tests/TestAttributeCache.cpp +++ b/src/app/tests/TestAttributeCache.cpp @@ -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(nullptr, path, &reader, status); + callback->OnAttributeData(nullptr, nullptr, path, &reader, status); } else { ChipLogProgress(DataManagement, "\t -- Generating Status"); status.mStatus = Protocols::InteractionModel::Status::Failure; - callback->OnAttributeData(nullptr, path, nullptr, status); + callback->OnAttributeData(nullptr, nullptr, path, nullptr, status); } index++; diff --git a/src/app/tests/TestBufferedReadCallback.cpp b/src/app/tests/TestBufferedReadCallback.cpp index 9bccd0eac3edf3..2d2189b7533723 100644 --- a/src/app/tests/TestBufferedReadCallback.cpp +++ b/src/app/tests/TestBufferedReadCallback.cpp @@ -80,8 +80,8 @@ class DataSeriesValidator : public BufferedReadCallback::Callback void OnReportBegin(const ReadClient * apReadClient) override; void OnReportEnd(const ReadClient * apReadClient) override; - void OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, - const StatusIB & aStatus) override; + void OnAttributeData(const ReadClient * apReadClient, DataVersion * apDataVersion, const ConcreteDataAttributePath & aPath, + TLV::TLVReader * apData, const StatusIB & aStatus) override; void OnDone(ReadClient * apClient) override {} std::vector mInstructionList; @@ -95,8 +95,9 @@ void DataSeriesValidator::OnReportBegin(const ReadClient * apReadClient) void DataSeriesValidator::OnReportEnd(const ReadClient * apReadClient) {} -void DataSeriesValidator::OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, - TLV::TLVReader * apData, const StatusIB & aStatus) +void DataSeriesValidator::OnAttributeData(const ReadClient * apReadClient, DataVersion * apDataVersion, + const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, + const StatusIB & aStatus) { uint32_t expectedListLength; @@ -402,7 +403,7 @@ void DataSeriesGenerator::Generate() path.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; status.mStatus = Protocols::InteractionModel::Status::Failure; hasData = false; - callback->OnAttributeData(nullptr, path, &reader, status); + callback->OnAttributeData(nullptr, nullptr, path, &reader, status); break; } @@ -413,7 +414,7 @@ void DataSeriesGenerator::Generate() path.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; status.mStatus = Protocols::InteractionModel::Status::Failure; hasData = false; - callback->OnAttributeData(nullptr, path, &reader, status); + callback->OnAttributeData(nullptr, nullptr, path, &reader, status); break; } @@ -431,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(nullptr, path, &reader, status); + callback->OnAttributeData(nullptr, nullptr, path, &reader, status); } ChipLogProgress(DataManagement, "\t -- Generating C0..C512"); @@ -454,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(nullptr, path, &reader, status); + callback->OnAttributeData(nullptr, nullptr, path, &reader, status); } break; @@ -474,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(nullptr, path, &reader, status); + callback->OnAttributeData(nullptr, nullptr, path, &reader, status); } ChipLogProgress(DataManagement, "\t -- Generating D0..D512"); @@ -493,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(nullptr, path, &reader, status); + callback->OnAttributeData(nullptr, nullptr, path, &reader, status); } break; @@ -508,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(nullptr, path, &reader, status); + callback->OnAttributeData(nullptr, nullptr, path, &reader, status); } index++; diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index fb019d3a6cb6cd..b33d52ea17e8fb 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -143,8 +143,9 @@ class MockInteractionModelApp : public chip::app::ReadClient::Callback, public c mGotEventResponse = true; } - void OnAttributeData(const chip::app::ReadClient * apReadClient, const chip::app::ConcreteDataAttributePath & aPath, - chip::TLV::TLVReader * apData, const chip::app::StatusIB & status) override + void OnAttributeData(const chip::app::ReadClient * apReadClient, chip::DataVersion * apVersion, + const chip::app::ConcreteDataAttributePath & aPath, 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 e00e3b8d65576d..56c121baae6e22 100644 --- a/src/app/tests/integration/chip_im_initiator.cpp +++ b/src/app/tests/integration/chip_im_initiator.cpp @@ -146,8 +146,9 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate, } } } - void OnAttributeData(const chip::app::ReadClient * apReadClient, const chip::app::ConcreteDataAttributePath & aPath, - chip::TLV::TLVReader * aData, const chip::app::StatusIB & status) override + void OnAttributeData(const chip::app::ReadClient * apReadClient, chip::DataVersion * apVersion, + const chip::app::ConcreteDataAttributePath & aPath, chip::TLV::TLVReader * aData, + const chip::app::StatusIB & status) override {} void OnError(const chip::app::ReadClient * apReadClient, CHIP_ERROR aError) override diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp index c9e092d82409f0..b1f361a2f7f926 100644 --- a/src/app/util/ember-compatibility-functions.cpp +++ b/src/app/util/ember-compatibility-functions.cpp @@ -24,6 +24,8 @@ #include #include #include +#include +#include #include #include #include @@ -58,7 +60,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); @@ -340,7 +341,13 @@ CHIP_ERROR ReadViaAccessInterface(FabricIndex aAccessingFabricIndex, const Concr // into status responses, unless our caller already does that. AttributeValueEncoder::AttributeEncodeState state = (aEncoderState == nullptr ? AttributeValueEncoder::AttributeEncodeState() : *aEncoderState); - AttributeValueEncoder valueEncoder(aAttributeReports, aAccessingFabricIndex, aPath, kTemporaryDataVersion, state); + DataVersion version = 0; + auto * versionStorage = GetClusterDataVersionPersistenceProvider(); + if (versionStorage != nullptr) + { + versionStorage->ReadValue(ConcreteClusterPath(aPath.mEndpointId, aPath.mClusterId), version); + } + AttributeValueEncoder valueEncoder(aAttributeReports, aAccessingFabricIndex, aPath, version, state); CHIP_ERROR err = aAccessInterface->Read(aPath, valueEncoder); if (err != CHIP_NO_ERROR) @@ -443,7 +450,13 @@ CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, c AttributeDataIB::Builder & attributeDataIBBuilder = attributeReport.CreateAttributeData(); ReturnErrorOnFailure(attributeDataIBBuilder.GetError()); - attributeDataIBBuilder.DataVersion(kTemporaryDataVersion); + DataVersion version = 0; + auto * versionStorage = GetClusterDataVersionPersistenceProvider(); + if (versionStorage != nullptr) + { + versionStorage->ReadValue(ConcreteClusterPath(aPath.mEndpointId, aPath.mClusterId), version); + } + attributeDataIBBuilder.DataVersion(version); ReturnErrorOnFailure(attributeDataIBBuilder.GetError()); AttributePathIB::Builder & attributePathIBBuilder = attributeDataIBBuilder.CreatePath(); @@ -912,13 +925,39 @@ void MatterReportingAttributeChangeCallback(EndpointId endpoint, ClusterId clust MatterReportingAttributeChangeCallback(endpoint, clusterId, attributeId); } +void UpdateClusterDataVersion(ClusterInfo & aClusterInfo) +{ + auto * versionStorage = GetClusterDataVersionPersistenceProvider(); + if (versionStorage == nullptr) + { + ChipLogError(DataManagement, "no cluster version storage provided"); + return; + } + DataVersion version = 0; + ConcreteClusterPath path(aClusterInfo.mEndpointId, aClusterInfo.mClusterId); + CHIP_ERROR err = versionStorage->ReadValue(path, version); + if (err != CHIP_NO_ERROR) + { + ChipLogError(DataManagement, " ReadValue fail with error %" CHIP_ERROR_FORMAT "!", err.Format()); + } + + version++; + err = versionStorage->WriteValue(path, version); + if (err != CHIP_NO_ERROR) + { + ChipLogError(DataManagement, "WriteValue fail with error %" CHIP_ERROR_FORMAT "!", err.Format()); + } + ChipLogDetail(DataManagement, "Endpoint %" PRIx16 ", Cluster %" PRIx32 " update version to %" PRIx32, path.mEndpointId, + path.mClusterId, version); +} + void MatterReportingAttributeChangeCallback(EndpointId endpoint, ClusterId clusterId, AttributeId attributeId) { ClusterInfo info; info.mClusterId = clusterId; info.mAttributeId = attributeId; info.mEndpointId = endpoint; - + UpdateClusterDataVersion(info); 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 ad609865a15393..8a61fdb25b1a76 100644 --- a/src/controller/TypedReadCallback.h +++ b/src/controller/TypedReadCallback.h @@ -58,8 +58,9 @@ class TypedReadAttributeCallback final : public app::ReadClient::Callback app::BufferedReadCallback & GetBufferedCallback() { return mBufferedReadAdapter; } private: - void OnAttributeData(const app::ReadClient * apReadClient, const app::ConcreteDataAttributePath & aPath, - TLV::TLVReader * apData, const app::StatusIB & aStatus) override + void OnAttributeData(const app::ReadClient * apReadClient, DataVersion * apVersion, + const app::ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, + const app::StatusIB & aStatus) override { CHIP_ERROR err = CHIP_NO_ERROR; DecodableAttributeType value; diff --git a/src/controller/python/chip/clusters/Attribute.py b/src/controller/python/chip/clusters/Attribute.py index f49fb670eadf7e..8b6d65bb56be5c 100644 --- a/src/controller/python/chip/clusters/Attribute.py +++ b/src/controller/python/chip/clusters/Attribute.py @@ -317,7 +317,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). ''' @@ -535,7 +535,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: @@ -550,14 +550,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: @@ -683,7 +683,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) @@ -698,10 +698,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 065aff5f701cf5..4a5d4c8bcbcf93 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,8 +78,8 @@ class ReadClientCallback : public ReadClient::Callback app::BufferedReadCallback * GetBufferedReadCallback() { return &mBufferedReadCallback; } - void OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, - const StatusIB & aStatus) override + void OnAttributeData(const ReadClient * apReadClient, DataVersion * apVersion, const ConcreteDataAttributePath & aPath, + 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 @@ -89,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) @@ -106,10 +107,11 @@ class ReadClientCallback : public ReadClient::Callback this->OnError(apReadClient, err); return; } - size = writer.GetLengthWritten(); + size = writer.GetLengthWritten(); + version = *apVersion; } - 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 a2d290336d1633..ad364fc0433230 100755 --- a/src/controller/python/test/test_scripts/mobile-device-test.py +++ b/src/controller/python/test/test_scripts/mobile-device-test.py @@ -122,18 +122,24 @@ def main(): FailIfNot(asyncio.run(ClusterObjectTests.RunTest(test.devCtrl)), "Failed when testing Python Cluster Object APIs") - logger.info("Testing attribute reading") + logger.info("Testing attribute reading basic") FailIfNot(test.TestReadBasicAttributes(nodeid=1, endpoint=ENDPOINT_ID, group=GROUP_ID), "Failed to test Read Basic Attributes") - logger.info("Testing attribute writing") + logger.info("Testing attribute writing basic") FailIfNot(test.TestWriteBasicAttributes(nodeid=1, endpoint=ENDPOINT_ID, 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 71ce2022de487c..e39e4fe3a6e32f 100644 --- a/src/controller/tests/TestReadChunking.cpp +++ b/src/controller/tests/TestReadChunking.cpp @@ -96,8 +96,9 @@ class TestReadCallback : public app::ReadClient::Callback { public: TestReadCallback() : mBufferedCallback(*this) {} - void OnAttributeData(const app::ReadClient * apReadClient, const app::ConcreteDataAttributePath & aPath, - TLV::TLVReader * apData, const app::StatusIB & aStatus) override; + void OnAttributeData(const app::ReadClient * apReadClient, DataVersion * apVersion, + const app::ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, + const app::StatusIB & aStatus) override; void OnDone(app::ReadClient * apReadClient) override; @@ -108,8 +109,9 @@ class TestReadCallback : public app::ReadClient::Callback app::BufferedReadCallback mBufferedCallback; }; -void TestReadCallback::OnAttributeData(const app::ReadClient * apReadClient, const app::ConcreteDataAttributePath & aPath, - TLV::TLVReader * apData, const app::StatusIB & aStatus) +void TestReadCallback::OnAttributeData(const app::ReadClient * apReadClient, DataVersion * apVersion, + const app::ConcreteDataAttributePath & aPath, 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 a79c645202ecc1..0137cdea49990a 100644 --- a/src/darwin/Framework/CHIP/CHIPDevice.mm +++ b/src/darwin/Framework/CHIP/CHIPDevice.mm @@ -94,8 +94,8 @@ - (instancetype)initWithDevice:(chip::DeviceProxy *)device void OnReportEnd(const ReadClient * apReadClient) override; - void OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, - const StatusIB & aStatus) override; + void OnAttributeData(const ReadClient * apReadClient, DataVersion * apVersion, const ConcreteDataAttributePath & aPath, + TLV::TLVReader * apData, const StatusIB & aStatus) override; void OnError(const ReadClient * apReadClient, CHIP_ERROR aError) override; @@ -213,8 +213,8 @@ - (instancetype)initWithPath:(const ConcreteDataAttributePath &)path value:(null // Else we have a pending error already. } -void SubscriptionCallback::OnAttributeData( - const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) +void SubscriptionCallback::OnAttributeData(const ReadClient * apReadClient, DataVersion * apVersion, + const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) { if (aPath.IsListItemOperation()) { ReportError(CHIP_ERROR_INCORRECT_STATE); diff --git a/src/lib/support/DefaultStorageKeyAllocator.h b/src/lib/support/DefaultStorageKeyAllocator.h index 08af377b7ef511..d7cf55f86c0bbe 100644 --- a/src/lib/support/DefaultStorageKeyAllocator.h +++ b/src/lib/support/DefaultStorageKeyAllocator.h @@ -17,6 +17,7 @@ #pragma once #include +#include #include #include #include @@ -55,6 +56,11 @@ class DefaultStorageKeyAllocator return Format("a/%" PRIx16 "/%" PRIx32 "/%" PRIx32, aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId); } + const char * ClusterValue(const app::ConcreteClusterPath & aPath) + { + return Format("a/%" PRIx16 "/%" PRIx32, aPath.mEndpointId, aPath.mClusterId); + } + private: static const size_t kKeyLengthMax = 32;