From 30812a9914670b39efa3a5db537fdb5116df55d7 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 7 May 2024 15:17:48 -0400 Subject: [PATCH 01/79] Initial copy with a clean history --- src/BUILD.gn | 1 + src/app/ConcreteClusterPath.h | 2 +- src/app/codegen-interaction-model/BUILD.gn | 22 ++ src/app/codegen-interaction-model/Model.cpp | 267 ++++++++++++++++ src/app/codegen-interaction-model/Model.h | 52 ++++ src/app/codegen-interaction-model/model.gni | 35 +++ .../codegen-interaction-model/tests/BUILD.gn | 35 +++ .../tests/TestCodegenModelViaMocks.cpp | 290 ++++++++++++++++++ src/app/interaction-model/IterationTypes.h | 16 +- src/app/interaction-model/tests/BUILD.gn | 2 + src/app/util/mock/attribute-storage.cpp | 1 + 11 files changed, 721 insertions(+), 2 deletions(-) create mode 100644 src/app/codegen-interaction-model/BUILD.gn create mode 100644 src/app/codegen-interaction-model/Model.cpp create mode 100644 src/app/codegen-interaction-model/Model.h create mode 100644 src/app/codegen-interaction-model/model.gni create mode 100644 src/app/codegen-interaction-model/tests/BUILD.gn create mode 100644 src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp diff --git a/src/BUILD.gn b/src/BUILD.gn index d1dfcc7856135e..8ff1e2cf1f463b 100644 --- a/src/BUILD.gn +++ b/src/BUILD.gn @@ -55,6 +55,7 @@ if (chip_build_tests) { chip_test_group("tests") { deps = [] tests = [ + "${chip_root}/src/app/codegen-interaction-model/tests", "${chip_root}/src/app/interaction-model/tests", "${chip_root}/src/access/tests", "${chip_root}/src/crypto/tests", diff --git a/src/app/ConcreteClusterPath.h b/src/app/ConcreteClusterPath.h index 58b2f5b477f139..47feae1fd67269 100644 --- a/src/app/ConcreteClusterPath.h +++ b/src/app/ConcreteClusterPath.h @@ -52,7 +52,7 @@ struct ConcreteClusterPath // to alignment requirements it's "free" in the sense of not needing more // memory to put it here. But we don't initialize it, because that // increases codesize for the non-consumers. - bool mExpanded; // NOTE: in between larger members + bool mExpanded = false; // NOTE: in between larger members ClusterId mClusterId = 0; }; diff --git a/src/app/codegen-interaction-model/BUILD.gn b/src/app/codegen-interaction-model/BUILD.gn new file mode 100644 index 00000000000000..bbb4a30e96d621 --- /dev/null +++ b/src/app/codegen-interaction-model/BUILD.gn @@ -0,0 +1,22 @@ +# Copyright (c) 2024 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. +import("//build_overrides/chip.gni") +# This source set is TIGHLY coupled with code-generated data models +# as generally implemented by `src/app/util` +# +# Corresponding functions defined in attribute-storace.cpp/attribute-table.cpp must +# be available at link time for this model to use +# Use `model.gni` to get access to: +# model.cpp +# model.h diff --git a/src/app/codegen-interaction-model/Model.cpp b/src/app/codegen-interaction-model/Model.cpp new file mode 100644 index 00000000000000..dc3fbd9cbca7d0 --- /dev/null +++ b/src/app/codegen-interaction-model/Model.cpp @@ -0,0 +1,267 @@ +/* + * Copyright (c) 2024 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. + */ +#include + +#include +#include +#include + +namespace chip { +namespace app { +namespace CodegenDataModel { + +namespace { + +/// Checks if the specified ember cluster mask corresponds to a valid +/// server cluster. +bool IsServerMask(EmberAfClusterMask mask) +{ + return (mask == 0) || ((mask & CLUSTER_MASK_SERVER) != 0); +} + +/// Load the cluster information into the specified destination +void LoadClusterInfo(const ConcreteClusterPath & path, const EmberAfCluster & cluster, InteractionModel::ClusterInfo * info) +{ + chip::DataVersion * versionPtr = emberAfDataVersionStorage(path); + if (versionPtr != nullptr) + { + info->dataVersion = *versionPtr; + } + else + { + ChipLogError(AppServer, "Failed to get data version for %d/" ChipLogFormatMEI, static_cast(path.mEndpointId), + ChipLogValueMEI(cluster.clusterId)); + info->dataVersion = 0; + } + + // TODO: set entry flags: + // info->flags.Set(ClusterQualityFlags::kDiagnosticsData) +} + +/// Converts a EmberAfCluster into a ClusterEntry +InteractionModel::ClusterEntry ClusterEntryFrom(EndpointId endpointId, const EmberAfCluster & cluster) +{ + InteractionModel::ClusterEntry entry; + + entry.path = ConcreteClusterPath(endpointId, cluster.clusterId); + LoadClusterInfo(entry.path, cluster, &entry.info); + + return entry; +} + +/// Finds the first server cluster entry for the given endpoint data starting at [start_index] +/// +/// Returns an invalid entry if no more server clusters are found +InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, const EmberAfEndpointType * endpoint, + uint16_t start_index) +{ + for (unsigned i = start_index; i < endpoint->clusterCount; i++) + { + const EmberAfCluster & cluster = endpoint->cluster[i]; + if (!IsServerMask(cluster.mask & CLUSTER_MASK_SERVER)) + { + continue; + } + + return ClusterEntryFrom(endpointId, cluster); + } + + return InteractionModel::ClusterEntry::Invalid(); +} + +/// Load the cluster information into the specified destination +void LoadAttributeInfo(const ConcreteAttributePath & path, const EmberAfAttributeMetadata & attribute, + InteractionModel::AttributeInfo * info) +{ + if (attribute.attributeType == ZCL_ARRAY_ATTRIBUTE_TYPE) + { + info->flags.Set(InteractionModel::AttributeQualityFlags::kListAttribute); + } + + // TODO: Set additional flags: + // info->flags.Set(InteractionModel::AttributeQualityFlags::kChangesOmitted) +} + +InteractionModel::AttributeEntry AttributeEntryFrom(const ConcreteClusterPath & clusterPath, + const EmberAfAttributeMetadata & attribute) +{ + InteractionModel::AttributeEntry entry; + + entry.path = ConcreteAttributePath(clusterPath.mEndpointId, clusterPath.mClusterId, attribute.attributeId); + LoadAttributeInfo(entry.path, attribute, &entry.info); + + return entry; +} + +} // namespace + +CHIP_ERROR Model::ReadAttribute(const InteractionModel::ReadAttributeRequest & request, InteractionModel::ReadState & state, + AttributeValueEncoder & encoder) +{ + // TODO: this needs an implementation + return CHIP_ERROR_NOT_IMPLEMENTED; +} + +CHIP_ERROR Model::WriteAttribute(const InteractionModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) +{ + // TODO: this needs an implementation + return CHIP_ERROR_NOT_IMPLEMENTED; +} + +CHIP_ERROR Model::Invoke(const InteractionModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments, + InteractionModel::InvokeReply & reply) +{ + // TODO: this needs an implementation + return CHIP_ERROR_NOT_IMPLEMENTED; +} + +EndpointId Model::FirstEndpoint() +{ + // find the first enabled index + const uint16_t lastEndpointIndex = emberAfEndpointCount(); + for (uint16_t endpointIndex = 0; endpointIndex < lastEndpointIndex; endpointIndex++) + { + if (emberAfEndpointIndexIsEnabled(endpointIndex)) + { + return emberAfEndpointFromIndex(endpointIndex); + } + } + + // No enabled endpoint found. Give up + return kInvalidEndpointId; +} + +EndpointId Model::NextEndpoint(EndpointId before) +{ + // find the first enabled index + bool beforeFound = false; + + const uint16_t lastEndpointIndex = emberAfEndpointCount(); + for (uint16_t endpointIndex = 0; endpointIndex < lastEndpointIndex; endpointIndex++) + { + if (!beforeFound) + { + beforeFound = (before == emberAfEndpointFromIndex(endpointIndex)); + continue; + } + + if (emberAfEndpointIndexIsEnabled(endpointIndex)) + { + return emberAfEndpointFromIndex(endpointIndex); + } + } + + // No enabled enpoint after "before" was found, give up + return kInvalidEndpointId; +} + +InteractionModel::ClusterEntry Model::FirstCluster(EndpointId endpointId) +{ + const EmberAfEndpointType * endpoint = emberAfFindEndpointType(endpointId); + VerifyOrReturnValue(endpoint != nullptr, InteractionModel::ClusterEntry::Invalid()); + VerifyOrReturnValue(endpoint->clusterCount > 0, InteractionModel::ClusterEntry::Invalid()); + VerifyOrReturnValue(endpoint->cluster != nullptr, InteractionModel::ClusterEntry::Invalid()); + + return FirstServerClusterEntry(endpointId, endpoint, 0); +} + +InteractionModel::ClusterEntry Model::NextCluster(const ConcreteClusterPath & before) +{ + const EmberAfEndpointType * endpoint = emberAfFindEndpointType(before.mEndpointId); + VerifyOrReturnValue(endpoint != nullptr, InteractionModel::ClusterEntry::Invalid()); + VerifyOrReturnValue(endpoint->clusterCount > 0, InteractionModel::ClusterEntry::Invalid()); + VerifyOrReturnValue(endpoint->cluster != nullptr, InteractionModel::ClusterEntry::Invalid()); + + for (uint16_t i = 0; i < endpoint->clusterCount; i++) + { + const EmberAfCluster & cluster = endpoint->cluster[i]; + if (IsServerMask(cluster.mask) && (cluster.clusterId == before.mClusterId)) + { + return FirstServerClusterEntry(before.mEndpointId, endpoint, i + 1); + } + } + + return InteractionModel::ClusterEntry::Invalid(); +} + +std::optional Model::GetClusterInfo(const ConcreteClusterPath & path) +{ + const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); + VerifyOrReturnValue(cluster != nullptr, std::nullopt); + + InteractionModel::ClusterInfo info; + LoadClusterInfo(path, *cluster, &info); + + return std::make_optional(info); +} + +InteractionModel::AttributeEntry Model::FirstAttribute(const ConcreteClusterPath & path) +{ + const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); + VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::Invalid()); + VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); + VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); + + return AttributeEntryFrom(path, cluster->attributes[0]); +} + +InteractionModel::AttributeEntry Model::NextAttribute(const ConcreteAttributePath & before) +{ + const EmberAfCluster * cluster = emberAfFindServerCluster(before.mEndpointId, before.mClusterId); + VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::Invalid()); + VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); + VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); + + // find the given attribute in the list and then return the next one + bool foundPosition = false; + const unsigned attributeCount = cluster->attributeCount; + for (unsigned i = 0; i < attributeCount; i++) + { + if (foundPosition) + { + return AttributeEntryFrom(before, cluster->attributes[i]); + } + + foundPosition = (cluster->attributes[i].attributeId == before.mAttributeId); + } + + return InteractionModel::AttributeEntry::Invalid(); +} + +std::optional Model::GetAttributeInfo(const ConcreteAttributePath & path) +{ + const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); + VerifyOrReturnValue(cluster != nullptr, std::nullopt); + VerifyOrReturnValue(cluster->attributeCount > 0, std::nullopt); + VerifyOrReturnValue(cluster->attributes != nullptr, std::nullopt); + const unsigned attributeCount = cluster->attributeCount; + for (unsigned i = 0; i < attributeCount; i++) + { + if (cluster->attributes[i].attributeId == path.mAttributeId) + { + InteractionModel::AttributeInfo info; + LoadAttributeInfo(path, cluster->attributes[i], &info); + return std::make_optional(info); + } + } + + return std::nullopt; +} + +} // namespace CodegenDataModel +} // namespace app +} // namespace chip diff --git a/src/app/codegen-interaction-model/Model.h b/src/app/codegen-interaction-model/Model.h new file mode 100644 index 00000000000000..3f48970852a7bc --- /dev/null +++ b/src/app/codegen-interaction-model/Model.h @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2024 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 { +namespace CodegenDataModel { + +class Model : public chip::app::InteractionModel::Model +{ +public: + /// Generic model implementations + CHIP_ERROR Shutdown() override { return CHIP_NO_ERROR; } + + CHIP_ERROR ReadAttribute(const InteractionModel::ReadAttributeRequest & request, InteractionModel::ReadState & state, + AttributeValueEncoder & encoder) override; + CHIP_ERROR WriteAttribute(const InteractionModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) override; + CHIP_ERROR Invoke(const InteractionModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments, + InteractionModel::InvokeReply & reply) override; + + /// attribute tree iteration + EndpointId FirstEndpoint() override; + EndpointId NextEndpoint(EndpointId before) override; + + InteractionModel::ClusterEntry FirstCluster(EndpointId endpoint) override; + InteractionModel::ClusterEntry NextCluster(const ConcreteClusterPath & before) override; + std::optional GetClusterInfo(const ConcreteClusterPath & path) override; + + InteractionModel::AttributeEntry FirstAttribute(const ConcreteClusterPath & cluster) override; + InteractionModel::AttributeEntry NextAttribute(const ConcreteAttributePath & before) override; + std::optional GetAttributeInfo(const ConcreteAttributePath & path) override; +}; + +} // namespace CodegenDataModel +} // namespace app +} // namespace chip diff --git a/src/app/codegen-interaction-model/model.gni b/src/app/codegen-interaction-model/model.gni new file mode 100644 index 00000000000000..ddf7b1b172cc5a --- /dev/null +++ b/src/app/codegen-interaction-model/model.gni @@ -0,0 +1,35 @@ +# Copyright (c) 2024 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. +import("//build_overrides/chip.gni") + +# The sources in this directory are TIGHLY coupled with code-generated data models +# as generally implemented by `src/app/util` +# +# Corresponding functions defined in attribute-storace.cpp/attribute-table.cpp must +# be available at link time for this model to use and constants heavily depend +# on `zap-generated/endpoint_config.h` (generally compile-time constants that +# are code generated) +# +# As a result, the files here are NOT a source_set or similar because they cannot +# be cleanly built as a stand-alone and instead have to be imported as part of +# a different data model or compilation unit. +codegen_interaction_model_SOURCES = [ + "${chip_root}/src/app/codegen-interaction-model/Model.h", + "${chip_root}/src/app/codegen-interaction-model/Model.cpp", +] + +codegen_interaction_model_PUBLIC_DEPS = [ + "${chip_root}/src/app/common:attribute-type", + "${chip_root}/src/app/interaction-model", +] diff --git a/src/app/codegen-interaction-model/tests/BUILD.gn b/src/app/codegen-interaction-model/tests/BUILD.gn new file mode 100644 index 00000000000000..f543bc8d0cc24b --- /dev/null +++ b/src/app/codegen-interaction-model/tests/BUILD.gn @@ -0,0 +1,35 @@ +# Copyright (c) 2024 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. +import("//build_overrides/chip.gni") +import("${chip_root}/build/chip/chip_test_suite.gni") +import("${chip_root}/src/app/codegen-interaction-model/model.gni") + +source_set("mock_model") { + sources = codegen_interaction_model_SOURCES + + public_deps = codegen_interaction_model_PUBLIC_DEPS + + # this ties in the codegen model to an actual ember implementation + public_deps += [ "${chip_root}/src/app/util/mock:mock_ember" ] +} + +chip_test_suite("tests") { + output_name = "libCodegenInteractionModelTests" + + test_sources = [ "TestCodegenModelViaMocks.cpp" ] + + cflags = [ "-Wconversion" ] + + public_deps = [ ":mock_model" ] +} diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp new file mode 100644 index 00000000000000..fc34b566963df2 --- /dev/null +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -0,0 +1,290 @@ +/* + * + * Copyright (c) 2024 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. + */ +#include + +#include +#include +#include + +#include + +using namespace chip; +using namespace chip::Test; +using namespace chip::app; +using namespace chip::app::InteractionModel; +using namespace chip::app::Clusters::Globals::Attributes; + +namespace { + +constexpr EndpointId kEndpointIdThatIsMissing = kMockEndpointMin - 1; + +static_assert(kEndpointIdThatIsMissing != kInvalidEndpointId); +static_assert(kEndpointIdThatIsMissing != kMockEndpoint1); +static_assert(kEndpointIdThatIsMissing != kMockEndpoint2); +static_assert(kEndpointIdThatIsMissing != kMockEndpoint3); + +// clang-format off +const MockNodeConfig gTestNodeConfig({ + MockEndpointConfig(kMockEndpoint1, { + MockClusterConfig(MockClusterId(1), { + ClusterRevision::Id, FeatureMap::Id, + }, { + MockEventId(1), MockEventId(2), + }), + MockClusterConfig(MockClusterId(2), { + ClusterRevision::Id, FeatureMap::Id, MockAttributeId(1), + }), + }), + MockEndpointConfig(kMockEndpoint2, { + MockClusterConfig(MockClusterId(1), { + ClusterRevision::Id, FeatureMap::Id, + }), + MockClusterConfig(MockClusterId(2), { + ClusterRevision::Id, + FeatureMap::Id, + MockAttributeId(1), + MockAttributeConfig(MockAttributeId(2), ZCL_ARRAY_ATTRIBUTE_TYPE), + }), + MockClusterConfig(MockClusterId(3), { + ClusterRevision::Id, FeatureMap::Id, MockAttributeId(1), MockAttributeId(2), MockAttributeId(3), + }), + }), + MockEndpointConfig(kMockEndpoint3, { + MockClusterConfig(MockClusterId(1), { + ClusterRevision::Id, FeatureMap::Id, MockAttributeId(1), + }), + MockClusterConfig(MockClusterId(2), { + ClusterRevision::Id, FeatureMap::Id, MockAttributeId(1), MockAttributeId(2), MockAttributeId(3), MockAttributeId(4), + }), + MockClusterConfig(MockClusterId(3), { + ClusterRevision::Id, FeatureMap::Id, + }), + MockClusterConfig(MockClusterId(4), { + ClusterRevision::Id, FeatureMap::Id, + }), + }), +}); +// clang-format on + +struct UseMockNodeConfig +{ + UseMockNodeConfig(const MockNodeConfig & config) { SetMockNodeConfig(config); } + ~UseMockNodeConfig() { ResetMockNodeConfig(); } +}; + +} // namespace + +TEST(TestCodegenModelViaMocks, IterateOverEndpoints) +{ + UseMockNodeConfig config(gTestNodeConfig); + chip::app::CodegenDataModel::Model model; + + // This iteration relies on the hard-coding that occurs when mock_ember is used + ASSERT_EQ(model.FirstEndpoint(), kMockEndpoint1); + ASSERT_EQ(model.NextEndpoint(kMockEndpoint1), kMockEndpoint2); + ASSERT_EQ(model.NextEndpoint(kMockEndpoint2), kMockEndpoint3); + ASSERT_EQ(model.NextEndpoint(kMockEndpoint3), kInvalidEndpointId); + + /// Some out of order requests should work as well + ASSERT_EQ(model.NextEndpoint(kMockEndpoint2), kMockEndpoint3); + ASSERT_EQ(model.NextEndpoint(kMockEndpoint2), kMockEndpoint3); + ASSERT_EQ(model.NextEndpoint(kMockEndpoint1), kMockEndpoint2); + ASSERT_EQ(model.NextEndpoint(kMockEndpoint1), kMockEndpoint2); + ASSERT_EQ(model.NextEndpoint(kMockEndpoint2), kMockEndpoint3); + ASSERT_EQ(model.NextEndpoint(kMockEndpoint1), kMockEndpoint2); + ASSERT_EQ(model.NextEndpoint(kMockEndpoint3), kInvalidEndpointId); + ASSERT_EQ(model.NextEndpoint(kMockEndpoint3), kInvalidEndpointId); + ASSERT_EQ(model.FirstEndpoint(), kMockEndpoint1); + ASSERT_EQ(model.FirstEndpoint(), kMockEndpoint1); +} + +TEST(TestCodegenModelViaMocks, IterateOverClusters) +{ + UseMockNodeConfig config(gTestNodeConfig); + chip::app::CodegenDataModel::Model model; + + chip::Test::ResetVersion(); + + ASSERT_FALSE(model.FirstCluster(kEndpointIdThatIsMissing).path.HasValidIds()); + ASSERT_FALSE(model.FirstCluster(kInvalidEndpointId).path.HasValidIds()); + + // mock endpoint 1 has 2 mock clusters: 1 and 2 + ClusterEntry entry = model.FirstCluster(kMockEndpoint1); + ASSERT_TRUE(entry.path.HasValidIds()); + ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint1); + ASSERT_EQ(entry.path.mClusterId, MockClusterId(1)); + ASSERT_EQ(entry.info.dataVersion, 0u); + ASSERT_EQ(entry.info.flags.Raw(), 0u); + + chip::Test::BumpVersion(); + + entry = model.NextCluster(entry.path); + ASSERT_TRUE(entry.path.HasValidIds()); + ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint1); + ASSERT_EQ(entry.path.mClusterId, MockClusterId(2)); + ASSERT_EQ(entry.info.dataVersion, 1u); + ASSERT_EQ(entry.info.flags.Raw(), 0u); + + entry = model.NextCluster(entry.path); + ASSERT_FALSE(entry.path.HasValidIds()); + + // mock endpoint 3 has 4 mock clusters: 1 through 4 + entry = model.FirstCluster(kMockEndpoint3); + for (uint16_t clusterId = 1; clusterId <= 4; clusterId++) + { + ASSERT_TRUE(entry.path.HasValidIds()); + ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint3); + ASSERT_EQ(entry.path.mClusterId, MockClusterId(clusterId)); + entry = model.NextCluster(entry.path); + } + ASSERT_FALSE(entry.path.HasValidIds()); + + // repeat calls should work + for (int i = 0; i < 10; i++) + { + entry = model.FirstCluster(kMockEndpoint1); + ASSERT_TRUE(entry.path.HasValidIds()); + ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint1); + ASSERT_EQ(entry.path.mClusterId, MockClusterId(1)); + } + + for (int i = 0; i < 10; i++) + { + ClusterEntry nextEntry = model.NextCluster(entry.path); + ASSERT_TRUE(nextEntry.path.HasValidIds()); + ASSERT_EQ(nextEntry.path.mEndpointId, kMockEndpoint1); + ASSERT_EQ(nextEntry.path.mClusterId, MockClusterId(2)); + } +} + +TEST(TestCodegenModelViaMocks, GetCluterInfo) +{ + + UseMockNodeConfig config(gTestNodeConfig); + chip::app::CodegenDataModel::Model model; + + chip::Test::ResetVersion(); + + ASSERT_FALSE(model.GetClusterInfo(ConcreteClusterPath(kInvalidEndpointId, kInvalidClusterId)).has_value()); + ASSERT_FALSE(model.GetClusterInfo(ConcreteClusterPath(kInvalidEndpointId, MockClusterId(1))).has_value()); + ASSERT_FALSE(model.GetClusterInfo(ConcreteClusterPath(kMockEndpoint1, kInvalidClusterId)).has_value()); + ASSERT_FALSE(model.GetClusterInfo(ConcreteClusterPath(kMockEndpoint1, MockClusterId(10))).has_value()); + + // now get the value + std::optional info = model.GetClusterInfo(ConcreteClusterPath(kMockEndpoint1, MockClusterId(1))); + ASSERT_TRUE(info.has_value()); + EXPECT_EQ(info->dataVersion, 0u); + EXPECT_EQ(info->flags.Raw(), 0u); + + chip::Test::BumpVersion(); + info = model.GetClusterInfo(ConcreteClusterPath(kMockEndpoint1, MockClusterId(1))); + ASSERT_TRUE(info.has_value()); + EXPECT_EQ(info->dataVersion, 1u); + EXPECT_EQ(info->flags.Raw(), 0u); +} + +TEST(TestCodegenModelViaMocks, IterateOverAttributes) +{ + UseMockNodeConfig config(gTestNodeConfig); + chip::app::CodegenDataModel::Model model; + + // invalid paths should return in "no more data" + ASSERT_FALSE(model.FirstAttribute(ConcreteClusterPath(kEndpointIdThatIsMissing, MockClusterId(1))).path.HasValidIds()); + ASSERT_FALSE(model.FirstAttribute(ConcreteClusterPath(kInvalidEndpointId, MockClusterId(1))).path.HasValidIds()); + ASSERT_FALSE(model.FirstAttribute(ConcreteClusterPath(kMockEndpoint1, MockClusterId(10))).path.HasValidIds()); + ASSERT_FALSE(model.FirstAttribute(ConcreteClusterPath(kMockEndpoint1, kInvalidClusterId)).path.HasValidIds()); + + // should be able to iterate over valid paths + AttributeEntry entry = model.FirstAttribute(ConcreteClusterPath(kMockEndpoint2, MockClusterId(2))); + ASSERT_TRUE(entry.path.HasValidIds()); + ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint2); + ASSERT_EQ(entry.path.mClusterId, MockClusterId(2)); + ASSERT_EQ(entry.path.mAttributeId, ClusterRevision::Id); + ASSERT_FALSE(entry.info.flags.Has(AttributeQualityFlags::kListAttribute)); + + entry = model.NextAttribute(entry.path); + ASSERT_TRUE(entry.path.HasValidIds()); + ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint2); + ASSERT_EQ(entry.path.mClusterId, MockClusterId(2)); + ASSERT_EQ(entry.path.mAttributeId, FeatureMap::Id); + ASSERT_FALSE(entry.info.flags.Has(AttributeQualityFlags::kListAttribute)); + + entry = model.NextAttribute(entry.path); + ASSERT_TRUE(entry.path.HasValidIds()); + ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint2); + ASSERT_EQ(entry.path.mClusterId, MockClusterId(2)); + ASSERT_EQ(entry.path.mAttributeId, MockAttributeId(1)); + ASSERT_FALSE(entry.info.flags.Has(AttributeQualityFlags::kListAttribute)); + + entry = model.NextAttribute(entry.path); + ASSERT_TRUE(entry.path.HasValidIds()); + ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint2); + ASSERT_EQ(entry.path.mClusterId, MockClusterId(2)); + ASSERT_EQ(entry.path.mAttributeId, MockAttributeId(2)); + ASSERT_TRUE(entry.info.flags.Has(AttributeQualityFlags::kListAttribute)); + + entry = model.NextAttribute(entry.path); + ASSERT_FALSE(entry.path.HasValidIds()); + + // repeated calls should work + for (int i = 0; i < 10; i++) + { + entry = model.FirstAttribute(ConcreteClusterPath(kMockEndpoint2, MockClusterId(2))); + ASSERT_TRUE(entry.path.HasValidIds()); + ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint2); + ASSERT_EQ(entry.path.mClusterId, MockClusterId(2)); + ASSERT_EQ(entry.path.mAttributeId, ClusterRevision::Id); + ASSERT_FALSE(entry.info.flags.Has(AttributeQualityFlags::kListAttribute)); + } + + for (int i = 0; i < 10; i++) + { + entry = model.NextAttribute(ConcreteAttributePath(kMockEndpoint2, MockClusterId(2), MockAttributeId(1))); + ASSERT_TRUE(entry.path.HasValidIds()); + ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint2); + ASSERT_EQ(entry.path.mClusterId, MockClusterId(2)); + ASSERT_EQ(entry.path.mAttributeId, MockAttributeId(2)); + ASSERT_TRUE(entry.info.flags.Has(AttributeQualityFlags::kListAttribute)); + } +} + +TEST(TestCodegenModelViaMocks, GetAttributeInfo) +{ + UseMockNodeConfig config(gTestNodeConfig); + chip::app::CodegenDataModel::Model model; + + // various non-existent or invalid paths should return no info data + ASSERT_FALSE( + model.GetAttributeInfo(ConcreteAttributePath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId)).has_value()); + ASSERT_FALSE(model.GetAttributeInfo(ConcreteAttributePath(kInvalidEndpointId, kInvalidClusterId, FeatureMap::Id)).has_value()); + ASSERT_FALSE(model.GetAttributeInfo(ConcreteAttributePath(kInvalidEndpointId, MockClusterId(1), FeatureMap::Id)).has_value()); + ASSERT_FALSE(model.GetAttributeInfo(ConcreteAttributePath(kMockEndpoint1, kInvalidClusterId, FeatureMap::Id)).has_value()); + ASSERT_FALSE(model.GetAttributeInfo(ConcreteAttributePath(kMockEndpoint1, MockClusterId(10), FeatureMap::Id)).has_value()); + ASSERT_FALSE(model.GetAttributeInfo(ConcreteAttributePath(kMockEndpoint1, MockClusterId(10), kInvalidAttributeId)).has_value()); + ASSERT_FALSE(model.GetAttributeInfo(ConcreteAttributePath(kMockEndpoint1, MockClusterId(1), MockAttributeId(10))).has_value()); + + // valid info + std::optional info = + model.GetAttributeInfo(ConcreteAttributePath(kMockEndpoint1, MockClusterId(1), FeatureMap::Id)); + ASSERT_TRUE(info.has_value()); + ASSERT_FALSE(info->flags.Has(AttributeQualityFlags::kListAttribute)); + + info = model.GetAttributeInfo(ConcreteAttributePath(kMockEndpoint2, MockClusterId(2), MockAttributeId(2))); + ASSERT_TRUE(info.has_value()); + ASSERT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute)); +} diff --git a/src/app/interaction-model/IterationTypes.h b/src/app/interaction-model/IterationTypes.h index 32ad8bc42b73be..c948d4bcb428f8 100644 --- a/src/app/interaction-model/IterationTypes.h +++ b/src/app/interaction-model/IterationTypes.h @@ -35,7 +35,7 @@ enum class ClusterQualityFlags : uint32_t struct ClusterInfo { - DataVersion dataVersion; // current version of this cluster + DataVersion dataVersion = 0; // current version of this cluster BitFlags flags; }; @@ -43,6 +43,13 @@ struct ClusterEntry { ConcreteClusterPath path; ClusterInfo info; + + static ClusterEntry Invalid() + { + ClusterEntry result; + result.path = ConcreteClusterPath(kInvalidEndpointId, kInvalidClusterId); + return result; + } }; enum class AttributeQualityFlags : uint32_t @@ -60,6 +67,13 @@ struct AttributeEntry { ConcreteAttributePath path; AttributeInfo info; + + static AttributeEntry Invalid() + { + AttributeEntry result; + result.path = ConcreteAttributePath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId); + return result; + } }; /// Provides metadata information for a data model diff --git a/src/app/interaction-model/tests/BUILD.gn b/src/app/interaction-model/tests/BUILD.gn index c7d36b4f1dcdca..4767d61dd12ed1 100644 --- a/src/app/interaction-model/tests/BUILD.gn +++ b/src/app/interaction-model/tests/BUILD.gn @@ -15,6 +15,8 @@ import("//build_overrides/chip.gni") import("${chip_root}/build/chip/chip_test_suite.gni") chip_test_suite("tests") { + output_name = "libIMInterfaceTests" + test_sources = [ "TestEventEmitting.cpp" ] cflags = [ "-Wconversion" ] diff --git a/src/app/util/mock/attribute-storage.cpp b/src/app/util/mock/attribute-storage.cpp index 2293a48a8403e4..131385be8373bb 100644 --- a/src/app/util/mock/attribute-storage.cpp +++ b/src/app/util/mock/attribute-storage.cpp @@ -414,6 +414,7 @@ void SetMockNodeConfig(const MockNodeConfig & config) mockConfig = &config; } +/// Resets the mock attribute storage to the default configuration. void ResetMockNodeConfig() { mockConfig = nullptr; From 53891f3f7bd03abd10dea2b2ac57520aa8f86458 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 7 May 2024 15:28:05 -0400 Subject: [PATCH 02/79] make linter happy --- src/app/codegen-interaction-model/BUILD.gn | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/app/codegen-interaction-model/BUILD.gn b/src/app/codegen-interaction-model/BUILD.gn index bbb4a30e96d621..7bacec118c55ad 100644 --- a/src/app/codegen-interaction-model/BUILD.gn +++ b/src/app/codegen-interaction-model/BUILD.gn @@ -12,11 +12,17 @@ # See the License for the specific language governing permissions and # limitations under the License. import("//build_overrides/chip.gni") + # This source set is TIGHLY coupled with code-generated data models # as generally implemented by `src/app/util` # # Corresponding functions defined in attribute-storace.cpp/attribute-table.cpp must # be available at link time for this model to use +# # Use `model.gni` to get access to: -# model.cpp -# model.h +# Model.cpp +# Model.h +# +# The abolve list of files exists to satisfy the "dependency linter" +# since those files should technically be "visible to gn" even though we +# are supposed to go through model.gni constants From 0947dd68091194f62afb2eaba0735714aa02a1db Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 7 May 2024 15:29:39 -0400 Subject: [PATCH 03/79] Restyle --- src/app/codegen-interaction-model/BUILD.gn | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/codegen-interaction-model/BUILD.gn b/src/app/codegen-interaction-model/BUILD.gn index 7bacec118c55ad..8b859afc6b2e48 100644 --- a/src/app/codegen-interaction-model/BUILD.gn +++ b/src/app/codegen-interaction-model/BUILD.gn @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. import("//build_overrides/chip.gni") - # This source set is TIGHLY coupled with code-generated data models # as generally implemented by `src/app/util` # From 5a2a10809c959778b9163297059631f8d15cb0dd Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 7 May 2024 15:54:47 -0400 Subject: [PATCH 04/79] Fix typo --- .../tests/TestCodegenModelViaMocks.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index fc34b566963df2..9baf73dd5a6b11 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -172,7 +172,7 @@ TEST(TestCodegenModelViaMocks, IterateOverClusters) } } -TEST(TestCodegenModelViaMocks, GetCluterInfo) +TEST(TestCodegenModelViaMocks, GetClusterInfo) { UseMockNodeConfig config(gTestNodeConfig); From 26bb33b36e7cfd47fc1403256fd91e8e311756b3 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 8 May 2024 08:54:47 -0400 Subject: [PATCH 05/79] Add nolint: assert will return before we use the underlying value --- .../tests/TestCodegenModelViaMocks.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index 9baf73dd5a6b11..6c1982678082f3 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -188,14 +188,14 @@ TEST(TestCodegenModelViaMocks, GetClusterInfo) // now get the value std::optional info = model.GetClusterInfo(ConcreteClusterPath(kMockEndpoint1, MockClusterId(1))); ASSERT_TRUE(info.has_value()); - EXPECT_EQ(info->dataVersion, 0u); - EXPECT_EQ(info->flags.Raw(), 0u); + EXPECT_EQ(info->dataVersion, 0u); // NOLINT(bugprone-unchecked-optional-access) + EXPECT_EQ(info->flags.Raw(), 0u); // NOLINT(bugprone-unchecked-optional-access) chip::Test::BumpVersion(); info = model.GetClusterInfo(ConcreteClusterPath(kMockEndpoint1, MockClusterId(1))); ASSERT_TRUE(info.has_value()); - EXPECT_EQ(info->dataVersion, 1u); - EXPECT_EQ(info->flags.Raw(), 0u); + EXPECT_EQ(info->dataVersion, 1u); // NOLINT(bugprone-unchecked-optional-access) + EXPECT_EQ(info->flags.Raw(), 0u); // NOLINT(bugprone-unchecked-optional-access) } TEST(TestCodegenModelViaMocks, IterateOverAttributes) From 4b55a2082d3249f12eee46eb716244bb2210e18b Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 8 May 2024 08:55:43 -0400 Subject: [PATCH 06/79] 2 more fixes regarding unchecked access --- .../tests/TestCodegenModelViaMocks.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index 6c1982678082f3..1e4f734b823256 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -282,9 +282,9 @@ TEST(TestCodegenModelViaMocks, GetAttributeInfo) std::optional info = model.GetAttributeInfo(ConcreteAttributePath(kMockEndpoint1, MockClusterId(1), FeatureMap::Id)); ASSERT_TRUE(info.has_value()); - ASSERT_FALSE(info->flags.Has(AttributeQualityFlags::kListAttribute)); + EXPECT_FALSE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access) info = model.GetAttributeInfo(ConcreteAttributePath(kMockEndpoint2, MockClusterId(2), MockAttributeId(2))); ASSERT_TRUE(info.has_value()); - ASSERT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute)); + EXPECT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access) } From d8b8433462a7e3f69d65b2f91e486692ea7768f6 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 8 May 2024 08:57:52 -0400 Subject: [PATCH 07/79] Switch some asserts to expects, for better test logic --- .../tests/TestCodegenModelViaMocks.cpp | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index 1e4f734b823256..a8b69e797a0d6f 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -95,8 +95,8 @@ TEST(TestCodegenModelViaMocks, IterateOverEndpoints) chip::app::CodegenDataModel::Model model; // This iteration relies on the hard-coding that occurs when mock_ember is used - ASSERT_EQ(model.FirstEndpoint(), kMockEndpoint1); - ASSERT_EQ(model.NextEndpoint(kMockEndpoint1), kMockEndpoint2); + EXPECT_EQ(model.FirstEndpoint(), kMockEndpoint1); + EXPECT_EQ(model.NextEndpoint(kMockEndpoint1), kMockEndpoint2); ASSERT_EQ(model.NextEndpoint(kMockEndpoint2), kMockEndpoint3); ASSERT_EQ(model.NextEndpoint(kMockEndpoint3), kInvalidEndpointId); @@ -120,55 +120,55 @@ TEST(TestCodegenModelViaMocks, IterateOverClusters) chip::Test::ResetVersion(); - ASSERT_FALSE(model.FirstCluster(kEndpointIdThatIsMissing).path.HasValidIds()); - ASSERT_FALSE(model.FirstCluster(kInvalidEndpointId).path.HasValidIds()); + EXPECT_FALSE(model.FirstCluster(kEndpointIdThatIsMissing).path.HasValidIds()); + EXPECT_FALSE(model.FirstCluster(kInvalidEndpointId).path.HasValidIds()); // mock endpoint 1 has 2 mock clusters: 1 and 2 ClusterEntry entry = model.FirstCluster(kMockEndpoint1); ASSERT_TRUE(entry.path.HasValidIds()); - ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint1); - ASSERT_EQ(entry.path.mClusterId, MockClusterId(1)); - ASSERT_EQ(entry.info.dataVersion, 0u); - ASSERT_EQ(entry.info.flags.Raw(), 0u); + EXPECT_EQ(entry.path.mEndpointId, kMockEndpoint1); + EXPECT_EQ(entry.path.mClusterId, MockClusterId(1)); + EXPECT_EQ(entry.info.dataVersion, 0u); + EXPECT_EQ(entry.info.flags.Raw(), 0u); chip::Test::BumpVersion(); entry = model.NextCluster(entry.path); ASSERT_TRUE(entry.path.HasValidIds()); - ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint1); - ASSERT_EQ(entry.path.mClusterId, MockClusterId(2)); - ASSERT_EQ(entry.info.dataVersion, 1u); - ASSERT_EQ(entry.info.flags.Raw(), 0u); + EXPECT_EQ(entry.path.mEndpointId, kMockEndpoint1); + EXPECT_EQ(entry.path.mClusterId, MockClusterId(2)); + EXPECT_EQ(entry.info.dataVersion, 1u); + EXPECT_EQ(entry.info.flags.Raw(), 0u); entry = model.NextCluster(entry.path); - ASSERT_FALSE(entry.path.HasValidIds()); + EXPECT_FALSE(entry.path.HasValidIds()); // mock endpoint 3 has 4 mock clusters: 1 through 4 entry = model.FirstCluster(kMockEndpoint3); for (uint16_t clusterId = 1; clusterId <= 4; clusterId++) { ASSERT_TRUE(entry.path.HasValidIds()); - ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint3); - ASSERT_EQ(entry.path.mClusterId, MockClusterId(clusterId)); + EXPECT_EQ(entry.path.mEndpointId, kMockEndpoint3); + EXPECT_EQ(entry.path.mClusterId, MockClusterId(clusterId)); entry = model.NextCluster(entry.path); } - ASSERT_FALSE(entry.path.HasValidIds()); + EXPECT_FALSE(entry.path.HasValidIds()); // repeat calls should work for (int i = 0; i < 10; i++) { entry = model.FirstCluster(kMockEndpoint1); ASSERT_TRUE(entry.path.HasValidIds()); - ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint1); - ASSERT_EQ(entry.path.mClusterId, MockClusterId(1)); + EXPECT_EQ(entry.path.mEndpointId, kMockEndpoint1); + EXPECT_EQ(entry.path.mClusterId, MockClusterId(1)); } for (int i = 0; i < 10; i++) { ClusterEntry nextEntry = model.NextCluster(entry.path); ASSERT_TRUE(nextEntry.path.HasValidIds()); - ASSERT_EQ(nextEntry.path.mEndpointId, kMockEndpoint1); - ASSERT_EQ(nextEntry.path.mClusterId, MockClusterId(2)); + EXPECT_EQ(nextEntry.path.mEndpointId, kMockEndpoint1); + EXPECT_EQ(nextEntry.path.mClusterId, MockClusterId(2)); } } From 27e106a520eb7e73aa6590e2a258dbda61245c79 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 14 May 2024 09:42:49 -0400 Subject: [PATCH 08/79] Model renames --- src/app/codegen-interaction-model/BUILD.gn | 4 +- src/app/codegen-interaction-model/Model.cpp | 267 ------------------ src/app/codegen-interaction-model/Model.h | 52 ---- src/app/codegen-interaction-model/model.gni | 4 +- .../tests/TestCodegenModelViaMocks.cpp | 12 +- src/app/interaction-model/IterationTypes.h | 2 +- 6 files changed, 11 insertions(+), 330 deletions(-) delete mode 100644 src/app/codegen-interaction-model/Model.cpp delete mode 100644 src/app/codegen-interaction-model/Model.h diff --git a/src/app/codegen-interaction-model/BUILD.gn b/src/app/codegen-interaction-model/BUILD.gn index 8b859afc6b2e48..65a672076e4a8a 100644 --- a/src/app/codegen-interaction-model/BUILD.gn +++ b/src/app/codegen-interaction-model/BUILD.gn @@ -19,8 +19,8 @@ import("//build_overrides/chip.gni") # be available at link time for this model to use # # Use `model.gni` to get access to: -# Model.cpp -# Model.h +# CodegenDataModel.cpp +# CodegenDataModel.h # # The abolve list of files exists to satisfy the "dependency linter" # since those files should technically be "visible to gn" even though we diff --git a/src/app/codegen-interaction-model/Model.cpp b/src/app/codegen-interaction-model/Model.cpp deleted file mode 100644 index dc3fbd9cbca7d0..00000000000000 --- a/src/app/codegen-interaction-model/Model.cpp +++ /dev/null @@ -1,267 +0,0 @@ -/* - * Copyright (c) 2024 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. - */ -#include - -#include -#include -#include - -namespace chip { -namespace app { -namespace CodegenDataModel { - -namespace { - -/// Checks if the specified ember cluster mask corresponds to a valid -/// server cluster. -bool IsServerMask(EmberAfClusterMask mask) -{ - return (mask == 0) || ((mask & CLUSTER_MASK_SERVER) != 0); -} - -/// Load the cluster information into the specified destination -void LoadClusterInfo(const ConcreteClusterPath & path, const EmberAfCluster & cluster, InteractionModel::ClusterInfo * info) -{ - chip::DataVersion * versionPtr = emberAfDataVersionStorage(path); - if (versionPtr != nullptr) - { - info->dataVersion = *versionPtr; - } - else - { - ChipLogError(AppServer, "Failed to get data version for %d/" ChipLogFormatMEI, static_cast(path.mEndpointId), - ChipLogValueMEI(cluster.clusterId)); - info->dataVersion = 0; - } - - // TODO: set entry flags: - // info->flags.Set(ClusterQualityFlags::kDiagnosticsData) -} - -/// Converts a EmberAfCluster into a ClusterEntry -InteractionModel::ClusterEntry ClusterEntryFrom(EndpointId endpointId, const EmberAfCluster & cluster) -{ - InteractionModel::ClusterEntry entry; - - entry.path = ConcreteClusterPath(endpointId, cluster.clusterId); - LoadClusterInfo(entry.path, cluster, &entry.info); - - return entry; -} - -/// Finds the first server cluster entry for the given endpoint data starting at [start_index] -/// -/// Returns an invalid entry if no more server clusters are found -InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, const EmberAfEndpointType * endpoint, - uint16_t start_index) -{ - for (unsigned i = start_index; i < endpoint->clusterCount; i++) - { - const EmberAfCluster & cluster = endpoint->cluster[i]; - if (!IsServerMask(cluster.mask & CLUSTER_MASK_SERVER)) - { - continue; - } - - return ClusterEntryFrom(endpointId, cluster); - } - - return InteractionModel::ClusterEntry::Invalid(); -} - -/// Load the cluster information into the specified destination -void LoadAttributeInfo(const ConcreteAttributePath & path, const EmberAfAttributeMetadata & attribute, - InteractionModel::AttributeInfo * info) -{ - if (attribute.attributeType == ZCL_ARRAY_ATTRIBUTE_TYPE) - { - info->flags.Set(InteractionModel::AttributeQualityFlags::kListAttribute); - } - - // TODO: Set additional flags: - // info->flags.Set(InteractionModel::AttributeQualityFlags::kChangesOmitted) -} - -InteractionModel::AttributeEntry AttributeEntryFrom(const ConcreteClusterPath & clusterPath, - const EmberAfAttributeMetadata & attribute) -{ - InteractionModel::AttributeEntry entry; - - entry.path = ConcreteAttributePath(clusterPath.mEndpointId, clusterPath.mClusterId, attribute.attributeId); - LoadAttributeInfo(entry.path, attribute, &entry.info); - - return entry; -} - -} // namespace - -CHIP_ERROR Model::ReadAttribute(const InteractionModel::ReadAttributeRequest & request, InteractionModel::ReadState & state, - AttributeValueEncoder & encoder) -{ - // TODO: this needs an implementation - return CHIP_ERROR_NOT_IMPLEMENTED; -} - -CHIP_ERROR Model::WriteAttribute(const InteractionModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) -{ - // TODO: this needs an implementation - return CHIP_ERROR_NOT_IMPLEMENTED; -} - -CHIP_ERROR Model::Invoke(const InteractionModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments, - InteractionModel::InvokeReply & reply) -{ - // TODO: this needs an implementation - return CHIP_ERROR_NOT_IMPLEMENTED; -} - -EndpointId Model::FirstEndpoint() -{ - // find the first enabled index - const uint16_t lastEndpointIndex = emberAfEndpointCount(); - for (uint16_t endpointIndex = 0; endpointIndex < lastEndpointIndex; endpointIndex++) - { - if (emberAfEndpointIndexIsEnabled(endpointIndex)) - { - return emberAfEndpointFromIndex(endpointIndex); - } - } - - // No enabled endpoint found. Give up - return kInvalidEndpointId; -} - -EndpointId Model::NextEndpoint(EndpointId before) -{ - // find the first enabled index - bool beforeFound = false; - - const uint16_t lastEndpointIndex = emberAfEndpointCount(); - for (uint16_t endpointIndex = 0; endpointIndex < lastEndpointIndex; endpointIndex++) - { - if (!beforeFound) - { - beforeFound = (before == emberAfEndpointFromIndex(endpointIndex)); - continue; - } - - if (emberAfEndpointIndexIsEnabled(endpointIndex)) - { - return emberAfEndpointFromIndex(endpointIndex); - } - } - - // No enabled enpoint after "before" was found, give up - return kInvalidEndpointId; -} - -InteractionModel::ClusterEntry Model::FirstCluster(EndpointId endpointId) -{ - const EmberAfEndpointType * endpoint = emberAfFindEndpointType(endpointId); - VerifyOrReturnValue(endpoint != nullptr, InteractionModel::ClusterEntry::Invalid()); - VerifyOrReturnValue(endpoint->clusterCount > 0, InteractionModel::ClusterEntry::Invalid()); - VerifyOrReturnValue(endpoint->cluster != nullptr, InteractionModel::ClusterEntry::Invalid()); - - return FirstServerClusterEntry(endpointId, endpoint, 0); -} - -InteractionModel::ClusterEntry Model::NextCluster(const ConcreteClusterPath & before) -{ - const EmberAfEndpointType * endpoint = emberAfFindEndpointType(before.mEndpointId); - VerifyOrReturnValue(endpoint != nullptr, InteractionModel::ClusterEntry::Invalid()); - VerifyOrReturnValue(endpoint->clusterCount > 0, InteractionModel::ClusterEntry::Invalid()); - VerifyOrReturnValue(endpoint->cluster != nullptr, InteractionModel::ClusterEntry::Invalid()); - - for (uint16_t i = 0; i < endpoint->clusterCount; i++) - { - const EmberAfCluster & cluster = endpoint->cluster[i]; - if (IsServerMask(cluster.mask) && (cluster.clusterId == before.mClusterId)) - { - return FirstServerClusterEntry(before.mEndpointId, endpoint, i + 1); - } - } - - return InteractionModel::ClusterEntry::Invalid(); -} - -std::optional Model::GetClusterInfo(const ConcreteClusterPath & path) -{ - const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); - VerifyOrReturnValue(cluster != nullptr, std::nullopt); - - InteractionModel::ClusterInfo info; - LoadClusterInfo(path, *cluster, &info); - - return std::make_optional(info); -} - -InteractionModel::AttributeEntry Model::FirstAttribute(const ConcreteClusterPath & path) -{ - const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); - VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::Invalid()); - VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); - VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); - - return AttributeEntryFrom(path, cluster->attributes[0]); -} - -InteractionModel::AttributeEntry Model::NextAttribute(const ConcreteAttributePath & before) -{ - const EmberAfCluster * cluster = emberAfFindServerCluster(before.mEndpointId, before.mClusterId); - VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::Invalid()); - VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); - VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); - - // find the given attribute in the list and then return the next one - bool foundPosition = false; - const unsigned attributeCount = cluster->attributeCount; - for (unsigned i = 0; i < attributeCount; i++) - { - if (foundPosition) - { - return AttributeEntryFrom(before, cluster->attributes[i]); - } - - foundPosition = (cluster->attributes[i].attributeId == before.mAttributeId); - } - - return InteractionModel::AttributeEntry::Invalid(); -} - -std::optional Model::GetAttributeInfo(const ConcreteAttributePath & path) -{ - const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); - VerifyOrReturnValue(cluster != nullptr, std::nullopt); - VerifyOrReturnValue(cluster->attributeCount > 0, std::nullopt); - VerifyOrReturnValue(cluster->attributes != nullptr, std::nullopt); - const unsigned attributeCount = cluster->attributeCount; - for (unsigned i = 0; i < attributeCount; i++) - { - if (cluster->attributes[i].attributeId == path.mAttributeId) - { - InteractionModel::AttributeInfo info; - LoadAttributeInfo(path, cluster->attributes[i], &info); - return std::make_optional(info); - } - } - - return std::nullopt; -} - -} // namespace CodegenDataModel -} // namespace app -} // namespace chip diff --git a/src/app/codegen-interaction-model/Model.h b/src/app/codegen-interaction-model/Model.h deleted file mode 100644 index 3f48970852a7bc..00000000000000 --- a/src/app/codegen-interaction-model/Model.h +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Copyright (c) 2024 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 { -namespace CodegenDataModel { - -class Model : public chip::app::InteractionModel::Model -{ -public: - /// Generic model implementations - CHIP_ERROR Shutdown() override { return CHIP_NO_ERROR; } - - CHIP_ERROR ReadAttribute(const InteractionModel::ReadAttributeRequest & request, InteractionModel::ReadState & state, - AttributeValueEncoder & encoder) override; - CHIP_ERROR WriteAttribute(const InteractionModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) override; - CHIP_ERROR Invoke(const InteractionModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments, - InteractionModel::InvokeReply & reply) override; - - /// attribute tree iteration - EndpointId FirstEndpoint() override; - EndpointId NextEndpoint(EndpointId before) override; - - InteractionModel::ClusterEntry FirstCluster(EndpointId endpoint) override; - InteractionModel::ClusterEntry NextCluster(const ConcreteClusterPath & before) override; - std::optional GetClusterInfo(const ConcreteClusterPath & path) override; - - InteractionModel::AttributeEntry FirstAttribute(const ConcreteClusterPath & cluster) override; - InteractionModel::AttributeEntry NextAttribute(const ConcreteAttributePath & before) override; - std::optional GetAttributeInfo(const ConcreteAttributePath & path) override; -}; - -} // namespace CodegenDataModel -} // namespace app -} // namespace chip diff --git a/src/app/codegen-interaction-model/model.gni b/src/app/codegen-interaction-model/model.gni index ddf7b1b172cc5a..f44beefd94079b 100644 --- a/src/app/codegen-interaction-model/model.gni +++ b/src/app/codegen-interaction-model/model.gni @@ -25,8 +25,8 @@ import("//build_overrides/chip.gni") # be cleanly built as a stand-alone and instead have to be imported as part of # a different data model or compilation unit. codegen_interaction_model_SOURCES = [ - "${chip_root}/src/app/codegen-interaction-model/Model.h", - "${chip_root}/src/app/codegen-interaction-model/Model.cpp", + "${chip_root}/src/app/codegen-interaction-model/CodegenDataModel.h", + "${chip_root}/src/app/codegen-interaction-model/CodegenDataModel.cpp", ] codegen_interaction_model_PUBLIC_DEPS = [ diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index a8b69e797a0d6f..9904939b5b0525 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -15,7 +15,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include +#include #include #include @@ -92,7 +92,7 @@ struct UseMockNodeConfig TEST(TestCodegenModelViaMocks, IterateOverEndpoints) { UseMockNodeConfig config(gTestNodeConfig); - chip::app::CodegenDataModel::Model model; + chip::app::CodegenDataModel model; // This iteration relies on the hard-coding that occurs when mock_ember is used EXPECT_EQ(model.FirstEndpoint(), kMockEndpoint1); @@ -116,7 +116,7 @@ TEST(TestCodegenModelViaMocks, IterateOverEndpoints) TEST(TestCodegenModelViaMocks, IterateOverClusters) { UseMockNodeConfig config(gTestNodeConfig); - chip::app::CodegenDataModel::Model model; + chip::app::CodegenDataModel model; chip::Test::ResetVersion(); @@ -176,7 +176,7 @@ TEST(TestCodegenModelViaMocks, GetClusterInfo) { UseMockNodeConfig config(gTestNodeConfig); - chip::app::CodegenDataModel::Model model; + chip::app::CodegenDataModel model; chip::Test::ResetVersion(); @@ -201,7 +201,7 @@ TEST(TestCodegenModelViaMocks, GetClusterInfo) TEST(TestCodegenModelViaMocks, IterateOverAttributes) { UseMockNodeConfig config(gTestNodeConfig); - chip::app::CodegenDataModel::Model model; + chip::app::CodegenDataModel model; // invalid paths should return in "no more data" ASSERT_FALSE(model.FirstAttribute(ConcreteClusterPath(kEndpointIdThatIsMissing, MockClusterId(1))).path.HasValidIds()); @@ -266,7 +266,7 @@ TEST(TestCodegenModelViaMocks, IterateOverAttributes) TEST(TestCodegenModelViaMocks, GetAttributeInfo) { UseMockNodeConfig config(gTestNodeConfig); - chip::app::CodegenDataModel::Model model; + chip::app::CodegenDataModel model; // various non-existent or invalid paths should return no info data ASSERT_FALSE( diff --git a/src/app/interaction-model/IterationTypes.h b/src/app/interaction-model/IterationTypes.h index c948d4bcb428f8..68862ed40f5bc6 100644 --- a/src/app/interaction-model/IterationTypes.h +++ b/src/app/interaction-model/IterationTypes.h @@ -35,7 +35,7 @@ enum class ClusterQualityFlags : uint32_t struct ClusterInfo { - DataVersion dataVersion = 0; // current version of this cluster + DataVersion dataVersion = 0; // current cluster data version BitFlags flags; }; From 5b73a1de1c16dd10e448c99e150e8631370bf1ad Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 14 May 2024 09:46:13 -0400 Subject: [PATCH 09/79] Add renamed files --- .../CodegenDataModel.cpp | 264 ++++++++++++++++++ .../CodegenDataModel.h | 50 ++++ 2 files changed, 314 insertions(+) create mode 100644 src/app/codegen-interaction-model/CodegenDataModel.cpp create mode 100644 src/app/codegen-interaction-model/CodegenDataModel.h diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp new file mode 100644 index 00000000000000..ab24dd265ac682 --- /dev/null +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -0,0 +1,264 @@ +/* + * Copyright (c) 2024 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. + */ +#include + +#include +#include +#include + +namespace chip { +namespace app { +namespace { + +/// Checks if the specified ember cluster mask corresponds to a valid +/// server cluster. +bool IsServerMask(EmberAfClusterMask mask) +{ + return (mask == 0) || ((mask & CLUSTER_MASK_SERVER) != 0); +} + +/// Load the cluster information into the specified destination +void LoadClusterInfo(const ConcreteClusterPath & path, const EmberAfCluster & cluster, InteractionModel::ClusterInfo * info) +{ + chip::DataVersion * versionPtr = emberAfDataVersionStorage(path); + if (versionPtr != nullptr) + { + info->dataVersion = *versionPtr; + } + else + { + ChipLogError(AppServer, "Failed to get data version for %d/" ChipLogFormatMEI, static_cast(path.mEndpointId), + ChipLogValueMEI(cluster.clusterId)); + info->dataVersion = 0; + } + + // TODO: set entry flags: + // info->flags.Set(ClusterQualityFlags::kDiagnosticsData) +} + +/// Converts a EmberAfCluster into a ClusterEntry +InteractionModel::ClusterEntry ClusterEntryFrom(EndpointId endpointId, const EmberAfCluster & cluster) +{ + InteractionModel::ClusterEntry entry; + + entry.path = ConcreteClusterPath(endpointId, cluster.clusterId); + LoadClusterInfo(entry.path, cluster, &entry.info); + + return entry; +} + +/// Finds the first server cluster entry for the given endpoint data starting at [start_index] +/// +/// Returns an invalid entry if no more server clusters are found +InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, const EmberAfEndpointType * endpoint, + uint16_t start_index) +{ + for (unsigned cluster_idx = start_index; cluster_idx < endpoint->clusterCount; cluster_idx++) + { + const EmberAfCluster & cluster = endpoint->cluster[cluster_idx]; + if (!IsServerMask(cluster.mask & CLUSTER_MASK_SERVER)) + { + continue; + } + + return ClusterEntryFrom(endpointId, cluster); + } + + return InteractionModel::ClusterEntry::Invalid(); +} + +/// Load the cluster information into the specified destination +void LoadAttributeInfo(const ConcreteAttributePath & path, const EmberAfAttributeMetadata & attribute, + InteractionModel::AttributeInfo * info) +{ + if (attribute.attributeType == ZCL_ARRAY_ATTRIBUTE_TYPE) + { + info->flags.Set(InteractionModel::AttributeQualityFlags::kListAttribute); + } + + // TODO: Set additional flags: + // info->flags.Set(InteractionModel::AttributeQualityFlags::kChangesOmitted) +} + +InteractionModel::AttributeEntry AttributeEntryFrom(const ConcreteClusterPath & clusterPath, + const EmberAfAttributeMetadata & attribute) +{ + InteractionModel::AttributeEntry entry; + + entry.path = ConcreteAttributePath(clusterPath.mEndpointId, clusterPath.mClusterId, attribute.attributeId); + LoadAttributeInfo(entry.path, attribute, &entry.info); + + return entry; +} + +} // namespace + +CHIP_ERROR CodegenDataModel::ReadAttribute(const InteractionModel::ReadAttributeRequest & request, InteractionModel::ReadState & state, + AttributeValueEncoder & encoder) +{ + // TODO: this needs an implementation + return CHIP_ERROR_NOT_IMPLEMENTED; +} + +CHIP_ERROR CodegenDataModel::WriteAttribute(const InteractionModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) +{ + // TODO: this needs an implementation + return CHIP_ERROR_NOT_IMPLEMENTED; +} + +CHIP_ERROR CodegenDataModel::Invoke(const InteractionModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments, + InteractionModel::InvokeReply & reply) +{ + // TODO: this needs an implementation + return CHIP_ERROR_NOT_IMPLEMENTED; +} + +EndpointId CodegenDataModel::FirstEndpoint() +{ + // find the first enabled index + const uint16_t lastEndpointIndex = emberAfEndpointCount(); + for (uint16_t endpoint_idx = 0; endpoint_idx < lastEndpointIndex; endpoint_idx++) + { + if (emberAfEndpointIndexIsEnabled(endpoint_idx)) + { + return emberAfEndpointFromIndex(endpoint_idx); + } + } + + // No enabled endpoint found. Give up + return kInvalidEndpointId; +} + +EndpointId CodegenDataModel::NextEndpoint(EndpointId before) +{ + // find the first enabled index + bool beforeFound = false; + + const uint16_t lastEndpointIndex = emberAfEndpointCount(); + for (uint16_t endpoint_idx = 0; endpoint_idx < lastEndpointIndex; endpoint_idx++) + { + if (!beforeFound) + { + beforeFound = (before == emberAfEndpointFromIndex(endpoint_idx)); + continue; + } + + if (emberAfEndpointIndexIsEnabled(endpoint_idx)) + { + return emberAfEndpointFromIndex(endpoint_idx); + } + } + + // No enabled enpoint after "before" was found, give up + return kInvalidEndpointId; +} + +InteractionModel::ClusterEntry CodegenDataModel::FirstCluster(EndpointId endpointId) +{ + const EmberAfEndpointType * endpoint = emberAfFindEndpointType(endpointId); + VerifyOrReturnValue(endpoint != nullptr, InteractionModel::ClusterEntry::Invalid()); + VerifyOrReturnValue(endpoint->clusterCount > 0, InteractionModel::ClusterEntry::Invalid()); + VerifyOrReturnValue(endpoint->cluster != nullptr, InteractionModel::ClusterEntry::Invalid()); + + return FirstServerClusterEntry(endpointId, endpoint, 0); +} + +InteractionModel::ClusterEntry CodegenDataModel::NextCluster(const ConcreteClusterPath & before) +{ + const EmberAfEndpointType * endpoint = emberAfFindEndpointType(before.mEndpointId); + VerifyOrReturnValue(endpoint != nullptr, InteractionModel::ClusterEntry::Invalid()); + VerifyOrReturnValue(endpoint->clusterCount > 0, InteractionModel::ClusterEntry::Invalid()); + VerifyOrReturnValue(endpoint->cluster != nullptr, InteractionModel::ClusterEntry::Invalid()); + + for (uint16_t cluster_idx = 0; cluster_idx < endpoint->clusterCount; cluster_idx++) + { + const EmberAfCluster & cluster = endpoint->cluster[cluster_idx]; + if (IsServerMask(cluster.mask) && (cluster.clusterId == before.mClusterId)) + { + return FirstServerClusterEntry(before.mEndpointId, endpoint, cluster_idx + 1); + } + } + + return InteractionModel::ClusterEntry::Invalid(); +} + +std::optional CodegenDataModel::GetClusterInfo(const ConcreteClusterPath & path) +{ + const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); + VerifyOrReturnValue(cluster != nullptr, std::nullopt); + + InteractionModel::ClusterInfo info; + LoadClusterInfo(path, *cluster, &info); + + return std::make_optional(info); +} + +InteractionModel::AttributeEntry CodegenDataModel::FirstAttribute(const ConcreteClusterPath & path) +{ + const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); + VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::Invalid()); + VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); + VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); + + return AttributeEntryFrom(path, cluster->attributes[0]); +} + +InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteAttributePath & before) +{ + const EmberAfCluster * cluster = emberAfFindServerCluster(before.mEndpointId, before.mClusterId); + VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::Invalid()); + VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); + VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); + + // find the given attribute in the list and then return the next one + bool foundPosition = false; + const unsigned attributeCount = cluster->attributeCount; + for (unsigned attribute_idx = 0; attribute_idx < attributeCount; attribute_idx++) + { + if (foundPosition) + { + return AttributeEntryFrom(before, cluster->attributes[attribute_idx]); + } + + foundPosition = (cluster->attributes[i].attributeId == before.mAttributeId); + } + + return InteractionModel::AttributeEntry::Invalid(); +} + +std::optional CodegenDataModel::GetAttributeInfo(const ConcreteAttributePath & path) +{ + const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); + VerifyOrReturnValue(cluster != nullptr, std::nullopt); + VerifyOrReturnValue(cluster->attributeCount > 0, std::nullopt); + VerifyOrReturnValue(cluster->attributes != nullptr, std::nullopt); + const unsigned attributeCount = cluster->attributeCount; + for (unsigned attribute_idx = 0; attribute_idx < attributeCount; attribute_idx++) + { + if (cluster->attributes[attribute_idx].attributeId == path.mAttributeId) + { + InteractionModel::AttributeInfo info; + LoadAttributeInfo(path, cluster->attributes[attribute_idx], &info); + return std::make_optional(info); + } + } + + return std::nullopt; +} + +} // namespace app +} // namespace chip diff --git a/src/app/codegen-interaction-model/CodegenDataModel.h b/src/app/codegen-interaction-model/CodegenDataModel.h new file mode 100644 index 00000000000000..490404f2e68a4a --- /dev/null +++ b/src/app/codegen-interaction-model/CodegenDataModel.h @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2024 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 { + +class CodegenDataModel : public chip::app::InteractionModel::Model +{ +public: + /// Generic model implementations + CHIP_ERROR Shutdown() override { return CHIP_NO_ERROR; } + + CHIP_ERROR ReadAttribute(const InteractionModel::ReadAttributeRequest & request, InteractionModel::ReadState & state, + AttributeValueEncoder & encoder) override; + CHIP_ERROR WriteAttribute(const InteractionModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) override; + CHIP_ERROR Invoke(const InteractionModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments, + InteractionModel::InvokeReply & reply) override; + + /// attribute tree iteration + EndpointId FirstEndpoint() override; + EndpointId NextEndpoint(EndpointId before) override; + + InteractionModel::ClusterEntry FirstCluster(EndpointId endpoint) override; + InteractionModel::ClusterEntry NextCluster(const ConcreteClusterPath & before) override; + std::optional GetClusterInfo(const ConcreteClusterPath & path) override; + + InteractionModel::AttributeEntry FirstAttribute(const ConcreteClusterPath & cluster) override; + InteractionModel::AttributeEntry NextAttribute(const ConcreteAttributePath & before) override; + std::optional GetAttributeInfo(const ConcreteAttributePath & path) override; +}; + +} // namespace app +} // namespace chip From 9bf0f85f4d9285e047c30e0b45c757bae3b9c38c Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 14 May 2024 09:51:21 -0400 Subject: [PATCH 10/79] Add some attribute iteration hint --- .../CodegenDataModel.cpp | 23 ++++++++++++++----- .../CodegenDataModel.h | 8 +++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index ab24dd265ac682..a322aa8c489477 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -107,21 +107,22 @@ InteractionModel::AttributeEntry AttributeEntryFrom(const ConcreteClusterPath & } // namespace -CHIP_ERROR CodegenDataModel::ReadAttribute(const InteractionModel::ReadAttributeRequest & request, InteractionModel::ReadState & state, - AttributeValueEncoder & encoder) +CHIP_ERROR CodegenDataModel::ReadAttribute(const InteractionModel::ReadAttributeRequest & request, + InteractionModel::ReadState & state, AttributeValueEncoder & encoder) { // TODO: this needs an implementation return CHIP_ERROR_NOT_IMPLEMENTED; } -CHIP_ERROR CodegenDataModel::WriteAttribute(const InteractionModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) +CHIP_ERROR CodegenDataModel::WriteAttribute(const InteractionModel::WriteAttributeRequest & request, + AttributeValueDecoder & decoder) { // TODO: this needs an implementation return CHIP_ERROR_NOT_IMPLEMENTED; } CHIP_ERROR CodegenDataModel::Invoke(const InteractionModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments, - InteractionModel::InvokeReply & reply) + InteractionModel::InvokeReply & reply) { // TODO: this needs an implementation return CHIP_ERROR_NOT_IMPLEMENTED; @@ -227,14 +228,24 @@ InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteA // find the given attribute in the list and then return the next one bool foundPosition = false; const unsigned attributeCount = cluster->attributeCount; - for (unsigned attribute_idx = 0; attribute_idx < attributeCount; attribute_idx++) + unsigned startIdx = 0; + + // Attempt to use the hint + if ((mAttributeIterationHint < attributeCount) && + (cluster->attributes[mAttributeIterationHint].attributeId == before.mAttributeId)) + { + startIdx = mAttributeIterationHint; + } + + for (unsigned attribute_idx = startIdx; attribute_idx < attributeCount; attribute_idx++) { if (foundPosition) { + mAttributeIterationHint = attribute_idx; return AttributeEntryFrom(before, cluster->attributes[attribute_idx]); } - foundPosition = (cluster->attributes[i].attributeId == before.mAttributeId); + foundPosition = (cluster->attributes[attribute_idx].attributeId == before.mAttributeId); } return InteractionModel::AttributeEntry::Invalid(); diff --git a/src/app/codegen-interaction-model/CodegenDataModel.h b/src/app/codegen-interaction-model/CodegenDataModel.h index 490404f2e68a4a..ae70ce6b997d92 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.h +++ b/src/app/codegen-interaction-model/CodegenDataModel.h @@ -44,6 +44,14 @@ class CodegenDataModel : public chip::app::InteractionModel::Model InteractionModel::AttributeEntry FirstAttribute(const ConcreteClusterPath & cluster) override; InteractionModel::AttributeEntry NextAttribute(const ConcreteAttributePath & before) override; std::optional GetAttributeInfo(const ConcreteAttributePath & path) override; + +private: + // Iteration is often done in a tight loop going through all values. + // To avoid N^2 iterations, cache a hint of where something is positioned + // uint16_t mEndpointIterationHint = 0; + // uint16_t mClusterIterationHint = 0; + unsigned mAttributeIterationHint = 0; + }; } // namespace app From 64f0db7afbc3440df5242c4d6c3c4b808413fd5e Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 14 May 2024 10:39:13 -0400 Subject: [PATCH 11/79] Make use of the attribute cache --- .../CodegenDataModel.cpp | 65 +++++++++++-------- .../CodegenDataModel.h | 4 ++ 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index a322aa8c489477..371d9d4b14c406 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -218,6 +218,29 @@ InteractionModel::AttributeEntry CodegenDataModel::FirstAttribute(const Concrete return AttributeEntryFrom(path, cluster->attributes[0]); } +std::optional CodegenDataModel::TryFindAttributeIndex(const EmberAfCluster * cluster, chip::AttributeId id) const +{ + const unsigned attributeCount = cluster->attributeCount; + + // attempt to find this based on the embedded hint + if ((mAttributeIterationHint < attributeCount) && (cluster->attributes[mAttributeIterationHint].attributeId == id)) + { + return std::make_optional(mAttributeIterationHint); + } + + // linear search is required. This may be slow + for (unsigned attribute_idx = 0; attribute_idx < attributeCount; attribute_idx++) + { + + if (cluster->attributes[attribute_idx].attributeId == id) + { + return std::make_optional(attribute_idx); + } + } + + return std::nullopt; +} + InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteAttributePath & before) { const EmberAfCluster * cluster = emberAfFindServerCluster(before.mEndpointId, before.mClusterId); @@ -226,29 +249,20 @@ InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteA VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); // find the given attribute in the list and then return the next one - bool foundPosition = false; - const unsigned attributeCount = cluster->attributeCount; - unsigned startIdx = 0; - - // Attempt to use the hint - if ((mAttributeIterationHint < attributeCount) && - (cluster->attributes[mAttributeIterationHint].attributeId == before.mAttributeId)) + std::optional attribute_idx = TryFindAttributeIndex(cluster, before.mAttributeId); + if (!attribute_idx.has_value()) { - startIdx = mAttributeIterationHint; + return InteractionModel::AttributeEntry::Invalid(); } - for (unsigned attribute_idx = startIdx; attribute_idx < attributeCount; attribute_idx++) + unsigned next_idx = *attribute_idx + 1; + if (next_idx >= cluster->attributeCount) { - if (foundPosition) - { - mAttributeIterationHint = attribute_idx; - return AttributeEntryFrom(before, cluster->attributes[attribute_idx]); - } - - foundPosition = (cluster->attributes[attribute_idx].attributeId == before.mAttributeId); + return InteractionModel::AttributeEntry::Invalid(); } - return InteractionModel::AttributeEntry::Invalid(); + mAttributeIterationHint = next_idx; + return AttributeEntryFrom(before, cluster->attributes[next_idx]); } std::optional CodegenDataModel::GetAttributeInfo(const ConcreteAttributePath & path) @@ -257,18 +271,17 @@ std::optional CodegenDataModel::GetAttributeInf VerifyOrReturnValue(cluster != nullptr, std::nullopt); VerifyOrReturnValue(cluster->attributeCount > 0, std::nullopt); VerifyOrReturnValue(cluster->attributes != nullptr, std::nullopt); - const unsigned attributeCount = cluster->attributeCount; - for (unsigned attribute_idx = 0; attribute_idx < attributeCount; attribute_idx++) + + std::optional attribute_idx = TryFindAttributeIndex(cluster, path.mAttributeId); + + if (!attribute_idx.has_value()) { - if (cluster->attributes[attribute_idx].attributeId == path.mAttributeId) - { - InteractionModel::AttributeInfo info; - LoadAttributeInfo(path, cluster->attributes[attribute_idx], &info); - return std::make_optional(info); - } + return std::nullopt; } - return std::nullopt; + InteractionModel::AttributeInfo info; + LoadAttributeInfo(path, cluster->attributes[*attribute_idx], &info); + return std::make_optional(info); } } // namespace app diff --git a/src/app/codegen-interaction-model/CodegenDataModel.h b/src/app/codegen-interaction-model/CodegenDataModel.h index ae70ce6b997d92..88375ebc99356e 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.h +++ b/src/app/codegen-interaction-model/CodegenDataModel.h @@ -18,6 +18,8 @@ #include +#include + namespace chip { namespace app { @@ -52,6 +54,8 @@ class CodegenDataModel : public chip::app::InteractionModel::Model // uint16_t mClusterIterationHint = 0; unsigned mAttributeIterationHint = 0; + /// Find the index of the given attribute id + std::optional TryFindAttributeIndex(const EmberAfCluster *cluster, chip::AttributeId search_for_id) const; }; } // namespace app From 777ee68114f5da384192a631146f25e445326dfe Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 14 May 2024 10:39:48 -0400 Subject: [PATCH 12/79] Restyle --- src/app/codegen-interaction-model/CodegenDataModel.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.h b/src/app/codegen-interaction-model/CodegenDataModel.h index 88375ebc99356e..8462e48ac99613 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.h +++ b/src/app/codegen-interaction-model/CodegenDataModel.h @@ -55,7 +55,7 @@ class CodegenDataModel : public chip::app::InteractionModel::Model unsigned mAttributeIterationHint = 0; /// Find the index of the given attribute id - std::optional TryFindAttributeIndex(const EmberAfCluster *cluster, chip::AttributeId search_for_id) const; + std::optional TryFindAttributeIndex(const EmberAfCluster * cluster, chip::AttributeId search_for_id) const; }; } // namespace app From 851e0a4bd81568f6c959969db32ad943ae70544e Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 14 May 2024 10:50:56 -0400 Subject: [PATCH 13/79] Add a cluster iteration hint --- .../CodegenDataModel.cpp | 42 +++++++++++++++---- .../CodegenDataModel.h | 7 +++- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 371d9d4b14c406..5c3b5936e3303e 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -65,7 +65,7 @@ InteractionModel::ClusterEntry ClusterEntryFrom(EndpointId endpointId, const Emb /// /// Returns an invalid entry if no more server clusters are found InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, const EmberAfEndpointType * endpoint, - uint16_t start_index) + uint16_t start_index, unsigned & found_index) { for (unsigned cluster_idx = start_index; cluster_idx < endpoint->clusterCount; cluster_idx++) { @@ -75,6 +75,7 @@ InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, co continue; } + found_index = cluster_idx; return ClusterEntryFrom(endpointId, cluster); } @@ -175,7 +176,33 @@ InteractionModel::ClusterEntry CodegenDataModel::FirstCluster(EndpointId endpoin VerifyOrReturnValue(endpoint->clusterCount > 0, InteractionModel::ClusterEntry::Invalid()); VerifyOrReturnValue(endpoint->cluster != nullptr, InteractionModel::ClusterEntry::Invalid()); - return FirstServerClusterEntry(endpointId, endpoint, 0); + return FirstServerClusterEntry(endpointId, endpoint, 0, mClusterIterationHint); +} + +std::optional CodegenDataModel::TryFindServerClusterIndex(const EmberAfEndpointType * endpoint, chip::ClusterId id) const +{ + const unsigned clusterCount = endpoint->clusterCount; + + if (mClusterIterationHint < clusterCount) + { + const EmberAfCluster & cluster = endpoint->cluster[mClusterIterationHint]; + if (IsServerMask(cluster.mask) && (cluster.clusterId == id)) + { + return std::make_optional(mClusterIterationHint); + } + } + + // linear search, this may be slow + for (unsigned cluster_idx = 0; cluster_idx < clusterCount; cluster_idx++) + { + const EmberAfCluster & cluster = endpoint->cluster[cluster_idx]; + if (IsServerMask(cluster.mask) && (cluster.clusterId == id)) + { + return std::make_optional(cluster_idx); + } + } + + return std::nullopt; } InteractionModel::ClusterEntry CodegenDataModel::NextCluster(const ConcreteClusterPath & before) @@ -185,16 +212,13 @@ InteractionModel::ClusterEntry CodegenDataModel::NextCluster(const ConcreteClust VerifyOrReturnValue(endpoint->clusterCount > 0, InteractionModel::ClusterEntry::Invalid()); VerifyOrReturnValue(endpoint->cluster != nullptr, InteractionModel::ClusterEntry::Invalid()); - for (uint16_t cluster_idx = 0; cluster_idx < endpoint->clusterCount; cluster_idx++) + std::optional cluster_idx = TryFindServerClusterIndex(endpoint, before.mClusterId); + if (!cluster_idx.has_value()) { - const EmberAfCluster & cluster = endpoint->cluster[cluster_idx]; - if (IsServerMask(cluster.mask) && (cluster.clusterId == before.mClusterId)) - { - return FirstServerClusterEntry(before.mEndpointId, endpoint, cluster_idx + 1); - } + return InteractionModel::ClusterEntry::Invalid(); } - return InteractionModel::ClusterEntry::Invalid(); + return FirstServerClusterEntry(before.mEndpointId, endpoint, *cluster_idx + 1, mClusterIterationHint); } std::optional CodegenDataModel::GetClusterInfo(const ConcreteClusterPath & path) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.h b/src/app/codegen-interaction-model/CodegenDataModel.h index 8462e48ac99613..2a5d2b9d7d9a14 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.h +++ b/src/app/codegen-interaction-model/CodegenDataModel.h @@ -51,11 +51,14 @@ class CodegenDataModel : public chip::app::InteractionModel::Model // Iteration is often done in a tight loop going through all values. // To avoid N^2 iterations, cache a hint of where something is positioned // uint16_t mEndpointIterationHint = 0; - // uint16_t mClusterIterationHint = 0; + unsigned mClusterIterationHint = 0; unsigned mAttributeIterationHint = 0; /// Find the index of the given attribute id - std::optional TryFindAttributeIndex(const EmberAfCluster * cluster, chip::AttributeId search_for_id) const; + std::optional TryFindAttributeIndex(const EmberAfCluster * cluster, chip::AttributeId id) const; + + /// Find the index of the given cluster id + std::optional TryFindServerClusterIndex(const EmberAfEndpointType * endpoint, chip::ClusterId id) const; }; } // namespace app From 5009d8ee4f3232aa99b10506363b48d9f667f358 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 14 May 2024 11:00:27 -0400 Subject: [PATCH 14/79] Add a few more hints. Ember code still contains loops though, so this may not be ideal still --- .../CodegenDataModel.cpp | 42 +++++++++++++++---- .../CodegenDataModel.h | 7 +++- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 5c3b5936e3303e..232355719b04e4 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -19,6 +19,7 @@ #include #include #include +#include namespace chip { namespace app { @@ -145,22 +146,49 @@ EndpointId CodegenDataModel::FirstEndpoint() return kInvalidEndpointId; } -EndpointId CodegenDataModel::NextEndpoint(EndpointId before) +std::optional CodegenDataModel::TryFindEndpointIndex(chip::EndpointId id) const { - // find the first enabled index - bool beforeFound = false; + const unsigned lastEndpointIndex = emberAfEndpointCount(); - const uint16_t lastEndpointIndex = emberAfEndpointCount(); - for (uint16_t endpoint_idx = 0; endpoint_idx < lastEndpointIndex; endpoint_idx++) + if ((mEndpointIterationHint < lastEndpointIndex) && emberAfEndpointIndexIsEnabled(mEndpointIterationHint) && + (id == emberAfEndpointFromIndex(mEndpointIterationHint))) { - if (!beforeFound) + return std::make_optional(mEndpointIterationHint); + } + + // Linear search, this may be slow + for (unsigned endpoint_idx = 0; endpoint_idx < lastEndpointIndex; endpoint_idx++) + { + if (!emberAfEndpointIndexIsEnabled(endpoint_idx)) { - beforeFound = (before == emberAfEndpointFromIndex(endpoint_idx)); continue; } + if (id == emberAfEndpointFromIndex(endpoint_idx)) + { + return std::make_optional(endpoint_idx); + } + } + + return std::nullopt; +} + +EndpointId CodegenDataModel::NextEndpoint(EndpointId before) +{ + const unsigned lastEndpointIndex = emberAfEndpointCount(); + + std::optional before_idx = TryFindEndpointIndex(before); + if (!before_idx.has_value()) + { + return kInvalidEndpointId; + } + + // find the first enabled index + for (unsigned endpoint_idx = *before_idx + 1; endpoint_idx < lastEndpointIndex; endpoint_idx++) + { if (emberAfEndpointIndexIsEnabled(endpoint_idx)) { + mEndpointIterationHint = endpoint_idx; return emberAfEndpointFromIndex(endpoint_idx); } } diff --git a/src/app/codegen-interaction-model/CodegenDataModel.h b/src/app/codegen-interaction-model/CodegenDataModel.h index 2a5d2b9d7d9a14..22a2d0369ae4ca 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.h +++ b/src/app/codegen-interaction-model/CodegenDataModel.h @@ -50,8 +50,8 @@ class CodegenDataModel : public chip::app::InteractionModel::Model private: // Iteration is often done in a tight loop going through all values. // To avoid N^2 iterations, cache a hint of where something is positioned - // uint16_t mEndpointIterationHint = 0; - unsigned mClusterIterationHint = 0; + uint16_t mEndpointIterationHint = 0; + unsigned mClusterIterationHint = 0; unsigned mAttributeIterationHint = 0; /// Find the index of the given attribute id @@ -59,6 +59,9 @@ class CodegenDataModel : public chip::app::InteractionModel::Model /// Find the index of the given cluster id std::optional TryFindServerClusterIndex(const EmberAfEndpointType * endpoint, chip::ClusterId id) const; + + /// Find the index of the given endpoint id + std::optional TryFindEndpointIndex(chip::EndpointId id) const; }; } // namespace app From 88e5bbcc3de8587952231433b3d8aea442ffbd10 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 14 May 2024 11:06:06 -0400 Subject: [PATCH 15/79] Add some TODO items for using faster iterations for data. Ember index vs value duality still needs some work --- .../CodegenDataModel.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 232355719b04e4..9c435dce9b5b6b 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -235,7 +235,10 @@ std::optional CodegenDataModel::TryFindServerClusterIndex(const EmberA InteractionModel::ClusterEntry CodegenDataModel::NextCluster(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, InteractionModel::ClusterEntry::Invalid()); VerifyOrReturnValue(endpoint->clusterCount > 0, InteractionModel::ClusterEntry::Invalid()); VerifyOrReturnValue(endpoint->cluster != nullptr, InteractionModel::ClusterEntry::Invalid()); @@ -251,7 +254,10 @@ InteractionModel::ClusterEntry CodegenDataModel::NextCluster(const ConcreteClust std::optional CodegenDataModel::GetClusterInfo(const ConcreteClusterPath & path) { + // TODO: This search still seems slow (ember will loop). Should use index hints as long + // as ember API supports it const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); + VerifyOrReturnValue(cluster != nullptr, std::nullopt); InteractionModel::ClusterInfo info; @@ -262,7 +268,10 @@ std::optional CodegenDataModel::GetClusterInfo(co InteractionModel::AttributeEntry CodegenDataModel::FirstAttribute(const ConcreteClusterPath & path) { + // TODO: This search still seems slow (ember will loop). Should use index hints as long + // as ember API supports it const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); + VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::Invalid()); VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); @@ -295,7 +304,10 @@ std::optional CodegenDataModel::TryFindAttributeIndex(const EmberAfClu InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteAttributePath & before) { + // TODO: This search still seems slow (ember will loop). Should use index hints as long + // as ember API supports it const EmberAfCluster * cluster = emberAfFindServerCluster(before.mEndpointId, before.mClusterId); + VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::Invalid()); VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); @@ -319,7 +331,10 @@ InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteA std::optional CodegenDataModel::GetAttributeInfo(const ConcreteAttributePath & path) { + // TODO: This search still seems slow (ember will loop). Should use index hints as long + // as ember API supports it const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); + VerifyOrReturnValue(cluster != nullptr, std::nullopt); VerifyOrReturnValue(cluster->attributeCount > 0, std::nullopt); VerifyOrReturnValue(cluster->attributes != nullptr, std::nullopt); From 7672d22e178c1dabde670e892811127d9b53b0de Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 14 May 2024 11:17:33 -0400 Subject: [PATCH 16/79] Add a cluster type cache as well. This relies on ember being reasonably static --- .../CodegenDataModel.cpp | 33 +++++++++++-------- .../CodegenDataModel.h | 17 ++++++++++ 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 9c435dce9b5b6b..7c139763d4306a 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -254,9 +254,7 @@ InteractionModel::ClusterEntry CodegenDataModel::NextCluster(const ConcreteClust std::optional CodegenDataModel::GetClusterInfo(const ConcreteClusterPath & path) { - // TODO: This search still seems slow (ember will loop). Should use index hints as long - // as ember API supports it - const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); + const EmberAfCluster * cluster = FindServerCluster(path); VerifyOrReturnValue(cluster != nullptr, std::nullopt); @@ -268,9 +266,7 @@ std::optional CodegenDataModel::GetClusterInfo(co InteractionModel::AttributeEntry CodegenDataModel::FirstAttribute(const ConcreteClusterPath & path) { - // TODO: This search still seems slow (ember will loop). Should use index hints as long - // as ember API supports it - const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); + const EmberAfCluster * cluster = FindServerCluster(path); VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::Invalid()); VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); @@ -302,12 +298,25 @@ std::optional CodegenDataModel::TryFindAttributeIndex(const EmberAfClu return std::nullopt; } -InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteAttributePath & before) +const EmberAfCluster * CodegenDataModel::FindServerCluster(const ConcreteClusterPath & path) { - // TODO: This search still seems slow (ember will loop). Should use index hints as long - // as ember API supports it - const EmberAfCluster * cluster = emberAfFindServerCluster(before.mEndpointId, before.mClusterId); + // cache things + if (mPreviouslyFoundCluster.has_value() && (mPreviouslyFoundCluster->path == path)) + { + return mPreviouslyFoundCluster->cluster; + } + const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); + if (cluster != nullptr) + { + mPreviouslyFoundCluster = std::make_optional(path, cluster); + } + return cluster; +} + +InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteAttributePath & before) +{ + const EmberAfCluster * cluster = FindServerCluster(before); VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::Invalid()); VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); @@ -331,9 +340,7 @@ InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteA std::optional CodegenDataModel::GetAttributeInfo(const ConcreteAttributePath & path) { - // TODO: This search still seems slow (ember will loop). Should use index hints as long - // as ember API supports it - const EmberAfCluster * cluster = emberAfFindServerCluster(path.mEndpointId, path.mClusterId); + const EmberAfCluster * cluster = FindServerCluster(path); VerifyOrReturnValue(cluster != nullptr, std::nullopt); VerifyOrReturnValue(cluster->attributeCount > 0, std::nullopt); diff --git a/src/app/codegen-interaction-model/CodegenDataModel.h b/src/app/codegen-interaction-model/CodegenDataModel.h index 22a2d0369ae4ca..723831b4368dcb 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.h +++ b/src/app/codegen-interaction-model/CodegenDataModel.h @@ -16,6 +16,7 @@ */ #pragma once +#include "app/ConcreteClusterPath.h" #include #include @@ -54,6 +55,22 @@ class CodegenDataModel : public chip::app::InteractionModel::Model unsigned mClusterIterationHint = 0; unsigned mAttributeIterationHint = 0; + // represents a remembered cluster reference that has been found as + // looking for clusters is very common (for every attribute iteration) + struct ClusterReference + { + ConcreteClusterPath path; + const EmberAfCluster * cluster; + + ClusterReference(const ConcreteClusterPath p, const EmberAfCluster * c) : path(p), cluster(c) {} + }; + std::optional mPreviouslyFoundCluster; + + /// Finds the specified ember cluster + /// + /// Effectively the same as `emberAfFindServerCluster` except with some caching capabilities + const EmberAfCluster * FindServerCluster(const ConcreteClusterPath & path); + /// Find the index of the given attribute id std::optional TryFindAttributeIndex(const EmberAfCluster * cluster, chip::AttributeId id) const; From 23a988d185186f51897124da5b7d58297ff424ed Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 14 May 2024 12:56:04 -0400 Subject: [PATCH 17/79] Add global attribute handling --- .../CodegenDataModel.cpp | 57 +++++++++++++++++-- .../tests/TestCodegenModelViaMocks.cpp | 34 +++++++++++ 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 7c139763d4306a..1bb1c1c0085028 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include namespace chip { @@ -107,6 +108,17 @@ InteractionModel::AttributeEntry AttributeEntryFrom(const ConcreteClusterPath & return entry; } +InteractionModel::AttributeEntry AttributeEntryForGlobalListAttribute(const ConcreteClusterPath & clusterPath, + chip::AttributeId globalAttributeId) +{ + InteractionModel::AttributeEntry entry; + + entry.path = ConcreteAttributePath(clusterPath.mEndpointId, clusterPath.mClusterId, globalAttributeId); + entry.info.flags.Set(InteractionModel::AttributeQualityFlags::kListAttribute); + + return entry; +} + } // namespace CHIP_ERROR CodegenDataModel::ReadAttribute(const InteractionModel::ReadAttributeRequest & request, @@ -321,6 +333,24 @@ InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteA VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); + // Handles global attribute iteration: if we got a global attribute, move to the next + switch (before.mAttributeId) + { + case Clusters::Globals::Attributes::GeneratedCommandList::Id: + return AttributeEntryForGlobalListAttribute(before, Clusters::Globals::Attributes::AcceptedCommandList::Id); + case Clusters::Globals::Attributes::AcceptedCommandList::Id: +#if CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE + return AttributeEntryForGlobalListAttribute(before, Clusters::Globals::Attributes::EventList::Id); + case Clusters::Globals::Attributes::EventList::Id: +#endif // CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE + return AttributeEntryForGlobalListAttribute(before, Clusters::Globals::Attributes::AttributeList::Id); + case Clusters::Globals::Attributes::AttributeList::Id: + return InteractionModel::AttributeEntry::Invalid(); + default: + // pass-through: not a global attribute, try to find a "regular" attribute + break; + } + // find the given attribute in the list and then return the next one std::optional attribute_idx = TryFindAttributeIndex(cluster, before.mAttributeId); if (!attribute_idx.has_value()) @@ -329,13 +359,15 @@ InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteA } unsigned next_idx = *attribute_idx + 1; - if (next_idx >= cluster->attributeCount) + if (next_idx < cluster->attributeCount) { - return InteractionModel::AttributeEntry::Invalid(); + mAttributeIterationHint = next_idx; + return AttributeEntryFrom(before, cluster->attributes[next_idx]); } - mAttributeIterationHint = next_idx; - return AttributeEntryFrom(before, cluster->attributes[next_idx]); + // We reach here if next_idx is just past the last attribute metadata. Return the first global + // attribute + return AttributeEntryForGlobalListAttribute(before, Clusters::Globals::Attributes::GeneratedCommandList::Id); } std::optional CodegenDataModel::GetAttributeInfo(const ConcreteAttributePath & path) @@ -346,6 +378,23 @@ std::optional CodegenDataModel::GetAttributeInf VerifyOrReturnValue(cluster->attributeCount > 0, std::nullopt); VerifyOrReturnValue(cluster->attributes != nullptr, std::nullopt); + switch (path.mAttributeId) + { + case Clusters::Globals::Attributes::GeneratedCommandList::Id: + case Clusters::Globals::Attributes::AcceptedCommandList::Id: +#if CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE + case Clusters::Globals::Attributes::EventList::Id: +#endif // CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE + case Clusters::Globals::Attributes::AttributeList::Id: { + InteractionModel::AttributeInfo info; + info.flags.Set(InteractionModel::AttributeQualityFlags::kListAttribute); + return info; + } + default: + // pass-through: not a global attribute, try to find a "regular" attribute + break; + } + std::optional attribute_idx = TryFindAttributeIndex(cluster, path.mAttributeId); if (!attribute_idx.has_value()) diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index 9904939b5b0525..c4cd5fa2977e79 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -15,6 +15,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +#include "app/GlobalAttributes.h" +#include "lib/core/DataModelTypes.h" #include #include @@ -238,6 +240,21 @@ TEST(TestCodegenModelViaMocks, IterateOverAttributes) ASSERT_EQ(entry.path.mAttributeId, MockAttributeId(2)); ASSERT_TRUE(entry.info.flags.Has(AttributeQualityFlags::kListAttribute)); + // Iteration MUST include global attributes. Ember does not provide those, so we + // assert here that we present them in order + for (auto globalAttributeId : GlobalAttributesNotInMetadata) + { + + entry = model.NextAttribute(entry.path); + ASSERT_TRUE(entry.path.HasValidIds()); + ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint2); + ASSERT_EQ(entry.path.mClusterId, MockClusterId(2)); + ASSERT_EQ(entry.path.mAttributeId, globalAttributeId); + + // all global attributes not in ember metadata are LIST typed + ASSERT_TRUE(entry.info.flags.Has(AttributeQualityFlags::kListAttribute)); + } + entry = model.NextAttribute(entry.path); ASSERT_FALSE(entry.path.HasValidIds()); @@ -288,3 +305,20 @@ TEST(TestCodegenModelViaMocks, GetAttributeInfo) ASSERT_TRUE(info.has_value()); EXPECT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access) } + +TEST(TestCodegenModelViaMocks, GlobalAttributeInfo) +{ + UseMockNodeConfig config(gTestNodeConfig); + chip::app::CodegenDataModel model; + + std::optional info = model.GetAttributeInfo( + ConcreteAttributePath(kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::GeneratedCommandList::Id)); + + ASSERT_TRUE(info.has_value()); + EXPECT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access) + + info = model.GetAttributeInfo( + ConcreteAttributePath(kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::AttributeList::Id)); + ASSERT_TRUE(info.has_value()); + EXPECT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access) +} From f4651fc612bc74be04c3e9911f173d7c8117687b Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 14 May 2024 13:14:08 -0400 Subject: [PATCH 18/79] Fix typing u16 vs unsigned --- src/app/codegen-interaction-model/CodegenDataModel.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 1bb1c1c0085028..c5bedcb75d68f5 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -67,7 +67,7 @@ InteractionModel::ClusterEntry ClusterEntryFrom(EndpointId endpointId, const Emb /// /// Returns an invalid entry if no more server clusters are found InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, const EmberAfEndpointType * endpoint, - uint16_t start_index, unsigned & found_index) + unsigned start_index, unsigned & found_index) { for (unsigned cluster_idx = start_index; cluster_idx < endpoint->clusterCount; cluster_idx++) { @@ -160,7 +160,7 @@ EndpointId CodegenDataModel::FirstEndpoint() std::optional CodegenDataModel::TryFindEndpointIndex(chip::EndpointId id) const { - const unsigned lastEndpointIndex = emberAfEndpointCount(); + const uint16_t lastEndpointIndex = emberAfEndpointCount(); if ((mEndpointIterationHint < lastEndpointIndex) && emberAfEndpointIndexIsEnabled(mEndpointIterationHint) && (id == emberAfEndpointFromIndex(mEndpointIterationHint))) @@ -169,7 +169,7 @@ std::optional CodegenDataModel::TryFindEndpointIndex(chip::EndpointId } // Linear search, this may be slow - for (unsigned endpoint_idx = 0; endpoint_idx < lastEndpointIndex; endpoint_idx++) + for (uint16_t endpoint_idx = 0; endpoint_idx < lastEndpointIndex; endpoint_idx++) { if (!emberAfEndpointIndexIsEnabled(endpoint_idx)) { @@ -196,7 +196,7 @@ EndpointId CodegenDataModel::NextEndpoint(EndpointId before) } // find the first enabled index - for (unsigned endpoint_idx = *before_idx + 1; endpoint_idx < lastEndpointIndex; endpoint_idx++) + for (uint16_t endpoint_idx = static_cast(*before_idx + 1); endpoint_idx < lastEndpointIndex; endpoint_idx++) { if (emberAfEndpointIndexIsEnabled(endpoint_idx)) { From eba45aefd3a961e99d9ad66601e708f18b5d3187 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 17 May 2024 14:32:30 -0400 Subject: [PATCH 19/79] Fix auto-added include names --- src/app/codegen-interaction-model/CodegenDataModel.h | 1 - .../tests/TestCodegenModelViaMocks.cpp | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.h b/src/app/codegen-interaction-model/CodegenDataModel.h index 723831b4368dcb..f3e9ca1c763426 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.h +++ b/src/app/codegen-interaction-model/CodegenDataModel.h @@ -16,7 +16,6 @@ */ #pragma once -#include "app/ConcreteClusterPath.h" #include #include diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index c4cd5fa2977e79..ace5236ebfd2a1 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -15,13 +15,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include "app/GlobalAttributes.h" -#include "lib/core/DataModelTypes.h" #include +#include #include #include #include +#include #include From 22e9df0e330b251a45a489cffc6992aa3b5e7763 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 21 May 2024 11:52:28 -0400 Subject: [PATCH 20/79] Remove back the initialization and make the comment more obvious --- src/app/ConcreteClusterPath.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/ConcreteClusterPath.h b/src/app/ConcreteClusterPath.h index 47feae1fd67269..8b701efa83967b 100644 --- a/src/app/ConcreteClusterPath.h +++ b/src/app/ConcreteClusterPath.h @@ -52,7 +52,7 @@ struct ConcreteClusterPath // to alignment requirements it's "free" in the sense of not needing more // memory to put it here. But we don't initialize it, because that // increases codesize for the non-consumers. - bool mExpanded = false; // NOTE: in between larger members + bool mExpanded; // NOTE: in between larger members, NOT initialized (see above) ClusterId mClusterId = 0; }; From 34066a65501ba815bf220b62e13ec9ee92c120be Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 29 May 2024 10:33:24 -0400 Subject: [PATCH 21/79] Update src/app/codegen-interaction-model/model.gni Co-authored-by: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com> --- src/app/codegen-interaction-model/model.gni | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/codegen-interaction-model/model.gni b/src/app/codegen-interaction-model/model.gni index f44beefd94079b..d1c4e85b90b433 100644 --- a/src/app/codegen-interaction-model/model.gni +++ b/src/app/codegen-interaction-model/model.gni @@ -13,7 +13,7 @@ # limitations under the License. import("//build_overrides/chip.gni") -# The sources in this directory are TIGHLY coupled with code-generated data models +# The sources in this directory are TIGHTLY coupled with code-generated data models # as generally implemented by `src/app/util` # # Corresponding functions defined in attribute-storace.cpp/attribute-table.cpp must From b2e057aecfaa1fb7f7f91654122380da0f3668fe Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 29 May 2024 10:41:46 -0400 Subject: [PATCH 22/79] Code review feedback: added comments --- src/app/codegen-interaction-model/CodegenDataModel.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.h b/src/app/codegen-interaction-model/CodegenDataModel.h index f3e9ca1c763426..c39a18d7dc2c96 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.h +++ b/src/app/codegen-interaction-model/CodegenDataModel.h @@ -23,6 +23,18 @@ namespace chip { namespace app { +/// An implementation of `InterationModel::Model` that relies on code-generation +/// via zap/ember. +/// +/// The Ember framework uses generated files (like endpoint-config.h and various +/// other generated metadata) to provide a cluster model. +/// +/// This class will use global functions generally residing in `app/util` +/// as well as application-specific overrides to provide data model functionality. +/// +/// Given that this relies on global data at link time, there generally can be +/// only one CodegenDataModel per application (you can create more instances, +/// however they would share the exact same underlying data and storage). class CodegenDataModel : public chip::app::InteractionModel::Model { public: From b98b88b2be556895af09573459b13063c9ec21d5 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 07:56:13 -0400 Subject: [PATCH 23/79] Update src/app/codegen-interaction-model/CodegenDataModel.h Co-authored-by: Boris Zbarsky --- src/app/codegen-interaction-model/CodegenDataModel.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.h b/src/app/codegen-interaction-model/CodegenDataModel.h index c39a18d7dc2c96..712405105daf09 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.h +++ b/src/app/codegen-interaction-model/CodegenDataModel.h @@ -23,7 +23,7 @@ namespace chip { namespace app { -/// An implementation of `InterationModel::Model` that relies on code-generation +/// An implementation of `InteractionModel::Model` that relies on code-generation /// via zap/ember. /// /// The Ember framework uses generated files (like endpoint-config.h and various From b3d53aab0cedfbd1ce132aa007abddb8228bff25 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 07:56:22 -0400 Subject: [PATCH 24/79] Update src/app/codegen-interaction-model/CodegenDataModel.cpp Co-authored-by: Boris Zbarsky --- src/app/codegen-interaction-model/CodegenDataModel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index c5bedcb75d68f5..5bd7741722c471 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -84,7 +84,7 @@ InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, co return InteractionModel::ClusterEntry::Invalid(); } -/// Load the cluster information into the specified destination +/// Load the attribute information into the specified destination void LoadAttributeInfo(const ConcreteAttributePath & path, const EmberAfAttributeMetadata & attribute, InteractionModel::AttributeInfo * info) { From c292e9ea890aa6c7dcddb94652d94122417fa99a Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 08:02:08 -0400 Subject: [PATCH 25/79] Update src/app/codegen-interaction-model/BUILD.gn Co-authored-by: Tennessee Carmel-Veilleux --- src/app/codegen-interaction-model/BUILD.gn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/codegen-interaction-model/BUILD.gn b/src/app/codegen-interaction-model/BUILD.gn index 65a672076e4a8a..418983a2fc8b8f 100644 --- a/src/app/codegen-interaction-model/BUILD.gn +++ b/src/app/codegen-interaction-model/BUILD.gn @@ -22,6 +22,6 @@ import("//build_overrides/chip.gni") # CodegenDataModel.cpp # CodegenDataModel.h # -# The abolve list of files exists to satisfy the "dependency linter" +# The above list of files exists to satisfy the "dependency linter" # since those files should technically be "visible to gn" even though we # are supposed to go through model.gni constants From 2b914edff2a6a6786bc8bec052446eb69aeac9f6 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 09:39:29 -0400 Subject: [PATCH 26/79] Some cleanup logic for event generation - naming and return values as eventid is not the same as event number --- src/app/interaction-model/Events.h | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index 255a55e97ee465..9c979205970519 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -25,6 +25,7 @@ #include #include +#include namespace chip { namespace app { @@ -45,8 +46,8 @@ class SimpleEventLoggingDelegate : public EventLoggingDelegate const T & mEventData; }; -template ::value, bool> = true> -EventNumber GenerateEvent(E & emittor, const T & aEventData, EndpointId aEndpoint) +template ::value, bool> = true> +std::optional GenerateEvent(G & generator, const T & aEventData, EndpointId aEndpoint) { internal::SimpleEventLoggingDelegate eventData(aEventData); ConcreteEventPath path(aEndpoint, aEventData.GetClusterId(), aEventData.GetEventId()); @@ -55,12 +56,11 @@ EventNumber GenerateEvent(E & emittor, const T & aEventData, EndpointId aEndpoin eventOptions.mPriority = aEventData.GetPriorityLevel(); eventOptions.mFabricIndex = aEventData.GetFabricIndex(); - // this skips logging the event if it's fabric-scoped but no fabric association exists yet. - + // this skips generating the event if it's fabric-scoped but no fabric association exists yet. if (eventOptions.mFabricIndex == kUndefinedFabricIndex) { ChipLogError(EventLogging, "Event encode failure: no fabric index for fabric scoped event"); - return kInvalidEventId; + return std::nullopt; } // @@ -72,18 +72,18 @@ EventNumber GenerateEvent(E & emittor, const T & aEventData, EndpointId aEndpoin // and used to match against the accessing fabric. // EventNumber eventNumber; - CHIP_ERROR err = emittor.GenerateEvent(&eventData, eventOptions, eventNumber); + CHIP_ERROR err = generator.GenerateEvent(&eventData, eventOptions, eventNumber); if (err != CHIP_NO_ERROR) { ChipLogError(EventLogging, "Failed to log event: %" CHIP_ERROR_FORMAT, err.Format()); - return kInvalidEventId; + return std::nullopt; } return eventNumber; } -template ::value, bool> = true> -EventNumber GenerateEvent(E & emittor, const T & aEventData, EndpointId endpointId) +template ::value, bool> = true> +std::optional GenerateEvent(G & generator, const T & aEventData, EndpointId endpointId) { internal::SimpleEventLoggingDelegate eventData(aEventData); ConcreteEventPath path(endpointId, aEventData.GetClusterId(), aEventData.GetEventId()); @@ -91,11 +91,11 @@ EventNumber GenerateEvent(E & emittor, const T & aEventData, EndpointId endpoint eventOptions.mPath = path; eventOptions.mPriority = aEventData.GetPriorityLevel(); EventNumber eventNumber; - CHIP_ERROR err = emittor.GenerateEvent(&eventData, eventOptions, eventNumber); + CHIP_ERROR err = generator.GenerateEvent(&eventData, eventOptions, eventNumber); if (err != CHIP_NO_ERROR) { ChipLogError(EventLogging, "Failed to log event: %" CHIP_ERROR_FORMAT, err.Format()); - return kInvalidEventId; + return std::nullopt; } return eventNumber; @@ -117,9 +117,10 @@ class Events EventNumber & generatedEventNumber) = 0; // Convenience methods for event logging using cluster-object structures - // On error, these log and return kInvalidEventId + // + // On error, these log and return nullopt. template - EventNumber GenerateEvent(const T & eventData, EndpointId endpointId) + std::optional GenerateEvent(const T & eventData, EndpointId endpointId) { return internal::GenerateEvent(*this, eventData, endpointId); } From 4d66cd1bf1fb95bf34f1b6b8d6851c50891fc2ee Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 09:40:57 -0400 Subject: [PATCH 27/79] Comment fix --- src/app/interaction-model/Events.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index 9c979205970519..15194863a8554b 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -56,7 +56,8 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En eventOptions.mPriority = aEventData.GetPriorityLevel(); eventOptions.mFabricIndex = aEventData.GetFabricIndex(); - // this skips generating the event if it's fabric-scoped but no fabric association exists yet. + // this skips generating the event if it is fabric-scoped however the event does not seem + // associated with any fabric. if (eventOptions.mFabricIndex == kUndefinedFabricIndex) { ChipLogError(EventLogging, "Event encode failure: no fabric index for fabric scoped event"); From 0ae3b5c9070d992d4567ce034b19a8a0250e039f Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 09:44:33 -0400 Subject: [PATCH 28/79] More naming updates --- src/app/interaction-model/Events.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index 15194863a8554b..86e121c9e9a327 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -24,8 +24,8 @@ #include #include -#include #include +#include namespace chip { namespace app { @@ -49,7 +49,7 @@ class SimpleEventLoggingDelegate : public EventLoggingDelegate template ::value, bool> = true> std::optional GenerateEvent(G & generator, const T & aEventData, EndpointId aEndpoint) { - internal::SimpleEventLoggingDelegate eventData(aEventData); + internal::SimpleEventLoggingDelegate eventPayloadWriter(aEventData); ConcreteEventPath path(aEndpoint, aEventData.GetClusterId(), aEventData.GetEventId()); EventOptions eventOptions; eventOptions.mPath = path; @@ -73,10 +73,10 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En // and used to match against the accessing fabric. // EventNumber eventNumber; - CHIP_ERROR err = generator.GenerateEvent(&eventData, eventOptions, eventNumber); + CHIP_ERROR err = generator.GenerateEvent(&eventPayloadWriter, eventOptions, eventNumber); if (err != CHIP_NO_ERROR) { - ChipLogError(EventLogging, "Failed to log event: %" CHIP_ERROR_FORMAT, err.Format()); + ChipLogError(EventLogging, "Failed to generate event: %" CHIP_ERROR_FORMAT, err.Format()); return std::nullopt; } @@ -86,16 +86,16 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En template ::value, bool> = true> std::optional GenerateEvent(G & generator, const T & aEventData, EndpointId endpointId) { - internal::SimpleEventLoggingDelegate eventData(aEventData); + internal::SimpleEventLoggingDelegate eventPayloadWriter(aEventData); ConcreteEventPath path(endpointId, aEventData.GetClusterId(), aEventData.GetEventId()); EventOptions eventOptions; eventOptions.mPath = path; eventOptions.mPriority = aEventData.GetPriorityLevel(); EventNumber eventNumber; - CHIP_ERROR err = generator.GenerateEvent(&eventData, eventOptions, eventNumber); + CHIP_ERROR err = generator.GenerateEvent(&eventPayloadWriter, eventOptions, eventNumber); if (err != CHIP_NO_ERROR) { - ChipLogError(EventLogging, "Failed to log event: %" CHIP_ERROR_FORMAT, err.Format()); + ChipLogError(EventLogging, "Failed to generate event: %" CHIP_ERROR_FORMAT, err.Format()); return std::nullopt; } @@ -114,11 +114,11 @@ class Events /// Events are generally expected to be sent to subscribed clients and also /// be available for read later until they get overwritten by new events /// that are being generated. - virtual CHIP_ERROR GenerateEvent(EventLoggingDelegate * eventContentWriter, const EventOptions & options, + virtual CHIP_ERROR GenerateEvent(EventLoggingDelegate * eventPayloadWriter, const EventOptions & options, EventNumber & generatedEventNumber) = 0; // Convenience methods for event logging using cluster-object structures - // + // // On error, these log and return nullopt. template std::optional GenerateEvent(const T & eventData, EndpointId endpointId) From 9cbabe171b9b9e4d8bd833d1d2517320bba5e93d Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:13:49 -0400 Subject: [PATCH 29/79] Several comment updates and renamed RequestContext to ActionContext --- .../{RequestContext.h => ActionContext.h} | 8 ++++---- src/app/interaction-model/Actions.h | 4 ++-- src/app/interaction-model/BUILD.gn | 2 +- src/app/interaction-model/InvokeResponder.h | 2 +- src/app/interaction-model/IterationTypes.h | 9 +++++++-- src/app/interaction-model/Model.h | 10 +++++----- src/app/interaction-model/OperationTypes.h | 4 ++-- src/app/interaction-model/Paths.h | 16 +++++++--------- 8 files changed, 29 insertions(+), 26 deletions(-) rename src/app/interaction-model/{RequestContext.h => ActionContext.h} (87%) diff --git a/src/app/interaction-model/RequestContext.h b/src/app/interaction-model/ActionContext.h similarity index 87% rename from src/app/interaction-model/RequestContext.h rename to src/app/interaction-model/ActionContext.h index 74fa9af9da0fc8..bfc2555870c780 100644 --- a/src/app/interaction-model/RequestContext.h +++ b/src/app/interaction-model/ActionContext.h @@ -22,13 +22,13 @@ namespace chip { namespace app { namespace InteractionModel { -// Context for a currently executing request -class RequestContext +// Context for a currently executing action +class ActionContext { public: - virtual ~RequestContext() = default; + virtual ~ActionContext() = default; - /// Valid ONLY during synchronous handling of a Read/Write/Invoke + /// Valid ONLY during synchronous handling of an action. /// /// Used sparingly, however some operations will require these. An example /// usage is "Operational Credentials aborting communications on removed fabrics" diff --git a/src/app/interaction-model/Actions.h b/src/app/interaction-model/Actions.h index 62021fbf7ccd32..51574f3596b935 100644 --- a/src/app/interaction-model/Actions.h +++ b/src/app/interaction-model/Actions.h @@ -18,7 +18,7 @@ #include #include -#include +#include namespace chip { namespace app { @@ -29,7 +29,7 @@ struct InteractionModelActions { Events * events; Paths * paths; - RequestContext * requestContext; + ActionContext * requestContext; }; } // namespace InteractionModel diff --git a/src/app/interaction-model/BUILD.gn b/src/app/interaction-model/BUILD.gn index c91c2aedac633e..7525fee12057f5 100644 --- a/src/app/interaction-model/BUILD.gn +++ b/src/app/interaction-model/BUILD.gn @@ -16,13 +16,13 @@ import("//build_overrides/chip.gni") source_set("interaction-model") { sources = [ "Actions.h", + "ActionContex.h", "Events.h", "InvokeResponder.h", "IterationTypes.h", "Model.h", "OperationTypes.h", "Paths.h", - "RequestContext.h", ] public_deps = [ diff --git a/src/app/interaction-model/InvokeResponder.h b/src/app/interaction-model/InvokeResponder.h index 0a399b24564e96..54ae9ba0f51a08 100644 --- a/src/app/interaction-model/InvokeResponder.h +++ b/src/app/interaction-model/InvokeResponder.h @@ -26,7 +26,7 @@ namespace InteractionModel { /// Handles encoding of an invoke response for a specific invoke request. /// -/// This class handles a single response (i.e. a CommandDataIB within the +/// This class handles a single request (i.e. a CommandDataIB within the /// matter protocol) and is responsible for constructing its corresponding /// response (i.e. a InvokeResponseIB within the matter protocol) /// diff --git a/src/app/interaction-model/IterationTypes.h b/src/app/interaction-model/IterationTypes.h index 32ad8bc42b73be..5accbf63fcd652 100644 --- a/src/app/interaction-model/IterationTypes.h +++ b/src/app/interaction-model/IterationTypes.h @@ -70,8 +70,13 @@ struct AttributeEntry /// Iteration rules: /// - kInvalidEndpointId will be returned when iteration ends (or generally kInvalid* for paths) /// - Any internal iteration errors are just logged (callers do not handle iteration CHIP_ERROR) -/// - Iteration order is NOT guaranteed, however uniqueness and completeness is (must iterate -/// over all possible distinct values as long as no internal structural changes occur) +/// - Iteration order is NOT guaranteed globally. Only the following is guaranteed: +/// - when iterating over an endpoint, ALL clusters of that endpoint will be iterated first, before +/// switching the endpoint (order of clusters ids themselves not guaranteed) +/// - when iterating over a cluster, ALL attributes of that cluster will be iterated first, before +/// switching to a new cluster +/// - uniqueness and completeness (iterate over all possible distinct values as long as no +/// internal structural changes occur) class AttributeTreeIterator { public: diff --git a/src/app/interaction-model/Model.h b/src/app/interaction-model/Model.h index b3a127c074aaa1..02d8a647e15871 100644 --- a/src/app/interaction-model/Model.h +++ b/src/app/interaction-model/Model.h @@ -53,7 +53,7 @@ class Model : public AttributeTreeIterator // During the transition phase, we expect a large subset of code to require access to // event emitting, path marking and other operations - virtual InteractionModelActions CurrentActions() { return mActions; } + virtual InteractionModelActions CurrentActions() const { return mActions; } /// List reading has specific handling logic: /// `state` contains in/out data about the current list reading. MUST start with kInvalidListIndex on first call @@ -94,14 +94,14 @@ class Model : public AttributeTreeIterator /// - `NeedsTimedInteraction` for writes that are not timed however are required to be so virtual CHIP_ERROR WriteAttribute(const WriteAttributeRequest & request, AttributeValueDecoder & decoder) = 0; - /// `responder` is used to send back the reply. + /// `reply` is used to send back the reply. /// - calling Reply() or ReplyAsync() will let the application control the reply /// - returning a CHIP_NO_ERROR without reply/reply_async implies a Status::Success reply without data - /// - returning a CHIP_*_ERROR implies an error reply (error and data are mutually exclusive) + /// - returning a value other than CHIP_NO_ERROR implies an error reply (error and data are mutually exclusive) /// /// See InvokeReply/AutoCompleteInvokeResponder for details on how to send back replies and expected - /// error handling. If you require knowledge if a response was successfully sent, use the underlying - /// `reply` object instead of returning an error codes from Invoke. + /// error handling. If you need to know weather a response was successfully sent, use the underlying + /// `reply` object instead of returning an error code from Invoke. /// /// Return codes /// CHIP_IM_GLOBAL_STATUS(code): diff --git a/src/app/interaction-model/OperationTypes.h b/src/app/interaction-model/OperationTypes.h index 115dc11a1ff3bc..640f183ef5ccff 100644 --- a/src/app/interaction-model/OperationTypes.h +++ b/src/app/interaction-model/OperationTypes.h @@ -67,7 +67,7 @@ struct ReadState enum class WriteFlags : uint32_t { kTimed = 0x0001, // Received as a 2nd command after a timed invoke - kListBegin = 0x0002, // This is the FIRST list data element in a series of data + kListBegin = 0x0002, // This is the FIRST list of data elements kListEnd = 0x0004, // This is the LAST list element to write }; @@ -79,7 +79,7 @@ struct WriteAttributeRequest : OperationRequest enum class InvokeFlags : uint32_t { - kTimed = 0x0001, // Received as a 2nd command after a timed invoke + kTimed = 0x0001, // Command received as part of a timed invoke interaction. }; struct InvokeRequest : OperationRequest diff --git a/src/app/interaction-model/Paths.h b/src/app/interaction-model/Paths.h index 2bf9f0c4158011..2cf6937bb4f250 100644 --- a/src/app/interaction-model/Paths.h +++ b/src/app/interaction-model/Paths.h @@ -22,23 +22,21 @@ namespace chip { namespace app { namespace InteractionModel { -/// Handles path attributes for interaction models. +/// Notification listener for attribute changes. /// -/// It allows a user of the class to mark specific paths -/// as having changed. The intended use is for some listener to -/// perform operations as a result of something having changed, -/// usually by forwarding updates (e.g. in case of subscriptions -/// that cover that path). +/// Used to notify that a specific attribute path (or several attributes +/// via wildcards) have changed their underlying content. /// -/// Methods on this class MUCH be called from within the matter +/// Methods on this class MUST be called from within the matter /// main loop as they will likely trigger interaction model -/// internal updates and subscription event updates. +/// internal updates and subscription data reporting. class Paths { public: virtual ~Paths() = 0; - /// Mark some specific attributes dirty. + /// Mark some specific attribute dirty (or several of attributes when using wildcards). + /// /// Wildcards are supported. virtual void MarkDirty(const AttributePathParams & path) = 0; }; From b40bf0a13d9c87cc9ca8973d80a97f4aa0f6e0bf Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:14:24 -0400 Subject: [PATCH 30/79] Restyle --- src/app/interaction-model/Actions.h | 2 +- src/app/interaction-model/BUILD.gn | 2 +- src/app/interaction-model/Paths.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/interaction-model/Actions.h b/src/app/interaction-model/Actions.h index 51574f3596b935..830b349e3b91d9 100644 --- a/src/app/interaction-model/Actions.h +++ b/src/app/interaction-model/Actions.h @@ -16,9 +16,9 @@ */ #pragma once +#include #include #include -#include namespace chip { namespace app { diff --git a/src/app/interaction-model/BUILD.gn b/src/app/interaction-model/BUILD.gn index 7525fee12057f5..6dd4df8e4afa1e 100644 --- a/src/app/interaction-model/BUILD.gn +++ b/src/app/interaction-model/BUILD.gn @@ -15,8 +15,8 @@ import("//build_overrides/chip.gni") source_set("interaction-model") { sources = [ - "Actions.h", "ActionContex.h", + "Actions.h", "Events.h", "InvokeResponder.h", "IterationTypes.h", diff --git a/src/app/interaction-model/Paths.h b/src/app/interaction-model/Paths.h index 2cf6937bb4f250..fdc61e89a2dd7a 100644 --- a/src/app/interaction-model/Paths.h +++ b/src/app/interaction-model/Paths.h @@ -25,7 +25,7 @@ namespace InteractionModel { /// Notification listener for attribute changes. /// /// Used to notify that a specific attribute path (or several attributes -/// via wildcards) have changed their underlying content. +/// via wildcards) have changed their underlying content. /// /// Methods on this class MUST be called from within the matter /// main loop as they will likely trigger interaction model From 4648b3ce3f0ebb226ae7a7d9c8a9539802b7eebf Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:18:32 -0400 Subject: [PATCH 31/79] Rename to InteractionModelContext --- src/app/interaction-model/BUILD.gn | 2 +- src/app/interaction-model/{Actions.h => Context.h} | 8 ++++++-- src/app/interaction-model/Model.h | 10 +++++----- 3 files changed, 12 insertions(+), 8 deletions(-) rename src/app/interaction-model/{Actions.h => Context.h} (78%) diff --git a/src/app/interaction-model/BUILD.gn b/src/app/interaction-model/BUILD.gn index 6dd4df8e4afa1e..83e6fbf7df419b 100644 --- a/src/app/interaction-model/BUILD.gn +++ b/src/app/interaction-model/BUILD.gn @@ -16,7 +16,7 @@ import("//build_overrides/chip.gni") source_set("interaction-model") { sources = [ "ActionContex.h", - "Actions.h", + "Context.h", "Events.h", "InvokeResponder.h", "IterationTypes.h", diff --git a/src/app/interaction-model/Actions.h b/src/app/interaction-model/Context.h similarity index 78% rename from src/app/interaction-model/Actions.h rename to src/app/interaction-model/Context.h index 830b349e3b91d9..5f7f442664ff20 100644 --- a/src/app/interaction-model/Actions.h +++ b/src/app/interaction-model/Context.h @@ -25,11 +25,15 @@ namespace app { namespace InteractionModel { /// Data provided to data models in order to interface with the interaction model environment. -struct InteractionModelActions +/// +/// Provides callback-style functionality to notify the interaction model of changes +/// (e.g. using paths to notify of attribute data changes or events to generate events) +/// as well as fetching current state (via actionContext) +struct InteractionModelContext { Events * events; Paths * paths; - ActionContext * requestContext; + ActionContext * actionContext; }; } // namespace InteractionModel diff --git a/src/app/interaction-model/Model.h b/src/app/interaction-model/Model.h index 02d8a647e15871..291e5967c8cfa3 100644 --- a/src/app/interaction-model/Model.h +++ b/src/app/interaction-model/Model.h @@ -22,7 +22,7 @@ #include #include -#include +#include #include #include #include @@ -44,16 +44,16 @@ class Model : public AttributeTreeIterator virtual ~Model() = default; // `actions` pointers will be guaranteed valid until Shutdown is called() - virtual CHIP_ERROR Startup(InteractionModelActions actions) + virtual CHIP_ERROR Startup(InteractionModelContext actions) { - mActions = actions; + mContext = actions; return CHIP_NO_ERROR; } virtual CHIP_ERROR Shutdown() = 0; // During the transition phase, we expect a large subset of code to require access to // event emitting, path marking and other operations - virtual InteractionModelActions CurrentActions() const { return mActions; } + virtual InteractionModelContext CurrentActions() const { return mContext; } /// List reading has specific handling logic: /// `state` contains in/out data about the current list reading. MUST start with kInvalidListIndex on first call @@ -115,7 +115,7 @@ class Model : public AttributeTreeIterator virtual CHIP_ERROR Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, InvokeReply & reply) = 0; private: - InteractionModelActions mActions = { nullptr }; + InteractionModelContext mContext = { nullptr }; }; } // namespace InteractionModel From 9ec35cf99b36e45dd44fd163cd341a9839ddbf33 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:19:44 -0400 Subject: [PATCH 32/79] one more rename --- src/app/interaction-model/Events.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index 86e121c9e9a327..efa72308bfee36 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -33,10 +33,10 @@ namespace InteractionModel { namespace internal { template -class SimpleEventLoggingDelegate : public EventLoggingDelegate +class SimpleEventPayloadWriter : public EventLoggingDelegate { public: - SimpleEventLoggingDelegate(const T & aEventData) : mEventData(aEventData){}; + SimpleEventPayloadWriter(const T & aEventData) : mEventData(aEventData){}; CHIP_ERROR WriteEvent(chip::TLV::TLVWriter & aWriter) final override { return DataModel::Encode(aWriter, TLV::ContextTag(EventDataIB::Tag::kData), mEventData); @@ -49,7 +49,7 @@ class SimpleEventLoggingDelegate : public EventLoggingDelegate template ::value, bool> = true> std::optional GenerateEvent(G & generator, const T & aEventData, EndpointId aEndpoint) { - internal::SimpleEventLoggingDelegate eventPayloadWriter(aEventData); + internal::SimpleEventPayloadWriter eventPayloadWriter(aEventData); ConcreteEventPath path(aEndpoint, aEventData.GetClusterId(), aEventData.GetEventId()); EventOptions eventOptions; eventOptions.mPath = path; @@ -86,7 +86,7 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En template ::value, bool> = true> std::optional GenerateEvent(G & generator, const T & aEventData, EndpointId endpointId) { - internal::SimpleEventLoggingDelegate eventPayloadWriter(aEventData); + internal::SimpleEventPayloadWriter eventPayloadWriter(aEventData); ConcreteEventPath path(endpointId, aEventData.GetClusterId(), aEventData.GetEventId()); EventOptions eventOptions; eventOptions.mPath = path; From 292c1c9bf2289a00f9ee188be483e1a5a05f0170 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:22:44 -0400 Subject: [PATCH 33/79] Fix typo --- src/app/interaction-model/BUILD.gn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/interaction-model/BUILD.gn b/src/app/interaction-model/BUILD.gn index 83e6fbf7df419b..a0967289c6c65c 100644 --- a/src/app/interaction-model/BUILD.gn +++ b/src/app/interaction-model/BUILD.gn @@ -15,7 +15,7 @@ import("//build_overrides/chip.gni") source_set("interaction-model") { sources = [ - "ActionContex.h", + "ActionContext.h", "Context.h", "Events.h", "InvokeResponder.h", From 55c715d47e78308e1f73d63d791357c167d1b029 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:25:52 -0400 Subject: [PATCH 34/79] Fix tests to compile --- .../tests/TestEventEmitting.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/app/interaction-model/tests/TestEventEmitting.cpp b/src/app/interaction-model/tests/TestEventEmitting.cpp index cb49dc25caa068..2bf9661fd27809 100644 --- a/src/app/interaction-model/tests/TestEventEmitting.cpp +++ b/src/app/interaction-model/tests/TestEventEmitting.cpp @@ -100,8 +100,10 @@ TEST(TestInteractionModelEventEmitting, TestBasicType) StartUpEventType event{ kFakeSoftwareVersion }; - EventNumber n1 = events->GenerateEvent(event, 0 /* EndpointId */); - ASSERT_EQ(n1, logOnlyEvents.CurrentEventNumber()); + std::optional n1 = events->GenerateEvent(event, 0 /* EndpointId */); + + ASSERT_TRUE(n1.has_value()); + ASSERT_EQ(*n1, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(0 /* endpointId */, StartUpEventType::GetClusterId(), StartUpEventType::GetEventId())); @@ -115,8 +117,9 @@ TEST(TestInteractionModelEventEmitting, TestBasicType) ASSERT_EQ(err, CHIP_NO_ERROR); ASSERT_EQ(decoded_event.softwareVersion, kFakeSoftwareVersion); - EventNumber n2 = events->GenerateEvent(event, /* endpointId = */ 1); - ASSERT_EQ(n2, logOnlyEvents.CurrentEventNumber()); + std::optional n2 = events->GenerateEvent(event, /* endpointId = */ 1); + ASSERT_TRUE(n2.has_value()); + ASSERT_EQ(*n2, logOnlyEvents.CurrentEventNumber()); ASSERT_NE(n1, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, @@ -137,14 +140,14 @@ TEST(TestInteractionModelEventEmitting, TestFabricScoped) event.adminNodeID = chip::app::DataModel::MakeNullable(kTestNodeId); event.adminPasscodeID = chip::app::DataModel::MakeNullable(kTestPasscode); - EventNumber n1 = events->GenerateEvent(event, 0 /* EndpointId */); + std::optional n1 = events->GenerateEvent(event, 0 /* EndpointId */); // encoding without a fabric ID MUST fail for fabric events - ASSERT_EQ(n1, kInvalidEventId); + ASSERT_FALSE(n1.has_value()); event.fabricIndex = kTestFabricIndex; n1 = events->GenerateEvent(event, /* endpointId = */ 0); - ASSERT_NE(n1, kInvalidEventId); + ASSERT_TRUE(n1.has_value()); ASSERT_EQ(n1, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(0 /* endpointId */, AccessControlEntryChangedType::GetClusterId(), From d34e51d1e20ea78d33ef85fd3a02b85cec980f86 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:29:04 -0400 Subject: [PATCH 35/79] More renames of actions to context --- src/app/interaction-model/Model.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/interaction-model/Model.h b/src/app/interaction-model/Model.h index 291e5967c8cfa3..151065f4c540d2 100644 --- a/src/app/interaction-model/Model.h +++ b/src/app/interaction-model/Model.h @@ -43,17 +43,17 @@ class Model : public AttributeTreeIterator public: virtual ~Model() = default; - // `actions` pointers will be guaranteed valid until Shutdown is called() - virtual CHIP_ERROR Startup(InteractionModelContext actions) + // `context` pointers will be guaranteed valid until Shutdown is called() + virtual CHIP_ERROR Startup(InteractionModelContext context) { - mContext = actions; + mContext = context; return CHIP_NO_ERROR; } virtual CHIP_ERROR Shutdown() = 0; // During the transition phase, we expect a large subset of code to require access to // event emitting, path marking and other operations - virtual InteractionModelContext CurrentActions() const { return mContext; } + virtual InteractionModelContext CurrentContext() const { return mContext; } /// List reading has specific handling logic: /// `state` contains in/out data about the current list reading. MUST start with kInvalidListIndex on first call From 5332c0b0d5a2a91edd44ce1518b500ca383a46cf Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 10:42:38 -0400 Subject: [PATCH 36/79] One more comment added --- src/app/interaction-model/Events.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index efa72308bfee36..f127f214c592ab 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -104,6 +104,10 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En } // namespace internal +/// Exposes event access capabilities. +/// +/// Allows callers to "generate events" which effectively notifies of an event having +/// ocurred. class Events { public: From b95086cb16c489195bcc71c178cff719a6ae85d2 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 11:39:38 -0400 Subject: [PATCH 37/79] Restyle --- src/app/interaction-model/Events.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index f127f214c592ab..87c479d831fd71 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -107,7 +107,7 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En /// Exposes event access capabilities. /// /// Allows callers to "generate events" which effectively notifies of an event having -/// ocurred. +/// ocurred. class Events { public: From 8ea6206f9ba97db5bedcca31e3724d7665b4e5e9 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 12:29:45 -0400 Subject: [PATCH 38/79] Address review comments --- .../CodegenDataModel.cpp | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 5bd7741722c471..2304be2f45e55a 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -36,7 +36,7 @@ bool IsServerMask(EmberAfClusterMask mask) /// Load the cluster information into the specified destination void LoadClusterInfo(const ConcreteClusterPath & path, const EmberAfCluster & cluster, InteractionModel::ClusterInfo * info) { - chip::DataVersion * versionPtr = emberAfDataVersionStorage(path); + DataVersion * versionPtr = emberAfDataVersionStorage(path); if (versionPtr != nullptr) { info->dataVersion = *versionPtr; @@ -109,7 +109,7 @@ InteractionModel::AttributeEntry AttributeEntryFrom(const ConcreteClusterPath & } InteractionModel::AttributeEntry AttributeEntryForGlobalListAttribute(const ConcreteClusterPath & clusterPath, - chip::AttributeId globalAttributeId) + AttributeId globalAttributeId) { InteractionModel::AttributeEntry entry; @@ -135,7 +135,7 @@ CHIP_ERROR CodegenDataModel::WriteAttribute(const InteractionModel::WriteAttribu return CHIP_ERROR_NOT_IMPLEMENTED; } -CHIP_ERROR CodegenDataModel::Invoke(const InteractionModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments, +CHIP_ERROR CodegenDataModel::Invoke(const InteractionModel::InvokeRequest & request, TLV::TLVReader & input_arguments, InteractionModel::InvokeReply & reply) { // TODO: this needs an implementation @@ -150,6 +150,7 @@ EndpointId CodegenDataModel::FirstEndpoint() { if (emberAfEndpointIndexIsEnabled(endpoint_idx)) { + mEndpointIterationHint = endpoint_idx; return emberAfEndpointFromIndex(endpoint_idx); } } @@ -158,7 +159,7 @@ EndpointId CodegenDataModel::FirstEndpoint() return kInvalidEndpointId; } -std::optional CodegenDataModel::TryFindEndpointIndex(chip::EndpointId id) const +std::optional CodegenDataModel::TryFindEndpointIndex(EndpointId id) const { const uint16_t lastEndpointIndex = emberAfEndpointCount(); @@ -169,20 +170,13 @@ std::optional CodegenDataModel::TryFindEndpointIndex(chip::EndpointId } // Linear search, this may be slow - for (uint16_t endpoint_idx = 0; endpoint_idx < lastEndpointIndex; endpoint_idx++) + uint16_t idx = emberAfIndexFromEndpoint(id); + if (idx == kEmberInvalidEndpointIndex) { - if (!emberAfEndpointIndexIsEnabled(endpoint_idx)) - { - continue; - } - - if (id == emberAfEndpointFromIndex(endpoint_idx)) - { - return std::make_optional(endpoint_idx); - } + return std::nullopt; } - - return std::nullopt; + + return std::make_optional(idx); } EndpointId CodegenDataModel::NextEndpoint(EndpointId before) @@ -219,7 +213,7 @@ InteractionModel::ClusterEntry CodegenDataModel::FirstCluster(EndpointId endpoin return FirstServerClusterEntry(endpointId, endpoint, 0, mClusterIterationHint); } -std::optional CodegenDataModel::TryFindServerClusterIndex(const EmberAfEndpointType * endpoint, chip::ClusterId id) const +std::optional CodegenDataModel::TryFindServerClusterIndex(const EmberAfEndpointType * endpoint, ClusterId id) const { const unsigned clusterCount = endpoint->clusterCount; @@ -233,6 +227,8 @@ std::optional CodegenDataModel::TryFindServerClusterIndex(const EmberA } // linear search, this may be slow + // does NOT use emberAfClusterIndex to not iteratoe over endpoints as we have + // already found the correct endpoint for (unsigned cluster_idx = 0; cluster_idx < clusterCount; cluster_idx++) { const EmberAfCluster & cluster = endpoint->cluster[cluster_idx]; @@ -284,10 +280,11 @@ InteractionModel::AttributeEntry CodegenDataModel::FirstAttribute(const Concrete VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); + mAttributeIterationHint = 0; return AttributeEntryFrom(path, cluster->attributes[0]); } -std::optional CodegenDataModel::TryFindAttributeIndex(const EmberAfCluster * cluster, chip::AttributeId id) const +std::optional CodegenDataModel::TryFindAttributeIndex(const EmberAfCluster * cluster, AttributeId id) const { const unsigned attributeCount = cluster->attributeCount; From 66589f5cd1c10a57cb21ef80fc406729ca66aa97 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 12:30:10 -0400 Subject: [PATCH 39/79] Restyle --- src/app/codegen-interaction-model/CodegenDataModel.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 2304be2f45e55a..cdc0cc3c184535 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -175,7 +175,7 @@ std::optional CodegenDataModel::TryFindEndpointIndex(EndpointId id) co { return std::nullopt; } - + return std::make_optional(idx); } @@ -227,7 +227,7 @@ std::optional CodegenDataModel::TryFindServerClusterIndex(const EmberA } // linear search, this may be slow - // does NOT use emberAfClusterIndex to not iteratoe over endpoints as we have + // does NOT use emberAfClusterIndex to not iteratoe over endpoints as we have // already found the correct endpoint for (unsigned cluster_idx = 0; cluster_idx < clusterCount; cluster_idx++) { From 81e268ddd1d91e0c1e40defec0c00abe579ee55d Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 13:09:22 -0400 Subject: [PATCH 40/79] make clang-tidy happy --- src/app/interaction-model/tests/TestEventEmitting.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/app/interaction-model/tests/TestEventEmitting.cpp b/src/app/interaction-model/tests/TestEventEmitting.cpp index 2bf9661fd27809..31632fbfece2bc 100644 --- a/src/app/interaction-model/tests/TestEventEmitting.cpp +++ b/src/app/interaction-model/tests/TestEventEmitting.cpp @@ -103,7 +103,7 @@ TEST(TestInteractionModelEventEmitting, TestBasicType) std::optional n1 = events->GenerateEvent(event, 0 /* EndpointId */); ASSERT_TRUE(n1.has_value()); - ASSERT_EQ(*n1, logOnlyEvents.CurrentEventNumber()); + ASSERT_EQ(*n1, logOnlyEvents.CurrentEventNumber()); // NOLINT(bugprone-unchecked-optional-access) ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(0 /* endpointId */, StartUpEventType::GetClusterId(), StartUpEventType::GetEventId())); @@ -119,7 +119,7 @@ TEST(TestInteractionModelEventEmitting, TestBasicType) std::optional n2 = events->GenerateEvent(event, /* endpointId = */ 1); ASSERT_TRUE(n2.has_value()); - ASSERT_EQ(*n2, logOnlyEvents.CurrentEventNumber()); + ASSERT_EQ(*n2, logOnlyEvents.CurrentEventNumber()); // NOLINT(bugprone-unchecked-optional-access) ASSERT_NE(n1, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, @@ -148,7 +148,7 @@ TEST(TestInteractionModelEventEmitting, TestFabricScoped) n1 = events->GenerateEvent(event, /* endpointId = */ 0); ASSERT_TRUE(n1.has_value()); - ASSERT_EQ(n1, logOnlyEvents.CurrentEventNumber()); + ASSERT_EQ(*n1, logOnlyEvents.CurrentEventNumber()); // NOLINT(bugprone-unchecked-optional-access) ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(0 /* endpointId */, AccessControlEntryChangedType::GetClusterId(), AccessControlEntryChangedType::GetEventId())); From 2f548a6849a5e2370e6053963d8c3b08158f2f0c Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 4 Jun 2024 13:11:41 -0400 Subject: [PATCH 41/79] Operator== exists on optional ... make use of that directly --- src/app/interaction-model/tests/TestEventEmitting.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/app/interaction-model/tests/TestEventEmitting.cpp b/src/app/interaction-model/tests/TestEventEmitting.cpp index 31632fbfece2bc..3196636f621653 100644 --- a/src/app/interaction-model/tests/TestEventEmitting.cpp +++ b/src/app/interaction-model/tests/TestEventEmitting.cpp @@ -102,8 +102,7 @@ TEST(TestInteractionModelEventEmitting, TestBasicType) std::optional n1 = events->GenerateEvent(event, 0 /* EndpointId */); - ASSERT_TRUE(n1.has_value()); - ASSERT_EQ(*n1, logOnlyEvents.CurrentEventNumber()); // NOLINT(bugprone-unchecked-optional-access) + ASSERT_EQ(n1, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(0 /* endpointId */, StartUpEventType::GetClusterId(), StartUpEventType::GetEventId())); @@ -118,9 +117,7 @@ TEST(TestInteractionModelEventEmitting, TestBasicType) ASSERT_EQ(decoded_event.softwareVersion, kFakeSoftwareVersion); std::optional n2 = events->GenerateEvent(event, /* endpointId = */ 1); - ASSERT_TRUE(n2.has_value()); - ASSERT_EQ(*n2, logOnlyEvents.CurrentEventNumber()); // NOLINT(bugprone-unchecked-optional-access) - ASSERT_NE(n1, logOnlyEvents.CurrentEventNumber()); + ASSERT_EQ(n2, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(1 /* endpointId */, StartUpEventType::GetClusterId(), StartUpEventType::GetEventId())); @@ -147,8 +144,7 @@ TEST(TestInteractionModelEventEmitting, TestFabricScoped) event.fabricIndex = kTestFabricIndex; n1 = events->GenerateEvent(event, /* endpointId = */ 0); - ASSERT_TRUE(n1.has_value()); - ASSERT_EQ(*n1, logOnlyEvents.CurrentEventNumber()); // NOLINT(bugprone-unchecked-optional-access) + ASSERT_EQ(n1, logOnlyEvents.CurrentEventNumber()); ASSERT_EQ(logOnlyEvents.LastOptions().mPath, ConcreteEventPath(0 /* endpointId */, AccessControlEntryChangedType::GetClusterId(), AccessControlEntryChangedType::GetEventId())); From 981b1a796f6b21076aeb3eb33cddb74ea4e6d593 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 13:41:27 -0400 Subject: [PATCH 42/79] Started renaming things --- src/app/interaction-model/BUILD.gn | 2 +- .../{IterationTypes.h => MetadataTypes.h} | 66 ++++++++++++++++--- 2 files changed, 57 insertions(+), 11 deletions(-) rename src/app/interaction-model/{IterationTypes.h => MetadataTypes.h} (55%) diff --git a/src/app/interaction-model/BUILD.gn b/src/app/interaction-model/BUILD.gn index a0967289c6c65c..058a61fb4ef0b7 100644 --- a/src/app/interaction-model/BUILD.gn +++ b/src/app/interaction-model/BUILD.gn @@ -19,7 +19,7 @@ source_set("interaction-model") { "Context.h", "Events.h", "InvokeResponder.h", - "IterationTypes.h", + "MetadataTypes.h", "Model.h", "OperationTypes.h", "Paths.h", diff --git a/src/app/interaction-model/IterationTypes.h b/src/app/interaction-model/MetadataTypes.h similarity index 55% rename from src/app/interaction-model/IterationTypes.h rename to src/app/interaction-model/MetadataTypes.h index d163dbaf06548a..93ce3fb0ccdb85 100644 --- a/src/app/interaction-model/IterationTypes.h +++ b/src/app/interaction-model/MetadataTypes.h @@ -19,8 +19,10 @@ #include #include +#include #include #include +#include #include #include @@ -54,13 +56,20 @@ struct ClusterEntry enum class AttributeQualityFlags : uint32_t { - kListAttribute = 0x0001, // This attribute is a list attribute - kChangesOmitted = 0x0002, // `C` quality on attributes + kListAttribute = 0x0004, // This attribute is a list attribute + kFabricScoped = 0x0008, // 'F' quality on attributes + kFabricSensitive = 0x0010, // 'S' quality on attributes + kChangesOmitted = 0x0020, // `C` quality on attributes + kTimed = 0x0040, // `T` quality on attributes (writes require timed interactions) }; struct AttributeInfo { BitFlags flags; + + // read/write access will be missing if read/write is NOT allowed + std::optional readPrivilege; // generally defaults to View if readable + std::optional writePrivilege; // generally defaults to Operate if writable }; struct AttributeEntry @@ -76,25 +85,50 @@ struct AttributeEntry } }; +enum class CommandQualityFlags : uint32_t +{ + kFabricScoped = 0x0001, + kFabricSensitive = 0x0002, + kTimed = 0x0004, // `T` quality on commands +}; + +struct CommandInfo +{ + BitFlags flags; + Access::Privilege invokePrivilege = Access::Privilege::kOperate; +}; + +struct CommandEntry +{ + ConcreteCommandPath path; + CommandInfo info; + + static CommandEntry Invalid() + { + CommandEntry result; + result.path = ConcreteCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId); + return result; + } +}; + /// Provides metadata information for a data model /// -/// The data model can be viewed as a tree of endpoint/cluster/attribute -/// where each element can be iterated through independently +/// The data model can be viewed as a tree of endpoint/cluster/(attribute+commands+events) +/// where each element can be iterated through independently. /// /// Iteration rules: /// - kInvalidEndpointId will be returned when iteration ends (or generally kInvalid* for paths) +/// - Global Attributes are NOT returned since they are implied /// - Any internal iteration errors are just logged (callers do not handle iteration CHIP_ERROR) /// - Iteration order is NOT guaranteed globally. Only the following is guaranteed: -/// - when iterating over an endpoint, ALL clusters of that endpoint will be iterated first, before -/// switching the endpoint (order of clusters ids themselves not guaranteed) -/// - when iterating over a cluster, ALL attributes of that cluster will be iterated first, before -/// switching to a new cluster +/// - Complete tree iteration (e.g. when iterating an endpoint, ALL clusters of that endpoint +/// are returned, when iterating over a cluster, all attributes/commands are iterated over) /// - uniqueness and completeness (iterate over all possible distinct values as long as no /// internal structural changes occur) -class AttributeTreeIterator +class DataModelMetadataTree { public: - virtual ~AttributeTreeIterator() = default; + virtual ~DataModelMetadataTree() = default; virtual EndpointId FirstEndpoint() = 0; virtual EndpointId NextEndpoint(EndpointId before) = 0; @@ -103,9 +137,21 @@ class AttributeTreeIterator virtual ClusterEntry NextCluster(const ConcreteClusterPath & before) = 0; virtual std::optional GetClusterInfo(const ConcreteClusterPath & path) = 0; + // Attribute iteration and accessors provide cluster-level access over + // attributes virtual AttributeEntry FirstAttribute(const ConcreteClusterPath & cluster) = 0; virtual AttributeEntry NextAttribute(const ConcreteAttributePath & before) = 0; virtual std::optional GetAttributeInfo(const ConcreteAttributePath & path) = 0; + + // Command iteration and accessors provide cluster-level access over commands + virtual CommandEntry FirstAcceptedCommand(const ConcreteClusterPath & cluster) = 0; + virtual CommandEntry NextAcceptedCommand(const ConcreteCommandPath & before) = 0; + virtual std::optional GetAcceptedCommandInfo(const ConcreteCommandPath & path) = 0; + + // "generated" commands are purely for reporting what types of command ids can be + // returned as responses. + virtual ConcreteCommandPath FirstGeneratedCommand(const ConcreteClusterPath & cluster) = 0; + virtual ConcreteCommandPath NextGeneratedCommand(const ConcreteCommandPath & before) = 0; }; } // namespace InteractionModel From 738e3c25c24ee29c144fb6a632fe28eef21ab560 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 13:42:17 -0400 Subject: [PATCH 43/79] Use the right types in Model.h --- src/app/interaction-model/Model.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/interaction-model/Model.h b/src/app/interaction-model/Model.h index 151065f4c540d2..0ce8e7fdd4f419 100644 --- a/src/app/interaction-model/Model.h +++ b/src/app/interaction-model/Model.h @@ -24,7 +24,7 @@ #include #include -#include +#include #include namespace chip { @@ -38,7 +38,7 @@ namespace InteractionModel { /// thread or equivalent /// - class is allowed to attempt to cache indexes/locations for faster /// lookups of things (e.g during iterations) -class Model : public AttributeTreeIterator +class Model : public DataModelMetadataTree { public: virtual ~Model() = default; From 18185e05065c3a4665d079ff1e3c2e19e2d1a93b Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 13:47:21 -0400 Subject: [PATCH 44/79] Make things compile --- .../CodegenDataModel.cpp | 30 +++++++++++++++++++ .../CodegenDataModel.h | 7 +++++ 2 files changed, 37 insertions(+) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index cdc0cc3c184535..1dc42bfd0b8a75 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -404,5 +404,35 @@ std::optional CodegenDataModel::GetAttributeInf return std::make_optional(info); } +InteractionModel::CommandEntry CodegenDataModel::FirstAcceptedCommand(const ConcreteClusterPath & cluster) +{ + // FIXME: implement + return InteractionModel::CommandEntry::Invalid(); +} + +InteractionModel::CommandEntry CodegenDataModel::NextAcceptedCommand(const ConcreteCommandPath & before) +{ + // FIXME: implement + return InteractionModel::CommandEntry::Invalid(); +} + +std::optional CodegenDataModel::GetAcceptedCommandInfo(const ConcreteCommandPath & path) +{ + // FIXME: implement + return std::nullopt; +} + +ConcreteCommandPath CodegenDataModel::FirstGeneratedCommand(const ConcreteClusterPath & cluster) +{ + // FIXME: implement + return ConcreteCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId); +} + +ConcreteCommandPath CodegenDataModel::NextGeneratedCommand(const ConcreteCommandPath & before) +{ + // FIXME: implement + return ConcreteCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId); +} + } // namespace app } // namespace chip diff --git a/src/app/codegen-interaction-model/CodegenDataModel.h b/src/app/codegen-interaction-model/CodegenDataModel.h index 712405105daf09..c57314c5f63bdc 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.h +++ b/src/app/codegen-interaction-model/CodegenDataModel.h @@ -59,6 +59,13 @@ class CodegenDataModel : public chip::app::InteractionModel::Model InteractionModel::AttributeEntry NextAttribute(const ConcreteAttributePath & before) override; std::optional GetAttributeInfo(const ConcreteAttributePath & path) override; + InteractionModel::CommandEntry FirstAcceptedCommand(const ConcreteClusterPath & cluster) override; + InteractionModel::CommandEntry NextAcceptedCommand(const ConcreteCommandPath & before) override; + std::optional GetAcceptedCommandInfo(const ConcreteCommandPath & path) override; + + ConcreteCommandPath FirstGeneratedCommand(const ConcreteClusterPath & cluster) override; + ConcreteCommandPath NextGeneratedCommand(const ConcreteCommandPath & before) override; + private: // Iteration is often done in a tight loop going through all values. // To avoid N^2 iterations, cache a hint of where something is positioned From f0a4893a3d50cfa37aa95a2f60b297d8e865677c Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 13:53:35 -0400 Subject: [PATCH 45/79] Skip global attribute handling, add TODOs for reading extra bits from ember --- .../CodegenDataModel.cpp | 59 ++++--------------- .../tests/TestCodegenModelViaMocks.cpp | 23 +------- 2 files changed, 13 insertions(+), 69 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 1dc42bfd0b8a75..c0d7f00a42c0f7 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -95,6 +95,14 @@ void LoadAttributeInfo(const ConcreteAttributePath & path, const EmberAfAttribut // TODO: Set additional flags: // info->flags.Set(InteractionModel::AttributeQualityFlags::kChangesOmitted) + // info->flags.Set(InteractionModel::AttributeQualityFlags::kFabricScoped) + // info->flags.Set(InteractionModel::AttributeQualityFlags::kFabricSensitive) + // info->flags.Set(InteractionModel::AttributeQualityFlags::kChangesOmitted) + // info->flags.Set(InteractionModel::AttributeQualityFlags::kTimed) + + // TODO: load privileges (and decide if readable/writable) + info->readPrivilege = Access::Privilege::kView; + info->writePrivilege = Access::Privilege::kOperate; } InteractionModel::AttributeEntry AttributeEntryFrom(const ConcreteClusterPath & clusterPath, @@ -108,17 +116,6 @@ InteractionModel::AttributeEntry AttributeEntryFrom(const ConcreteClusterPath & return entry; } -InteractionModel::AttributeEntry AttributeEntryForGlobalListAttribute(const ConcreteClusterPath & clusterPath, - AttributeId globalAttributeId) -{ - InteractionModel::AttributeEntry entry; - - entry.path = ConcreteAttributePath(clusterPath.mEndpointId, clusterPath.mClusterId, globalAttributeId); - entry.info.flags.Set(InteractionModel::AttributeQualityFlags::kListAttribute); - - return entry; -} - } // namespace CHIP_ERROR CodegenDataModel::ReadAttribute(const InteractionModel::ReadAttributeRequest & request, @@ -330,24 +327,6 @@ InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteA VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); - // Handles global attribute iteration: if we got a global attribute, move to the next - switch (before.mAttributeId) - { - case Clusters::Globals::Attributes::GeneratedCommandList::Id: - return AttributeEntryForGlobalListAttribute(before, Clusters::Globals::Attributes::AcceptedCommandList::Id); - case Clusters::Globals::Attributes::AcceptedCommandList::Id: -#if CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE - return AttributeEntryForGlobalListAttribute(before, Clusters::Globals::Attributes::EventList::Id); - case Clusters::Globals::Attributes::EventList::Id: -#endif // CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE - return AttributeEntryForGlobalListAttribute(before, Clusters::Globals::Attributes::AttributeList::Id); - case Clusters::Globals::Attributes::AttributeList::Id: - return InteractionModel::AttributeEntry::Invalid(); - default: - // pass-through: not a global attribute, try to find a "regular" attribute - break; - } - // find the given attribute in the list and then return the next one std::optional attribute_idx = TryFindAttributeIndex(cluster, before.mAttributeId); if (!attribute_idx.has_value()) @@ -362,9 +341,8 @@ InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteA return AttributeEntryFrom(before, cluster->attributes[next_idx]); } - // We reach here if next_idx is just past the last attribute metadata. Return the first global - // attribute - return AttributeEntryForGlobalListAttribute(before, Clusters::Globals::Attributes::GeneratedCommandList::Id); + // iteration complete + return InteractionModel::AttributeEntry::Invalid(); } std::optional CodegenDataModel::GetAttributeInfo(const ConcreteAttributePath & path) @@ -375,23 +353,6 @@ std::optional CodegenDataModel::GetAttributeInf VerifyOrReturnValue(cluster->attributeCount > 0, std::nullopt); VerifyOrReturnValue(cluster->attributes != nullptr, std::nullopt); - switch (path.mAttributeId) - { - case Clusters::Globals::Attributes::GeneratedCommandList::Id: - case Clusters::Globals::Attributes::AcceptedCommandList::Id: -#if CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE - case Clusters::Globals::Attributes::EventList::Id: -#endif // CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE - case Clusters::Globals::Attributes::AttributeList::Id: { - InteractionModel::AttributeInfo info; - info.flags.Set(InteractionModel::AttributeQualityFlags::kListAttribute); - return info; - } - default: - // pass-through: not a global attribute, try to find a "regular" attribute - break; - } - std::optional attribute_idx = TryFindAttributeIndex(cluster, path.mAttributeId); if (!attribute_idx.has_value()) diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index ace5236ebfd2a1..62a6958a3b69fa 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -17,7 +17,6 @@ */ #include -#include #include #include #include @@ -240,21 +239,6 @@ TEST(TestCodegenModelViaMocks, IterateOverAttributes) ASSERT_EQ(entry.path.mAttributeId, MockAttributeId(2)); ASSERT_TRUE(entry.info.flags.Has(AttributeQualityFlags::kListAttribute)); - // Iteration MUST include global attributes. Ember does not provide those, so we - // assert here that we present them in order - for (auto globalAttributeId : GlobalAttributesNotInMetadata) - { - - entry = model.NextAttribute(entry.path); - ASSERT_TRUE(entry.path.HasValidIds()); - ASSERT_EQ(entry.path.mEndpointId, kMockEndpoint2); - ASSERT_EQ(entry.path.mClusterId, MockClusterId(2)); - ASSERT_EQ(entry.path.mAttributeId, globalAttributeId); - - // all global attributes not in ember metadata are LIST typed - ASSERT_TRUE(entry.info.flags.Has(AttributeQualityFlags::kListAttribute)); - } - entry = model.NextAttribute(entry.path); ASSERT_FALSE(entry.path.HasValidIds()); @@ -306,6 +290,7 @@ TEST(TestCodegenModelViaMocks, GetAttributeInfo) EXPECT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access) } +// global attributes are EXPLICITLY not supported TEST(TestCodegenModelViaMocks, GlobalAttributeInfo) { UseMockNodeConfig config(gTestNodeConfig); @@ -314,11 +299,9 @@ TEST(TestCodegenModelViaMocks, GlobalAttributeInfo) std::optional info = model.GetAttributeInfo( ConcreteAttributePath(kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::GeneratedCommandList::Id)); - ASSERT_TRUE(info.has_value()); - EXPECT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access) + ASSERT_FALSE(info.has_value()); info = model.GetAttributeInfo( ConcreteAttributePath(kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::AttributeList::Id)); - ASSERT_TRUE(info.has_value()); - EXPECT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access) + ASSERT_FALSE(info.has_value()); } From 54273270a19238f11aa1f0ca4df07185e309daac Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 13:56:13 -0400 Subject: [PATCH 46/79] Typo fix --- src/app/codegen-interaction-model/CodegenDataModel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index c0d7f00a42c0f7..089ae04f08dcc0 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -224,7 +224,7 @@ std::optional CodegenDataModel::TryFindServerClusterIndex(const EmberA } // linear search, this may be slow - // does NOT use emberAfClusterIndex to not iteratoe over endpoints as we have + // does NOT use emberAfClusterIndex to not iterate over endpoints as we have // already found the correct endpoint for (unsigned cluster_idx = 0; cluster_idx < clusterCount; cluster_idx++) { From 543c45620fd27e2c5fc05254bac3166c65ffbbe5 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 14:12:04 -0400 Subject: [PATCH 47/79] Several flags and correct loading of privileges for attributes --- .../CodegenDataModel.cpp | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 089ae04f08dcc0..0458b41521d7f0 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -16,6 +16,7 @@ */ #include +#include #include #include #include @@ -85,24 +86,37 @@ InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, co } /// 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). void LoadAttributeInfo(const ConcreteAttributePath & path, const EmberAfAttributeMetadata & attribute, InteractionModel::AttributeInfo * info) { + info->readPrivilege = RequiredPrivilege::ForReadAttribute(path); + if (attribute.IsReadOnly()) + { + info->writePrivilege = RequiredPrivilege::ForWriteAttribute(path); + } + if (attribute.attributeType == ZCL_ARRAY_ATTRIBUTE_TYPE) { info->flags.Set(InteractionModel::AttributeQualityFlags::kListAttribute); } + if (attribute.MustUseTimedWrite()) + { + info->flags.Set(InteractionModel::AttributeQualityFlags::kTimed); + } + + // NOTE: we do NOT provide additional info for: + // - IsExternal/IsSingleton/IsAutomaticallyPersisted is not used by IM handling + // - IsSingleton spec defines it for CLUSTERS where as we have it for ATTRIBUTES + // - Several specification flags are not available (reportable, quieter reporting, + // fixed, source attribution) + // TODO: Set additional flags: - // info->flags.Set(InteractionModel::AttributeQualityFlags::kChangesOmitted) // info->flags.Set(InteractionModel::AttributeQualityFlags::kFabricScoped) // info->flags.Set(InteractionModel::AttributeQualityFlags::kFabricSensitive) // info->flags.Set(InteractionModel::AttributeQualityFlags::kChangesOmitted) - // info->flags.Set(InteractionModel::AttributeQualityFlags::kTimed) - - // TODO: load privileges (and decide if readable/writable) - info->readPrivilege = Access::Privilege::kView; - info->writePrivilege = Access::Privilege::kOperate; } InteractionModel::AttributeEntry AttributeEntryFrom(const ConcreteClusterPath & clusterPath, From fc9ad52d359427b51e70f564d1015ff6b3916271 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 14:22:07 -0400 Subject: [PATCH 48/79] Start implementing command iteration ... still feels awkward and caching will be a pain --- .../CodegenDataModel.cpp | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 0458b41521d7f0..9aa23f34c7c50f 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -16,8 +16,8 @@ */ #include -#include #include +#include #include #include #include @@ -130,6 +130,25 @@ InteractionModel::AttributeEntry AttributeEntryFrom(const ConcreteClusterPath & return entry; } +InteractionModel::CommandEntry CommandEntryFrom(const ConcreteClusterPath & clusterPath, CommandId clusterCommandId, + const EmberAfCluster * emberCluster) +{ + InteractionModel::CommandEntry entry; + entry.path = ConcreteCommandPath(clusterPath.mEndpointId, clusterPath.mClusterId, clusterCommandId); + entry.info.invokePrivilege = RequiredPrivilege::ForInvokeCommand(entry.path); + + if (CommandNeedsTimedInvoke(clusterPath.mClusterId, clusterCommandId)) + { + entry.info.flags.Set(InteractionModel::CommandQualityFlags::kTimed); + } + + // TODO: Set additional flags: + // entry.info.flags.Set(InteractionModel::CommandQualityFlags::kFabricScoped) + // entry.info.flags.Set(InteractionModel::CommandQualityFlags::kFabricSensitive) + + return entry; +} + } // namespace CHIP_ERROR CodegenDataModel::ReadAttribute(const InteractionModel::ReadAttributeRequest & request, @@ -379,10 +398,15 @@ std::optional CodegenDataModel::GetAttributeInf return std::make_optional(info); } -InteractionModel::CommandEntry CodegenDataModel::FirstAcceptedCommand(const ConcreteClusterPath & cluster) +InteractionModel::CommandEntry CodegenDataModel::FirstAcceptedCommand(const ConcreteClusterPath & path) { - // FIXME: implement - return InteractionModel::CommandEntry::Invalid(); + const EmberAfCluster * cluster = FindServerCluster(path); + + VerifyOrReturnValue(cluster != nullptr, InteractionModel::CommandEntry::Invalid()); + VerifyOrReturnValue(cluster->acceptedCommandList != nullptr, InteractionModel::CommandEntry::Invalid()); + VerifyOrReturnValue(cluster->acceptedCommandList[0] != kInvalidCommandId, InteractionModel::CommandEntry::Invalid()); + + return CommandEntryFrom(path, cluster->acceptedCommandList[0], cluster); } InteractionModel::CommandEntry CodegenDataModel::NextAcceptedCommand(const ConcreteCommandPath & before) From 7d750da396587687ab211e2383958fc110f38521 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 14:24:16 -0400 Subject: [PATCH 49/79] We seem to also support fabric scoping detection --- src/app/codegen-interaction-model/CodegenDataModel.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 9aa23f34c7c50f..1f68959466fe6e 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -142,8 +142,12 @@ InteractionModel::CommandEntry CommandEntryFrom(const ConcreteClusterPath & clus entry.info.flags.Set(InteractionModel::CommandQualityFlags::kTimed); } + if (CommandIsFabricScoped(clusterPath.mClusterId, clusterCommandId)) + { + entry.info.flags.Set(InteractionModel::CommandQualityFlags::kFabricScoped); + } + // TODO: Set additional flags: - // entry.info.flags.Set(InteractionModel::CommandQualityFlags::kFabricScoped) // entry.info.flags.Set(InteractionModel::CommandQualityFlags::kFabricSensitive) return entry; From 848861c801689663e10b33a24f31f4a84f86feac Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 15:25:29 -0400 Subject: [PATCH 50/79] implementation is in theory done, need unit tests --- .../CodegenDataModel.cpp | 116 +++++++++++++++--- .../CodegenDataModel.h | 29 +++++ 2 files changed, 131 insertions(+), 14 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 1f68959466fe6e..3642f3c557db39 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -130,8 +130,7 @@ InteractionModel::AttributeEntry AttributeEntryFrom(const ConcreteClusterPath & return entry; } -InteractionModel::CommandEntry CommandEntryFrom(const ConcreteClusterPath & clusterPath, CommandId clusterCommandId, - const EmberAfCluster * emberCluster) +InteractionModel::CommandEntry CommandEntryFrom(const ConcreteClusterPath & clusterPath, CommandId clusterCommandId) { InteractionModel::CommandEntry entry; entry.path = ConcreteCommandPath(clusterPath.mEndpointId, clusterPath.mClusterId, clusterCommandId); @@ -153,8 +152,75 @@ InteractionModel::CommandEntry CommandEntryFrom(const ConcreteClusterPath & clus return entry; } +const ConcreteCommandPath kInvalidCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId); + } // namespace +std::optional CodegenDataModel::EmberCommandListIterator::First(const CommandId * list) +{ + VerifyOrReturnValue(list != nullptr, std::nullopt); + mCurrentList = mCurrentHint = list; + + VerifyOrReturnValue(*mCurrentList != kInvalidCommandId, std::nullopt); + return *mCurrentList; +} + +std::optional CodegenDataModel::EmberCommandListIterator::Next(const CommandId * list, CommandId previousId) +{ + VerifyOrReturnValue(list != nullptr, std::nullopt); + VerifyOrReturnValue(previousId != kInvalidCommandId, std::nullopt); + + if (mCurrentList != list) + { + // invalidate the hint if switching lists... + mCurrentHint = nullptr; + mCurrentList = list; + } + + if ((mCurrentHint == nullptr) || (*mCurrentHint != previousId)) + { + // we did not find a usable hint. Search from the to set the hint + mCurrentHint = mCurrentList; + while ((*mCurrentHint != kInvalidCommandId) && (*mCurrentHint != previousId)) + { + mCurrentHint++; + } + } + + VerifyOrReturnValue(*mCurrentHint == previousId, std::nullopt); + + // hint is valid and can be used immediately + mCurrentHint++; // this is the next value + return (*mCurrentHint == kInvalidCommandId) ? std::nullopt : std::make_optional(*mCurrentHint); +} + +bool CodegenDataModel::EmberCommandListIterator::Exists(const CommandId * list, CommandId toCheck) +{ + VerifyOrReturnValue(list != nullptr, false); + VerifyOrReturnValue(toCheck != kInvalidCommandId, false); + + if (mCurrentList != list) + { + // invalidate the hint if switching lists... + mCurrentHint = nullptr; + mCurrentList = list; + } + + // maybe already positioned correctly + if ((mCurrentHint != nullptr) && (*mCurrentHint == toCheck)) + { + return true; + } + + // move and try to find it + while ((*mCurrentHint != kInvalidCommandId) && (*mCurrentHint != toCheck)) + { + mCurrentHint++; + } + + return (*mCurrentHint == toCheck); +} + CHIP_ERROR CodegenDataModel::ReadAttribute(const InteractionModel::ReadAttributeRequest & request, InteractionModel::ReadState & state, AttributeValueEncoder & encoder) { @@ -407,34 +473,56 @@ InteractionModel::CommandEntry CodegenDataModel::FirstAcceptedCommand(const Conc const EmberAfCluster * cluster = FindServerCluster(path); VerifyOrReturnValue(cluster != nullptr, InteractionModel::CommandEntry::Invalid()); - VerifyOrReturnValue(cluster->acceptedCommandList != nullptr, InteractionModel::CommandEntry::Invalid()); - VerifyOrReturnValue(cluster->acceptedCommandList[0] != kInvalidCommandId, InteractionModel::CommandEntry::Invalid()); - return CommandEntryFrom(path, cluster->acceptedCommandList[0], cluster); + std::optional commandId = mAcceptedCommandsIterator.First(cluster->acceptedCommandList); + VerifyOrReturnValue(commandId.has_value(), InteractionModel::CommandEntry::Invalid()); + + return CommandEntryFrom(path, *commandId); } InteractionModel::CommandEntry CodegenDataModel::NextAcceptedCommand(const ConcreteCommandPath & before) { - // FIXME: implement - return InteractionModel::CommandEntry::Invalid(); + const EmberAfCluster * cluster = FindServerCluster(before); + + VerifyOrReturnValue(cluster != nullptr, InteractionModel::CommandEntry::Invalid()); + + std::optional commandId = mAcceptedCommandsIterator.Next(cluster->acceptedCommandList, before.mCommandId); + VerifyOrReturnValue(commandId.has_value(), InteractionModel::CommandEntry::Invalid()); + + return CommandEntryFrom(before, *commandId); } std::optional CodegenDataModel::GetAcceptedCommandInfo(const ConcreteCommandPath & path) { - // FIXME: implement - return std::nullopt; + const EmberAfCluster * cluster = FindServerCluster(path); + + VerifyOrReturnValue(cluster != nullptr, std::nullopt); + VerifyOrReturnValue(mAcceptedCommandsIterator.Exists(cluster->acceptedCommandList, path.mCommandId), std::nullopt); + + return CommandEntryFrom(path, path.mCommandId).info; } -ConcreteCommandPath CodegenDataModel::FirstGeneratedCommand(const ConcreteClusterPath & cluster) +ConcreteCommandPath CodegenDataModel::FirstGeneratedCommand(const ConcreteClusterPath & path) { - // FIXME: implement - return ConcreteCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId); + const EmberAfCluster * cluster = FindServerCluster(path); + + VerifyOrReturnValue(cluster != nullptr, kInvalidCommandPath); + + std::optional commandId = mGeneratedCommandsIterator.First(cluster->generatedCommandList); + VerifyOrReturnValue(commandId.has_value(), kInvalidCommandPath); + return ConcreteCommandPath(path.mEndpointId, path.mClusterId, *commandId); } ConcreteCommandPath CodegenDataModel::NextGeneratedCommand(const ConcreteCommandPath & before) { - // FIXME: implement - return ConcreteCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId); + const EmberAfCluster * cluster = FindServerCluster(before); + + VerifyOrReturnValue(cluster != nullptr, kInvalidCommandPath); + + std::optional commandId = mAcceptedCommandsIterator.Next(cluster->acceptedCommandList, before.mCommandId); + VerifyOrReturnValue(commandId.has_value(), kInvalidCommandPath); + + return ConcreteCommandPath(before.mEndpointId, before.mClusterId, *commandId); } } // namespace app diff --git a/src/app/codegen-interaction-model/CodegenDataModel.h b/src/app/codegen-interaction-model/CodegenDataModel.h index c57314c5f63bdc..43117fa48d2312 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.h +++ b/src/app/codegen-interaction-model/CodegenDataModel.h @@ -37,6 +37,33 @@ namespace app { /// however they would share the exact same underlying data and storage). class CodegenDataModel : public chip::app::InteractionModel::Model { +private: + /// Ember commands are stored as a `CommandId *` pointer that is either null (i.e. no commands) + /// or is terminated with 0xFFFF_FFFF aka kInvalidCommandId + /// + /// Since iterator implementations in the data model use Next(before_path) calls, iterating + /// such lists from the beginning would be very inefficient as O(n^2). + /// + /// This class maintains a cached position inside such iteration, such that `Next` calls + /// can be faster. + class EmberCommandListIterator + { + private: + const CommandId * mCurrentList = nullptr; + const CommandId * mCurrentHint = nullptr; // Invariant: mCurrentHint is INSIDE mCurrentList + public: + EmberCommandListIterator() = default; + + /// Returns the first command in the given list (or nullopt if list is null or starts with 0xFFFFFFF) + std::optional First(const CommandId * list); + + /// Returns the command after `previousId` in the given list + std::optional Next(const CommandId * list, CommandId previousId); + + /// Checks if the given command id exists in the given list + bool Exists(const CommandId * list, CommandId toCheck); + }; + public: /// Generic model implementations CHIP_ERROR Shutdown() override { return CHIP_NO_ERROR; } @@ -72,6 +99,8 @@ class CodegenDataModel : public chip::app::InteractionModel::Model uint16_t mEndpointIterationHint = 0; unsigned mClusterIterationHint = 0; unsigned mAttributeIterationHint = 0; + EmberCommandListIterator mAcceptedCommandsIterator; + EmberCommandListIterator mGeneratedCommandsIterator; // represents a remembered cluster reference that has been found as // looking for clusters is very common (for every attribute iteration) From c8e319127bba4ecd383ee63a9d32192d59085066 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 15:26:01 -0400 Subject: [PATCH 51/79] Fix iterator name --- src/app/codegen-interaction-model/CodegenDataModel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 3642f3c557db39..ec73ce0ef72feb 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -519,7 +519,7 @@ ConcreteCommandPath CodegenDataModel::NextGeneratedCommand(const ConcreteCommand VerifyOrReturnValue(cluster != nullptr, kInvalidCommandPath); - std::optional commandId = mAcceptedCommandsIterator.Next(cluster->acceptedCommandList, before.mCommandId); + std::optional commandId = mGeneratedCommandsIterator.Next(cluster->acceptedCommandList, before.mCommandId); VerifyOrReturnValue(commandId.has_value(), kInvalidCommandPath); return ConcreteCommandPath(before.mEndpointId, before.mClusterId, *commandId); From 312d522c2d45daa3fde8a996c9d057c781ee109e Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 15:49:58 -0400 Subject: [PATCH 52/79] Mock support for accepted/generated commands, start having unit tests --- .../tests/TestCodegenModelViaMocks.cpp | 52 ++++++++++++++++--- src/app/util/mock/MockNodeConfig.cpp | 30 +++++++++-- src/app/util/mock/MockNodeConfig.h | 5 +- 3 files changed, 77 insertions(+), 10 deletions(-) diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index 62a6958a3b69fa..f0ce44383163e4 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -55,12 +55,18 @@ const MockNodeConfig gTestNodeConfig({ MockClusterConfig(MockClusterId(1), { ClusterRevision::Id, FeatureMap::Id, }), - MockClusterConfig(MockClusterId(2), { - ClusterRevision::Id, - FeatureMap::Id, - MockAttributeId(1), - MockAttributeConfig(MockAttributeId(2), ZCL_ARRAY_ATTRIBUTE_TYPE), - }), + MockClusterConfig( + MockClusterId(2), + { + ClusterRevision::Id, + FeatureMap::Id, + MockAttributeId(1), + MockAttributeConfig(MockAttributeId(2), ZCL_ARRAY_ATTRIBUTE_TYPE), + }, /* attributes */ + {}, /* events */ + {1, 2, 23}, /* acceptedCommands */ + {2, 10} /* generatedCommands */ + ), MockClusterConfig(MockClusterId(3), { ClusterRevision::Id, FeatureMap::Id, MockAttributeId(1), MockAttributeId(2), MockAttributeId(3), }), @@ -305,3 +311,37 @@ TEST(TestCodegenModelViaMocks, GlobalAttributeInfo) ConcreteAttributePath(kMockEndpoint1, MockClusterId(1), Clusters::Globals::Attributes::AttributeList::Id)); ASSERT_FALSE(info.has_value()); } + +TEST(TestCodegenModelViaMocks, IterateOverAcceptedCommands) +{ + UseMockNodeConfig config(gTestNodeConfig); + chip::app::CodegenDataModel model; + + // invalid paths should return in "no more data" + ASSERT_FALSE(model.FirstAcceptedCommand(ConcreteClusterPath(kEndpointIdThatIsMissing, MockClusterId(1))).path.HasValidIds()); + ASSERT_FALSE(model.FirstAcceptedCommand(ConcreteClusterPath(kInvalidEndpointId, MockClusterId(1))).path.HasValidIds()); + ASSERT_FALSE(model.FirstAcceptedCommand(ConcreteClusterPath(kMockEndpoint1, MockClusterId(10))).path.HasValidIds()); + ASSERT_FALSE(model.FirstAcceptedCommand(ConcreteClusterPath(kMockEndpoint1, kInvalidClusterId)).path.HasValidIds()); + + // should be able to iterate over valid paths + CommandEntry entry = model.FirstAcceptedCommand(ConcreteClusterPath(kMockEndpoint2, MockClusterId(2))); + ASSERT_TRUE(entry.path.HasValidIds()); + EXPECT_EQ(entry.path.mEndpointId, kMockEndpoint2); + EXPECT_EQ(entry.path.mClusterId, MockClusterId(2)); + EXPECT_EQ(entry.path.mCommandId, 1u); + + entry = model.NextAcceptedCommand(entry.path); + ASSERT_TRUE(entry.path.HasValidIds()); + EXPECT_EQ(entry.path.mEndpointId, kMockEndpoint2); + EXPECT_EQ(entry.path.mClusterId, MockClusterId(2)); + EXPECT_EQ(entry.path.mCommandId, 2u); + + entry = model.NextAcceptedCommand(entry.path); + ASSERT_TRUE(entry.path.HasValidIds()); + EXPECT_EQ(entry.path.mEndpointId, kMockEndpoint2); + EXPECT_EQ(entry.path.mClusterId, MockClusterId(2)); + EXPECT_EQ(entry.path.mCommandId, 23u); + + entry = model.NextAcceptedCommand(entry.path); + ASSERT_FALSE(entry.path.HasValidIds()); +} diff --git a/src/app/util/mock/MockNodeConfig.cpp b/src/app/util/mock/MockNodeConfig.cpp index 79c886f532a8a2..5670966e2ebb29 100644 --- a/src/app/util/mock/MockNodeConfig.cpp +++ b/src/app/util/mock/MockNodeConfig.cpp @@ -54,9 +54,12 @@ const T * findById(const std::vector & vector, decltype(std::declval().id) } // namespace MockClusterConfig::MockClusterConfig(ClusterId aId, std::initializer_list aAttributes, - std::initializer_list aEvents) : + std::initializer_list aEvents, + std::initializer_list aAcceptedCommands, + std::initializer_list aGeneratedCommands) : id(aId), - attributes(aAttributes), events(aEvents), mEmberCluster{} + attributes(aAttributes), events(aEvents), mEmberCluster{}, mAcceptedCommands(aAcceptedCommands), + mGeneratedCommands(aGeneratedCommands) { VerifyOrDie(aAttributes.size() < UINT16_MAX); @@ -71,6 +74,18 @@ MockClusterConfig::MockClusterConfig(ClusterId aId, std::initializer_list(mEmberEventList.size()); mEmberCluster.eventList = mEmberEventList.data(); + if (!mAcceptedCommands.empty()) + { + mAcceptedCommands.push_back(kInvalidCommandId); + mEmberCluster.acceptedCommandList = mAcceptedCommands.data(); + } + + if (!mGeneratedCommands.empty()) + { + mGeneratedCommands.push_back(kInvalidCommandId); + mEmberCluster.generatedCommandList = mGeneratedCommands.data(); + } + for (auto & attr : attributes) { mAttributeMetaData.push_back(attr.attributeMetaData); @@ -82,10 +97,19 @@ MockClusterConfig::MockClusterConfig(ClusterId aId, std::initializer_list aAttributes = {}, - std::initializer_list aEvents = {}); + std::initializer_list aEvents = {}, std::initializer_list aAcceptedCommands = {}, + std::initializer_list aGeneratedCommands = {}); // Cluster-config is self-referential: mEmberCluster.attributes references mAttributeMetaData.data() MockClusterConfig(const MockClusterConfig & other); @@ -86,6 +87,8 @@ struct MockClusterConfig EmberAfCluster mEmberCluster; std::vector mEmberEventList; std::vector mAttributeMetaData; + std::vector mAcceptedCommands; + std::vector mGeneratedCommands; }; struct MockEndpointConfig From 1e45ed085718779424b81160b55cbf0cbcc9b4dd Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 15:53:30 -0400 Subject: [PATCH 53/79] Better iteration tests on accepted commands --- .../tests/TestCodegenModelViaMocks.cpp | 43 +++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index f0ce44383163e4..5a4b51d8e742af 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -67,9 +67,15 @@ const MockNodeConfig gTestNodeConfig({ {1, 2, 23}, /* acceptedCommands */ {2, 10} /* generatedCommands */ ), - MockClusterConfig(MockClusterId(3), { - ClusterRevision::Id, FeatureMap::Id, MockAttributeId(1), MockAttributeId(2), MockAttributeId(3), - }), + MockClusterConfig( + MockClusterId(3), + { + ClusterRevision::Id, FeatureMap::Id, MockAttributeId(1), MockAttributeId(2), MockAttributeId(3), + }, /* attributes */ + {}, /* events */ + {11}, /* acceptedCommands */ + {4} /* generatedCommands */ + ), }), MockEndpointConfig(kMockEndpoint3, { MockClusterConfig(MockClusterId(1), { @@ -344,4 +350,35 @@ TEST(TestCodegenModelViaMocks, IterateOverAcceptedCommands) entry = model.NextAcceptedCommand(entry.path); ASSERT_FALSE(entry.path.HasValidIds()); + + // attempt some out-of-order requests as well + entry = model.FirstAcceptedCommand(ConcreteClusterPath(kMockEndpoint2, MockClusterId(3))); + ASSERT_TRUE(entry.path.HasValidIds()); + EXPECT_EQ(entry.path.mEndpointId, kMockEndpoint2); + EXPECT_EQ(entry.path.mClusterId, MockClusterId(3)); + EXPECT_EQ(entry.path.mCommandId, 11u); + + for (int i = 0; i < 10; i++) + { + entry = model.NextAcceptedCommand(ConcreteCommandPath(kMockEndpoint2, MockClusterId(2), 2)); + ASSERT_TRUE(entry.path.HasValidIds()); + EXPECT_EQ(entry.path.mEndpointId, kMockEndpoint2); + EXPECT_EQ(entry.path.mClusterId, MockClusterId(2)); + EXPECT_EQ(entry.path.mCommandId, 23u); + } + + for (int i = 0; i < 10; i++) + { + entry = model.NextAcceptedCommand(ConcreteCommandPath(kMockEndpoint2, MockClusterId(2), 1)); + ASSERT_TRUE(entry.path.HasValidIds()); + EXPECT_EQ(entry.path.mEndpointId, kMockEndpoint2); + EXPECT_EQ(entry.path.mClusterId, MockClusterId(2)); + EXPECT_EQ(entry.path.mCommandId, 2u); + } + + for (int i = 0; i < 10; i++) + { + entry = model.NextAcceptedCommand(ConcreteCommandPath(kMockEndpoint2, MockClusterId(3), 11)); + EXPECT_FALSE(entry.path.HasValidIds()); + } } From e62aa6e75030c290e2b705b652151778942ef1c1 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 16:01:37 -0400 Subject: [PATCH 54/79] More unit tests and fix bugs --- .../CodegenDataModel.cpp | 2 +- .../tests/TestCodegenModelViaMocks.cpp | 63 ++++++++++++++++++- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index ec73ce0ef72feb..bb11991ee0b494 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -519,7 +519,7 @@ ConcreteCommandPath CodegenDataModel::NextGeneratedCommand(const ConcreteCommand VerifyOrReturnValue(cluster != nullptr, kInvalidCommandPath); - std::optional commandId = mGeneratedCommandsIterator.Next(cluster->acceptedCommandList, before.mCommandId); + std::optional commandId = mGeneratedCommandsIterator.Next(cluster->generatedCommandList, before.mCommandId); VerifyOrReturnValue(commandId.has_value(), kInvalidCommandPath); return ConcreteCommandPath(before.mEndpointId, before.mClusterId, *commandId); diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index 5a4b51d8e742af..19f0dda75ac579 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -74,7 +74,7 @@ const MockNodeConfig gTestNodeConfig({ }, /* attributes */ {}, /* events */ {11}, /* acceptedCommands */ - {4} /* generatedCommands */ + {4, 6} /* generatedCommands */ ), }), MockEndpointConfig(kMockEndpoint3, { @@ -378,7 +378,66 @@ TEST(TestCodegenModelViaMocks, IterateOverAcceptedCommands) for (int i = 0; i < 10; i++) { - entry = model.NextAcceptedCommand(ConcreteCommandPath(kMockEndpoint2, MockClusterId(3), 11)); + entry = model.NextAcceptedCommand(ConcreteCommandPath(kMockEndpoint2, MockClusterId(3), 10)); EXPECT_FALSE(entry.path.HasValidIds()); } } + +TEST(TestCodegenModelViaMocks, IterateOverGeneratedCommands) +{ + UseMockNodeConfig config(gTestNodeConfig); + chip::app::CodegenDataModel model; + + // invalid paths should return in "no more data" + ASSERT_FALSE(model.FirstGeneratedCommand(ConcreteClusterPath(kEndpointIdThatIsMissing, MockClusterId(1))).HasValidIds()); + ASSERT_FALSE(model.FirstGeneratedCommand(ConcreteClusterPath(kInvalidEndpointId, MockClusterId(1))).HasValidIds()); + ASSERT_FALSE(model.FirstGeneratedCommand(ConcreteClusterPath(kMockEndpoint1, MockClusterId(10))).HasValidIds()); + ASSERT_FALSE(model.FirstGeneratedCommand(ConcreteClusterPath(kMockEndpoint1, kInvalidClusterId)).HasValidIds()); + + // should be able to iterate over valid paths + ConcreteCommandPath path = model.FirstGeneratedCommand(ConcreteClusterPath(kMockEndpoint2, MockClusterId(2))); + ASSERT_TRUE(path.HasValidIds()); + EXPECT_EQ(path.mEndpointId, kMockEndpoint2); + EXPECT_EQ(path.mClusterId, MockClusterId(2)); + EXPECT_EQ(path.mCommandId, 2u); + + path = model.NextGeneratedCommand(path); + ASSERT_TRUE(path.HasValidIds()); + EXPECT_EQ(path.mEndpointId, kMockEndpoint2); + EXPECT_EQ(path.mClusterId, MockClusterId(2)); + EXPECT_EQ(path.mCommandId, 10u); + + path = model.NextGeneratedCommand(path); + ASSERT_FALSE(path.HasValidIds()); + + // attempt some out-of-order requests as well + path = model.FirstGeneratedCommand(ConcreteClusterPath(kMockEndpoint2, MockClusterId(3))); + ASSERT_TRUE(path.HasValidIds()); + EXPECT_EQ(path.mEndpointId, kMockEndpoint2); + EXPECT_EQ(path.mClusterId, MockClusterId(3)); + EXPECT_EQ(path.mCommandId, 4u); + + for (int i = 0; i < 10; i++) + { + path = model.NextGeneratedCommand(ConcreteCommandPath(kMockEndpoint2, MockClusterId(2), 2)); + ASSERT_TRUE(path.HasValidIds()); + EXPECT_EQ(path.mEndpointId, kMockEndpoint2); + EXPECT_EQ(path.mClusterId, MockClusterId(2)); + EXPECT_EQ(path.mCommandId, 10u); + } + + for (int i = 0; i < 10; i++) + { + path = model.NextGeneratedCommand(ConcreteCommandPath(kMockEndpoint2, MockClusterId(3), 4)); + ASSERT_TRUE(path.HasValidIds()); + EXPECT_EQ(path.mEndpointId, kMockEndpoint2); + EXPECT_EQ(path.mClusterId, MockClusterId(3)); + EXPECT_EQ(path.mCommandId, 6u); + } + + for (int i = 0; i < 10; i++) + { + path = model.NextGeneratedCommand(ConcreteCommandPath(kMockEndpoint2, MockClusterId(3), 6)); + EXPECT_FALSE(path.HasValidIds()); + } +} From f116aae7ea76fc165368177baf010373ec992bbf Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 16:01:50 -0400 Subject: [PATCH 55/79] Restyle --- .../tests/TestCodegenModelViaMocks.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index 19f0dda75ac579..96f42364255f80 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -56,7 +56,7 @@ const MockNodeConfig gTestNodeConfig({ ClusterRevision::Id, FeatureMap::Id, }), MockClusterConfig( - MockClusterId(2), + MockClusterId(2), { ClusterRevision::Id, FeatureMap::Id, @@ -68,7 +68,7 @@ const MockNodeConfig gTestNodeConfig({ {2, 10} /* generatedCommands */ ), MockClusterConfig( - MockClusterId(3), + MockClusterId(3), { ClusterRevision::Id, FeatureMap::Id, MockAttributeId(1), MockAttributeId(2), MockAttributeId(3), }, /* attributes */ From bea8c164e900c86d9b26319af9780f51a5f7ddad Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 16:16:25 -0400 Subject: [PATCH 56/79] More tests, one iteration bug fix --- .../CodegenDataModel.cpp | 1 + .../tests/TestCodegenModelViaMocks.cpp | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index bb11991ee0b494..133d001baa1ee9 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -213,6 +213,7 @@ bool CodegenDataModel::EmberCommandListIterator::Exists(const CommandId * list, } // move and try to find it + mCurrentHint = mCurrentList; while ((*mCurrentHint != kInvalidCommandId) && (*mCurrentHint != toCheck)) { mCurrentHint++; diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index 96f42364255f80..bbc770ca3599ba 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -383,6 +383,34 @@ TEST(TestCodegenModelViaMocks, IterateOverAcceptedCommands) } } +TEST(TestCodegenModelViaMocks, AcceptedCommandInfo) +{ + UseMockNodeConfig config(gTestNodeConfig); + chip::app::CodegenDataModel model; + + // invalid paths should return in "no more data" + ASSERT_FALSE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kEndpointIdThatIsMissing, MockClusterId(1), 1)).has_value()); + ASSERT_FALSE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kInvalidEndpointId, MockClusterId(1), 1)).has_value()); + ASSERT_FALSE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint1, MockClusterId(10), 1)).has_value()); + ASSERT_FALSE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint1, kInvalidClusterId, 1)).has_value()); + ASSERT_FALSE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint1, MockClusterId(1), kInvalidCommandId)).has_value()); + + std::optional info = model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint2, MockClusterId(2), 1u)); + ASSERT_TRUE(info.has_value()); + + info = model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint2, MockClusterId(2), 2u)); + ASSERT_TRUE(info.has_value()); + + info = model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint2, MockClusterId(2), 1u)); + ASSERT_TRUE(info.has_value()); + + info = model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint2, MockClusterId(2), 23u)); + ASSERT_TRUE(info.has_value()); + + info = model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint2, MockClusterId(2), 1234u)); + ASSERT_FALSE(info.has_value()); +} + TEST(TestCodegenModelViaMocks, IterateOverGeneratedCommands) { UseMockNodeConfig config(gTestNodeConfig); From 20fc0de919fcb459e2281e66eca569f52c185ffa Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 16:18:37 -0400 Subject: [PATCH 57/79] Slight update again --- .../tests/TestCodegenModelViaMocks.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index bbc770ca3599ba..bd35802ce34e6a 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -404,6 +404,9 @@ TEST(TestCodegenModelViaMocks, AcceptedCommandInfo) info = model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint2, MockClusterId(2), 1u)); ASSERT_TRUE(info.has_value()); + info = model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint2, MockClusterId(2), 1u)); + ASSERT_TRUE(info.has_value()); + info = model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint2, MockClusterId(2), 23u)); ASSERT_TRUE(info.has_value()); From e17e516dcbb2a9a77b53c5addd770f6e6c6ae952 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 16:24:05 -0400 Subject: [PATCH 58/79] Aiming for more test coverage --- .../tests/TestCodegenModelViaMocks.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index bd35802ce34e6a..49b953ff60ccf2 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -135,6 +135,9 @@ TEST(TestCodegenModelViaMocks, IterateOverClusters) EXPECT_FALSE(model.FirstCluster(kEndpointIdThatIsMissing).path.HasValidIds()); EXPECT_FALSE(model.FirstCluster(kInvalidEndpointId).path.HasValidIds()); + EXPECT_FALSE(model.NextCluster(ConcreteClusterPath(kInvalidEndpointId, 123)).path.HasValidIds()); + EXPECT_FALSE(model.NextCluster(ConcreteClusterPath(kMockEndpoint1, kInvalidClusterId)).path.HasValidIds()); + EXPECT_FALSE(model.NextCluster(ConcreteClusterPath(kMockEndpoint1, 981u)).path.HasValidIds()); // mock endpoint 1 has 2 mock clusters: 1 and 2 ClusterEntry entry = model.FirstCluster(kMockEndpoint1); From 1207126b82a8ab2fb838453c4e043c1a42c825a8 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 16:33:05 -0400 Subject: [PATCH 59/79] More test coverage for edge cases in iteration --- .../tests/TestCodegenModelViaMocks.cpp | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index 49b953ff60ccf2..fac065aa0f1856 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -110,20 +110,24 @@ TEST(TestCodegenModelViaMocks, IterateOverEndpoints) // This iteration relies on the hard-coding that occurs when mock_ember is used EXPECT_EQ(model.FirstEndpoint(), kMockEndpoint1); EXPECT_EQ(model.NextEndpoint(kMockEndpoint1), kMockEndpoint2); - ASSERT_EQ(model.NextEndpoint(kMockEndpoint2), kMockEndpoint3); - ASSERT_EQ(model.NextEndpoint(kMockEndpoint3), kInvalidEndpointId); + EXPECT_EQ(model.NextEndpoint(kMockEndpoint2), kMockEndpoint3); + EXPECT_EQ(model.NextEndpoint(kMockEndpoint3), kInvalidEndpointId); /// Some out of order requests should work as well - ASSERT_EQ(model.NextEndpoint(kMockEndpoint2), kMockEndpoint3); - ASSERT_EQ(model.NextEndpoint(kMockEndpoint2), kMockEndpoint3); - ASSERT_EQ(model.NextEndpoint(kMockEndpoint1), kMockEndpoint2); - ASSERT_EQ(model.NextEndpoint(kMockEndpoint1), kMockEndpoint2); - ASSERT_EQ(model.NextEndpoint(kMockEndpoint2), kMockEndpoint3); - ASSERT_EQ(model.NextEndpoint(kMockEndpoint1), kMockEndpoint2); - ASSERT_EQ(model.NextEndpoint(kMockEndpoint3), kInvalidEndpointId); - ASSERT_EQ(model.NextEndpoint(kMockEndpoint3), kInvalidEndpointId); - ASSERT_EQ(model.FirstEndpoint(), kMockEndpoint1); - ASSERT_EQ(model.FirstEndpoint(), kMockEndpoint1); + EXPECT_EQ(model.NextEndpoint(kMockEndpoint2), kMockEndpoint3); + EXPECT_EQ(model.NextEndpoint(kMockEndpoint2), kMockEndpoint3); + EXPECT_EQ(model.NextEndpoint(kMockEndpoint1), kMockEndpoint2); + EXPECT_EQ(model.NextEndpoint(kMockEndpoint1), kMockEndpoint2); + EXPECT_EQ(model.NextEndpoint(kMockEndpoint2), kMockEndpoint3); + EXPECT_EQ(model.NextEndpoint(kMockEndpoint1), kMockEndpoint2); + EXPECT_EQ(model.NextEndpoint(kMockEndpoint3), kInvalidEndpointId); + EXPECT_EQ(model.NextEndpoint(kMockEndpoint3), kInvalidEndpointId); + EXPECT_EQ(model.FirstEndpoint(), kMockEndpoint1); + EXPECT_EQ(model.FirstEndpoint(), kMockEndpoint1); + + // invalid endpoiunts + EXPECT_EQ(model.NextEndpoint(kInvalidEndpointId), kInvalidEndpointId); + EXPECT_EQ(model.NextEndpoint(987u), kInvalidEndpointId); } TEST(TestCodegenModelViaMocks, IterateOverClusters) @@ -225,6 +229,12 @@ TEST(TestCodegenModelViaMocks, IterateOverAttributes) ASSERT_FALSE(model.FirstAttribute(ConcreteClusterPath(kMockEndpoint1, MockClusterId(10))).path.HasValidIds()); ASSERT_FALSE(model.FirstAttribute(ConcreteClusterPath(kMockEndpoint1, kInvalidClusterId)).path.HasValidIds()); + ASSERT_FALSE(model.NextAttribute(ConcreteAttributePath(kEndpointIdThatIsMissing, MockClusterId(1), 1u)).path.HasValidIds()); + ASSERT_FALSE(model.NextAttribute(ConcreteAttributePath(kInvalidEndpointId, MockClusterId(1), 1u)).path.HasValidIds()); + ASSERT_FALSE(model.NextAttribute(ConcreteAttributePath(kMockEndpoint1, MockClusterId(10), 1u)).path.HasValidIds()); + ASSERT_FALSE(model.NextAttribute(ConcreteAttributePath(kMockEndpoint1, kInvalidClusterId, 1u)).path.HasValidIds()); + ASSERT_FALSE(model.NextAttribute(ConcreteAttributePath(kMockEndpoint1, MockClusterId(1), 987u)).path.HasValidIds()); + // should be able to iterate over valid paths AttributeEntry entry = model.FirstAttribute(ConcreteClusterPath(kMockEndpoint2, MockClusterId(2))); ASSERT_TRUE(entry.path.HasValidIds()); From b29105663f5dbff3076165eac8248183169259bb Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 16:37:08 -0400 Subject: [PATCH 60/79] Fix code review comment --- src/app/codegen-interaction-model/CodegenDataModel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 133d001baa1ee9..95f80eb5dad7d2 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -73,7 +73,7 @@ InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, co for (unsigned cluster_idx = start_index; cluster_idx < endpoint->clusterCount; cluster_idx++) { const EmberAfCluster & cluster = endpoint->cluster[cluster_idx]; - if (!IsServerMask(cluster.mask & CLUSTER_MASK_SERVER)) + if (!IsServerMask(cluster.mask)) { continue; } From 949302a6935abdaea66e9eb23e32223f08fb91b9 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 6 Jun 2024 16:38:55 -0400 Subject: [PATCH 61/79] Restyle --- .../tests/TestCodegenModelViaMocks.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp index fac065aa0f1856..e8175bf8d3f340 100644 --- a/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-interaction-model/tests/TestCodegenModelViaMocks.cpp @@ -406,7 +406,8 @@ TEST(TestCodegenModelViaMocks, AcceptedCommandInfo) ASSERT_FALSE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kInvalidEndpointId, MockClusterId(1), 1)).has_value()); ASSERT_FALSE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint1, MockClusterId(10), 1)).has_value()); ASSERT_FALSE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint1, kInvalidClusterId, 1)).has_value()); - ASSERT_FALSE(model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint1, MockClusterId(1), kInvalidCommandId)).has_value()); + ASSERT_FALSE( + model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint1, MockClusterId(1), kInvalidCommandId)).has_value()); std::optional info = model.GetAcceptedCommandInfo(ConcreteCommandPath(kMockEndpoint2, MockClusterId(2), 1u)); ASSERT_TRUE(info.has_value()); From 2b32cd4cc232e7c97c4b6a217e2d4e6e53238c8d Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Jun 2024 14:24:15 -0400 Subject: [PATCH 62/79] Update src/app/interaction-model/Events.h Co-authored-by: Boris Zbarsky --- src/app/interaction-model/Events.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/interaction-model/Events.h b/src/app/interaction-model/Events.h index 87c479d831fd71..0fc3fc56b974bd 100644 --- a/src/app/interaction-model/Events.h +++ b/src/app/interaction-model/Events.h @@ -56,7 +56,7 @@ std::optional GenerateEvent(G & generator, const T & aEventData, En eventOptions.mPriority = aEventData.GetPriorityLevel(); eventOptions.mFabricIndex = aEventData.GetFabricIndex(); - // this skips generating the event if it is fabric-scoped however the event does not seem + // this skips generating the event if it is fabric-scoped but the provided event data is not // associated with any fabric. if (eventOptions.mFabricIndex == kUndefinedFabricIndex) { From a9f5680e1ac7d5213be35c38fc3d7589563361ce Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Jun 2024 14:25:33 -0400 Subject: [PATCH 63/79] Update src/app/interaction-model/IterationTypes.h Co-authored-by: Boris Zbarsky --- src/app/interaction-model/IterationTypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/interaction-model/IterationTypes.h b/src/app/interaction-model/IterationTypes.h index 5accbf63fcd652..441dd3acb7b81f 100644 --- a/src/app/interaction-model/IterationTypes.h +++ b/src/app/interaction-model/IterationTypes.h @@ -72,7 +72,7 @@ struct AttributeEntry /// - Any internal iteration errors are just logged (callers do not handle iteration CHIP_ERROR) /// - Iteration order is NOT guaranteed globally. Only the following is guaranteed: /// - when iterating over an endpoint, ALL clusters of that endpoint will be iterated first, before -/// switching the endpoint (order of clusters ids themselves not guaranteed) +/// switching the endpoint (order of clusters themselves not guaranteed) /// - when iterating over a cluster, ALL attributes of that cluster will be iterated first, before /// switching to a new cluster /// - uniqueness and completeness (iterate over all possible distinct values as long as no From b1f6bebf651f802cec1c9c5a99a487b98a39a874 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Jun 2024 14:25:42 -0400 Subject: [PATCH 64/79] Update src/app/interaction-model/Paths.h Co-authored-by: Boris Zbarsky --- src/app/interaction-model/Paths.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/interaction-model/Paths.h b/src/app/interaction-model/Paths.h index fdc61e89a2dd7a..9241222a990cb5 100644 --- a/src/app/interaction-model/Paths.h +++ b/src/app/interaction-model/Paths.h @@ -35,7 +35,7 @@ class Paths public: virtual ~Paths() = 0; - /// Mark some specific attribute dirty (or several of attributes when using wildcards). + /// Mark all attributes matching the given path (which may be a wildcard) dirty. /// /// Wildcards are supported. virtual void MarkDirty(const AttributePathParams & path) = 0; From e26503ff7016a2d8e49f3b579da1aaf5ae2af727 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Jun 2024 14:35:31 -0400 Subject: [PATCH 65/79] Fix comment about validity --- src/app/interaction-model/MetadataTypes.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/app/interaction-model/MetadataTypes.h b/src/app/interaction-model/MetadataTypes.h index 93ce3fb0ccdb85..50e2357eba2bf3 100644 --- a/src/app/interaction-model/MetadataTypes.h +++ b/src/app/interaction-model/MetadataTypes.h @@ -52,6 +52,8 @@ struct ClusterEntry result.path = ConcreteClusterPath(kInvalidEndpointId, kInvalidClusterId); return result; } + + bool IsValid() const { return path.HasValidIds(); } }; enum class AttributeQualityFlags : uint32_t @@ -83,6 +85,8 @@ struct AttributeEntry result.path = ConcreteAttributePath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId); return result; } + + bool IsValid() const { return path.HasValidIds(); } }; enum class CommandQualityFlags : uint32_t @@ -109,6 +113,8 @@ struct CommandEntry result.path = ConcreteCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId); return result; } + + bool IsValid() const { return path.HasValidIds(); } }; /// Provides metadata information for a data model @@ -117,7 +123,9 @@ struct CommandEntry /// where each element can be iterated through independently. /// /// Iteration rules: -/// - kInvalidEndpointId will be returned when iteration ends (or generally kInvalid* for paths) +/// - Invalid paths will be returned when iteration ends (IDs will be kInvalid* and in particular +/// mEndpointId will be kInvalidEndpointId). See `::Invalid()` methods for entries and +/// can use ::IsValid() to determine if the entry is valid or not. /// - Global Attributes are NOT returned since they are implied /// - Any internal iteration errors are just logged (callers do not handle iteration CHIP_ERROR) /// - Iteration order is NOT guaranteed globally. Only the following is guaranteed: From 154d15d25ea7cb223645d37568942d9f9699fe4f Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Jun 2024 14:41:43 -0400 Subject: [PATCH 66/79] Some kListBegin/End comment updates --- src/app/interaction-model/OperationTypes.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/interaction-model/OperationTypes.h b/src/app/interaction-model/OperationTypes.h index 640f183ef5ccff..578444e2bd292a 100644 --- a/src/app/interaction-model/OperationTypes.h +++ b/src/app/interaction-model/OperationTypes.h @@ -67,8 +67,8 @@ struct ReadState enum class WriteFlags : uint32_t { kTimed = 0x0001, // Received as a 2nd command after a timed invoke - kListBegin = 0x0002, // This is the FIRST list of data elements - kListEnd = 0x0004, // This is the LAST list element to write + kListBegin = 0x0002, // Flag is for the first write in a list attribute + kListEnd = 0x0004, // Flag is set for the last write in a list attribute (and that write may be empty) }; struct WriteAttributeRequest : OperationRequest From 7ee3828508c8ac3fb4bbc95c3d3f0ead03cabaa6 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Jun 2024 14:45:16 -0400 Subject: [PATCH 67/79] Drop kListBegin/End alltogether --- src/app/interaction-model/Model.h | 5 ----- src/app/interaction-model/OperationTypes.h | 2 -- 2 files changed, 7 deletions(-) diff --git a/src/app/interaction-model/Model.h b/src/app/interaction-model/Model.h index 0ce8e7fdd4f419..5ab973901a433b 100644 --- a/src/app/interaction-model/Model.h +++ b/src/app/interaction-model/Model.h @@ -77,11 +77,6 @@ class Model : public DataModelMetadataTree /// When this is invoked, caller is expected to have already done some validations: /// - cluster `data version` has been checked for the incoming request if applicable /// - /// List operation support: - /// - the first list write will have `request.writeFlags.Has(WriteFlags::kListBegin)` - /// - the last list write will have `request.writeFlags.Has(WriteFlags::kListEnd)` - /// - the last list write MAY have empty data (no list items) - /// /// When `request.writeFlags.Has(WriteFlags::kForceInternal)` the request is from an internal app update /// and SHOULD bypass some internal checks (like timed enforcement, potentially read-only restrictions) /// diff --git a/src/app/interaction-model/OperationTypes.h b/src/app/interaction-model/OperationTypes.h index 578444e2bd292a..e9d3e6cf2ad63c 100644 --- a/src/app/interaction-model/OperationTypes.h +++ b/src/app/interaction-model/OperationTypes.h @@ -67,8 +67,6 @@ struct ReadState enum class WriteFlags : uint32_t { kTimed = 0x0001, // Received as a 2nd command after a timed invoke - kListBegin = 0x0002, // Flag is for the first write in a list attribute - kListEnd = 0x0004, // Flag is set for the last write in a list attribute (and that write may be empty) }; struct WriteAttributeRequest : OperationRequest From 7fda3b6c49289b5944f478f453602e4d1e25eb39 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Jun 2024 14:46:36 -0400 Subject: [PATCH 68/79] Drop groupId --- src/app/interaction-model/OperationTypes.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/interaction-model/OperationTypes.h b/src/app/interaction-model/OperationTypes.h index e9d3e6cf2ad63c..296a9ce9e0c7b5 100644 --- a/src/app/interaction-model/OperationTypes.h +++ b/src/app/interaction-model/OperationTypes.h @@ -83,7 +83,6 @@ enum class InvokeFlags : uint32_t struct InvokeRequest : OperationRequest { ConcreteCommandPath path; - std::optional groupRequestId; // set if and only if this was a group request BitFlags invokeFlags; }; From 1e0314358ec8828052118904b740a0da4c627ac5 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 7 Jun 2024 16:51:41 -0400 Subject: [PATCH 69/79] Comment update --- src/app/interaction-model/OperationTypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/interaction-model/OperationTypes.h b/src/app/interaction-model/OperationTypes.h index 640f183ef5ccff..57499a8dbafb49 100644 --- a/src/app/interaction-model/OperationTypes.h +++ b/src/app/interaction-model/OperationTypes.h @@ -66,7 +66,7 @@ struct ReadState enum class WriteFlags : uint32_t { - kTimed = 0x0001, // Received as a 2nd command after a timed invoke + kTimed = 0x0001, // Write is a timed write (i.e. a Timed Request Action preceeded it) kListBegin = 0x0002, // This is the FIRST list of data elements kListEnd = 0x0004, // This is the LAST list element to write }; From 4e7019ebf8462d6ce42d5ea97ba91e00e8333a9a Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jun 2024 08:17:35 -0400 Subject: [PATCH 70/79] Update for data version to be mandatory, add more error reporting and logging --- .../CodegenDataModel.cpp | 78 +++++++++++++------ src/app/interaction-model/BUILD.gn | 1 + src/app/interaction-model/MetadataTypes.cpp | 31 ++++++++ src/app/interaction-model/MetadataTypes.h | 15 ++-- 4 files changed, 94 insertions(+), 31 deletions(-) create mode 100644 src/app/interaction-model/MetadataTypes.cpp diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 95f80eb5dad7d2..2cf478bc6c2343 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -21,7 +21,9 @@ #include #include #include + #include +#include namespace chip { namespace app { @@ -35,33 +37,41 @@ bool IsServerMask(EmberAfClusterMask mask) } /// Load the cluster information into the specified destination -void LoadClusterInfo(const ConcreteClusterPath & path, const EmberAfCluster & cluster, InteractionModel::ClusterInfo * info) +std::variant LoadClusterInfo(const ConcreteClusterPath & path, + const EmberAfCluster & cluster) { DataVersion * versionPtr = emberAfDataVersionStorage(path); - if (versionPtr != nullptr) - { - info->dataVersion = *versionPtr; - } - else + if (versionPtr == nullptr) { ChipLogError(AppServer, "Failed to get data version for %d/" ChipLogFormatMEI, static_cast(path.mEndpointId), ChipLogValueMEI(cluster.clusterId)); - info->dataVersion = 0; + return CHIP_ERROR_NOT_FOUND; } + InteractionModel::ClusterInfo info(*versionPtr); + // TODO: set entry flags: // info->flags.Set(ClusterQualityFlags::kDiagnosticsData) + + return info; } /// Converts a EmberAfCluster into a ClusterEntry -InteractionModel::ClusterEntry ClusterEntryFrom(EndpointId endpointId, const EmberAfCluster & cluster) +std::variant ClusterEntryFrom(EndpointId endpointId, const EmberAfCluster & cluster) { - InteractionModel::ClusterEntry entry; + ConcreteClusterPath clusterPath(endpointId, cluster.clusterId); + auto info = LoadClusterInfo(clusterPath, cluster); - entry.path = ConcreteClusterPath(endpointId, cluster.clusterId); - LoadClusterInfo(entry.path, cluster, &entry.info); + if (CHIP_ERROR * err = std::get_if(&info)) + { + return *err; + } - return entry; + if (InteractionModel::ClusterInfo * infoValue = std::get_if(&info)) + { + return InteractionModel::ClusterEntry(clusterPath, *infoValue); + } + return CHIP_ERROR_INCORRECT_STATE; } /// Finds the first server cluster entry for the given endpoint data starting at [start_index] @@ -79,10 +89,25 @@ InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, co } found_index = cluster_idx; - return ClusterEntryFrom(endpointId, cluster); + auto entry = ClusterEntryFrom(endpointId, cluster); + + if (InteractionModel::ClusterEntry * entryValue = std::get_if(&entry)) + { + return *entryValue; + } + + if (CHIP_ERROR * errValue = std::get_if(&entry)) + { + ChipLogError(AppServer, "Failed to load cluster entry: %" CHIP_ERROR_FORMAT, errValue->Format()); + } + else + { + // Should NOT be possible: entryFrom has only 2 variants + ChipLogError(AppServer, "Failed to load cluster entry, UNKNOWN entry return type"); + } } - return InteractionModel::ClusterEntry::Invalid(); + return InteractionModel::ClusterEntry::kInvalid; } /// Load the attribute information into the specified destination @@ -307,9 +332,9 @@ EndpointId CodegenDataModel::NextEndpoint(EndpointId before) InteractionModel::ClusterEntry CodegenDataModel::FirstCluster(EndpointId endpointId) { const EmberAfEndpointType * endpoint = emberAfFindEndpointType(endpointId); - VerifyOrReturnValue(endpoint != nullptr, InteractionModel::ClusterEntry::Invalid()); - VerifyOrReturnValue(endpoint->clusterCount > 0, InteractionModel::ClusterEntry::Invalid()); - VerifyOrReturnValue(endpoint->cluster != nullptr, InteractionModel::ClusterEntry::Invalid()); + VerifyOrReturnValue(endpoint != nullptr, InteractionModel::ClusterEntry::kInvalid); + VerifyOrReturnValue(endpoint->clusterCount > 0, InteractionModel::ClusterEntry::kInvalid); + VerifyOrReturnValue(endpoint->cluster != nullptr, InteractionModel::ClusterEntry::kInvalid); return FirstServerClusterEntry(endpointId, endpoint, 0, mClusterIterationHint); } @@ -348,14 +373,14 @@ InteractionModel::ClusterEntry CodegenDataModel::NextCluster(const ConcreteClust // as ember API supports it const EmberAfEndpointType * endpoint = emberAfFindEndpointType(before.mEndpointId); - VerifyOrReturnValue(endpoint != nullptr, InteractionModel::ClusterEntry::Invalid()); - VerifyOrReturnValue(endpoint->clusterCount > 0, InteractionModel::ClusterEntry::Invalid()); - VerifyOrReturnValue(endpoint->cluster != nullptr, InteractionModel::ClusterEntry::Invalid()); + VerifyOrReturnValue(endpoint != nullptr, InteractionModel::ClusterEntry::kInvalid); + VerifyOrReturnValue(endpoint->clusterCount > 0, InteractionModel::ClusterEntry::kInvalid); + VerifyOrReturnValue(endpoint->cluster != nullptr, InteractionModel::ClusterEntry::kInvalid); std::optional cluster_idx = TryFindServerClusterIndex(endpoint, before.mClusterId); if (!cluster_idx.has_value()) { - return InteractionModel::ClusterEntry::Invalid(); + return InteractionModel::ClusterEntry::kInvalid; } return FirstServerClusterEntry(before.mEndpointId, endpoint, *cluster_idx + 1, mClusterIterationHint); @@ -367,10 +392,15 @@ std::optional CodegenDataModel::GetClusterInfo(co VerifyOrReturnValue(cluster != nullptr, std::nullopt); - InteractionModel::ClusterInfo info; - LoadClusterInfo(path, *cluster, &info); + auto info = LoadClusterInfo(path, *cluster); - return std::make_optional(info); + if (CHIP_ERROR * err = std::get_if(&info)) + { + ChipLogError(AppServer, "Failed to load cluster info: %" CHIP_ERROR_FORMAT, err->Format()); + return std::nullopt; + } + + return std::make_optional(std::get(info)); } InteractionModel::AttributeEntry CodegenDataModel::FirstAttribute(const ConcreteClusterPath & path) diff --git a/src/app/interaction-model/BUILD.gn b/src/app/interaction-model/BUILD.gn index 058a61fb4ef0b7..19dd3de6c26291 100644 --- a/src/app/interaction-model/BUILD.gn +++ b/src/app/interaction-model/BUILD.gn @@ -19,6 +19,7 @@ source_set("interaction-model") { "Context.h", "Events.h", "InvokeResponder.h", + "MetadataTypes.cpp", "MetadataTypes.h", "Model.h", "OperationTypes.h", diff --git a/src/app/interaction-model/MetadataTypes.cpp b/src/app/interaction-model/MetadataTypes.cpp new file mode 100644 index 00000000000000..83098c1959bb85 --- /dev/null +++ b/src/app/interaction-model/MetadataTypes.cpp @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2024 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. + */ +#include + +namespace chip { +namespace app { +namespace InteractionModel { + +const ClusterEntry ClusterEntry::kInvalid +{ + .path = ConcreteClusterPath(kInvalidEndpointId, kInvalidClusterId), + .info = ClusterInfo(0 /* version */), // version of invalid cluster entry does not matter +}; + +} // namespace InteractionModel +} // namespace app +} // namespace chip diff --git a/src/app/interaction-model/MetadataTypes.h b/src/app/interaction-model/MetadataTypes.h index 50e2357eba2bf3..9ad3e802c6b84a 100644 --- a/src/app/interaction-model/MetadataTypes.h +++ b/src/app/interaction-model/MetadataTypes.h @@ -37,8 +37,12 @@ enum class ClusterQualityFlags : uint32_t struct ClusterInfo { - DataVersion dataVersion = 0; // current cluster data version + DataVersion dataVersion; // current cluster data version, BitFlags flags; + + /// Constructor that marks data version as mandatory + /// for this structure. + ClusterInfo(DataVersion version) : dataVersion(version) {} }; struct ClusterEntry @@ -46,14 +50,11 @@ struct ClusterEntry ConcreteClusterPath path; ClusterInfo info; - static ClusterEntry Invalid() - { - ClusterEntry result; - result.path = ConcreteClusterPath(kInvalidEndpointId, kInvalidClusterId); - return result; - } + ClusterEntry(ConcreteClusterPath clusterPath, ClusterInfo clusterInfo) : path(clusterPath), info(clusterInfo) {} bool IsValid() const { return path.HasValidIds(); } + + static const ClusterEntry kInvalid; }; enum class AttributeQualityFlags : uint32_t From 2c6a5bac0f9d19707e969d39ec23d06651695ac6 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jun 2024 08:23:09 -0400 Subject: [PATCH 71/79] Update to use kInvalid instead of Invalid method --- .../CodegenDataModel.cpp | 29 ++++++++++--------- src/app/interaction-model/MetadataTypes.cpp | 11 ++++++- src/app/interaction-model/MetadataTypes.h | 22 ++++---------- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 2cf478bc6c2343..ca1cc26ad7904c 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -69,7 +69,10 @@ std::variant ClusterEntryFrom(Endpoi if (InteractionModel::ClusterInfo * infoValue = std::get_if(&info)) { - return InteractionModel::ClusterEntry(clusterPath, *infoValue); + return InteractionModel::ClusterEntry{ + .path = clusterPath, + .info = *infoValue, + }; } return CHIP_ERROR_INCORRECT_STATE; } @@ -407,9 +410,9 @@ InteractionModel::AttributeEntry CodegenDataModel::FirstAttribute(const Concrete { const EmberAfCluster * cluster = FindServerCluster(path); - VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::Invalid()); - VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); - VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); + VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::kInvalid); + VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::kInvalid); + VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::kInvalid); mAttributeIterationHint = 0; return AttributeEntryFrom(path, cluster->attributes[0]); @@ -457,15 +460,15 @@ const EmberAfCluster * CodegenDataModel::FindServerCluster(const ConcreteCluster InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteAttributePath & before) { const EmberAfCluster * cluster = FindServerCluster(before); - VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::Invalid()); - VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::Invalid()); - VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::Invalid()); + VerifyOrReturnValue(cluster != nullptr, InteractionModel::AttributeEntry::kInvalid); + VerifyOrReturnValue(cluster->attributeCount > 0, InteractionModel::AttributeEntry::kInvalid); + VerifyOrReturnValue(cluster->attributes != nullptr, InteractionModel::AttributeEntry::kInvalid); // find the given attribute in the list and then return the next one std::optional attribute_idx = TryFindAttributeIndex(cluster, before.mAttributeId); if (!attribute_idx.has_value()) { - return InteractionModel::AttributeEntry::Invalid(); + return InteractionModel::AttributeEntry::kInvalid; } unsigned next_idx = *attribute_idx + 1; @@ -476,7 +479,7 @@ InteractionModel::AttributeEntry CodegenDataModel::NextAttribute(const ConcreteA } // iteration complete - return InteractionModel::AttributeEntry::Invalid(); + return InteractionModel::AttributeEntry::kInvalid; } std::optional CodegenDataModel::GetAttributeInfo(const ConcreteAttributePath & path) @@ -503,10 +506,10 @@ InteractionModel::CommandEntry CodegenDataModel::FirstAcceptedCommand(const Conc { const EmberAfCluster * cluster = FindServerCluster(path); - VerifyOrReturnValue(cluster != nullptr, InteractionModel::CommandEntry::Invalid()); + VerifyOrReturnValue(cluster != nullptr, InteractionModel::CommandEntry::kInvalid); std::optional commandId = mAcceptedCommandsIterator.First(cluster->acceptedCommandList); - VerifyOrReturnValue(commandId.has_value(), InteractionModel::CommandEntry::Invalid()); + VerifyOrReturnValue(commandId.has_value(), InteractionModel::CommandEntry::kInvalid); return CommandEntryFrom(path, *commandId); } @@ -515,10 +518,10 @@ InteractionModel::CommandEntry CodegenDataModel::NextAcceptedCommand(const Concr { const EmberAfCluster * cluster = FindServerCluster(before); - VerifyOrReturnValue(cluster != nullptr, InteractionModel::CommandEntry::Invalid()); + VerifyOrReturnValue(cluster != nullptr, InteractionModel::CommandEntry::kInvalid); std::optional commandId = mAcceptedCommandsIterator.Next(cluster->acceptedCommandList, before.mCommandId); - VerifyOrReturnValue(commandId.has_value(), InteractionModel::CommandEntry::Invalid()); + VerifyOrReturnValue(commandId.has_value(), InteractionModel::CommandEntry::kInvalid); return CommandEntryFrom(before, *commandId); } diff --git a/src/app/interaction-model/MetadataTypes.cpp b/src/app/interaction-model/MetadataTypes.cpp index 83098c1959bb85..1dde4be7b5f86d 100644 --- a/src/app/interaction-model/MetadataTypes.cpp +++ b/src/app/interaction-model/MetadataTypes.cpp @@ -20,8 +20,17 @@ namespace chip { namespace app { namespace InteractionModel { -const ClusterEntry ClusterEntry::kInvalid +const AttributeEntry AttributeEntry::kInvalid { + .path = ConcreteAttributePath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId) +}; + +const CommandEntry CommandEntry::kInvalid +{ + .path = ConcreteCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId) +}; + +const ClusterEntry ClusterEntry::kInvalid{ .path = ConcreteClusterPath(kInvalidEndpointId, kInvalidClusterId), .info = ClusterInfo(0 /* version */), // version of invalid cluster entry does not matter }; diff --git a/src/app/interaction-model/MetadataTypes.h b/src/app/interaction-model/MetadataTypes.h index 9ad3e802c6b84a..b1f582b215ef3f 100644 --- a/src/app/interaction-model/MetadataTypes.h +++ b/src/app/interaction-model/MetadataTypes.h @@ -50,8 +50,6 @@ struct ClusterEntry ConcreteClusterPath path; ClusterInfo info; - ClusterEntry(ConcreteClusterPath clusterPath, ClusterInfo clusterInfo) : path(clusterPath), info(clusterInfo) {} - bool IsValid() const { return path.HasValidIds(); } static const ClusterEntry kInvalid; @@ -80,14 +78,9 @@ struct AttributeEntry ConcreteAttributePath path; AttributeInfo info; - static AttributeEntry Invalid() - { - AttributeEntry result; - result.path = ConcreteAttributePath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId); - return result; - } - bool IsValid() const { return path.HasValidIds(); } + + static const AttributeEntry kInvalid; }; enum class CommandQualityFlags : uint32_t @@ -108,14 +101,9 @@ struct CommandEntry ConcreteCommandPath path; CommandInfo info; - static CommandEntry Invalid() - { - CommandEntry result; - result.path = ConcreteCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId); - return result; - } - bool IsValid() const { return path.HasValidIds(); } + + static const CommandEntry kInvalid; }; /// Provides metadata information for a data model @@ -125,7 +113,7 @@ struct CommandEntry /// /// Iteration rules: /// - Invalid paths will be returned when iteration ends (IDs will be kInvalid* and in particular -/// mEndpointId will be kInvalidEndpointId). See `::Invalid()` methods for entries and +/// mEndpointId will be kInvalidEndpointId). See `::kInvalid` constants for entries and /// can use ::IsValid() to determine if the entry is valid or not. /// - Global Attributes are NOT returned since they are implied /// - Any internal iteration errors are just logged (callers do not handle iteration CHIP_ERROR) From d94cde3e6e839398cac1d80729255b530cd8794c Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jun 2024 08:27:18 -0400 Subject: [PATCH 72/79] Update flags.set --- .../CodegenDataModel.cpp | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index ca1cc26ad7904c..054cc893b274fe 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -125,15 +125,8 @@ void LoadAttributeInfo(const ConcreteAttributePath & path, const EmberAfAttribut info->writePrivilege = RequiredPrivilege::ForWriteAttribute(path); } - if (attribute.attributeType == ZCL_ARRAY_ATTRIBUTE_TYPE) - { - info->flags.Set(InteractionModel::AttributeQualityFlags::kListAttribute); - } - - if (attribute.MustUseTimedWrite()) - { - info->flags.Set(InteractionModel::AttributeQualityFlags::kTimed); - } + info->flags.Set(InteractionModel::AttributeQualityFlags::kListAttribute, (attribute.attributeType == ZCL_ARRAY_ATTRIBUTE_TYPE)); + info->flags.Set(InteractionModel::AttributeQualityFlags::kTimed, attribute.MustUseTimedWrite()); // NOTE: we do NOT provide additional info for: // - IsExternal/IsSingleton/IsAutomaticallyPersisted is not used by IM handling @@ -164,15 +157,11 @@ InteractionModel::CommandEntry CommandEntryFrom(const ConcreteClusterPath & clus entry.path = ConcreteCommandPath(clusterPath.mEndpointId, clusterPath.mClusterId, clusterCommandId); entry.info.invokePrivilege = RequiredPrivilege::ForInvokeCommand(entry.path); - if (CommandNeedsTimedInvoke(clusterPath.mClusterId, clusterCommandId)) - { - entry.info.flags.Set(InteractionModel::CommandQualityFlags::kTimed); - } + entry.info.flags.Set(InteractionModel::CommandQualityFlags::kTimed, + CommandNeedsTimedInvoke(clusterPath.mClusterId, clusterCommandId)); - if (CommandIsFabricScoped(clusterPath.mClusterId, clusterCommandId)) - { - entry.info.flags.Set(InteractionModel::CommandQualityFlags::kFabricScoped); - } + entry.info.flags.Set(InteractionModel::CommandQualityFlags::kFabricScoped, + CommandIsFabricScoped(clusterPath.mClusterId, clusterCommandId)); // TODO: Set additional flags: // entry.info.flags.Set(InteractionModel::CommandQualityFlags::kFabricSensitive) From 8ff725d00a70515b9f5a134bb68cfabe06b05935 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jun 2024 08:31:09 -0400 Subject: [PATCH 73/79] Use IsServerMask on clusterr class --- .../codegen-interaction-model/CodegenDataModel.cpp | 13 +++---------- src/app/interaction-model/MetadataTypes.cpp | 11 +++-------- src/app/util/af-types.h | 3 +++ 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 054cc893b274fe..bc9a7579a3960d 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -29,13 +29,6 @@ namespace chip { namespace app { namespace { -/// Checks if the specified ember cluster mask corresponds to a valid -/// server cluster. -bool IsServerMask(EmberAfClusterMask mask) -{ - return (mask == 0) || ((mask & CLUSTER_MASK_SERVER) != 0); -} - /// Load the cluster information into the specified destination std::variant LoadClusterInfo(const ConcreteClusterPath & path, const EmberAfCluster & cluster) @@ -86,7 +79,7 @@ InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, co for (unsigned cluster_idx = start_index; cluster_idx < endpoint->clusterCount; cluster_idx++) { const EmberAfCluster & cluster = endpoint->cluster[cluster_idx]; - if (!IsServerMask(cluster.mask)) + if (!cluster.IsServerMask()) { continue; } @@ -338,7 +331,7 @@ std::optional CodegenDataModel::TryFindServerClusterIndex(const EmberA if (mClusterIterationHint < clusterCount) { const EmberAfCluster & cluster = endpoint->cluster[mClusterIterationHint]; - if (IsServerMask(cluster.mask) && (cluster.clusterId == id)) + if (cluster.IsServerMask() && (cluster.clusterId == id)) { return std::make_optional(mClusterIterationHint); } @@ -350,7 +343,7 @@ std::optional CodegenDataModel::TryFindServerClusterIndex(const EmberA for (unsigned cluster_idx = 0; cluster_idx < clusterCount; cluster_idx++) { const EmberAfCluster & cluster = endpoint->cluster[cluster_idx]; - if (IsServerMask(cluster.mask) && (cluster.clusterId == id)) + if (cluster.IsServerMask() && (cluster.clusterId == id)) { return std::make_optional(cluster_idx); } diff --git a/src/app/interaction-model/MetadataTypes.cpp b/src/app/interaction-model/MetadataTypes.cpp index 1dde4be7b5f86d..48c2e3db733a52 100644 --- a/src/app/interaction-model/MetadataTypes.cpp +++ b/src/app/interaction-model/MetadataTypes.cpp @@ -20,15 +20,10 @@ namespace chip { namespace app { namespace InteractionModel { -const AttributeEntry AttributeEntry::kInvalid -{ - .path = ConcreteAttributePath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId) -}; +const AttributeEntry AttributeEntry::kInvalid{ .path = ConcreteAttributePath(kInvalidEndpointId, kInvalidClusterId, + kInvalidAttributeId) }; -const CommandEntry CommandEntry::kInvalid -{ - .path = ConcreteCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId) -}; +const CommandEntry CommandEntry::kInvalid{ .path = ConcreteCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId) }; const ClusterEntry ClusterEntry::kInvalid{ .path = ConcreteClusterPath(kInvalidEndpointId, kInvalidClusterId), diff --git a/src/app/util/af-types.h b/src/app/util/af-types.h index c608854c346fba..57fe81de1783f4 100644 --- a/src/app/util/af-types.h +++ b/src/app/util/af-types.h @@ -23,6 +23,7 @@ * @{ */ +#include "att-storage.h" #include // For bool #include // For various uint*_t types @@ -116,6 +117,8 @@ typedef struct * Total number of events supported by the cluster instance (in eventList array). */ uint16_t eventCount; + + bool IsServerMask() const { return (mask & CLUSTER_MASK_SERVER) != 0; } } EmberAfCluster; /** From b52b362a1fff6c7b00d42340952680c77e393bcf Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jun 2024 08:57:03 -0400 Subject: [PATCH 74/79] Use a struct instead of a typedef --- src/app/util/af-types.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/util/af-types.h b/src/app/util/af-types.h index 57fe81de1783f4..4509803561d9a3 100644 --- a/src/app/util/af-types.h +++ b/src/app/util/af-types.h @@ -64,7 +64,7 @@ typedef void (*EmberAfGenericClusterFunction)(void); /** * @brief Struct describing cluster */ -typedef struct +struct EmberAfCluster { /** * ID of cluster according to ZCL spec @@ -119,7 +119,7 @@ typedef struct uint16_t eventCount; bool IsServerMask() const { return (mask & CLUSTER_MASK_SERVER) != 0; } -} EmberAfCluster; +}; /** * @brief Struct that represents a logical device type consisting From 41012d9df3ddeeb6bf5d98e18fafb781069fd3cd Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jun 2024 10:27:55 -0400 Subject: [PATCH 75/79] Fix compile without error logging --- src/app/codegen-interaction-model/CodegenDataModel.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index bc9a7579a3960d..b4dee9e61181d3 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -92,6 +92,7 @@ InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, co return *entryValue; } +#if CHIP_ERROR_LOGGING if (CHIP_ERROR * errValue = std::get_if(&entry)) { ChipLogError(AppServer, "Failed to load cluster entry: %" CHIP_ERROR_FORMAT, errValue->Format()); @@ -101,6 +102,7 @@ InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, co // Should NOT be possible: entryFrom has only 2 variants ChipLogError(AppServer, "Failed to load cluster entry, UNKNOWN entry return type"); } +#endif } return InteractionModel::ClusterEntry::kInvalid; @@ -381,7 +383,11 @@ std::optional CodegenDataModel::GetClusterInfo(co if (CHIP_ERROR * err = std::get_if(&info)) { +#if CHIP_ERROR_LOGGING ChipLogError(AppServer, "Failed to load cluster info: %" CHIP_ERROR_FORMAT, err->Format()); +#else + (void)err->Format(); +#endif return std::nullopt; } From 988d4d5b318183115f5027ef30e24f19d8c6cd5b Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jun 2024 10:43:06 -0400 Subject: [PATCH 76/79] Restyle --- src/app/codegen-interaction-model/CodegenDataModel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index b4dee9e61181d3..2ab453ecb2030f 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -386,7 +386,7 @@ std::optional CodegenDataModel::GetClusterInfo(co #if CHIP_ERROR_LOGGING ChipLogError(AppServer, "Failed to load cluster info: %" CHIP_ERROR_FORMAT, err->Format()); #else - (void)err->Format(); + (void) err->Format(); #endif return std::nullopt; } From 3acac047f76053a760d5d7ed9fd9746b71f2a1b2 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jun 2024 11:14:09 -0400 Subject: [PATCH 77/79] Remove command quality that is not supported --- src/app/codegen-interaction-model/CodegenDataModel.cpp | 3 --- src/app/interaction-model/MetadataTypes.h | 3 +-- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 2ab453ecb2030f..2f65f09c53fd8f 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -158,9 +158,6 @@ InteractionModel::CommandEntry CommandEntryFrom(const ConcreteClusterPath & clus entry.info.flags.Set(InteractionModel::CommandQualityFlags::kFabricScoped, CommandIsFabricScoped(clusterPath.mClusterId, clusterCommandId)); - // TODO: Set additional flags: - // entry.info.flags.Set(InteractionModel::CommandQualityFlags::kFabricSensitive) - return entry; } diff --git a/src/app/interaction-model/MetadataTypes.h b/src/app/interaction-model/MetadataTypes.h index b1f582b215ef3f..1d30958159f7c1 100644 --- a/src/app/interaction-model/MetadataTypes.h +++ b/src/app/interaction-model/MetadataTypes.h @@ -86,8 +86,7 @@ struct AttributeEntry enum class CommandQualityFlags : uint32_t { kFabricScoped = 0x0001, - kFabricSensitive = 0x0002, - kTimed = 0x0004, // `T` quality on commands + kTimed = 0x0002, // `T` quality on commands }; struct CommandInfo From 82a68241885c1d3de325f250a2b5a45662d56025 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jun 2024 11:20:26 -0400 Subject: [PATCH 78/79] Restyle --- src/app/interaction-model/MetadataTypes.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/interaction-model/MetadataTypes.h b/src/app/interaction-model/MetadataTypes.h index 1d30958159f7c1..5b3c62f0be2247 100644 --- a/src/app/interaction-model/MetadataTypes.h +++ b/src/app/interaction-model/MetadataTypes.h @@ -85,8 +85,8 @@ struct AttributeEntry enum class CommandQualityFlags : uint32_t { - kFabricScoped = 0x0001, - kTimed = 0x0002, // `T` quality on commands + kFabricScoped = 0x0001, + kTimed = 0x0002, // `T` quality on commands }; struct CommandInfo From 10c48ca0d235babbade54fb75f5168ad8d399949 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Mon, 10 Jun 2024 16:08:29 -0400 Subject: [PATCH 79/79] Rename IsServerMask to IsServer --- src/app/codegen-interaction-model/CodegenDataModel.cpp | 6 +++--- src/app/util/af-types.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app/codegen-interaction-model/CodegenDataModel.cpp b/src/app/codegen-interaction-model/CodegenDataModel.cpp index 2f65f09c53fd8f..f917a8e9c9c4b0 100644 --- a/src/app/codegen-interaction-model/CodegenDataModel.cpp +++ b/src/app/codegen-interaction-model/CodegenDataModel.cpp @@ -79,7 +79,7 @@ InteractionModel::ClusterEntry FirstServerClusterEntry(EndpointId endpointId, co for (unsigned cluster_idx = start_index; cluster_idx < endpoint->clusterCount; cluster_idx++) { const EmberAfCluster & cluster = endpoint->cluster[cluster_idx]; - if (!cluster.IsServerMask()) + if (!cluster.IsServer()) { continue; } @@ -330,7 +330,7 @@ std::optional CodegenDataModel::TryFindServerClusterIndex(const EmberA if (mClusterIterationHint < clusterCount) { const EmberAfCluster & cluster = endpoint->cluster[mClusterIterationHint]; - if (cluster.IsServerMask() && (cluster.clusterId == id)) + if (cluster.IsServer() && (cluster.clusterId == id)) { return std::make_optional(mClusterIterationHint); } @@ -342,7 +342,7 @@ std::optional CodegenDataModel::TryFindServerClusterIndex(const EmberA for (unsigned cluster_idx = 0; cluster_idx < clusterCount; cluster_idx++) { const EmberAfCluster & cluster = endpoint->cluster[cluster_idx]; - if (cluster.IsServerMask() && (cluster.clusterId == id)) + if (cluster.IsServer() && (cluster.clusterId == id)) { return std::make_optional(cluster_idx); } diff --git a/src/app/util/af-types.h b/src/app/util/af-types.h index 4509803561d9a3..929ad055d0fc1e 100644 --- a/src/app/util/af-types.h +++ b/src/app/util/af-types.h @@ -118,7 +118,7 @@ struct EmberAfCluster */ uint16_t eventCount; - bool IsServerMask() const { return (mask & CLUSTER_MASK_SERVER) != 0; } + bool IsServer() const { return (mask & CLUSTER_MASK_SERVER) != 0; } }; /**