From e150e067de18e0a07cef604162ce9f424452de27 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 17 Oct 2024 11:11:51 -0400 Subject: [PATCH] Add `emberAfMetadataStructureGeneration` for codegen data model. (#36104) CodegenDataModel is using caching to speed up queries on clusters. However dynamic endpoints can be created/removed at will and in those cases old cluster pointers should not be re-used. --- .../CodegenDataModelProvider.cpp | 8 +++++--- .../CodegenDataModelProvider.h | 1 + src/app/util/attribute-storage.cpp | 13 +++++++++++++ src/app/util/attribute-storage.h | 8 ++++++++ src/app/util/mock/attribute-storage.cpp | 12 ++++++++++-- 5 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp index 7f7b7e14b8b752..2fb542c768aac0 100644 --- a/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp +++ b/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp @@ -619,8 +619,9 @@ std::optional CodegenDataModelProvider::TryFindAttributeIndex(const Em const EmberAfCluster * CodegenDataModelProvider::FindServerCluster(const ConcreteClusterPath & path) { - // cache things - if (mPreviouslyFoundCluster.has_value() && (mPreviouslyFoundCluster->path == path)) + if (mPreviouslyFoundCluster.has_value() && (mPreviouslyFoundCluster->path == path) && + (mEmberMetadataStructureGeneration == emberAfMetadataStructureGeneration())) + { return mPreviouslyFoundCluster->cluster; } @@ -628,7 +629,8 @@ const EmberAfCluster * CodegenDataModelProvider::FindServerCluster(const Concret const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); if (cluster != nullptr) { - mPreviouslyFoundCluster = std::make_optional(path, cluster); + mPreviouslyFoundCluster = std::make_optional(path, cluster); + mEmberMetadataStructureGeneration = emberAfMetadataStructureGeneration(); } return cluster; } diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider.h b/src/app/codegen-data-model-provider/CodegenDataModelProvider.h index f6d6e4e4ed4be3..085ae67ec392ab 100644 --- a/src/app/codegen-data-model-provider/CodegenDataModelProvider.h +++ b/src/app/codegen-data-model-provider/CodegenDataModelProvider.h @@ -137,6 +137,7 @@ class CodegenDataModelProvider : public chip::app::DataModel::Provider ClusterReference(const ConcreteClusterPath p, const EmberAfCluster * c) : path(p), cluster(c) {} }; std::optional mPreviouslyFoundCluster; + unsigned mEmberMetadataStructureGeneration = 0; /// Finds the specified ember cluster /// diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp index adc728157c3a1e..51c01c7e43fa80 100644 --- a/src/app/util/attribute-storage.cpp +++ b/src/app/util/attribute-storage.cpp @@ -93,6 +93,11 @@ uint8_t singletonAttributeData[ACTUAL_SINGLETONS_SIZE]; uint16_t emberEndpointCount = 0; +/// Determines a incremental unique index for ember +/// metadata that is increased whenever a structural change is made to the +/// ember metadata (e.g. changing dynamic endpoints or enabling/disabling endpoints) +unsigned emberMetadataStructureGeneration = 0; + // If we have attributes that are more than 4 bytes, then // we need this data block for the defaults #if (defined(GENERATED_DEFAULTS) && GENERATED_DEFAULTS_COUNT) @@ -315,6 +320,7 @@ CHIP_ERROR emberAfSetDynamicEndpoint(uint16_t index, EndpointId id, const EmberA // Now enable the endpoint. emberAfEndpointEnableDisable(id, true); + emberMetadataStructureGeneration++; return CHIP_NO_ERROR; } @@ -332,6 +338,7 @@ EndpointId emberAfClearDynamicEndpoint(uint16_t index) emAfEndpoints[index].endpoint = kInvalidEndpointId; } + emberMetadataStructureGeneration++; return ep; } @@ -944,9 +951,15 @@ bool emberAfEndpointEnableDisable(EndpointId endpoint, bool enable) emberAfGlobalInteractionModelAttributesChangedListener()); } + emberMetadataStructureGeneration++; return true; } +unsigned emberAfMetadataStructureGeneration() +{ + return emberMetadataStructureGeneration; +} + // Returns the index of a given endpoint. Does not consider disabled endpoints. uint16_t emberAfIndexFromEndpoint(EndpointId endpoint) { diff --git a/src/app/util/attribute-storage.h b/src/app/util/attribute-storage.h index 9d2dcc60bb3f05..711f6a7cff768d 100644 --- a/src/app/util/attribute-storage.h +++ b/src/app/util/attribute-storage.h @@ -312,6 +312,14 @@ void emberAfAttributeChanged(chip::EndpointId endpoint, chip::ClusterId clusterI /// any cluster data versions. void emberAfEndpointChanged(chip::EndpointId endpoint, chip::app::AttributesChangedListener * listener); +/// Maintains a increasing index of structural changes within ember +/// that determine if existing "indexes" and metadata pointers within ember +/// are still valid or not. +/// +/// Changes to metadata structure (e.g. endpoint enable/disable and dynamic endpoint changes) +/// are reflected in this generation count changing. +unsigned emberAfMetadataStructureGeneration(); + namespace chip { namespace app { diff --git a/src/app/util/mock/attribute-storage.cpp b/src/app/util/mock/attribute-storage.cpp index 256d0a3a880886..57f3d1e5216264 100644 --- a/src/app/util/mock/attribute-storage.cpp +++ b/src/app/util/mock/attribute-storage.cpp @@ -61,8 +61,9 @@ using namespace Clusters::Globals::Attributes; namespace { -DataVersion dataVersion = 0; -const MockNodeConfig * mockConfig = nullptr; +unsigned metadataStructureGeneration = 0; +DataVersion dataVersion = 0; +const MockNodeConfig * mockConfig = nullptr; const MockNodeConfig & DefaultMockNodeConfig() { @@ -342,6 +343,11 @@ void emberAfAttributeChanged(EndpointId endpoint, ClusterId clusterId, Attribute listener->MarkDirty(AttributePathParams(endpoint, clusterId, attributeId)); } +unsigned emberAfMetadataStructureGeneration() +{ + return metadataStructureGeneration; +} + namespace chip { namespace app { @@ -494,12 +500,14 @@ CHIP_ERROR ReadSingleMockClusterData(FabricIndex aAccessingFabricIndex, const Co void SetMockNodeConfig(const MockNodeConfig & config) { + metadataStructureGeneration++; mockConfig = &config; } /// Resets the mock attribute storage to the default configuration. void ResetMockNodeConfig() { + metadataStructureGeneration++; mockConfig = nullptr; }