Add compile-time code generation using zap for app-templates #23962
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
- add support for gn builds - add support for cmake builds - update esp32 build logic - update pre-generation script - split out app and idl codegen - update template used so zap_generate_all only generates IDLs for apps - updated CI unit tests
github-actions
bot
added
app
config
controller
darwin
examples
github
gn
lib
scripts
shell
workflows
labels
pullapprove
bot
requested review from
anush-apple,
arkq,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis,
franck-apple,
gjc13,
harsha-rajendran,
hawk248,
isiu-apple and
jelderton
|
PR #23962: Size comparison from a25af90 to bef4097 Increases (19 builds for bl602, bl702, cc13x2_26x2, efr32, linux, mbed, psoc6, telink)
Decreases (26 builds for bl602, bl702, cc13x2_26x2, esp32, linux, nrfconnect, psoc6, qpg, telink)
Full report (69 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
You mean Darwin in CI, or when building locally?
So what are these tests actually testing? |
… contain what files get formatted
|
PR #23962: Size comparison from c8a9f1a to 9f7e343 Increases (19 builds for bl602, bl702, cc13x2_26x2, esp32, linux, mbed, nrfconnect, telink)
Decreases (24 builds for bl602, bl702, cc13x2_26x2, linux, nrfconnect, psoc6, telink)
Full report (69 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
|
PR #23962: Size comparison from c8a9f1a to 5025c38 Increases (18 builds for bl602, bl702, cc13x2_26x2, esp32, linux, mbed, nrfconnect, telink)
Decreases (26 builds for bl602, bl702, cc13x2_26x2, linux, nrfconnect, psoc6, telink)
Full report (69 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
|
PR #23962: Size comparison from 9fbcf1c to 8db3711 Increases (21 builds for bl602, bl702, cc13x2_26x2, linux, mbed, nrfconnect, telink)
Decreases (23 builds for bl702, cc13x2_26x2, esp32, linux, nrfconnect, psoc6, qpg, telink)
Full report (69 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
There was a problem hiding this comment.
This is looking very close, but one critical issue remains: running env ZAP_DEVELOPMENT_PATH=/Users/bzbarsky/zap ./gn_build.sh is_asan=true enable_host_clang_build=false enable_host_gcc_mbedtls_crypto_tests=false enable_host_clang_boringssl_crypto_tests=false chip_enable_dnssd_tests=false errors out with:
FAILED: mac_arm64_gcc/gen/src/controller/data_model/zap_pregen/zap-generated/access.h mac_arm64_gcc/gen/src/controller/data_model/zap_pregen/zap-generated/gen_config.h mac_arm64_gcc/gen/src/controller/data_model/zap_pregen/zap-generated/endpoint_config.h mac_arm64_gcc/gen/src/controller/data_model/zap_pregen/zap-generated/CHIPClientCallbacks.h mac_arm64_gcc/gen/src/controller/data_model/zap_pregen/zap-generated/CHIPClusters.h mac_arm64_gcc/gen/src/controller/data_model/zap_pregen/zap-generated/IMClusterCommandHandler.cpp
python3 ../../third_party/pigweed/repo/pw_build/py/pw_build/python_runner.py --gn-root ../../ --current-path ../../src/controller/data_model --default-toolchain=//third_party/pigweed/repo/pw_toolchain/default:default --current-toolchain=//build/toolchain/host:mac_arm64_gcc --capture-output --python-dep-list-files mac_arm64_gcc/gen/src/controller/data_model/data_model_zapgen_zap_pregen_metadata_path_list.txt -- ../../scripts/tools/zap/generate.py --no-prettify-output --templates /Users/bzbarsky/connectedhomeip/src/app/zap-templates/app-templates.json --output-dir /Users/bzbarsky/connectedhomeip/out/debug/mac_arm64_gcc/gen/src/controller/data_model/zap_pregen/zap-generated --lock-file /Users/bzbarsky/connectedhomeip/out/debug/mac_arm64_gcc/zap_gen.lock --parallel /Users/bzbarsky/connectedhomeip/src/controller/data_model/controller-clusters.zap
Searching for zcl file from /Users/bzbarsky/connectedhomeip/src/controller/data_model/controller-clusters.zap
Failed to find `Version: ....` line from ['node', 'src-script/zap-start.js'] --version
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
@bzbarsky-apple I am not sure what is going on since on linux my devel zap can report a version ... but for now I just made the version check not run when a development path is set. If one sets ZAP_DEVELOPMENT it sounds like it is deliberate and the person knows what is happening. |
bzbarsky-apple
approved these changes
Jan 17, 2023
yunhanw-google
approved these changes
Jan 17, 2023
lecndav
pushed a commit
to lecndav/connectedhomeip
that referenced
this pull request
Mar 22, 2023
…-chip#23962) * Add compile-time code generation using zap for app-templates - add support for gn builds - add support for cmake builds - update esp32 build logic - update pre-generation script - split out app and idl codegen - update template used so zap_generate_all only generates IDLs for apps - updated CI unit tests * Fix tizen build dependencies for all-clusters and minimal * Remove one more unnedded include directory from tizen * Fix tv casting app include * Restyle * Bump up openiotsdk dockerfile to get zap-cli * Fix include for efr32 Lcd painter * Test out a zap installation workflow in darwin builds * Fix trailing slash * Multiline to make mac install more redable * make sure all subdirs are created when installing zap * Use sudo to create the zap install directory * unzip only zap-cli, to hopefully make the unzip faster * Install zap on all darwin CI runs * Add `"$(TEMP_DIR)/out/gen/src/controller/data_model/zapgen/"` to xcode project header search paths * Do not run clang-format for compile-time generated files * Update code generation documentation a bit * More documentation with examples of how to invoke and set pregeneration variables * Restyle * Fix typo in command for code pregen * Fix typo * Add a runnable check that is intended to codegen * make unit tests with golden images run (but not yet pass) * Update golden copies * Update documentation and restyle * Bump up build apps time... seems like bimodal timing distribution and this occasionally fails at 1h, leave to 1.5hrs for now * Fix spelling * make zap-cli --version run on mac as part of the install since I seem to get package conflict errors periodically * Try to use tempState for mac parallel runs * Restyle * Several code review changes * Fix typo in path for templates * Undo third party submodule changes * Move common python zap logic into one file, to have the zap path logic centralized and to be more easily be able to add version checks * Restyle * Add version check capabilities to zap execution and generate.py * Restyle * Update zap version and set skip_real_version based on merge with master * Also update darwin zap installation path * Update more paths with the new zap version * Restyle * Golden image update for regeneration * Do not restyle test outputs for zap generation tests * Restyle * Restyled by isort * Fix up one more include for obsolete paths * Update generated files for unit tests * Re-generate unit test output data * Fix telink contact sensor build * Update unit test data * Revert wrong diff marker * Bump CI docker to 0.6.31 to use the latest zap * Bump up image versions for non-chip-build images as well * Re-generate using latest zap from 0.6.31 image * Keep codegen for controller clusters, for darwin specifically * patch the matter xcode project to update the include path to use the darwin-spefici codegen for controller cluster paths * Restyle * Update to make template checker happy, update min version for zap execution * One more manual restyle attempt * Ran regen again for tests * Re-generated this time using CI steps for checking * See if CI can run zap tests on mac as well * Do not set skip real version for the version check * Add a better comment of why IMClusterCommandHandler is odd * Restyle * Start developing a simpler ZAP version update script as this is likely often used * Off by one fix and update search logic a bit * Add support for min version update as well * Update a comment and a min version * Comment updates * Another comment regarding version update * Fix gen files for unit tests. We should figure out why environment matters here ... this is odd * Update clang-format version comment: it is 15 not 14 now in pigweed * Force clang-format-15 or clang-format (assuming we use pigweed) * Regenerate in pigweed environment, so clang-format is consistently 15 * Report clang-format version when formatting, to understand any format differences * Re-gen all after clang-format-15 was applied * Remove zzz_generated from restyled.yaml since we clang-format already during zap gen * Zap regen app2 * Regenerated (after restyle changed permissions..) and updated logs to contain what files get formatted * Update CI image for coverage run * Serialize zap execution to work around zap darwin bugs * Fix copy and paste error for cmake: remove extra comma * Update version update to control what files to update: usage or docker * Update scripts/tools/zap/generate.py Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Skip ZAP version check when ZAP_DEVELOPMENT_PATH is set * Restyle Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Andrei Litvin <andreilitvin@google.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
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.
Why
Compile time code generation makes code dependencies clearer (and also highlights oddities ... like we generate several CHIPClusters.h with different content and different uses throughout)
zap file updates for apps will not conflict as much during codegen (but data model clusters still will)
Faster edit/compile cycle for zap edits (zap-regen has a much narrower scope ... still needs .matter files update, however not other files).
Changes
generate.pyoutputs over timeFuture (not this PR):
Update effects
zap-clibecomes a mandatory tool during building (unless one uses a pre-generated directory).