-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ServerCluster base class and Ember adapter #33480
base: master
Are you sure you want to change the base?
Add ServerCluster base class and Ember adapter #33480
Conversation
PR #33480: Size comparison from be581ef to fe2b2c6 Increases (2 builds for linux)
Decreases (4 builds for efr32, linux)
Full report (83 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, mbed, nrfconnect, nxp, psoc6, qpg, stm32, telink)
|
@@ -328,6 +328,8 @@ static_library("app") { | |||
"OTAUserConsentCommon.h", | |||
"ReadHandler.cpp", | |||
"SafeAttributePersistenceProvider.h", | |||
"ServerCluster.cpp", | |||
"ServerCluster.h", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wraps AAI ... seems better to split it out of "app" ... maybe it should be tied to attribute-access
instead. We should stop having "app" being a container of everything.
@@ -23,6 +23,7 @@ source_set("types") { | |||
"attribute-metadata.h", | |||
"attribute-storage-null-handling.h", | |||
"basic-types.h", | |||
"ember-server-cluster.h", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Types generally should not depend on generated code. This depends on app-common/zap-generated/callback.h
so it seems it should not be here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah these dependencies are tricky... my goal is to have the ember adapters statically allocated based on the MATTER_DM_*_SERVER_ENDPOINT_COUNT
without having to manually write "stub" files for each cluster.
A different possible approach would be to address this is in the zap templates directly, maybe by listing the clusters that are based on ServerCluster
in config-data.yaml and then have the code generation do the right thing automatically.
@ksperling-apple please provide a description on include dependencies for these things. We still have the ember and generated code dependencies very hard to untangle, so it looks like we need to have a solid plan on how things include each-other. GN does not seem to enforce |
This adds a
ServerCluster
base class that extendsAttributeAccessInterface
andCommandHandlerInterface
, and automatically registers the handlers inInit
.Additionally, a
DECLARE_SERVER_CLUSTER
macro is provided that defines a global holder object with storage forMATTER_DM_##NAME##_SERVER_ENDPOINT_COUNT
instances of the concreteServerCluster
sub-class, and defines theemberAf...ClusterServerInitCallback
andMatter...ClusterServerShutdownCallback
functions to initialize those cluster instances based on ZAP configuration.