-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Introduce the CodeDrivenDataModelProvider #39974
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
base: master
Are you sure you want to change the base?
Introduce the CodeDrivenDataModelProvider #39974
Conversation
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.
Code Review
This pull request introduces the CodeDrivenDataModelProvider
, enabling a code-based approach to define data models within the Matter SDK. The code includes unit tests and overall looks good.
PR #39974: Size comparison from 48704fe to 82a694c Full report (32 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, psoc6, qpg, stm32, telink, tizen)
|
void Temporary_ReportAttributeChanged(const AttributePathParams & path) override; | ||
|
||
/** | ||
* @brief Sets the persistent storage delegate for the provider. |
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 needs some explanation on why we do this. It is clear that SetPersistentStorageDelegate
sets the persistent storage delegate
.
I imagine this is so that we can build a ServerClusterContext ... but then I wonder if we should have a separate Init
call to provide the PersistentStorageDelegate AND the AttributePersistenceProvider to it.
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.
Agree, this doesn't look great.
Should we just make it an argument of CodeDrivenDataModelProvider::Startup()
and get rid of the Set
function?
PR #39974: Size comparison from 48704fe to 0c0e641 Full report (56 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, linux, nxp, psoc6, qpg, stm32, telink, tizen)
|
Summary
This Pull Request (PR) introduces the
CodeDrivenDataModelProvider
, enabling a code-based approach to define data models within the Matter SDK.The
CodeDrivenDataModelProvider
operates over endpoints. It uses anEndpointInterfaceRegistry
to manageEndpointInterface
instances. EachEndpointInterface
, such as theSpanEndpoint
, can be configured in code to define its device types, semantic tags, and server/client clusters. This approach contrasts with the existing more static, codegen-based (zap) approach.The
EndpointInterface
,EndpointInterfaceRegistry
andSpanEndpoint
were introduced earlier in pull/39396 and pull/39751 and they provide the basic mechanisms where applications can define its Data Model by organizing them by endpoints and adding to the CodeDrivenDataModelProvider.To build a real application that's fully code-based/dynamic, we need all of the cluster implementations to be code-driven and implement the
ServerClusterInterface
. The conversion work is ongoing in parallel. Meanwhile, while this work is not finished, we've introduced in pull/39393 theServerClusterShim
which offers a way to wrap ember/codegen-based clusters under theServerClusterInterface
so they can be used with theCodeDrivenDataModelProvider
.The following diagram describes how all these components are related to each other:
Testing
A comprehensive set of unit tests has been added. To test the
CodeDrivenDataModelProvider
, run:ninja -C out src/data-model-providers/codedriven/tests:TestCodeDrivenDataModelProvider && ./out/tests/TestCodeDrivenDataModelProvider