Skip to content

Commit f36dfde

Browse files
committed
Squashed commit of a server cluster interface.
This defines a ServerClusterInterface class along with a registry. Also slight update to build_coverage to run on more (specifically my) machines without errors. Implementation notes: - Registry does NOT allow anymore to have "wildcard register" on endpoints. This is because we expect to have attribute members (no more "free/implicit" storage from ember buffers). - Interface now includes metadata for attributes (AAI does not) and maintains the cluster version. - This does not yet include changes in #37526 (so no AAI replacement for list begin/end) as that interface gets finalized. It will have it once that PR is finalized and merged. - Registry has no "cache" but uses a "move found item to front of list". This is ok for individual requests however for full iterations it would be slower (about 2x if we always iterate over all clusters ... since then average search would be N instead of N/2 for a list that never changes). We can revisit this approach at any time This is currently unused. I expect usages to add flash & RAM cost, that will be slowly offset (especially RAM) as we move clusters around: - RAM for linked list will be one list instead of 2 (AAI to CHI) so any cluster moved that uses both would save 1 pointer metadata will increase slightly, can be reduced if we get ember to stop generating metadata for included clusters - RAM can decrease by FeatureMap and ClusterRevision if we stop ember from allocating space for it (however we will offset this by flash "encode const value"). - RAM usage increases slightly during conversion for "store version data" which ember currently allocates for all clusters. - Very long term: if we replace all clusters, we can drop AAI/CHI and that would save some flash. Still expect no savings because new interface does all of AAI and CHI and a bit more. TLDR on resourcing: probably ok on RAM over time, there is a flash overhead for this, claiming this is important for testable and maintainable code in the future.
1 parent 4e3f1cd commit f36dfde

28 files changed

+1313
-29
lines changed

docs/ids_and_codes/ERROR_CODES.md

+1
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ This file was **AUTOMATICALLY** generated by
5555
| 37 | 0x25 | `CHIP_ERROR_UNKNOWN_IMPLICIT_TLV_TAG` |
5656
| 38 | 0x26 | `CHIP_ERROR_WRONG_TLV_TYPE` |
5757
| 39 | 0x27 | `CHIP_ERROR_TLV_CONTAINER_OPEN` |
58+
| 40 | 0x28 | `CHIP_ERROR_IN_USE` |
5859
| 42 | 0x2A | `CHIP_ERROR_INVALID_MESSAGE_TYPE` |
5960
| 43 | 0x2B | `CHIP_ERROR_UNEXPECTED_TLV_ELEMENT` |
6061
| 45 | 0x2D | `CHIP_ERROR_NOT_IMPLEMENTED` |

scripts/build_coverage.sh

