[cmake][common] Adding optional support for --zcl argument in chip_zapgen() cmake function #40537
Conversation
…function Signed-off-by: Dina Benamar <dina.benamarelmaaroufi@nxp.com>
…ples to allow default ZCL path to be passed as --zcl argument with chip_zapgen Signed-off-by: Dina Benamar <dina.benamarelmaaroufi@nxp.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces optional support for the --zcl argument in the chip_zapgen() cmake function, which is a valuable addition for handling builds in different environments. The implementation is well-structured, using a new CHIP_ENABLE_ZCL_ARG variable for global control and allowing per-target overrides. The changes in chip_codegen.cmake, app_common.cmake, and chip_data_model.cmake are logical and correctly implement the described feature.
I have one suggestion in build/chip/chip_codegen.cmake to simplify the logic for adding the --zcl argument, which should improve readability. Overall, this is a solid contribution.
|
PR #40537: Size comparison from 0832b19 to 4c1814f Full report (4 builds for cc32xx, nrfconnect, stm32)
|
Signed-off-by: Dina Benamar <dina.benamarelmaaroufi@nxp.com>
|
PR #40537: Size comparison from 0832b19 to 21950a6 Full report (57 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40537 +/- ##
==========================================
- Coverage 51.05% 50.71% -0.34%
==========================================
Files 1344 1355 +11
Lines 98443 99303 +860
Branches 12672 12877 +205
==========================================
+ Hits 50257 50366 +109
- Misses 48186 48937 +751 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Please add documentation for the added arguments given that documentation already exists.
For the testing section in the PR summary, you included a command that invokes a build script that was not modified by this PR. Could you explain how that works? are there local changes? It was not immediately obvious to me how the changes in this PR translate in the command line you gave working differently ... some explanation/links on how things work would help
Thank you for the review. I have updated the documentation accordingly. As for the testing section, I added in the description the changes required for testing each configuration. These changes are applied on application cmake level, and do not require the command line to be changed. |
…ip_zapgen() and chip_configure_data_model() Signed-off-by: Dina Benamar <dina.benamarelmaaroufi@nxp.com>
… argument with chip_configure_data_model Signed-off-by: Dina Benamar <dina.benamarelmaaroufi@nxp.com>
Please describe why these arguments exist, so that people understand their use and intent. It is obvious that an argument In particular right now I do not know why this option is needed and why we have it in cmake only?
So please document things for real:
|
…ure_data_model to explain the purpose of using ZCL_PATH argument Signed-off-by: Dina Benamar <dina.benamarelmaaroufi@nxp.com>
I updated the documentation to clarify why the ZCL_PATH is needed, and also mentioned it in the PR description, in the "context" section. For the testing part, the manual commands were provided as an example to reproduce, but the change is already being covered by the CI workflows of this PR. Specifically, the NXP examples have switched to using these configurations, so the new behavior is exercised automatically. I’ve added this clarification to the PR description as well. |
Summary
The aim of this PR is to introduce optional support for passing "--zcl" argument to the zapgen "generate.py" script, without impacting existing example behavior.
It supports three levels of configuration:
1- Default behavior : No "--zcl" argument is passed, preserving current behavior for all examples.
2- Global opt-in : If the "CHIP_ENABLE_ZCL_ARG" cmake variable is set to ON, the default ZCL path "${CHIP_ROOT}/src/app/zap-templates/zcl/zcl.json" is used.
3- Custom override : Examples can pass a specific "ZCL_PATH" via the "chip_configure_data_model()" API to use a custom "zcl.json".
Example of use to add a custom ZCL_PATH :
Context
This change helps address build issues in freestanding examples, where the "generate.py" (called in chip_zapgen) relies on relative paths embedded in the ".zap" files to locate the "zcl.json" file. These paths may be invalid if the ".zap" file is moved or used outside its original context. Explicitly passing the ZCL path ensures consistent and portable builds.
Testing
These changes were tested by building the following configurations :
- Tests in CI : All NXP examples (except all-clusters-app) are using this configuration.
- Tests in CI : NXP all-clusters-app example is using this configuration.
Example of command used to build: