diff --git a/src/app/clusters/descriptor/descriptor.cpp b/src/app/clusters/descriptor/descriptor.cpp index 814c1649093087..67a1a3be546ccd 100644 --- a/src/app/clusters/descriptor/descriptor.cpp +++ b/src/app/clusters/descriptor/descriptor.cpp @@ -183,12 +183,11 @@ CHIP_ERROR DescriptorAttrAccess::ReadClientServerAttribute(EndpointId endpoint, } else { - ConcreteClusterPath clusterPath = - InteractionModelEngine::GetInstance()->GetDataModelProvider()->FirstClientCluster(endpoint); - while (clusterPath.HasValidIds()) + auto it = InteractionModelEngine::GetInstance()->GetDataModelProvider()->GetClientClusters(endpoint); + + for (auto clusterID = it->Next(); clusterID.has_value(); clusterID = it->Next()) { - ReturnErrorOnFailure(encoder.Encode(clusterPath.mClusterId)); - clusterPath = InteractionModelEngine::GetInstance()->GetDataModelProvider()->NextClientCluster(clusterPath); + ReturnErrorOnFailure(encoder.Encode(*clusterID)); } } diff --git a/src/app/data-model-provider/MetadataTypes.h b/src/app/data-model-provider/MetadataTypes.h index c37fe4d9bd0dbc..019ffa311e6428 100644 --- a/src/app/data-model-provider/MetadataTypes.h +++ b/src/app/data-model-provider/MetadataTypes.h @@ -203,23 +203,21 @@ class ProviderMetadataTree // - Lookups should be performed using `Get...` and `SeekTo`. // ///////////////////////////////////////////////////////////////////////// + virtual std::unique_ptr> GetDeviceTypes(EndpointId endpointId) = 0; + virtual std::unique_ptr> GetSemanticTags(EndpointId endpointId) = 0; + virtual std::unique_ptr> GetClientClusters(EndpointId endpointId) = 0; + + // TODO: below items MUST transition to pure virtual and have implementations everywhere virtual std::unique_ptr> GetEndpoints() { return std::make_unique>(); } - virtual std::unique_ptr> GetDeviceTypes(EndpointId endpointId) = 0; - virtual std::unique_ptr> GetSemanticTags(EndpointId endpointId) = 0; virtual std::unique_ptr> GetServerClusters(EndpointId endpointId) { return std::make_unique>(); } - virtual std::unique_ptr> GetClientClusters(EndpointId endpointId) - { - return std::make_unique>(); - } - virtual std::unique_ptr> GetAttributes(ConcreteClusterPath clusterPath) { return std::make_unique>(); @@ -244,12 +242,6 @@ class ProviderMetadataTree virtual ClusterEntry NextServerCluster(const ConcreteClusterPath & before) = 0; virtual std::optional GetServerClusterInfo(const ConcreteClusterPath & path) = 0; - // This iteration will list all client clusters on a given endpoint - // As the client cluster is only a client without any attributes/commands, - // these functions only return the cluster path. - virtual ConcreteClusterPath FirstClientCluster(EndpointId endpoint) = 0; - virtual ConcreteClusterPath NextClientCluster(const ConcreteClusterPath & before) = 0; - // Attribute iteration and accessors provide cluster-level access over // attributes virtual AttributeEntry FirstAttribute(const ConcreteClusterPath & cluster) = 0; diff --git a/src/app/tests/test-interaction-model-api.cpp b/src/app/tests/test-interaction-model-api.cpp index 19e6909d121915..b4cc632217ee79 100644 --- a/src/app/tests/test-interaction-model-api.cpp +++ b/src/app/tests/test-interaction-model-api.cpp @@ -179,14 +179,9 @@ std::optional TestImCustomDataModel::GetServerClusterInfo(const Con return CodegenDataModelProviderInstance(nullptr /* delegate */)->GetServerClusterInfo(path); } -ConcreteClusterPath TestImCustomDataModel::FirstClientCluster(EndpointId endpoint) +std::unique_ptr> TestImCustomDataModel::GetClientClusters(EndpointId endpointId) { - return CodegenDataModelProviderInstance(nullptr /* delegate */)->FirstClientCluster(endpoint); -} - -ConcreteClusterPath TestImCustomDataModel::NextClientCluster(const ConcreteClusterPath & before) -{ - return CodegenDataModelProviderInstance(nullptr /* delegate */)->NextClientCluster(before); + return CodegenDataModelProviderInstance(nullptr /* delegate */)->GetClientClusters(endpointId); } AttributeEntry TestImCustomDataModel::FirstAttribute(const ConcreteClusterPath & cluster) diff --git a/src/app/tests/test-interaction-model-api.h b/src/app/tests/test-interaction-model-api.h index bb9a7ba11adb92..3e13064cbc93f8 100644 --- a/src/app/tests/test-interaction-model-api.h +++ b/src/app/tests/test-interaction-model-api.h @@ -110,14 +110,13 @@ class TestImCustomDataModel : public DataModel::Provider std::optional Invoke(const DataModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments, CommandHandler * handler) override; + std::unique_ptr> GetClientClusters(EndpointId endpointId) override; DataModel::EndpointEntry FirstEndpoint() override; DataModel::EndpointEntry NextEndpoint(EndpointId before) override; std::optional GetEndpointInfo(EndpointId endpoint) override; DataModel::ClusterEntry FirstServerCluster(EndpointId endpoint) override; DataModel::ClusterEntry NextServerCluster(const ConcreteClusterPath & before) override; std::optional GetServerClusterInfo(const ConcreteClusterPath & path) override; - ConcreteClusterPath FirstClientCluster(EndpointId endpoint) override; - ConcreteClusterPath NextClientCluster(const ConcreteClusterPath & before) override; DataModel::AttributeEntry FirstAttribute(const ConcreteClusterPath & cluster) override; DataModel::AttributeEntry NextAttribute(const ConcreteAttributePath & before) override; std::optional GetAttributeInfo(const ConcreteAttributePath & path) override; diff --git a/src/controller/tests/data_model/DataModelFixtures.cpp b/src/controller/tests/data_model/DataModelFixtures.cpp index 24d85d1f1676ec..873922763cdd2e 100644 --- a/src/controller/tests/data_model/DataModelFixtures.cpp +++ b/src/controller/tests/data_model/DataModelFixtures.cpp @@ -507,14 +507,9 @@ std::optional CustomDataModel::GetServerClusterInfo(const ConcreteC return CodegenDataModelProviderInstance(nullptr /* delegate */)->GetServerClusterInfo(path); } -ConcreteClusterPath CustomDataModel::FirstClientCluster(EndpointId endpoint) +std::unique_ptr> CustomDataModel::GetClientClusters(EndpointId endpointId) { - return CodegenDataModelProviderInstance(nullptr /* delegate */)->FirstClientCluster(endpoint); -} - -ConcreteClusterPath CustomDataModel::NextClientCluster(const ConcreteClusterPath & before) -{ - return CodegenDataModelProviderInstance(nullptr /* delegate */)->NextClientCluster(before); + return CodegenDataModelProviderInstance(nullptr /* delegate */)->GetClientClusters(endpointId); } AttributeEntry CustomDataModel::FirstAttribute(const ConcreteClusterPath & cluster) diff --git a/src/controller/tests/data_model/DataModelFixtures.h b/src/controller/tests/data_model/DataModelFixtures.h index abb826bdcd556f..2061e16f5971e8 100644 --- a/src/controller/tests/data_model/DataModelFixtures.h +++ b/src/controller/tests/data_model/DataModelFixtures.h @@ -122,14 +122,13 @@ class CustomDataModel : public DataModel::Provider std::optional Invoke(const DataModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments, CommandHandler * handler) override; + std::unique_ptr> GetClientClusters(EndpointId endpointId) override; DataModel::EndpointEntry FirstEndpoint() override; DataModel::EndpointEntry NextEndpoint(EndpointId before) override; std::optional GetEndpointInfo(EndpointId endpoint) override; DataModel::ClusterEntry FirstServerCluster(EndpointId endpoint) override; DataModel::ClusterEntry NextServerCluster(const ConcreteClusterPath & before) override; std::optional GetServerClusterInfo(const ConcreteClusterPath & path) override; - ConcreteClusterPath FirstClientCluster(EndpointId endpoint) override; - ConcreteClusterPath NextClientCluster(const ConcreteClusterPath & before) override; DataModel::AttributeEntry FirstAttribute(const ConcreteClusterPath & cluster) override; DataModel::AttributeEntry NextAttribute(const ConcreteAttributePath & before) override; std::optional GetAttributeInfo(const ConcreteAttributePath & path) override; diff --git a/src/data-model-providers/codegen/CodegenDataModelProvider.cpp b/src/data-model-providers/codegen/CodegenDataModelProvider.cpp index a3bce143aade07..d95cf3fa918854 100644 --- a/src/data-model-providers/codegen/CodegenDataModelProvider.cpp +++ b/src/data-model-providers/codegen/CodegenDataModelProvider.cpp @@ -211,23 +211,6 @@ DataModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, const Emb return DataModel::ClusterEntry::kInvalid; } -ClusterId FirstClientClusterId(const EmberAfEndpointType * endpoint, unsigned start_index, unsigned & found_index) -{ - for (unsigned cluster_idx = start_index; cluster_idx < endpoint->clusterCount; cluster_idx++) - { - const EmberAfCluster & cluster = endpoint->cluster[cluster_idx]; - if (!cluster.IsClient()) - { - continue; - } - - found_index = cluster_idx; - return cluster.clusterId; - } - - return kInvalidClusterId; -} - /// Load the attribute information into the specified destination /// /// `info` is assumed to be default-constructed/clear (i.e. this sets flags, but does not reset them). @@ -367,6 +350,35 @@ class SemanticTagIterator : public DataModel::ElementIterator +{ +public: + ClientClusterIdIterator() = default; + ClientClusterIdIterator(Span clusters) : mClusters(clusters) {} + + std::optional Next() override + { + while (true) + { + if (mClusters.empty()) + { + return std::nullopt; + } + + if (mClusters.front().IsClient()) + { + ClusterId id = mClusters.front().clusterId; + mClusters = mClusters.SubSpan(1); + return id; + } + mClusters = mClusters.SubSpan(1); + } + } + +private: + Span mClusters; +}; + DefaultAttributePersistenceProvider gDefaultAttributePersistence; } // namespace @@ -629,33 +641,15 @@ std::optional CodegenDataModelProvider::GetServerCluster return std::make_optional(std::get(info)); } -ConcreteClusterPath CodegenDataModelProvider::FirstClientCluster(EndpointId endpointId) +std::unique_ptr> CodegenDataModelProvider::GetClientClusters(EndpointId endpointId) { const EmberAfEndpointType * endpoint = emberAfFindEndpointType(endpointId); - VerifyOrReturnValue(endpoint != nullptr, ConcreteClusterPath(endpointId, kInvalidClusterId)); - VerifyOrReturnValue(endpoint->clusterCount > 0, ConcreteClusterPath(endpointId, kInvalidClusterId)); - VerifyOrReturnValue(endpoint->cluster != nullptr, ConcreteClusterPath(endpointId, kInvalidClusterId)); - - return ConcreteClusterPath(endpointId, FirstClientClusterId(endpoint, 0, mClientClusterIterationHint)); -} - -ConcreteClusterPath CodegenDataModelProvider::NextClientCluster(const ConcreteClusterPath & before) -{ - // TODO: This search still seems slow (ember will loop). Should use index hints as long - // as ember API supports it - const EmberAfEndpointType * endpoint = emberAfFindEndpointType(before.mEndpointId); - - VerifyOrReturnValue(endpoint != nullptr, ConcreteClusterPath(before.mEndpointId, kInvalidClusterId)); - VerifyOrReturnValue(endpoint->clusterCount > 0, ConcreteClusterPath(before.mEndpointId, kInvalidClusterId)); - VerifyOrReturnValue(endpoint->cluster != nullptr, ConcreteClusterPath(before.mEndpointId, kInvalidClusterId)); - std::optional cluster_idx = TryFindClusterIndex(endpoint, before.mClusterId, ClusterSide::kClient); - if (!cluster_idx.has_value()) - { - return ConcreteClusterPath(before.mEndpointId, kInvalidClusterId); - } + VerifyOrReturnValue(endpoint != nullptr, std::make_unique()); + VerifyOrReturnValue(endpoint->clusterCount > 0, std::make_unique()); + VerifyOrReturnValue(endpoint->cluster != nullptr, std::make_unique()); - return ConcreteClusterPath(before.mEndpointId, FirstClientClusterId(endpoint, *cluster_idx + 1, mClientClusterIterationHint)); + return std::make_unique(Span(endpoint->cluster, endpoint->clusterCount)); } DataModel::AttributeEntry CodegenDataModelProvider::FirstAttribute(const ConcreteClusterPath & path) diff --git a/src/data-model-providers/codegen/CodegenDataModelProvider.h b/src/data-model-providers/codegen/CodegenDataModelProvider.h index 70bf958f231651..e548282b52f4c4 100644 --- a/src/data-model-providers/codegen/CodegenDataModelProvider.h +++ b/src/data-model-providers/codegen/CodegenDataModelProvider.h @@ -157,6 +157,7 @@ class CodegenDataModelProvider : public DataModel::Provider /// attribute tree iteration std::unique_ptr> GetDeviceTypes(EndpointId endpointId) override; std::unique_ptr> GetSemanticTags(EndpointId endpointId) override; + std::unique_ptr> GetClientClusters(EndpointId endpointId) override; DataModel::EndpointEntry FirstEndpoint() override; DataModel::EndpointEntry NextEndpoint(EndpointId before) override; @@ -167,9 +168,6 @@ class CodegenDataModelProvider : public DataModel::Provider DataModel::ClusterEntry NextServerCluster(const ConcreteClusterPath & before) override; std::optional GetServerClusterInfo(const ConcreteClusterPath & path) override; - ConcreteClusterPath FirstClientCluster(EndpointId endpoint) override; - ConcreteClusterPath NextClientCluster(const ConcreteClusterPath & before) override; - DataModel::AttributeEntry FirstAttribute(const ConcreteClusterPath & cluster) override; DataModel::AttributeEntry NextAttribute(const ConcreteAttributePath & before) override; std::optional GetAttributeInfo(const ConcreteAttributePath & path) override; diff --git a/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp b/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp index e080d43fde803e..51c1cc54d52ac9 100644 --- a/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp +++ b/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp @@ -15,6 +15,7 @@ * limitations under the License. */ +#include "pw_unit_test/framework.h" #include #include @@ -1079,53 +1080,33 @@ TEST(TestCodegenModelViaMocks, IterateOverClientClusters) UseMockNodeConfig config(gTestNodeConfig); CodegenDataModelProviderWithContext model; - EXPECT_FALSE(model.FirstClientCluster(kEndpointIdThatIsMissing).HasValidIds()); - EXPECT_FALSE(model.FirstClientCluster(kInvalidEndpointId).HasValidIds()); - EXPECT_FALSE(model.NextClientCluster(ConcreteClusterPath(kInvalidEndpointId, 123)).HasValidIds()); - EXPECT_FALSE(model.NextClientCluster(ConcreteClusterPath(kMockEndpoint1, kInvalidClusterId)).HasValidIds()); - EXPECT_FALSE(model.NextClientCluster(ConcreteClusterPath(kMockEndpoint1, 981u)).HasValidIds()); + EXPECT_FALSE(model.GetClientClusters(kEndpointIdThatIsMissing)->Next().has_value()); + EXPECT_FALSE(model.GetClientClusters(kInvalidEndpointId)->Next().has_value()); // mock endpoint 1 has 2 mock client clusters: 3 and 4 - ConcreteClusterPath path = model.FirstClientCluster(kMockEndpoint1); - ASSERT_TRUE(path.HasValidIds()); - EXPECT_EQ(path.mEndpointId, kMockEndpoint1); - EXPECT_EQ(path.mClusterId, MockClusterId(3)); + auto it = model.GetClientClusters(kMockEndpoint1); + auto clusterId = it->Next(); + ASSERT_TRUE(clusterId.has_value()); + EXPECT_EQ(*clusterId, MockClusterId(3)); // NOLINT(bugprone-unchecked-optional-access) - path = model.NextClientCluster(path); - ASSERT_TRUE(path.HasValidIds()); - EXPECT_EQ(path.mEndpointId, kMockEndpoint1); - EXPECT_EQ(path.mClusterId, MockClusterId(4)); + clusterId = it->Next(); + ASSERT_TRUE(clusterId.has_value()); + EXPECT_EQ(*clusterId, MockClusterId(4)); // NOLINT(bugprone-unchecked-optional-access) - path = model.NextClientCluster(path); - EXPECT_FALSE(path.HasValidIds()); + clusterId = it->Next(); + EXPECT_FALSE(clusterId.has_value()); // mock endpoint 2 has 1 mock client clusters: 3(has server side at the same time) and 4 - path = model.FirstClientCluster(kMockEndpoint2); - for (uint16_t clusterId = 3; clusterId <= 4; clusterId++) + it = model.GetClientClusters(kMockEndpoint2); + clusterId = it->Next(); + for (uint16_t expectedId = 3; expectedId <= 4; expectedId++) { - ASSERT_TRUE(path.HasValidIds()); - EXPECT_EQ(path.mEndpointId, kMockEndpoint2); - EXPECT_EQ(path.mClusterId, MockClusterId(clusterId)); - path = model.NextClientCluster(path); - } - EXPECT_FALSE(path.HasValidIds()); + ASSERT_TRUE(clusterId.has_value()); + EXPECT_EQ(*clusterId, MockClusterId(expectedId)); // NOLINT(bugprone-unchecked-optional-access) - // repeat calls should work - for (int i = 0; i < 10; i++) - { - path = model.FirstClientCluster(kMockEndpoint1); - ASSERT_TRUE(path.HasValidIds()); - EXPECT_EQ(path.mEndpointId, kMockEndpoint1); - EXPECT_EQ(path.mClusterId, MockClusterId(3)); - } - - for (int i = 0; i < 10; i++) - { - ConcreteClusterPath nextPath = model.NextClientCluster(path); - ASSERT_TRUE(nextPath.HasValidIds()); - EXPECT_EQ(nextPath.mEndpointId, kMockEndpoint1); - EXPECT_EQ(nextPath.mClusterId, MockClusterId(4)); + clusterId = it->Next(); } + ASSERT_FALSE(clusterId.has_value()); } TEST(TestCodegenModelViaMocks, IterateOverAttributes)