+4-1
Original file line numberDiff line numberDiff line change
@@ -222,16 +222,19 @@ lcov --initial --capture --directory "$OUTPUT_ROOT/obj/src" \
222222
--exclude="$PWD"/zzz_generated/* \
223223
--exclude="$PWD"/third_party/* \
224224
--exclude=/usr/include/* \
225+
--ignore-errors inconsistent \
225226
--output-file "$COVERAGE_ROOT/lcov_base.info"
226227

227228
lcov --capture --directory "$OUTPUT_ROOT/obj/src" \
228229
--ignore-errors inconsistent \
229230
--exclude="$PWD"/zzz_generated/* \
230231
--exclude="$PWD"/third_party/* \
231232
--exclude=/usr/include/* \
233+
--ignore-errors inconsistent \
232234
--output-file "$COVERAGE_ROOT/lcov_test.info"
233235

234-
lcov --add-tracefile "$COVERAGE_ROOT/lcov_base.info" \
236+
lcov --ignore-errors inconsistent \
237+
--add-tracefile "$COVERAGE_ROOT/lcov_base.info" \
235238
--add-tracefile "$COVERAGE_ROOT/lcov_test.info" \
236239
--ignore-errors inconsistent \
237240
--output-file "$COVERAGE_ROOT/lcov_final.info"

src/BUILD.gn

+10-2
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,14 @@ if (chip_build_tests) {
5151
deps = []
5252
tests = [
5353
"${chip_root}/src/access/tests",
54-
"${chip_root}/src/app/data-model/tests",
5554
"${chip_root}/src/app/cluster-building-blocks/tests",
5655
"${chip_root}/src/app/data-model-provider/tests",
56+
"${chip_root}/src/app/data-model/tests",
5757
"${chip_root}/src/app/icd/server/tests",
5858
"${chip_root}/src/crypto/tests",
5959
"${chip_root}/src/inet/tests",
6060
"${chip_root}/src/lib/address_resolve/tests",
61+
"${chip_root}/src/app/server-cluster/tests",
6162
"${chip_root}/src/lib/asn1/tests",
6263
"${chip_root}/src/lib/core/tests",
6364
"${chip_root}/src/lib/format/tests",
@@ -94,12 +95,19 @@ if (chip_build_tests) {
9495
# symbols for ember mocks which are used by other tests.
9596

9697
tests += [
97-
"${chip_root}/src/data-model-providers/codegen/tests",
9898
"${chip_root}/src/setup_payload/tests",
9999
"${chip_root}/src/transport/raw/tests",
100100
]
101101
}
102102

103+
if (current_os != "zephyr" && current_os != "mbed" &&
104+
chip_device_platform != "efr32") {
105+
# Disabled from "one single binary" since the "mocks" contain duplicate sybmols
106+
# with ember
107+
# Disabled on EFR32 since "include <random>" fails with `::fabs` not defined
108+
tests += [ "${chip_root}/src/data-model-providers/codegen/tests" ]
109+
}
110+
103111
if (chip_device_platform != "efr32") {
104112
tests += [
105113
"${chip_root}/src/app/tests",

src/app/InteractionModelEngine.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1734,7 +1734,7 @@ void InteractionModelEngine::DispatchCommand(CommandHandlerImpl & apCommandObj,
17341734
request.invokeFlags.Set(DataModel::InvokeFlags::kTimed, apCommandObj.IsTimedInvoke());
17351735
request.subjectDescriptor = &subjectDescriptor;
17361736

1737-
std::optional<DataModel::ActionReturnStatus> status = GetDataModelProvider()->Invoke(request, apPayload, &apCommandObj);
1737+
std::optional<DataModel::ActionReturnStatus> status = GetDataModelProvider()->InvokeCommand(request, apPayload, &apCommandObj);
17381738

17391739
// Provider indicates that handler status or data was already set (or will be set asynchronously) by
17401740
// returning std::nullopt. If any other value is returned, it is requesting that a status is set. This

src/app/data-model-provider/Provider.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,8 @@ class Provider : public ProviderMetadataTree
114114
/// - if a value is returned (not nullopt) then the handler response MUST NOT be filled. The caller
115115
/// will then issue `handler->AddStatus(request.path, <return_value>->GetStatusCode())`. This is a
116116
/// convenience to make writing Invoke calls easier.
117-
virtual std::optional<ActionReturnStatus> Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments,
118-
CommandHandler * handler) = 0;
117+
virtual std::optional<ActionReturnStatus> InvokeCommand(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments,
118+
CommandHandler * handler) = 0;
119119

120120
private:
121121
InteractionModelContext mContext = { nullptr };

src/app/data-model-provider/tests/TestActionReturnStatus.cpp

+2-4
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,11 @@
1515
* See the License for the specific language governing permissions and
1616
* limitations under the License.
1717
*/
18-
19-
#include "lib/core/CHIPError.h"
20-
#include "protocols/interaction_model/StatusCode.h"
21-
#include "pw_unit_test/framework_backend.h"
2218
#include <app/data-model-provider/ActionReturnStatus.h>
2319
#include <app/data-model-provider/StringBuilderAdapters.h>
20+
#include <lib/core/CHIPError.h>
2421
#include <lib/support/CodeUtils.h>
22+
#include <protocols/interaction_model/StatusCode.h>
2523

2624
#include <pw_unit_test/framework.h>
2725

