chip_data_model.gni: add option for custom-implemented clusters #22042
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
PR #22042: Size comparison from 67d6821 to 9de81ca Increases (4 builds for esp32, nrfconnect, psoc6, telink)
Decreases (3 builds for efr32, k32w, psoc6)
Full report (43 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, telink)
|
andy31415
previously requested changes
Sep 7, 2022
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
I still think this is an important feature. Its further review was postponed to post-1.0, which would be now ;-) |
The PR is marked as draft - should this not be draft anymore? |
|
@plan44 - please remove this from draft if that is appropriate - that should get the bot start asking for reviews. |
andy31415
reviewed
Nov 29, 2022
I will, as soon as I find time to rebase and re-test on master. Thanks for having had a look already :-) |
plan44
force-pushed
the
PR/build_option_for_custom_clusters
branch
2 times, most recently
from
6bbf3ff to
928a69b
Compare
Add `zap_clusters_with_custom_implementation` option to `chip_data_model()` template, which allows to list clusters that need to be available in a device, but don't use the standard implementation in `src/app/clusters/<clustername>/<clustername>.cpp`.
# Usage
To exclude a particular cluster's standard implementation in favor of an application level implementation:
- in the BUILD.gn file for the application specific `.zap` file
- in the instantiation of the `chip_data_model` template
- add a variable `zap_clusters_with_custom_implementation`
- assign it a list of strings with cluster names, corresponding to directories in the `src/app/clusters` directory.
- this causes the cluster implementation file(s) not to be included in the build, allowing re-implementation at the application level.
Example, excluding standard implementation of level-control:
```
chip_data_model("zap") {
zap_file = "myproject.zap"
zap_clusters_with_custom_implementation = [ "level-control" ]
zap_pregenerated_dir = "//zap-generated"
}
```
# Background
Some clusters, for example Level Control, are implemented with the assumption of direct low-level access to the device hardware. For bridged devices, the actual interface might be much more high level, as in my case where I have lights which are fully capable of performing parametrized smooth transitions but are NOT capable of receiving output changes in millisecond intervals. In such cases, a cluster might need application-specific re-implementation.
This changeset allows, from the application's ZAP BUILD.gn, to exclude the standard cluster implementation file, allowing re-implementation of the needed ember callbacks in a application specific file outside the connectedhomeip repo.
|
PR #22042: Size comparison from 9e371e6 to 928a69b Increases (5 builds for bl602, psoc6, telink)
Decreases (6 builds for bl602, esp32, nrfconnect, psoc6, telink)
Full report (53 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
plan44
force-pushed
the
PR/build_option_for_custom_clusters
branch
from
928a69b to
f24e31c
Compare
|
PR #22042: Size comparison from 9e371e6 to f24e31c Increases (13 builds for bl602, bl702, psoc6, telink)
Decreases (2 builds for cyw30739, esp32)
Full report (53 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
|
@andy31415 I've rebased and retested a few days ago, and removed the draft status. Checks look fine except for pullaprove/license/cla, which I do not understand, as I did agree to the CLA a while ago and some PRs of mine are merged already. I have no idea how to re-agree to the CLA, if that's what the status is asking for. |
|
@plan44, @andy31415 is on vacation right now, as is @woody-apple, and they're the obvious people to be able to override the CLA bits.... In practice you probably want to ping people after Jan 3. |
|
@bzbarsky-apple thanks for the info - no problem waiting until next year. Or rather than waiting, maybe finding time to prepare the libev and gaps-in-endpointId PRs ;-) |
|
@andy31415 @woody-apple - is anything but the CLA holding back this PR? I can't do anything regarding the CLA because I have agreed to it, and other PRs went through. I have no idea why the bot thinks in this particular PR the CLA was not agreed to. |
andy31415
dismissed
their stale review
1.0 released, removing change requested
|
CLA seems stuck ... not sure if there is a way to kick it. I will assume CLA is signed for the purposes of merging. Still need reviews. |
pullapprove
bot
requested review from
ksperling-apple,
lazarkov,
lpbeliveau-silabs,
LuDuda,
mrjerryjohns,
msandstedt,
mspang,
pjzander-signify,
rgoliver,
robszewczyk,
saurabhst,
selissia,
tcarmelveilleux,
tecimovic,
tehampson,
vijs,
vivien-apple,
woody-apple,
xylophone21 and
yufengwangca
bzbarsky-apple
approved these changes
Jan 25, 2023
jmartinez-silabs
approved these changes
Jan 26, 2023
lecndav
pushed a commit
to lecndav/connectedhomeip
that referenced
this pull request
Mar 22, 2023
…ect-chip#22042) * chip_data_model.gni: add option for custom-implemented clusters Add `zap_clusters_with_custom_implementation` option to `chip_data_model()` template, which allows to list clusters that need to be available in a device, but don't use the standard implementation in `src/app/clusters/<clustername>/<clustername>.cpp`. # Usage To exclude a particular cluster's standard implementation in favor of an application level implementation: - in the BUILD.gn file for the application specific `.zap` file - in the instantiation of the `chip_data_model` template - add a variable `zap_clusters_with_custom_implementation` - assign it a list of strings with cluster names, corresponding to directories in the `src/app/clusters` directory. - this causes the cluster implementation file(s) not to be included in the build, allowing re-implementation at the application level. Example, excluding standard implementation of level-control: ``` chip_data_model("zap") { zap_file = "myproject.zap" zap_clusters_with_custom_implementation = [ "level-control" ] zap_pregenerated_dir = "//zap-generated" } ``` # Background Some clusters, for example Level Control, are implemented with the assumption of direct low-level access to the device hardware. For bridged devices, the actual interface might be much more high level, as in my case where I have lights which are fully capable of performing parametrized smooth transitions but are NOT capable of receiving output changes in millisecond intervals. In such cases, a cluster might need application-specific re-implementation. This changeset allows, from the application's ZAP BUILD.gn, to exclude the standard cluster implementation file, allowing re-implementation of the needed ember callbacks in a application specific file outside the connectedhomeip repo. * Include suggestions from @bzbarsky-apple
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add
zap_clusters_with_custom_implementationoption tochip_data_model()template, which allows to list clusters that need to be available in a device, but don't use the standard implementation insrc/app/clusters/<clustername>/<clustername>.cpp.Usage
To exclude a particular cluster's standard implementation in favor of an application level implementation:
.zapfilechip_data_modeltemplatezap_clusters_with_custom_implementationsrc/app/clustersdirectory.Example, excluding standard implementation of level-control:
Background
Some clusters, for example Level Control, are implemented with the assumption of direct low-level access to the device hardware. For bridged devices, the actual interface might be much more high level, as in my case where I have lights which are fully capable of performing parametrized smooth transitions but are NOT capable of receiving output changes in millisecond intervals. In such cases, a cluster might need application-specific re-implementation.
This changeset allows, from the application's ZAP BUILD.gn, to exclude the standard cluster implementation file, allowing re-implementation of the needed ember callbacks in a application specific file outside the connectedhomeip repo.
Testing