Skip to content

Commit c1f377d

Browse files
zaid-googlerestyled-commits
zaid-google
and
restyled-commits
authored
Migrate Descriptor Cluster to be Code Driven (#40935)
* Update zcl json and config-data * Codegen * Update code driven DMP and span endpoint for EndpointUniqueId * Update descriptor to be code driven, add unit test * Remove old descriptor * Restyled by whitespace * Restyled by clang-format * Restyled by gn * Restyled by prettier-json * Address review comments * Restyled by whitespace * Restyled by clang-format * Address review comments * Fix EndpointUniqueID() with char span and build issues * Restyled by clang-format * Address review comments * Fix rebase diff * Codegen * Update CodegenIntegration to match latest refactor * More CodegenIntegration and config-data updates * Codegen * Restyled by clang-format * Darwin changes * Put tag list attribute behind feature flag * Restyled by clang-format * Make feature map match old impl, and revert a darwin change * Restyled by clang-format * Fix unit test to startup with TestServerClusterContext * CodegenIntegration fixes for darwin * Restyled by clang-format * Darwin change with emberAfReadAttribute * Call descriptor codegenintegration bits * Remove app specfic codegen header * Restyled by whitespace * Restyled by clang-format * MTRIMDispatch changes * Restyled by whitespace * Update order of data on darwin test * More darwin test ordering fixes * Address some review comments * Fix namespace * Fix Codegen after rebase * Restyled by whitespace * Restyled by clang-format * Address more review comments * Restyled by clang-format * Add comment on lifetime * Flash reduction with encode list calls * Restyled by whitespace * Restyled by prettier-json * Revert "Flash reduction with encode list calls" This reverts commit c460a82. * Changes to semantic tags * Restyled by whitespace * Restyled by clang-format * Fix typename * Fix build errors * More build issues * Restyled by whitespace * Fix include and alias * More build fixes * Restyled by whitespace * Restyled by clang-format * Restyled by prettier-json * Fix build issues with semantic tag struct * Address review comments * Restyled by whitespace * Restyled by clang-format * Fix unit tests with global attribute ordering * Address some review comments * Restyled by clang-format * Build fix * Make a descriptor cluster sub-class that can use ember semantic tag calls * Restyled by clang-format * Add comment on semantic tag Span lifetime * Restyled by whitespace * Restyled by clang-format * Add comment about ember descriptor cluster * Restyled by whitespace * Make GetSemanticTagsForEndpoint be called once * Restyled by clang-format * Update comment * Restyled by whitespace * Restyled by clang-format * Codegen * Restyled by whitespace * Restyled by prettier-json * Fix FakeDefaultServerCluster constructor --------- Co-authored-by: Restyled.io <[email protected]>
1 parent 685d31a commit c1f377d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+772
-506
lines changed

‎.github/workflows/lint.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ jobs:
351351
type-safe getters
352352
if: always()
353353
run: |
354-
git grep -I -n 'emberAfReadAttribute' -- './*' ':(exclude).github/workflows/lint.yml' ':(exclude)src/app/util/attribute-table.h' ':(exclude)zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp' ':(exclude)src/app/zap-templates/templates/app/attributes/Accessors.cpp.zapt' ':(exclude)src/app/util/attribute-table.cpp' ':(exclude)src/app/dynamic_server/DynamicDispatcher.cpp' ':(exclude)src/data-model-providers/codegen/ClusterIntegration.cpp' && exit 1 || exit 0
354+
git grep -I -n 'emberAfReadAttribute' -- './*' ':(exclude).github/workflows/lint.yml' ':(exclude)src/app/util/attribute-table.h' ':(exclude)zzz_generated/app-common/app-common/zap-generated/attributes/Accessors.cpp' ':(exclude)src/app/zap-templates/templates/app/attributes/Accessors.cpp.zapt' ':(exclude)src/app/util/attribute-table.cpp' ':(exclude)src/app/dynamic_server/DynamicDispatcher.cpp' ':(exclude)src/data-model-providers/codegen/ClusterIntegration.cpp' ':(exclude)src/darwin/Framework/CHIP/ServerEndpoint/MTRIMDispatch.mm' && exit 1 || exit 0
355355
356356
# git grep exits with 0 if it finds a match, but we want
357357
# to fail (exit nonzero) on match. And we want to exclude this file,

‎scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/CodeDrivenCallback.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ void MatterIdentifyClusterInitCallback(chip::EndpointId endpointId);
2727

2828
void MatterIdentifyClusterShutdownCallback(chip::EndpointId endpointId);
2929

30+
void MatterDescriptorClusterInitCallback(chip::EndpointId endpointId);
31+
32+
void MatterDescriptorClusterShutdownCallback(chip::EndpointId endpointId);
33+
3034
void MatterBindingClusterInitCallback(chip::EndpointId endpointId);
3135

3236
void MatterBindingClusterShutdownCallback(chip::EndpointId endpointId);

‎scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/CodeDrivenInitShutdown.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ void MatterClusterServerInitCallback(EndpointId endpoint, ClusterId clusterId)
3030
case app::Clusters::Identify::Id:
3131
MatterIdentifyClusterInitCallback(endpoint);
3232
break;
33+
case app::Clusters::Descriptor::Id:
34+
MatterDescriptorClusterInitCallback(endpoint);
35+
break;
3336
case app::Clusters::Binding::Id:
3437
MatterBindingClusterInitCallback(endpoint);
3538
break;
@@ -91,6 +94,9 @@ void MatterClusterServerShutdownCallback(EndpointId endpoint, ClusterId clusterI
9194
case app::Clusters::Identify::Id:
9295
MatterIdentifyClusterShutdownCallback(endpoint);
9396
break;
97+
case app::Clusters::Descriptor::Id:
98+
MatterDescriptorClusterShutdownCallback(endpoint);
99+
break;
94100
case app::Clusters::Binding::Id:
95101
MatterBindingClusterShutdownCallback(endpoint);
96102
break;

‎scripts/tools/zap/tests/outputs/lighting-app/app-templates/CodeDrivenCallback.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ void MatterIdentifyClusterInitCallback(chip::EndpointId endpointId);
2727

2828
void MatterIdentifyClusterShutdownCallback(chip::EndpointId endpointId);
2929

30+
void MatterDescriptorClusterInitCallback(chip::EndpointId endpointId);
31+
32+
void MatterDescriptorClusterShutdownCallback(chip::EndpointId endpointId);
33+
3034
void MatterAccessControlClusterInitCallback(chip::EndpointId endpointId);
3135

3236
void MatterAccessControlClusterShutdownCallback(chip::EndpointId endpointId);

‎scripts/tools/zap/tests/outputs/lighting-app/app-templates/CodeDrivenInitShutdown.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ void MatterClusterServerInitCallback(EndpointId endpoint, ClusterId clusterId)
3030
case app::Clusters::Identify::Id:
3131
MatterIdentifyClusterInitCallback(endpoint);
3232
break;
33+
case app::Clusters::Descriptor::Id:
34+
MatterDescriptorClusterInitCallback(endpoint);
35+
break;
3336
case app::Clusters::AccessControl::Id:
3437
MatterAccessControlClusterInitCallback(endpoint);
3538
break;
@@ -85,6 +88,9 @@ void MatterClusterServerShutdownCallback(EndpointId endpoint, ClusterId clusterI
8588
case app::Clusters::Identify::Id:
8689
MatterIdentifyClusterShutdownCallback(endpoint);
8790
break;
91+
case app::Clusters::Descriptor::Id:
92+
MatterDescriptorClusterShutdownCallback(endpoint);
93+
break;
8894
case app::Clusters::AccessControl::Id:
8995
MatterAccessControlClusterShutdownCallback(endpoint);
9096
break;

‎src/BUILD.gn‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ if (chip_build_tests) {
5555
"${chip_root}/src/app/clusters/access-control-server/tests",
5656
"${chip_root}/src/app/clusters/basic-information/tests",
5757
"${chip_root}/src/app/clusters/bindings/tests",
58+
"${chip_root}/src/app/clusters/descriptor/tests",
5859
"${chip_root}/src/app/clusters/general-commissioning-server/tests",
5960
"${chip_root}/src/app/clusters/general-diagnostics-server/tests",
6061
"${chip_root}/src/app/clusters/group-key-mgmt-server/tests",

‎src/access/tests/TestProviderDeviceTypeResolver.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ class FakeProvider final : public Provider
6767
}
6868

6969
// The following methods are not used in this test, but must be implemented as they are pure virtual in the Provider interface.
70-
CHIP_ERROR SemanticTags(EndpointId, ReadOnlyBufferBuilder<SemanticTag> &) override { return CHIP_NO_ERROR; }
7170
CHIP_ERROR ClientClusters(EndpointId, ReadOnlyBufferBuilder<ClusterId> &) override { return CHIP_NO_ERROR; }
7271
CHIP_ERROR ServerClusters(EndpointId, ReadOnlyBufferBuilder<ServerClusterEntry> &) override { return CHIP_NO_ERROR; }
7372
CHIP_ERROR Endpoints(ReadOnlyBufferBuilder<EndpointEntry> &) override { return CHIP_NO_ERROR; }

‎src/app/clusters/descriptor/BUILD.gn‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,24 @@
1111
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
14-
group("descriptor") {
14+
import("//build_overrides/build.gni")
15+
import("//build_overrides/chip.gni")
16+
17+
source_set("descriptor") {
18+
sources = [
19+
"descriptor-cluster.cpp",
20+
"descriptor-cluster.h",
21+
]
22+
23+
public_deps = [
24+
"${chip_root}/src/app:attribute-access",
25+
"${chip_root}/src/app/data-model-provider:metadata",
26+
"${chip_root}/src/app/server",
27+
"${chip_root}/src/app/server-cluster",
28+
"${chip_root}/src/lib/core:error",
29+
"${chip_root}/src/lib/support",
30+
"${chip_root}/zzz_generated/app-common/clusters/Descriptor",
31+
]
32+
33+
public_configs = [ "${chip_root}/src:includes" ]
1534
}

‎src/app/clusters/descriptor/CodegenIntegration.cpp‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
/**
2+
*
3+
* Copyright (c) 2025 Project CHIP Authors
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+
18+
#include <app/clusters/descriptor/descriptor-cluster.h>
19+
20+
#if !CHIP_CONFIG_SKIP_APP_SPECIFIC_GENERATED_HEADER_INCLUDES
21+
#include <app/static-cluster-config/Descriptor.h>
22+
#endif // CHIP_CONFIG_SKIP_APP_SPECIFIC_GENERATED_HEADER_INCLUDES
23+
24+
#include <app/util/attribute-storage.h>
25+
#include <app/util/config.h>
26+
#include <data-model-providers/codegen/ClusterIntegration.h>
27+
#include <lib/core/CHIPConfig.h>
28+
29+
using namespace chip;
30+
using namespace chip::app;
31+
using namespace chip::app::Clusters;
32+
using namespace chip::app::Clusters::Descriptor;
33+
34+
namespace {
35+
36+
/**
37+
* This is a DescriptorCluster class made specifically to fetch the tag list once through ember before one of either the
38+
* Attributes() or ReadAttribute() functions are called. This can NOT be called before endpoint init and passed to the constructor
39+
* of the regular DescriptorCluster class. This is because for fixed endpoints, we define endpoints in emberAfEndpointConfigure()
40+
* and init them in emberAfInit() with back to back calls in InitDataModelHandler(). For dynamic endpoints, we init endpoints in
41+
* emberAfSetDynamicEndpointWithEpUniqueId() by calling emberAfEndpointEnableDisable(), which calls initializeEndpoint(). The tag
42+
* list is a fixed attribute, but to maintain backwards compatiblility we get that information within the functions here.
43+
*/
44+
class EmberDescriptorCluster : public DescriptorCluster
45+
{
46+
public:
47+
EmberDescriptorCluster(EndpointId endpointId, DescriptorCluster::OptionalAttributesSet optionalAttributeSet,
48+
Span<const SemanticTag> semanticTags) :
49+
DescriptorCluster(endpointId, optionalAttributeSet, semanticTags)
50+
{}
51+
52+
CHIP_ERROR Attributes(const ConcreteClusterPath & path, ReadOnlyBufferBuilder<DataModel::AttributeEntry> & builder) override
53+
{
54+
if (!mFetchedSemanticTags)
55+
{
56+
GetSemanticTagsForEndpoint(path.mEndpointId, mSemanticTags);
57+
mFetchedSemanticTags = true;
58+
}
59+
return DescriptorCluster::Attributes(path, builder);
60+
}
61+
62+
DataModel::ActionReturnStatus ReadAttribute(const DataModel::ReadAttributeRequest & request,
63+
AttributeValueEncoder & encoder) override
64+
{
65+
if (!mFetchedSemanticTags)
66+
{
67+
GetSemanticTagsForEndpoint(request.path.mEndpointId, mSemanticTags);
68+
mFetchedSemanticTags = true;
69+
}
70+
return DescriptorCluster::ReadAttribute(request, encoder);
71+
}
72+
73+
private:
74+
bool mFetchedSemanticTags = false;
75+
};
76+
77+
#if CHIP_CONFIG_SKIP_APP_SPECIFIC_GENERATED_HEADER_INCLUDES
78+
static constexpr size_t kDescriptorFixedClusterCount = 0;
79+
#else
80+
static constexpr size_t kDescriptorFixedClusterCount = Descriptor::StaticApplicationConfig::kFixedClusterConfig.size();
81+
#endif
82+
static constexpr size_t kDescriptorMaxClusterCount = kDescriptorFixedClusterCount + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT;
83+
84+
LazyRegisteredServerCluster<EmberDescriptorCluster> gServers[kDescriptorMaxClusterCount];
85+
86+
class IntegrationDelegate : public CodegenClusterIntegration::Delegate
87+
{
88+
public:
89+
ServerClusterRegistration & CreateRegistration(EndpointId endpointId, unsigned clusterInstanceIndex,
90+
uint32_t optionalAttributeBits, uint32_t featureMap) override
91+
{
92+
gServers[clusterInstanceIndex].Create(endpointId, DescriptorCluster::OptionalAttributesSet(optionalAttributeBits),
93+
Span<const Clusters::Descriptor::Structs::SemanticTagStruct::Type>());
94+
return gServers[clusterInstanceIndex].Registration();
95+
}
96+
97+
ServerClusterInterface * FindRegistration(unsigned clusterInstanceIndex) override
98+
{
99+
VerifyOrReturnValue(gServers[clusterInstanceIndex].IsConstructed(), nullptr);
100+
return &gServers[clusterInstanceIndex].Cluster();
101+
}
102+
void ReleaseRegistration(unsigned clusterInstanceIndex) override { gServers[clusterInstanceIndex].Destroy(); }
103+
};
104+
} // namespace
105+
106+
void MatterDescriptorClusterInitCallback(EndpointId endpointId)
107+
{
108+
IntegrationDelegate integrationDelegate;
109+
110+
CodegenClusterIntegration::RegisterServer(
111+
{
112+
.endpointId = endpointId,
113+
.clusterId = Descriptor::Id,
114+
.fixedClusterInstanceCount = kDescriptorFixedClusterCount,
115+
.maxClusterInstanceCount = kDescriptorMaxClusterCount,
116+
.fetchFeatureMap = false,
117+
.fetchOptionalAttributes = true,
118+
},
119+
integrationDelegate);
120+
}
121+
122+
void MatterDescriptorClusterShutdownCallback(EndpointId endpointId)
123+
{
124+
IntegrationDelegate integrationDelegate;
125+
126+
CodegenClusterIntegration::UnregisterServer(
127+
{
128+
.endpointId = endpointId,
129+
.clusterId = Descriptor::Id,
130+
.fixedClusterInstanceCount = kDescriptorFixedClusterCount,
131+
.maxClusterInstanceCount = kDescriptorMaxClusterCount,
132+
},
133+
integrationDelegate);
134+
}
135+
136+
void MatterDescriptorPluginServerInitCallback() {}
137+
138+
void MatterDescriptorPluginServerShutdownCallback() {}

‎src/app/clusters/descriptor/app_config_dependent_sources.cmake‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,13 @@
1616
TARGET_SOURCES(
1717
${APP_TARGET}
1818
PRIVATE
19-
"${CLUSTER_DIR}/descriptor.cpp"
19+
"${CLUSTER_DIR}/CodegenIntegration.cpp"
20+
)
21+
22+
# These are the things that BUILD.gn dependencies would pull
23+
TARGET_SOURCES(
24+
${APP_TARGET}
25+
PRIVATE
26+
"${CLUSTER_DIR}/descriptor-cluster.h"
27+
"${CLUSTER_DIR}/descriptor-cluster.cpp"
2028
)

0 commit comments

Comments
 (0)