src/app/server-cluster/BUILD.gn

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Copyright (c) 2025 Project CHIP Authors
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
import("//build_overrides/chip.gni")
15+
16+
source_set("server-cluster") {
17+
sources = [
18+
"DefaultServerCluster.cpp",
19+
"DefaultServerCluster.h",
20+
"ServerClusterInterface.h",
21+
]
22+
23+
public_deps = [
24+
"${chip_root}/src/app:attribute-access",
25+
"${chip_root}/src/app:command-handler-interface",
26+
"${chip_root}/src/app:paths",
27+
"${chip_root}/src/app/common:ids",
28+
"${chip_root}/src/app/data-model-provider",
29+
"${chip_root}/src/app/data-model-provider:metadata",
30+
"${chip_root}/src/crypto",
31+
"${chip_root}/src/lib/core:error",
32+
"${chip_root}/src/lib/core:types",
33+
"${chip_root}/src/lib/support",
34+
]
35+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/*
2+
* Copyright (c) 2025 Project CHIP Authors
3+
* All rights reserved.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
#include <app/server-cluster/DefaultServerCluster.h>
18+
19+
#include <access/Privilege.h>
20+
#include <app-common/zap-generated/ids/Attributes.h>
21+
#include <app/data-model-provider/MetadataTypes.h>
22+
#include <crypto/RandUtils.h>
23+
#include <lib/support/BitFlags.h>
24+
#include <optional>
25+
#include <protocols/interaction_model/StatusCode.h>
26+
27+
namespace chip {
28+
namespace app {
29+
namespace {
30+
31+
using namespace chip::app::Clusters;
32+
using namespace chip::app::DataModel;
33+
34+
constexpr std::array<AttributeEntry, 5> kGlobalAttributeEntries{ {
35+
{
36+
Globals::Attributes::ClusterRevision::Id,
37+
BitFlags<AttributeQualityFlags>(),
38+
Access::Privilege::kView,
39+
std::nullopt,
40+
},
41+
{
42+
Globals::Attributes::FeatureMap::Id,
43+
BitFlags<AttributeQualityFlags>(),
44+
Access::Privilege::kView,
45+
std::nullopt,
46+
},
47+
{
48+
Globals::Attributes::AttributeList::Id,
49+
BitFlags<AttributeQualityFlags>(AttributeQualityFlags::kListAttribute),
50+
Access::Privilege::kView,
51+
std::nullopt,
52+
},
53+
{
54+
Globals::Attributes::AcceptedCommandList::Id,
55+
BitFlags<AttributeQualityFlags>(AttributeQualityFlags::kListAttribute),
56+
Access::Privilege::kView,
57+
std::nullopt,
58+
},
59+
{
60+
Globals::Attributes::GeneratedCommandList::Id,
61+
BitFlags<AttributeQualityFlags>(AttributeQualityFlags::kListAttribute),
62+
Access::Privilege::kView,
63+
std::nullopt,
64+
},
65+
} };
66+
67+
} // namespace
68+
69+
DefaultServerCluster::DefaultServerCluster()
70+
{
71+
// SPEC - 7.10.3. Cluster Data Version
72+
// A cluster data version SHALL be initialized randomly when it is first published.
73+
mDataVersion = Crypto::GetRandU32();
74+
}
75+
76+
CHIP_ERROR DefaultServerCluster::Attributes(const ConcreteClusterPath & path, DataModel::ListBuilder<AttributeEntry> & builder)
77+
{
78+
79+
return builder.ReferenceExisting(kGlobalAttributeEntries);
80+
}
81+
82+
BitFlags<ClusterQualityFlags> DefaultServerCluster::GetClusterFlags() const
83+
{
84+
return {};
85+
}
86+
87+
ActionReturnStatus DefaultServerCluster::WriteAttribute(const WriteAttributeRequest & request, AttributeValueDecoder & decoder)
88+
{
89+
return Protocols::InteractionModel::Status::UnsupportedWrite;
90+
}
91+
92+
std::optional<ActionReturnStatus>
93+
DefaultServerCluster::InvokeCommand(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, CommandHandler * handler)
94+
{
95+
return Protocols::InteractionModel::Status::UnsupportedCommand;
96+
}
97+
98+
CHIP_ERROR DefaultServerCluster::AcceptedCommands(const ConcreteClusterPath & path,
99+
DataModel::ListBuilder<AcceptedCommandEntry> & builder)
100+
{
101+
return CHIP_NO_ERROR;
102+
}
103+
104+
CHIP_ERROR DefaultServerCluster::GeneratedCommands(const ConcreteClusterPath & path, DataModel::ListBuilder<CommandId> & builder)
105+
{
106+
return CHIP_NO_ERROR;
107+
}
108+
109+
} // namespace app
110+
} // namespace chip
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/*
2+
* Copyright (c) 2025 Project CHIP Authors
3+
* All rights reserved.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
#pragma once
18+
19+
#include <app/server-cluster/ServerClusterInterface.h>
20+
21+
namespace chip {
22+
namespace app {
23+
24+
/// Provides an implementation of most methods for a `ServerClusterInterface`
25+
/// to make it easier to implement spec-compliant classes.
26+
///
27+
/// In particular it does:
28+
/// - maintains a data version and provides `IncreaseDataVersion`. Ensures this
29+
/// version is spec-compliant initialized (with a random value)
30+
/// - Provides default implementations for most virtual methods EXCEPT:
31+
/// - ReadAttribute (since that one needs to handle featuremap and revision)
32+
/// - GetClusterId (since every implementation is for different clusters)
33+
///
34+
///
35+
class DefaultServerCluster : public ServerClusterInterface
36+
{
37+
public:
38+
DefaultServerCluster();
39+
~DefaultServerCluster() override = default;
40+
41+
//////////////////////////// ServerClusterInterface implementation ////////////////////////////////////////
42+
43+
[[nodiscard]] DataVersion GetDataVersion() const override { return mDataVersion; }
44+
[[nodiscard]] BitFlags<DataModel::ClusterQualityFlags> GetClusterFlags() const override;
45+
46+
/// Default implementation errors out with an unsupported write on every attribute.
47+
DataModel::ActionReturnStatus WriteAttribute(const DataModel::WriteAttributeRequest & request,
48+
AttributeValueDecoder & decoder) override;
49+
50+
/// Must only be implemented if support for any non-global attributes
51+
/// is required.
52+
///
53+
/// Default implementation just returns the global attributes required by the API contract.
54+
CHIP_ERROR Attributes(const ConcreteClusterPath & path, DataModel::ListBuilder<DataModel::AttributeEntry> & builder) override;
55+
56+
///////////////////////////////////// Command Support /////////////////////////////////////////////////////////
57+
58+
/// Must only be implemented if commands are supported by the cluster
59+
///
60+
/// Default implementation errors out with an UnsupportedCommand error.
61+
std::optional<DataModel::ActionReturnStatus> InvokeCommand(const DataModel::InvokeRequest & request,
62+
chip::TLV::TLVReader & input_arguments,
63+
CommandHandler * handler) override;
64+
65+
/// Must only be implemented if commands are supported by the cluster
66+
///
67+
/// Default implementation is a NOOP (no list items generated)
68+
CHIP_ERROR AcceptedCommands(const ConcreteClusterPath & path,
69+
DataModel::ListBuilder<DataModel::AcceptedCommandEntry> & builder) override;
70+
71+
/// Must only be implemented if commands that return values are supported by the cluster.
72+
///
73+
/// Default implementation is a NOOP (no list items generated)
74+
CHIP_ERROR GeneratedCommands(const ConcreteClusterPath & path, DataModel::ListBuilder<CommandId> & builder) override;
75+
76+
protected:
77+
void IncreaseDataVersion() { mDataVersion++; }
78+
79+
private:
80+
DataVersion mDataVersion; // will be random-initialized as per spec
81+
};
82+
83+
} // namespace app
84+
} // namespace chip

0 commit comments

Comments
 (0)