From a61852427856800bc1b33ee90050a1f1e0df219c Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 26 Feb 2025 11:00:08 -0500 Subject: [PATCH 1/3] Example cleaner server cluster interface --- docs/ids_and_codes/ERROR_CODES.md | 1 + scripts/build_coverage.sh | 5 +- src/BUILD.gn | 5 +- src/app/InteractionModelEngine.cpp | 2 +- src/app/data-model-provider/Provider.h | 4 +- .../tests/TestActionReturnStatus.cpp | 6 +- src/app/server-cluster/BUILD.gn | 35 ++ .../server-cluster/DefaultServerCluster.cpp | 110 +++++++ src/app/server-cluster/DefaultServerCluster.h | 89 +++++ .../server-cluster/ServerClusterInterface.h | 124 +++++++ src/app/server-cluster/tests/BUILD.gn | 31 ++ .../tests/TestDefaultServerCluster.cpp | 150 +++++++++ src/app/tests/TestCommandInteraction.cpp | 7 +- src/app/tests/test-interaction-model-api.cpp | 5 +- src/app/tests/test-interaction-model-api.h | 5 +- .../tests/data_model/DataModelFixtures.cpp | 4 +- .../tests/data_model/DataModelFixtures.h | 5 +- src/data-model-providers/codegen/BUILD.gn | 14 + .../codegen/CodegenDataModelProvider.cpp | 6 +- .../codegen/CodegenDataModelProvider.h | 4 +- .../ServerClusterInterfaceRegistry.cpp | 224 +++++++++++++ .../codegen/ServerClusterInterfaceRegistry.h | 141 ++++++++ src/data-model-providers/codegen/model.gni | 2 + .../codegen/tests/BUILD.gn | 1 + .../tests/TestCodegenModelViaMocks.cpp | 4 +- .../TestServerClusterInterfaceRegistry.cpp | 307 ++++++++++++++++++ src/lib/core/CHIPError.cpp | 3 + src/lib/core/CHIPError.h | 11 +- 28 files changed, 1277 insertions(+), 28 deletions(-) create mode 100644 src/app/server-cluster/BUILD.gn create mode 100644 src/app/server-cluster/DefaultServerCluster.cpp create mode 100644 src/app/server-cluster/DefaultServerCluster.h create mode 100644 src/app/server-cluster/ServerClusterInterface.h create mode 100644 src/app/server-cluster/tests/BUILD.gn create mode 100644 src/app/server-cluster/tests/TestDefaultServerCluster.cpp create mode 100644 src/data-model-providers/codegen/ServerClusterInterfaceRegistry.cpp create mode 100644 src/data-model-providers/codegen/ServerClusterInterfaceRegistry.h create mode 100644 src/data-model-providers/codegen/tests/TestServerClusterInterfaceRegistry.cpp diff --git a/docs/ids_and_codes/ERROR_CODES.md b/docs/ids_and_codes/ERROR_CODES.md index ff549e6df6b6dc..5bd5807e4ed442 100644 --- a/docs/ids_and_codes/ERROR_CODES.md +++ b/docs/ids_and_codes/ERROR_CODES.md @@ -55,6 +55,7 @@ This file was **AUTOMATICALLY** generated by | 37 | 0x25 | `CHIP_ERROR_UNKNOWN_IMPLICIT_TLV_TAG` | | 38 | 0x26 | `CHIP_ERROR_WRONG_TLV_TYPE` | | 39 | 0x27 | `CHIP_ERROR_TLV_CONTAINER_OPEN` | +| 40 | 0x28 | `CHIP_ERROR_IN_USE` | | 42 | 0x2A | `CHIP_ERROR_INVALID_MESSAGE_TYPE` | | 43 | 0x2B | `CHIP_ERROR_UNEXPECTED_TLV_ELEMENT` | | 45 | 0x2D | `CHIP_ERROR_NOT_IMPLEMENTED` | diff --git a/scripts/build_coverage.sh b/scripts/build_coverage.sh index d21aef5a77eb7e..5c29e7a9482649 100755 --- a/scripts/build_coverage.sh +++ b/scripts/build_coverage.sh @@ -222,6 +222,7 @@ lcov --initial --capture --directory "$OUTPUT_ROOT/obj/src" \ --exclude="$PWD"/zzz_generated/* \ --exclude="$PWD"/third_party/* \ --exclude=/usr/include/* \ + --ignore-errors inconsistent \ --output-file "$COVERAGE_ROOT/lcov_base.info" lcov --capture --directory "$OUTPUT_ROOT/obj/src" \ @@ -229,9 +230,11 @@ lcov --capture --directory "$OUTPUT_ROOT/obj/src" \ --exclude="$PWD"/zzz_generated/* \ --exclude="$PWD"/third_party/* \ --exclude=/usr/include/* \ + --ignore-errors inconsistent \ --output-file "$COVERAGE_ROOT/lcov_test.info" -lcov --add-tracefile "$COVERAGE_ROOT/lcov_base.info" \ +lcov --ignore-errors inconsistent \ + --add-tracefile "$COVERAGE_ROOT/lcov_base.info" \ --add-tracefile "$COVERAGE_ROOT/lcov_test.info" \ --ignore-errors inconsistent \ --output-file "$COVERAGE_ROOT/lcov_final.info" diff --git a/src/BUILD.gn b/src/BUILD.gn index ba05860d6a110e..83c8a5a18647a6 100644 --- a/src/BUILD.gn +++ b/src/BUILD.gn @@ -51,9 +51,9 @@ if (chip_build_tests) { deps = [] tests = [ "${chip_root}/src/access/tests", - "${chip_root}/src/app/data-model/tests", "${chip_root}/src/app/cluster-building-blocks/tests", "${chip_root}/src/app/data-model-provider/tests", + "${chip_root}/src/app/data-model/tests", "${chip_root}/src/app/icd/server/tests", "${chip_root}/src/crypto/tests", "${chip_root}/src/inet/tests", @@ -108,6 +108,9 @@ if (chip_build_tests) { # https://github.com/project-chip/connectedhomeip/issues/35624 "${chip_root}/src/credentials/tests", "${chip_root}/src/lib/support/tests", + + # Disabled on EFR32 since "include " fails with `::fabs` not defined + "${chip_root}/src/app/server-cluster/tests", ] } diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index fe7105846c8892..cc50f932bd2109 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -1701,7 +1701,7 @@ void InteractionModelEngine::DispatchCommand(CommandHandlerImpl & apCommandObj, request.invokeFlags.Set(DataModel::InvokeFlags::kTimed, apCommandObj.IsTimedInvoke()); request.subjectDescriptor = &subjectDescriptor; - std::optional status = GetDataModelProvider()->Invoke(request, apPayload, &apCommandObj); + std::optional status = GetDataModelProvider()->InvokeCommand(request, apPayload, &apCommandObj); // Provider indicates that handler status or data was already set (or will be set asynchronously) by // returning std::nullopt. If any other value is returned, it is requesting that a status is set. This diff --git a/src/app/data-model-provider/Provider.h b/src/app/data-model-provider/Provider.h index fc5a204c869785..728759018a9ea4 100644 --- a/src/app/data-model-provider/Provider.h +++ b/src/app/data-model-provider/Provider.h @@ -105,8 +105,8 @@ class Provider : public ProviderMetadataTree /// - if a value is returned (not nullopt) then the handler response MUST NOT be filled. The caller /// will then issue `handler->AddStatus(request.path, ->GetStatusCode())`. This is a /// convenience to make writing Invoke calls easier. - virtual std::optional Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, - CommandHandler * handler) = 0; + virtual std::optional InvokeCommand(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, + CommandHandler * handler) = 0; private: InteractionModelContext mContext = { nullptr }; diff --git a/src/app/data-model-provider/tests/TestActionReturnStatus.cpp b/src/app/data-model-provider/tests/TestActionReturnStatus.cpp index 8ca1993d149e1a..451c65640c55d7 100644 --- a/src/app/data-model-provider/tests/TestActionReturnStatus.cpp +++ b/src/app/data-model-provider/tests/TestActionReturnStatus.cpp @@ -15,13 +15,11 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -#include "lib/core/CHIPError.h" -#include "protocols/interaction_model/StatusCode.h" -#include "pw_unit_test/framework_backend.h" #include #include +#include #include +#include #include diff --git a/src/app/server-cluster/BUILD.gn b/src/app/server-cluster/BUILD.gn new file mode 100644 index 00000000000000..cd8b7e3b48f816 --- /dev/null +++ b/src/app/server-cluster/BUILD.gn @@ -0,0 +1,35 @@ +# Copyright (c) 2025 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") + +source_set("server-cluster") { + sources = [ + "DefaultServerCluster.cpp", + "DefaultServerCluster.h", + "ServerClusterInterface.h", + ] + + public_deps = [ + "${chip_root}/src/app:attribute-access", + "${chip_root}/src/app:command-handler-interface", + "${chip_root}/src/app:paths", + "${chip_root}/src/app/common:ids", + "${chip_root}/src/app/data-model-provider", + "${chip_root}/src/app/data-model-provider:metadata", + "${chip_root}/src/crypto", + "${chip_root}/src/lib/core:error", + "${chip_root}/src/lib/core:types", + "${chip_root}/src/lib/support", + ] +} diff --git a/src/app/server-cluster/DefaultServerCluster.cpp b/src/app/server-cluster/DefaultServerCluster.cpp new file mode 100644 index 00000000000000..db2b7d3b3cf4ae --- /dev/null +++ b/src/app/server-cluster/DefaultServerCluster.cpp @@ -0,0 +1,110 @@ +/* + * Copyright (c) 2025 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 +#include +#include +#include + +namespace chip { +namespace app { +namespace { + +using namespace chip::app::Clusters; +using namespace chip::app::DataModel; + +constexpr std::array kGlobalAttributeEntries{ { + { + Globals::Attributes::ClusterRevision::Id, + BitFlags(), + Access::Privilege::kView, + std::nullopt, + }, + { + Globals::Attributes::FeatureMap::Id, + BitFlags(), + Access::Privilege::kView, + std::nullopt, + }, + { + Globals::Attributes::AttributeList::Id, + BitFlags(AttributeQualityFlags::kListAttribute), + Access::Privilege::kView, + std::nullopt, + }, + { + Globals::Attributes::AcceptedCommandList::Id, + BitFlags(AttributeQualityFlags::kListAttribute), + Access::Privilege::kView, + std::nullopt, + }, + { + Globals::Attributes::GeneratedCommandList::Id, + BitFlags(AttributeQualityFlags::kListAttribute), + Access::Privilege::kView, + std::nullopt, + }, +} }; + +} // namespace + +DefaultServerCluster::DefaultServerCluster() +{ + // SPEC - 7.10.3. Cluster Data Version + // A cluster data version SHALL be initialized randomly when it is first published. + mDataVersion = Crypto::GetRandU32(); +} + +CHIP_ERROR DefaultServerCluster::Attributes(const ConcreteClusterPath & path, DataModel::ListBuilder & builder) +{ + + return builder.ReferenceExisting(kGlobalAttributeEntries); +} + +BitFlags DefaultServerCluster::GetClusterFlags() const +{ + return {}; +} + +ActionReturnStatus DefaultServerCluster::WriteAttribute(const WriteAttributeRequest & request, AttributeValueDecoder & decoder) +{ + return Protocols::InteractionModel::Status::UnsupportedWrite; +} + +std::optional +DefaultServerCluster::InvokeCommand(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, CommandHandler * handler) +{ + return Protocols::InteractionModel::Status::UnsupportedCommand; +} + +CHIP_ERROR DefaultServerCluster::AcceptedCommands(const ConcreteClusterPath & path, + DataModel::ListBuilder & builder) +{ + return CHIP_NO_ERROR; +} + +CHIP_ERROR DefaultServerCluster::GeneratedCommands(const ConcreteClusterPath & path, DataModel::ListBuilder & builder) +{ + return CHIP_NO_ERROR; +} + +} // namespace app +} // namespace chip diff --git a/src/app/server-cluster/DefaultServerCluster.h b/src/app/server-cluster/DefaultServerCluster.h new file mode 100644 index 00000000000000..852aa9efcf6d89 --- /dev/null +++ b/src/app/server-cluster/DefaultServerCluster.h @@ -0,0 +1,89 @@ +/* + * Copyright (c) 2025 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 { + +/// Provides an implementation of most methods for a `ServerClusterInterface` +/// to make it easier to implement spec-compliant classes. +/// +/// In particular it does: +/// - maintains a data version and provides `IncreaseDataVersion`. Ensures this +/// version is spec-compliant initialized (with a random value) +/// - Provides default implementations for most virtual methods EXCEPT: +/// - ReadAttribute (since that one needs to handle featuremap and revision) +/// - GetClusterId (since every implementation is for different clusters) +/// +/// +class DefaultServerCluster : public ServerClusterInterface +{ +public: + DefaultServerCluster(); + ~DefaultServerCluster() override = default; + + DefaultServerCluster(DefaultServerCluster && other) = default; + DefaultServerCluster & operator=(DefaultServerCluster && other) = default; + + DefaultServerCluster(const DefaultServerCluster & other) = delete; + DefaultServerCluster & operator=(const DefaultServerCluster & other) = delete; + + void IncreaseDataVersion() { mDataVersion++; } + + //////////////////////////// ServerClusterInterface implementation //////////////////////////////////////// + + [[nodiscard]] DataVersion GetDataVersion() const override { return mDataVersion; } + [[nodiscard]] BitFlags GetClusterFlags() const override; + + /// Default implementation errors out with an unsupported write on every attribute. + DataModel::ActionReturnStatus WriteAttribute(const DataModel::WriteAttributeRequest & request, + AttributeValueDecoder & decoder) override; + + /// Must only be implemented if support for any non-global attributes + /// is required. + /// + /// Default implementation just returns the above global attributes. + CHIP_ERROR Attributes(const ConcreteClusterPath & path, DataModel::ListBuilder & builder) override; + + ///////////////////////////////////// Command Support ///////////////////////////////////////////////////////// + + /// Must only be implemented if commands are supported by the cluster + /// + /// Default implementation errors out with an UnspportedCommand error. + std::optional InvokeCommand(const DataModel::InvokeRequest & request, + chip::TLV::TLVReader & input_arguments, + CommandHandler * handler) override; + + /// Must only be implemented if commands are supported by the cluster + /// + /// Default implementation is a NOOP (no list items generated) + CHIP_ERROR AcceptedCommands(const ConcreteClusterPath & path, + DataModel::ListBuilder & builder) override; + + /// Must only be implemented if commands are supported by the cluster that return values + /// + /// Default implementation is a NOOP (no list items generated) + CHIP_ERROR GeneratedCommands(const ConcreteClusterPath & path, DataModel::ListBuilder & builder) override; + +private: + DataVersion mDataVersion; // will be random-initialized as per spec +}; + +} // namespace app +} // namespace chip diff --git a/src/app/server-cluster/ServerClusterInterface.h b/src/app/server-cluster/ServerClusterInterface.h new file mode 100644 index 00000000000000..e867c3daf85957 --- /dev/null +++ b/src/app/server-cluster/ServerClusterInterface.h @@ -0,0 +1,124 @@ +/* + * Copyright (c) 2025 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 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace chip { +namespace app { + +/// Handles cluster interactions for a specific cluster id. +/// +/// A `ServerClusterInterface` is generally associated with a single endpointId and represents +/// a cluster that exists at a given `endpointId/clusterId` path. +/// +/// Provides metadata as well as interaction processing (attribute read/write and command handling). +class ServerClusterInterface +{ +public: + ServerClusterInterface() = default; + virtual ~ServerClusterInterface() = default; + + ServerClusterInterface(ServerClusterInterface && other) = default; + ServerClusterInterface & operator=(ServerClusterInterface && other) = default; + + ServerClusterInterface(const ServerClusterInterface & other) = delete; + ServerClusterInterface & operator=(const ServerClusterInterface & other) = delete; + + ///////////////////////////////////// Cluster Metadata Support ////////////////////////////////////////////////// + [[nodiscard]] virtual ClusterId GetClusterId() const = 0; + + // Every cluster instance must have a data version. + // + // SPEC - 7.10.3. Cluster Data Version + // A cluster data version is a metadata increment-only counter value, maintained for each cluster instance. + // [...] + // A cluster data version SHALL increment or be set (wrap) to zero + // if incrementing would exceed its maximum value. A cluster data version + // SHALL be maintained for each cluster instance. + // [...] + // A cluster data version SHALL be incremented if any attribute data changes. + // + [[nodiscard]] virtual DataVersion GetDataVersion() const = 0; + + [[nodiscard]] virtual BitFlags GetClusterFlags() const = 0; + + ///////////////////////////////////// Attribute Support //////////////////////////////////////////////////////// + + /// ReadAttribute MUST be done on an "existent" attribute path: only on attributes that are + /// returned in an `Attributes` call for this cluster. + /// + /// `request.path` is expected to have `GetClusterId` as the cluster id as well as an attribute that is + /// included in a `Attributes` call. + /// + /// This MUST HANDLE the following global attributes: + /// - FeatureMap::Id + /// - ClusterRevision::Id + /// + /// This function WILL NOT be called for attributes that can be built out of cluster metadata. + /// Specifically this WILL NOT be called (and does not need to implement handling for) the + /// following attribute IDs: + /// - AcceptedCommandList::Id + /// - AttributeList::Id + /// - GeneratedCommandList::Id + virtual DataModel::ActionReturnStatus ReadAttribute(const DataModel::ReadAttributeRequest & request, + AttributeValueEncoder & encoder) = 0; + + /// WriteAttribute MUST be done on an "existent" attribute path: only on attributes that are + /// returned in an `Attributes` call for this cluster. + /// + /// `request.path` is expected to have `GetClusterId` as the cluster id as well as an attribute that is + /// included in a `Attributes` call. + virtual DataModel::ActionReturnStatus WriteAttribute(const DataModel::WriteAttributeRequest & request, + AttributeValueDecoder & decoder) = 0; + + /// Attribute list MUST contain global attributes. + /// + /// Specifically these attributes MUST always exist in the list for all clusters: + /// - ClusterRevision::Id + /// - FeatureMap::Id + /// - AcceptedCommandList::Id + /// - AttributeList::Id + /// - GeneratedCommandList::Id + /// See SPEC 7.13 Global Elements: `Global Attributes` table + /// + virtual CHIP_ERROR Attributes(const ConcreteClusterPath & path, + DataModel::ListBuilder & builder) = 0; + + ///////////////////////////////////// Command Support ///////////////////////////////////////////////////////// + + virtual std::optional + InvokeCommand(const DataModel::InvokeRequest & request, chip::TLV::TLVReader & input_arguments, CommandHandler * handler) = 0; + + virtual CHIP_ERROR AcceptedCommands(const ConcreteClusterPath & path, + DataModel::ListBuilder & builder) = 0; + + virtual CHIP_ERROR GeneratedCommands(const ConcreteClusterPath & path, DataModel::ListBuilder & builder) = 0; +}; + +} // namespace app +} // namespace chip diff --git a/src/app/server-cluster/tests/BUILD.gn b/src/app/server-cluster/tests/BUILD.gn new file mode 100644 index 00000000000000..d9f7af30ec5e45 --- /dev/null +++ b/src/app/server-cluster/tests/BUILD.gn @@ -0,0 +1,31 @@ +# Copyright (c) 2025 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/build.gni") +import("//build_overrides/chip.gni") +import("${chip_root}/build/chip/chip_test_suite.gni") + +chip_test_suite("tests") { + output_name = "libServerClusterInterfaceTests" + + test_sources = [ "TestDefaultServerCluster.cpp" ] + + cflags = [ "-Wconversion" ] + + public_deps = [ + "${chip_root}/src/app/common:ids", + "${chip_root}/src/app/data-model-provider/tests:encode-decode", + "${chip_root}/src/app/server-cluster", + "${chip_root}/src/lib/core:string-builder-adapters", + ] +} diff --git a/src/app/server-cluster/tests/TestDefaultServerCluster.cpp b/src/app/server-cluster/tests/TestDefaultServerCluster.cpp new file mode 100644 index 00000000000000..899c3584e79ffb --- /dev/null +++ b/src/app/server-cluster/tests/TestDefaultServerCluster.cpp @@ -0,0 +1,150 @@ +/* + * Copyright (c) 2025 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 +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +using namespace chip; +using namespace chip::app; +using namespace chip::app::Clusters; +using namespace chip::app::DataModel; +using namespace chip::app::Testing; +using namespace chip::Protocols::InteractionModel; + +namespace { + +class FakeDefaultServerCluster : public DefaultServerCluster +{ +public: + FakeDefaultServerCluster(ClusterId id) : mClusterId(id) {} + + ClusterId GetClusterId() const override { return mClusterId; } + + DataModel::ActionReturnStatus ReadAttribute(const DataModel::ReadAttributeRequest & request, + AttributeValueEncoder & encoder) override + { + switch (request.path.mAttributeId) + { + case Globals::Attributes::FeatureMap::Id: + return encoder.Encode(0); + case Globals::Attributes::ClusterRevision::Id: + return encoder.Encode(123); + } + return CHIP_ERROR_INVALID_ARGUMENT; + } + +private: + ClusterId mClusterId; +}; + +} // namespace + +TEST(TestDefaultServerCluster, TestDataVersion) +{ + FakeDefaultServerCluster cluster(1); + + DataVersion v1 = cluster.GetDataVersion(); + cluster.IncreaseDataVersion(); + ASSERT_EQ(cluster.GetDataVersion(), v1 + 1); +} + +TEST(TestDefaultServerCluster, TestFlagsDefault) +{ + FakeDefaultServerCluster cluster(1); + ASSERT_EQ(cluster.GetClusterFlags().Raw(), 0u); +} + +TEST(TestDefaultServerCluster, AttributesDefault) +{ + FakeDefaultServerCluster cluster(1); + + DataModel::ListBuilder attributes; + + ASSERT_EQ(cluster.Attributes({ 1, 1 }, attributes), CHIP_NO_ERROR); + + ReadOnlyBuffer data = attributes.TakeBuffer(); + + // 5 global attributes are currently supported. Ensure they are makred + ASSERT_EQ(data.size(), 5u); + + ASSERT_EQ(data[0].attributeId, Globals::Attributes::ClusterRevision::Id); + ASSERT_EQ(data[1].attributeId, Globals::Attributes::FeatureMap::Id); + ASSERT_EQ(data[2].attributeId, Globals::Attributes::AttributeList::Id); + ASSERT_EQ(data[3].attributeId, Globals::Attributes::AcceptedCommandList::Id); + ASSERT_EQ(data[4].attributeId, Globals::Attributes::GeneratedCommandList::Id); + + // first 2 are normal, the rest are list + for (size_t i = 0; i < 5; i++) + { + ASSERT_EQ(data[i].flags.Has(AttributeQualityFlags::kListAttribute), (i >= 2)); + ASSERT_EQ(data[i].readPrivilege, Access::Privilege::kView); + ASSERT_FALSE(data[i].writePrivilege.has_value()); + } +} + +TEST(TestDefaultServerCluster, CommandsDefault) +{ + FakeDefaultServerCluster cluster(1); + + DataModel::ListBuilder acceptedCommands; + ASSERT_EQ(cluster.AcceptedCommands({ 1, 1 }, acceptedCommands), CHIP_NO_ERROR); + ASSERT_TRUE(acceptedCommands.TakeBuffer().empty()); + + DataModel::ListBuilder generatedCommands; + ASSERT_EQ(cluster.GeneratedCommands({ 1, 1 }, generatedCommands), CHIP_NO_ERROR); + ASSERT_TRUE(generatedCommands.TakeBuffer().empty()); +} + +TEST(TestDefaultServerCluster, WriteAttributeDefault) +{ + FakeDefaultServerCluster cluster(1); + + WriteOperation test(0 /* endpoint */, 1 /* cluster */, 1234 /* attribute */); + test.SetSubjectDescriptor(kAdminSubjectDescriptor); + + AttributeValueDecoder decoder = test.DecoderFor(12345); + + ASSERT_EQ(cluster.WriteAttribute(test.GetRequest(), decoder), Status::UnsupportedWrite); + ASSERT_FALSE(decoder.TriedDecode()); +} + +TEST(TestDefaultServerCluster, InvokeDefault) +{ + FakeDefaultServerCluster cluster(1); + + TLV::TLVReader tlvReader; + InvokeRequest request; + + request.path = { 0 /* endpoint */, 1 /* cluster */, 1234 /* command */ }; + + ASSERT_EQ(cluster.InvokeCommand(request, tlvReader, nullptr /* command handler, assumed unused here */), + Status::UnsupportedCommand); +} diff --git a/src/app/tests/TestCommandInteraction.cpp b/src/app/tests/TestCommandInteraction.cpp index 363b43ccb4d5cf..65cab41b4848f9 100644 --- a/src/app/tests/TestCommandInteraction.cpp +++ b/src/app/tests/TestCommandInteraction.cpp @@ -403,10 +403,11 @@ class TestCommandInteractionModel : public TestImCustomDataModel return &instance; } - TestCommandInteractionModel() {} + TestCommandInteractionModel() = default; - std::optional Invoke(const DataModel::InvokeRequest & request, - chip::TLV::TLVReader & input_arguments, CommandHandler * handler) + std::optional InvokeCommand(const DataModel::InvokeRequest & request, + chip::TLV::TLVReader & input_arguments, + CommandHandler * handler) override { DispatchSingleClusterCommand(request.path, input_arguments, handler); return std::nullopt; // handler status is set by the dispatch diff --git a/src/app/tests/test-interaction-model-api.cpp b/src/app/tests/test-interaction-model-api.cpp index d5659420dd6a8a..2c7f393856b825 100644 --- a/src/app/tests/test-interaction-model-api.cpp +++ b/src/app/tests/test-interaction-model-api.cpp @@ -143,8 +143,9 @@ ActionReturnStatus TestImCustomDataModel::WriteAttribute(const WriteAttributeReq return CHIP_NO_ERROR; } -std::optional TestImCustomDataModel::Invoke(const InvokeRequest & request, - chip::TLV::TLVReader & input_arguments, CommandHandler * handler) +std::optional TestImCustomDataModel::InvokeCommand(const InvokeRequest & request, + chip::TLV::TLVReader & input_arguments, + CommandHandler * handler) { return std::make_optional(CHIP_ERROR_NOT_IMPLEMENTED); } diff --git a/src/app/tests/test-interaction-model-api.h b/src/app/tests/test-interaction-model-api.h index 54efc316da95a1..f3ad060286ee50 100644 --- a/src/app/tests/test-interaction-model-api.h +++ b/src/app/tests/test-interaction-model-api.h @@ -108,8 +108,9 @@ class TestImCustomDataModel : public CodegenDataModelProvider AttributeValueEncoder & encoder) override; DataModel::ActionReturnStatus WriteAttribute(const DataModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) override; - std::optional Invoke(const DataModel::InvokeRequest & request, - chip::TLV::TLVReader & input_arguments, CommandHandler * handler) override; + std::optional InvokeCommand(const DataModel::InvokeRequest & request, + chip::TLV::TLVReader & input_arguments, + CommandHandler * handler) override; }; } // namespace app diff --git a/src/controller/tests/data_model/DataModelFixtures.cpp b/src/controller/tests/data_model/DataModelFixtures.cpp index 930c74690392e2..55bc4e33fe06db 100644 --- a/src/controller/tests/data_model/DataModelFixtures.cpp +++ b/src/controller/tests/data_model/DataModelFixtures.cpp @@ -470,8 +470,8 @@ ActionReturnStatus CustomDataModel::WriteAttribute(const WriteAttributeRequest & return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; } -std::optional CustomDataModel::Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, - CommandHandler * handler) +std::optional CustomDataModel::InvokeCommand(const InvokeRequest & request, + chip::TLV::TLVReader & input_arguments, CommandHandler * handler) { DispatchSingleClusterCommand(request.path, input_arguments, handler); return std::nullopt; // handler status is set by the dispatch diff --git a/src/controller/tests/data_model/DataModelFixtures.h b/src/controller/tests/data_model/DataModelFixtures.h index d847e4b57439cf..c11cb7bfa92c5f 100644 --- a/src/controller/tests/data_model/DataModelFixtures.h +++ b/src/controller/tests/data_model/DataModelFixtures.h @@ -120,8 +120,9 @@ class CustomDataModel : public CodegenDataModelProvider AttributeValueEncoder & encoder) override; DataModel::ActionReturnStatus WriteAttribute(const DataModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) override; - std::optional Invoke(const DataModel::InvokeRequest & request, - chip::TLV::TLVReader & input_arguments, CommandHandler * handler) override; + std::optional InvokeCommand(const DataModel::InvokeRequest & request, + chip::TLV::TLVReader & input_arguments, + CommandHandler * handler) override; }; } // namespace DataModelTests diff --git a/src/data-model-providers/codegen/BUILD.gn b/src/data-model-providers/codegen/BUILD.gn index 3b414c595b9f67..d6517dea8366c0 100644 --- a/src/data-model-providers/codegen/BUILD.gn +++ b/src/data-model-providers/codegen/BUILD.gn @@ -45,3 +45,17 @@ source_set("instance-header") { # generally being unit tests or data_model.gni/data_model.cmake files) sources = [ "Instance.h" ] } + +source_set("registry") { + sources = [ + "ServerClusterInterfaceRegistry.cpp", + "ServerClusterInterfaceRegistry.h", + ] + + public_deps = [ + "${chip_root}/src/app:paths", + "${chip_root}/src/app/server-cluster", + "${chip_root}/src/lib/core:types", + "${chip_root}/src/lib/support", + ] +} diff --git a/src/data-model-providers/codegen/CodegenDataModelProvider.cpp b/src/data-model-providers/codegen/CodegenDataModelProvider.cpp index 6913214ac803cc..8c84fa1740374d 100644 --- a/src/data-model-providers/codegen/CodegenDataModelProvider.cpp +++ b/src/data-model-providers/codegen/CodegenDataModelProvider.cpp @@ -158,9 +158,9 @@ CHIP_ERROR CodegenDataModelProvider::Startup(DataModel::InteractionModelContext return CHIP_NO_ERROR; } -std::optional CodegenDataModelProvider::Invoke(const DataModel::InvokeRequest & request, - TLV::TLVReader & input_arguments, - CommandHandler * handler) +std::optional CodegenDataModelProvider::InvokeCommand(const DataModel::InvokeRequest & request, + TLV::TLVReader & input_arguments, + CommandHandler * handler) { CommandHandlerInterface * handler_interface = CommandHandlerInterfaceRegistry::Instance().GetCommandHandler(request.path.mEndpointId, request.path.mClusterId); diff --git a/src/data-model-providers/codegen/CodegenDataModelProvider.h b/src/data-model-providers/codegen/CodegenDataModelProvider.h index 764f5f74874673..5f683d56b9875a 100644 --- a/src/data-model-providers/codegen/CodegenDataModelProvider.h +++ b/src/data-model-providers/codegen/CodegenDataModelProvider.h @@ -63,8 +63,8 @@ class CodegenDataModelProvider : public DataModel::Provider AttributeValueEncoder & encoder) override; DataModel::ActionReturnStatus WriteAttribute(const DataModel::WriteAttributeRequest & request, AttributeValueDecoder & decoder) override; - std::optional Invoke(const DataModel::InvokeRequest & request, TLV::TLVReader & input_arguments, - CommandHandler * handler) override; + std::optional InvokeCommand(const DataModel::InvokeRequest & request, + TLV::TLVReader & input_arguments, CommandHandler * handler) override; /// attribute tree iteration CHIP_ERROR Endpoints(DataModel::ListBuilder & out) override; diff --git a/src/data-model-providers/codegen/ServerClusterInterfaceRegistry.cpp b/src/data-model-providers/codegen/ServerClusterInterfaceRegistry.cpp new file mode 100644 index 00000000000000..3d8f3b5fc2e720 --- /dev/null +++ b/src/data-model-providers/codegen/ServerClusterInterfaceRegistry.cpp @@ -0,0 +1,224 @@ +/* + * Copyright (c) 2025 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 +#include +#include + +namespace chip { +namespace app { + +void ServerClusterInterfaceRegistry::ClearSingleLinkedList(RegisteredServerClusterInterface * clusters) +{ + while (clusters != nullptr) + { + RegisteredServerClusterInterface * next = clusters->next; + Platform::Delete(clusters); + clusters = next; + } +} + +ServerClusterInterfaceRegistry::~ServerClusterInterfaceRegistry() +{ + while (mEndpoints != nullptr) + { + UnregisterAllFromEndpoint(mEndpoints->endpointId); + } +} + +CHIP_ERROR ServerClusterInterfaceRegistry::AllocateNewEndpointClusters(EndpointId endpointId, EndpointClusters *& dest) +{ + auto result = Platform::New(); + VerifyOrReturnError(result != nullptr, CHIP_ERROR_NO_MEMORY); + + result->endpointId = endpointId; + result->firstCluster = nullptr; + result->next = mEndpoints; + mEndpoints = result; + dest = result; + + return CHIP_NO_ERROR; +} + +CHIP_ERROR ServerClusterInterfaceRegistry::Register(EndpointId endpointId, ServerClusterInterface * cluster) +{ + VerifyOrReturnError(cluster != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(endpointId != kInvalidEndpointId, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(cluster->GetClusterId() != kInvalidClusterId, CHIP_ERROR_INVALID_ARGUMENT); + + // duplicate registrations are disallowed + VerifyOrReturnError(Get(ConcreteClusterPath(endpointId, cluster->GetClusterId())) == nullptr, CHIP_ERROR_DUPLICATE_KEY_ID); + + EndpointClusters * endpointClusters = FindClusters(endpointId); + if (endpointClusters == nullptr) + { + ReturnErrorOnFailure(AllocateNewEndpointClusters(endpointId, endpointClusters)); + } + + auto entry = Platform::New(cluster, endpointClusters->firstCluster); + VerifyOrReturnError(entry != nullptr, CHIP_ERROR_NO_MEMORY); + + endpointClusters->firstCluster = entry; + + return CHIP_NO_ERROR; +} + +ServerClusterInterface * ServerClusterInterfaceRegistry::Unregister(const ConcreteClusterPath & path) +{ + EndpointClusters * endpointClusters = FindClusters(path.mEndpointId); + VerifyOrReturnValue(endpointClusters != nullptr, nullptr); + VerifyOrReturnValue(endpointClusters->firstCluster != nullptr, nullptr); + + RegisteredServerClusterInterface * prev = nullptr; + RegisteredServerClusterInterface * current = endpointClusters->firstCluster; + + while (current != nullptr) + { + if (current->serverClusterInterface->GetClusterId() == path.mClusterId) + { + // take the item out of the current list and return it. + RegisteredServerClusterInterface * next = current->next; + + if (prev == nullptr) + { + endpointClusters->firstCluster = next; + } + else + { + prev->next = next; + } + + if (mCachedInterface == current->serverClusterInterface) + { + mCachedClusterEndpointId = kInvalidEndpointId; + mCachedInterface = nullptr; + } + + ServerClusterInterface * result = current->serverClusterInterface; + Platform::MemoryFree(current); + + return result; + } + + prev = current; + current = current->next; + } + + // Not found. + return nullptr; +} + +ServerClusterInterfaceRegistry::ClustersList ServerClusterInterfaceRegistry::ClustersOnEndpoint(EndpointId endpointId) +{ + EndpointClusters * clusters = FindClusters(endpointId); + + if (clusters == nullptr) + { + return nullptr; + } + + return clusters->firstCluster; +} + +void ServerClusterInterfaceRegistry::UnregisterAllFromEndpoint(EndpointId endpointId) +{ + if (mCachedClusterEndpointId == endpointId) + { + // all clusters on the given endpoint will be unregistered. + mCachedInterface = nullptr; + mCachedClusterEndpointId = kInvalidEndpointId; + } + + EndpointClusters * prev = nullptr; + EndpointClusters * current = mEndpoints; + while (current != nullptr) + { + if (current->endpointId == endpointId) + { + if (prev == nullptr) + { + mEndpoints = current->next; + } + else + { + prev->next = current->next; + } + ClearSingleLinkedList(current->firstCluster); + Platform::Delete(current); + return; + } + + prev = current; + current = current->next; + } +} + +ServerClusterInterface * ServerClusterInterfaceRegistry::Get(const ConcreteClusterPath & path) +{ + EndpointClusters * endpointClusters = FindClusters(path.mEndpointId); + VerifyOrReturnValue(endpointClusters != nullptr, nullptr); + VerifyOrReturnValue(endpointClusters->firstCluster != nullptr, nullptr); + + // Check the cache to speed things up + if ((mCachedClusterEndpointId == path.mEndpointId) && (mCachedInterface != nullptr) && + (mCachedInterface->GetClusterId() == path.mClusterId)) + { + return mCachedInterface; + } + + // The cluster searched for is not cached, do a linear search for it + RegisteredServerClusterInterface * current = endpointClusters->firstCluster; + + while (current != nullptr) + { + if (current->serverClusterInterface->GetClusterId() == path.mClusterId) + { + mCachedClusterEndpointId = path.mEndpointId; + mCachedInterface = current->serverClusterInterface; + return mCachedInterface; + } + + current = current->next; + } + + // not found + return nullptr; +} + +ServerClusterInterfaceRegistry::EndpointClusters * ServerClusterInterfaceRegistry::FindClusters(EndpointId endpointId) +{ + // invalid cluster id is NOT acceptable (since static allocation uses the + // invalid cluster id as a marker of "not used") + VerifyOrReturnValue(endpointId != kInvalidEndpointId, nullptr); + + for (EndpointClusters * p = mEndpoints; p != nullptr; p = p->next) + { + if (p->endpointId == endpointId) + { + return p; + } + } + + return nullptr; +} + +} // namespace app +} // namespace chip diff --git a/src/data-model-providers/codegen/ServerClusterInterfaceRegistry.h b/src/data-model-providers/codegen/ServerClusterInterfaceRegistry.h new file mode 100644 index 00000000000000..dfbcebad855b96 --- /dev/null +++ b/src/data-model-providers/codegen/ServerClusterInterfaceRegistry.h @@ -0,0 +1,141 @@ +/* + * Copyright (c) 2025 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 +#include +#include +#include + +namespace chip { +namespace app { + +/// Allows registering and retrieving ServerClusterInterface instances for specific cluster paths. +class ServerClusterInterfaceRegistry +{ +private: + /// A single-linked list of registered server cluster interfaces + struct RegisteredServerClusterInterface + { + // A single-linked list of clusters registered for the given `endpointId` + ServerClusterInterface * serverClusterInterface = nullptr; + RegisteredServerClusterInterface * next = nullptr; + + constexpr RegisteredServerClusterInterface(ServerClusterInterface * cluster, RegisteredServerClusterInterface * n) : + serverClusterInterface(cluster), next(n) + {} + }; + +public: + /// represents an iterable list of clusters + class ClustersList + { + public: + class Iterator + { + public: + Iterator(RegisteredServerClusterInterface * interface) : mInterface(interface) {} + + Iterator & operator++() + { + if (mInterface != nullptr) + { + mInterface = mInterface->next; + } + return *this; + } + bool operator==(const Iterator & other) const { return mInterface == other.mInterface; } + bool operator!=(const Iterator & other) const { return mInterface != other.mInterface; } + ServerClusterInterface * operator*() { return mInterface->serverClusterInterface; } + + private: + RegisteredServerClusterInterface * mInterface; + }; + + ClustersList(RegisteredServerClusterInterface * start) : mStart(start) {} + Iterator begin() { return mStart; } + Iterator end() { return nullptr; } + + private: + RegisteredServerClusterInterface * mStart; + }; + + ~ServerClusterInterfaceRegistry(); + + /// Associate a specific interface with the given endpoint. + /// + /// A `ServerClusterInterface` may only be associated on a single endpointId and + /// there can be only a single registration for a given `endpointId/clusterId` path. + /// + /// This means Register WILL RETURN AN ERROR if: + /// - A registration on the given `endpointId/cluster->GetClusterID()` already exists + /// - The given `cluster` is already registered for some enpoint id + /// + /// Registrations need a valid endpointId and cluster MUST return a valid cluster id. + [[nodiscard]] CHIP_ERROR Register(EndpointId endpointId, ServerClusterInterface * cluster); + + /// Remove an existing registration for a given endpoint/cluster path. + /// + /// Returns the previous registration if any exists (or nullptr if nothing + /// to unregister) + ServerClusterInterface * Unregister(const ConcreteClusterPath & path); + + /// Return the interface registered for the given cluster path or nullptr if one does not exist + ServerClusterInterface * Get(const ConcreteClusterPath & path); + + /// ClustersList is valid as a snapshot only and should not be saved. + ClustersList ClustersOnEndpoint(EndpointId endpointId); + + /// Unregister all registrations for the given endpoint. + void UnregisterAllFromEndpoint(EndpointId endpointId); + +private: + /// tracks clusters registered to a particular endpoint + struct EndpointClusters + { + // The endpointId for this registration. kInvalidEndpointId means + // not allocated/used + EndpointId endpointId = kInvalidEndpointId; + + // A single-linked list of clusters registered for the given `endpointId` + RegisteredServerClusterInterface * firstCluster = nullptr; + + /// A single-linked list of endpoint clusters that is dynamically + /// allocated. + EndpointClusters * next; + }; + + // Dynamic allocated endpoint cluters, once static allocation is used up + EndpointClusters * mEndpoints = nullptr; + + // a one-element cache to speed up finding a cluster within an endpoint. + // The endpointId specifies which endpoint the cache belongs to. + ClusterId mCachedClusterEndpointId = kInvalidEndpointId; + ServerClusterInterface * mCachedInterface = nullptr; + + /// returns nullptr if not found + EndpointClusters * FindClusters(EndpointId endpointId); + + /// Get a new usable endpoint cluster + CHIP_ERROR AllocateNewEndpointClusters(EndpointId endpointId, EndpointClusters *& dest); + + /// Clear and free memory for the given linked list + static void ClearSingleLinkedList(RegisteredServerClusterInterface * clusters); +}; + +} // namespace app +} // namespace chip diff --git a/src/data-model-providers/codegen/model.gni b/src/data-model-providers/codegen/model.gni index 51305a5fe30b5a..f9a8295fcb8593 100644 --- a/src/data-model-providers/codegen/model.gni +++ b/src/data-model-providers/codegen/model.gni @@ -39,6 +39,8 @@ codegen_data_model_SOURCES = [ codegen_data_model_PUBLIC_DEPS = [ "${chip_root}/src/app/common:attribute-type", "${chip_root}/src/app/data-model-provider", + "${chip_root}/src/app/server-cluster", "${chip_root}/src/data-model-providers/codegen:instance-header", + "${chip_root}/src/data-model-providers/codegen:registry", "${chip_root}/src/app/util/persistence", ] diff --git a/src/data-model-providers/codegen/tests/BUILD.gn b/src/data-model-providers/codegen/tests/BUILD.gn index 57facbee09ec10..a2d4fdec2cbee1 100644 --- a/src/data-model-providers/codegen/tests/BUILD.gn +++ b/src/data-model-providers/codegen/tests/BUILD.gn @@ -54,6 +54,7 @@ chip_test_suite("tests") { test_sources = [ "TestCodegenModelViaMocks.cpp", "TestEmberAttributeDataBuffer.cpp", + "TestServerClusterInterfaceRegistry.cpp", ] cflags = [ "-Wconversion" ] diff --git a/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp b/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp index bcd9d64d88d86d..b3dfcbff972d02 100644 --- a/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp +++ b/src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp @@ -2351,7 +2351,7 @@ TEST_F(TestCodegenModelViaMocks, EmberInvokeTest) const uint32_t kDispatchCountPre = chip::Test::DispatchCount(); // Using a handler set to nullptr as it is not used by the impl - ASSERT_EQ(model.Invoke(kInvokeRequest, tlvReader, /* handler = */ nullptr), std::nullopt); + ASSERT_EQ(model.InvokeCommand(kInvokeRequest, tlvReader, /* handler = */ nullptr), std::nullopt); EXPECT_EQ(chip::Test::DispatchCount(), kDispatchCountPre + 1); // single dispatch EXPECT_EQ(chip::Test::GetLastDispatchPath(), kCommandPath); // for the right path @@ -2365,7 +2365,7 @@ TEST_F(TestCodegenModelViaMocks, EmberInvokeTest) const uint32_t kDispatchCountPre = chip::Test::DispatchCount(); // Using a handler set to nullpotr as it is not used by the impl - ASSERT_EQ(model.Invoke(kInvokeRequest, tlvReader, /* handler = */ nullptr), std::nullopt); + ASSERT_EQ(model.InvokeCommand(kInvokeRequest, tlvReader, /* handler = */ nullptr), std::nullopt); EXPECT_EQ(chip::Test::DispatchCount(), kDispatchCountPre + 1); // single dispatch EXPECT_EQ(chip::Test::GetLastDispatchPath(), kCommandPath); // for the right path diff --git a/src/data-model-providers/codegen/tests/TestServerClusterInterfaceRegistry.cpp b/src/data-model-providers/codegen/tests/TestServerClusterInterfaceRegistry.cpp new file mode 100644 index 00000000000000..6731cf3a757f1d --- /dev/null +++ b/src/data-model-providers/codegen/tests/TestServerClusterInterfaceRegistry.cpp @@ -0,0 +1,307 @@ +/* + * Copyright (c) 2025 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 +#include +#include +#include + +#include +#include +#include + +using namespace chip; +using namespace chip::app; +using namespace chip::app::DataModel; +using namespace chip::app::Clusters; + +namespace { + +constexpr chip::EndpointId kEp1 = 1; +constexpr chip::EndpointId kEp2 = 2; +constexpr chip::EndpointId kEp3 = 3; +constexpr chip::ClusterId kCluster1 = 1; +constexpr chip::ClusterId kCluster2 = 2; +constexpr chip::ClusterId kCluster3 = 3; + +class FakeServerClusterInterface : public DefaultServerCluster +{ +public: + FakeServerClusterInterface(ClusterId id) : mClusterId(id) {} + + ClusterId GetClusterId() const override { return mClusterId; } + + DataModel::ActionReturnStatus ReadAttribute(const DataModel::ReadAttributeRequest & request, + AttributeValueEncoder & encoder) override + { + switch (request.path.mAttributeId) + { + case Globals::Attributes::FeatureMap::Id: + return encoder.Encode(0); + case Globals::Attributes::ClusterRevision::Id: + return encoder.Encode(123); + } + return CHIP_ERROR_INVALID_ARGUMENT; + } + +private: + ClusterId mClusterId; +}; + +struct TestServerClusterInterfaceRegistry : public ::testing::Test +{ + static void SetUpTestSuite() { ASSERT_EQ(chip::Platform::MemoryInit(), CHIP_NO_ERROR); } + static void TearDownTestSuite() { chip::Platform::MemoryShutdown(); } +}; + +} // namespace + +TEST_F(TestServerClusterInterfaceRegistry, BasicTest) +{ + // NOTE: tests DO NOT use the global registry and validate implementation + // details. + + ServerClusterInterfaceRegistry registry; + + FakeServerClusterInterface cluster1(kCluster1); + FakeServerClusterInterface cluster2(kCluster2); + FakeServerClusterInterface cluster3(kCluster3); + + // there should be nothing registered to start with. + EXPECT_EQ(registry.Get(ConcreteClusterPath(kEp1, kCluster1)), nullptr); + EXPECT_EQ(registry.Get(ConcreteClusterPath(kEp1, kCluster2)), nullptr); + EXPECT_EQ(registry.Get(ConcreteClusterPath(kEp2, kCluster2)), nullptr); + EXPECT_EQ(registry.Get(ConcreteClusterPath(kEp2, kCluster3)), nullptr); + EXPECT_EQ(registry.Get(ConcreteClusterPath(kInvalidEndpointId, kCluster2)), nullptr); + EXPECT_EQ(registry.Get(ConcreteClusterPath(kEp1, kInvalidClusterId)), nullptr); + + // registration of invalid values is not acceptable + EXPECT_EQ(registry.Register(kEp1, nullptr), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(registry.Register(kInvalidEndpointId, nullptr), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(registry.Register(kInvalidEndpointId, &cluster1), CHIP_ERROR_INVALID_ARGUMENT); + EXPECT_EQ(registry.Register(kInvalidEndpointId, &cluster2), CHIP_ERROR_INVALID_ARGUMENT); + { + FakeServerClusterInterface badCluster(kInvalidClusterId); + EXPECT_EQ(registry.Register(kInvalidEndpointId, &badCluster), CHIP_ERROR_INVALID_ARGUMENT); + } + + // should be able to register + EXPECT_EQ(registry.Register(kEp1, &cluster1), CHIP_NO_ERROR); + EXPECT_EQ(registry.Register(kEp2, &cluster2), CHIP_NO_ERROR); + EXPECT_EQ(registry.Register(kEp2, &cluster3), CHIP_NO_ERROR); + + // cannot register two implementations on the same path + { + FakeServerClusterInterface another1(kCluster1); + EXPECT_EQ(registry.Register(kEp1, &another1), CHIP_ERROR_DUPLICATE_KEY_ID); + } + + // Items can be found back + EXPECT_EQ(registry.Get({ kEp1, kCluster1 }), &cluster1); + EXPECT_EQ(registry.Get({ kEp2, kCluster2 }), &cluster2); + EXPECT_EQ(registry.Get({ kEp2, kCluster3 }), &cluster3); + + EXPECT_EQ(registry.Get({ kEp2, kCluster1 }), nullptr); + EXPECT_EQ(registry.Get({ kEp1, kCluster2 }), nullptr); + + // repeated calls work + EXPECT_EQ(registry.Get({ kEp1, kCluster2 }), nullptr); + EXPECT_EQ(registry.Get({ kEp1, kCluster2 }), nullptr); + EXPECT_EQ(registry.Get({ kEp1, kCluster2 }), nullptr); + EXPECT_EQ(registry.Get({ kEp2, kCluster1 }), nullptr); + EXPECT_EQ(registry.Get({ kEp2, kCluster1 }), nullptr); + EXPECT_EQ(registry.Get({ kEp2, kCluster1 }), nullptr); + + // remove registrations + EXPECT_EQ(registry.Unregister({ kEp2, kCluster2 }), &cluster2); + EXPECT_EQ(registry.Unregister({ kEp2, kCluster2 }), nullptr); + + // Re-adding works exactly once. + EXPECT_EQ(registry.Get({ kEp2, kCluster2 }), nullptr); + EXPECT_EQ(registry.Register(kEp3, &cluster2), CHIP_NO_ERROR); + EXPECT_EQ(registry.Get({ kEp3, kCluster2 }), &cluster2); + EXPECT_EQ(registry.Unregister({ kEp3, kCluster2 }), &cluster2); + EXPECT_EQ(registry.Get({ kEp2, kCluster2 }), nullptr); + EXPECT_EQ(registry.Unregister({ kEp3, kCluster2 }), nullptr); + EXPECT_EQ(registry.Get({ kEp2, kCluster2 }), nullptr); + + // cannot get it anymore once removed, others are still valid + EXPECT_EQ(registry.Get({ kEp2, kCluster2 }), nullptr); + EXPECT_EQ(registry.Get({ kEp2, kCluster3 }), &cluster3); + + // clean of an entire endpoint works + registry.UnregisterAllFromEndpoint(kEp2); + EXPECT_EQ(registry.Get({ kEp1, kCluster1 }), &cluster1); + EXPECT_EQ(registry.Get({ kEp2, kCluster2 }), nullptr); + EXPECT_EQ(registry.Get({ kEp2, kCluster3 }), nullptr); + + registry.UnregisterAllFromEndpoint(kEp1); + EXPECT_EQ(registry.Get({ kEp1, kCluster1 }), nullptr); + EXPECT_EQ(registry.Get({ kEp2, kCluster2 }), nullptr); + EXPECT_EQ(registry.Get({ kEp2, kCluster3 }), nullptr); +} + +TEST_F(TestServerClusterInterfaceRegistry, StressTest) +{ + // make the test repeatable + srand(1234); + + std::vector items; + + static constexpr ClusterId kClusterTestCount = 200; + static constexpr EndpointId kEndpointTestCount = 10; + static constexpr size_t kTestIterations = 4; + + static_assert(kInvalidClusterId > kClusterTestCount, "Tests assume all clusters IDs [0...] are valid"); + static_assert(kTestIterations > 1, "Tests use different unregister methods. Need 2 or more passes."); + + items.reserve(kClusterTestCount); + for (ClusterId i = 0; i < kClusterTestCount; i++) + { + items.emplace_back(i); + } + + ServerClusterInterfaceRegistry registry; + + for (size_t test = 0; test < kTestIterations; test++) + { + std::vector endpoint_placement; + endpoint_placement.reserve(kClusterTestCount); + + for (ClusterId i = 0; i < kClusterTestCount; i++) + { + auto endpointId = static_cast(rand() % kEndpointTestCount); + endpoint_placement.push_back(endpointId); + ASSERT_EQ(registry.Register(endpointId, &items[i]), CHIP_NO_ERROR); + } + + // test that getters work + for (ClusterId cluster = 0; cluster < kClusterTestCount; cluster++) + { + for (EndpointId ep = 0; ep < kEndpointTestCount; ep++) + { + if (endpoint_placement[cluster] == ep) + { + ASSERT_EQ(registry.Get({ ep, cluster }), &items[cluster]); + } + else + { + ASSERT_EQ(registry.Get({ ep, cluster }), nullptr); + } + } + } + + // clear endpoints. Stress test, unregister in different ways (bulk vs individual) + if (test % 2 == 1) + { + // shuffle unregister + std::vector unregister_order; + unregister_order.reserve(kClusterTestCount); + for (size_t i = 0; i < kClusterTestCount; i++) + { + unregister_order.push_back(i); + } + + std::default_random_engine eng(static_cast(rand())); + std::shuffle(unregister_order.begin(), unregister_order.end(), eng); + + // unregister + for (auto cluster : unregister_order) + { + // item MUST exist and be accessible + ASSERT_EQ(registry.Get({ endpoint_placement[cluster], static_cast(cluster) }), &items[cluster]); + ASSERT_EQ(registry.Unregister({ endpoint_placement[cluster], static_cast(cluster) }), &items[cluster]); + + // once unregistered, it is not there anymore + ASSERT_EQ(registry.Get({ endpoint_placement[cluster], static_cast(cluster) }), nullptr); + ASSERT_EQ(registry.Unregister({ endpoint_placement[cluster], static_cast(cluster) }), nullptr); + } + } + else + { + // bulk unregister + for (EndpointId ep = 0; ep < kEndpointTestCount; ep++) + { + registry.UnregisterAllFromEndpoint(ep); + } + } + + // all endpoints should be clear + for (ClusterId cluster = 0; cluster < kClusterTestCount; cluster++) + { + for (EndpointId ep = 0; ep < kEndpointTestCount; ep++) + { + ASSERT_EQ(registry.Get({ ep, cluster }), nullptr); + } + } + } +} + +TEST_F(TestServerClusterInterfaceRegistry, ClustersOnEndpoint) +{ + std::vector items; + + static constexpr ClusterId kClusterTestCount = 200; + static constexpr EndpointId kEndpointTestCount = 10; + + static_assert(kInvalidClusterId > kClusterTestCount, "Tests assume all clusters IDs [0...] are valid"); + + items.reserve(kClusterTestCount); + for (ClusterId i = 0; i < kClusterTestCount; i++) + { + items.emplace_back(i); + } + + ServerClusterInterfaceRegistry registry; + + // place the clusters on the respecitve endpoints + for (ClusterId i = 0; i < kClusterTestCount; i++) + { + ASSERT_EQ(registry.Register(static_cast(i % kEndpointTestCount), &items[i]), CHIP_NO_ERROR); + } + + // this IS implementation defined: we always register at "HEAD" so the listing is in + // INVERSE order of registering. + for (EndpointId ep = 0; ep < kEndpointTestCount; ep++) + { + // Move to the end since we iterate in reverse order + ClusterId expectedClusterId = ep + kEndpointTestCount * (kClusterTestCount / kEndpointTestCount); + if (expectedClusterId >= kClusterTestCount) + { + expectedClusterId -= kEndpointTestCount; + } + + // ensure that iteration happens exactly as we expect: reverse order and complete + for (auto cluster : registry.ClustersOnEndpoint(ep)) + { + ASSERT_LT(expectedClusterId, kClusterTestCount); + ASSERT_EQ(cluster->GetClusterId(), expectedClusterId); + expectedClusterId -= kEndpointTestCount; // next expected/registered cluster + } + + // Iterated through all : we overflowed and got a large number + ASSERT_GE(expectedClusterId, kClusterTestCount); + } + + // invalid index works and iteration on empty lists is ok + auto clusters = registry.ClustersOnEndpoint(kEndpointTestCount + 1); + ASSERT_EQ(clusters.begin(), clusters.end()); +} diff --git a/src/lib/core/CHIPError.cpp b/src/lib/core/CHIPError.cpp index 2068a9fbc68c42..de1f0b3862c5e3 100644 --- a/src/lib/core/CHIPError.cpp +++ b/src/lib/core/CHIPError.cpp @@ -178,6 +178,9 @@ bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err) case CHIP_ERROR_TLV_CONTAINER_OPEN.AsInteger(): desc = "TLV container open"; break; + case CHIP_ERROR_IN_USE.AsInteger(): + desc = "In use"; + break; case CHIP_ERROR_INVALID_MESSAGE_TYPE.AsInteger(): desc = "Invalid message type"; break; diff --git a/src/lib/core/CHIPError.h b/src/lib/core/CHIPError.h index 03217378eecb87..d1e1c12d562a10 100644 --- a/src/lib/core/CHIPError.h +++ b/src/lib/core/CHIPError.h @@ -837,7 +837,16 @@ using CHIP_ERROR = ::chip::ChipError; */ #define CHIP_ERROR_TLV_CONTAINER_OPEN CHIP_CORE_ERROR(0x27) -// AVAILABLE: 0x28 +/** + * @def CHIP_ERROR_IN_USE + * + * @brief + * A value is already used. Generally indicates an unavailable resrouce. + * As opposed to CHIP_ERROR_BUSY, the use is not considered transient/temporary. + * + */ +#define CHIP_ERROR_IN_USE CHIP_CORE_ERROR(0x28) + // AVAILABLE: 0x29 /** From 2848d2ebf637d187637a51e572ae8d9a8324473b Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 26 Feb 2025 11:07:17 -0500 Subject: [PATCH 2/3] Fix up test registration logic --- src/BUILD.gn | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/BUILD.gn b/src/BUILD.gn index 83c8a5a18647a6..4235064bfaccaa 100644 --- a/src/BUILD.gn +++ b/src/BUILD.gn @@ -58,6 +58,7 @@ if (chip_build_tests) { "${chip_root}/src/crypto/tests", "${chip_root}/src/inet/tests", "${chip_root}/src/lib/address_resolve/tests", + "${chip_root}/src/app/server-cluster/tests", "${chip_root}/src/lib/asn1/tests", "${chip_root}/src/lib/core/tests", "${chip_root}/src/lib/format/tests", @@ -94,12 +95,19 @@ if (chip_build_tests) { # symbols for ember mocks which are used by other tests. tests += [ - "${chip_root}/src/data-model-providers/codegen/tests", "${chip_root}/src/setup_payload/tests", "${chip_root}/src/transport/raw/tests", ] } + if (current_os != "zephyr" && current_os != "mbed" && + chip_device_platform != "efr32") { + # Disabled from "one single binary" since the "mocks" contain duplicate sybmols + # with ember + # Disabled on EFR32 since "include " fails with `::fabs` not defined + tests += [ "${chip_root}/src/data-model-providers/codegen/tests" ] + } + if (chip_device_platform != "efr32") { tests += [ "${chip_root}/src/app/tests", @@ -108,9 +116,6 @@ if (chip_build_tests) { # https://github.com/project-chip/connectedhomeip/issues/35624 "${chip_root}/src/credentials/tests", "${chip_root}/src/lib/support/tests", - - # Disabled on EFR32 since "include " fails with `::fabs` not defined - "${chip_root}/src/app/server-cluster/tests", ] } From ae9ddf799ce1c70415c59e82402cb12b370f07e8 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 26 Feb 2025 11:24:01 -0500 Subject: [PATCH 3/3] No need to override the assignment/constructors anymore since no more intrusiveness --- src/app/server-cluster/DefaultServerCluster.h | 6 ------ src/app/server-cluster/ServerClusterInterface.h | 7 ------- 2 files changed, 13 deletions(-) diff --git a/src/app/server-cluster/DefaultServerCluster.h b/src/app/server-cluster/DefaultServerCluster.h index 852aa9efcf6d89..71cf61fa42790c 100644 --- a/src/app/server-cluster/DefaultServerCluster.h +++ b/src/app/server-cluster/DefaultServerCluster.h @@ -38,12 +38,6 @@ class DefaultServerCluster : public ServerClusterInterface DefaultServerCluster(); ~DefaultServerCluster() override = default; - DefaultServerCluster(DefaultServerCluster && other) = default; - DefaultServerCluster & operator=(DefaultServerCluster && other) = default; - - DefaultServerCluster(const DefaultServerCluster & other) = delete; - DefaultServerCluster & operator=(const DefaultServerCluster & other) = delete; - void IncreaseDataVersion() { mDataVersion++; } //////////////////////////// ServerClusterInterface implementation //////////////////////////////////////// diff --git a/src/app/server-cluster/ServerClusterInterface.h b/src/app/server-cluster/ServerClusterInterface.h index e867c3daf85957..bc588deac4792f 100644 --- a/src/app/server-cluster/ServerClusterInterface.h +++ b/src/app/server-cluster/ServerClusterInterface.h @@ -40,15 +40,8 @@ namespace app { class ServerClusterInterface { public: - ServerClusterInterface() = default; virtual ~ServerClusterInterface() = default; - ServerClusterInterface(ServerClusterInterface && other) = default; - ServerClusterInterface & operator=(ServerClusterInterface && other) = default; - - ServerClusterInterface(const ServerClusterInterface & other) = delete; - ServerClusterInterface & operator=(const ServerClusterInterface & other) = delete; - ///////////////////////////////////// Cluster Metadata Support ////////////////////////////////////////////////// [[nodiscard]] virtual ClusterId GetClusterId() const = 0;