Skip to content
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

Move ClusterWriteMapping.java to be generated from matter.idl #25773

Merged

Conversation

andy31415
Copy link
Contributor

@andy31415 andy31415 commented Mar 21, 2023

We had code generation incompatibilities in java because source of truth between matter id and zap templates may use different content. In this case the error shows up in #25737: just chaing .matter idl to contain all possible attributes on client clusters is insufficient since zap code still relies on enabling flags. #25693 is looking to fix some of these, however updating all helpers is reasonably painful and it is likely easier to get out of sync using 2 generation methods.

Start moving some more files than strictly required due to OOM to have a single source of truth for codegen. Unfortunately we have a lot of java-specific codegen (14 templates), so this has to be done incrementally.

Changes:

  • split java-jni (c++) and java-class (java files) code generation
  • Create an equivalent ClusterWriteMapping.java template. I did validate that after formatting, old and new java file is identical (except minor comment saying 'generated by zap')
  • Added a basic golden image unit test for ClusterWriteMapping generation just in case (the fact that we check in ClusterWriteMapping for now makes this somewhat redundant, however it will be useful if we move things to compile-time generation)
  • Use the new java file instead of the old one
  • Add zap_regen_all.py changes to also generate via codegen
  • remove the restyle requirement for this file

I was unable to make the java files be generated at compile-time since our current android/java library did not have file generation support and build/chip/java/write_build_config.py also seems to make dependency assuptions. So for now similar codegen logic as before for just, just different generator/template.

@github-actions
Copy link

PR #25773: Size comparison from 2a9fa2a to 80baf11

Full report (1 build for cc32xx)
platform target config section 2a9fa2a 80baf11 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 645601 645601 0 0.0
(read/write) 203848 203848 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197248 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 930292 930292 0 0.0
.debug_aranges 87400 87400 0 0.0
.debug_frame 300320 300320 0 0.0
.debug_info 20248022 20248022 0 0.0
.debug_line 2661345 2661345 0 0.0
.debug_loc 2805489 2805489 0 0.0
.debug_ranges 283264 283264 0 0.0
.debug_str 3027174 3027174 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105993 105993 0 0.0
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 380421 380421 0 0.0
.symtab 257408 257408 0 0.0
.text 537488 537488 0 0.0

@github-actions
Copy link

PR #25773: Size comparison from 2a9fa2a to a9e2f2b

Decreases (1 build for cc32xx)
platform target config section 2a9fa2a a9e2f2b change % change
cc32xx lock CC3235SF_LAUNCHXL .debug_info 20248022 20248021 -1 -0.0
Full report (1 build for cc32xx)
platform target config section 2a9fa2a a9e2f2b change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 645601 645601 0 0.0
(read/write) 203848 203848 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197248 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 930292 930292 0 0.0
.debug_aranges 87400 87400 0 0.0
.debug_frame 300320 300320 0 0.0
.debug_info 20248022 20248021 -1 -0.0
.debug_line 2661345 2661345 0 0.0
.debug_loc 2805489 2805489 0 0.0
.debug_ranges 283264 283264 0 0.0
.debug_str 3027174 3027174 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105993 105993 0 0.0
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 380421 380421 0 0.0
.symtab 257408 257408 0 0.0
.text 537488 537488 0 0.0

@github-actions
Copy link

PR #25773: Size comparison from 2a9fa2a to ad4aa7c

Full report (1 build for cc32xx)
platform target config section 2a9fa2a ad4aa7c change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 645601 645601 0 0.0
(read/write) 203848 203848 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197248 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 930292 930292 0 0.0
.debug_aranges 87400 87400 0 0.0
.debug_frame 300320 300320 0 0.0
.debug_info 20248022 20248022 0 0.0
.debug_line 2661345 2661345 0 0.0
.debug_loc 2805489 2805489 0 0.0
.debug_ranges 283264 283264 0 0.0
.debug_str 3027174 3027174 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105993 105993 0 0.0
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 380421 380421 0 0.0
.symtab 257408 257408 0 0.0
.text 537488 537488 0 0.0

@github-actions
Copy link

PR #25773: Size comparison from 2a9fa2a to 46380e3

Decreases (1 build for qpg)
platform target config section 2a9fa2a 46380e3 change % change
qpg lock-app qpg6105+debug (read/write) 1121320 1121312 -8 -0.0
.text 568420 568412 -8 -0.0
Full report (4 builds for cc32xx, mbed, qpg)
platform target config section 2a9fa2a 46380e3 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 645601 645601 0 0.0
(read/write) 203848 203848 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197248 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 930292 930292 0 0.0
.debug_aranges 87400 87400 0 0.0
.debug_frame 300320 300320 0 0.0
.debug_info 20248022 20248022 0 0.0
.debug_line 2661345 2661345 0 0.0
.debug_loc 2805489 2805489 0 0.0
.debug_ranges 283264 283264 0 0.0
.debug_str 3027174 3027174 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105993 105993 0 0.0
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 380421 380421 0 0.0
.symtab 257408 257408 0 0.0
.text 537488 537488 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2468424 2468424 0 0.0
.bss 215964 215964 0 0.0
.data 5880 5880 0 0.0
.text 1431068 1431068 0 0.0
qpg lighting-app qpg6105+debug (read/write) 1152768 1152768 0 0.0
.bss 96036 96036 0 0.0
.data 852 852 0 0.0
.text 599864 599864 0 0.0
lock-app qpg6105+debug (read/write) 1121320 1121312 -8 -0.0
.bss 91172 91172 0 0.0
.data 856 856 0 0.0
.text 568420 568412 -8 -0.0

@github-actions
Copy link

PR #25773: Size comparison from 2a9fa2a to 6fc9632

Increases (1 build for cc32xx)
platform target config section 2a9fa2a 6fc9632 change % change
cc32xx lock CC3235SF_LAUNCHXL (read only) 645601 645825 224 0.0
.debug_frame 300320 300336 16 0.0
.debug_info 20248022 20251232 3210 0.0
.debug_line 2661345 2661959 614 0.0
.debug_loc 2805489 2806733 1244 0.0
.debug_ranges 283264 283424 160 0.1
.debug_str 3027174 3027534 360 0.0
.strtab 380421 380469 48 0.0
.text 537488 537712 224 0.0
Decreases (1 build for cc32xx)
platform target config section 2a9fa2a 6fc9632 change % change
cc32xx lock CC3235SF_LAUNCHXL .debug_abbrev 930292 930286 -6 -0.0
Full report (1 build for cc32xx)
platform target config section 2a9fa2a 6fc9632 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 645601 645825 224 0.0
(read/write) 203848 203848 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197248 197248 0 0.0
.comment 194 194 0 0.0
.data 1480 1480 0 0.0
.debug_abbrev 930292 930286 -6 -0.0
.debug_aranges 87400 87400 0 0.0
.debug_frame 300320 300336 16 0.0
.debug_info 20248022 20251232 3210 0.0
.debug_line 2661345 2661959 614 0.0
.debug_loc 2805489 2806733 1244 0.0
.debug_ranges 283264 283424 160 0.1
.debug_str 3027174 3027534 360 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105993 105993 0 0.0
.shstrtab 232 232 0 0.0
.stab 204 204 0 0.0
.stabstr 441 441 0 0.0
.stack 2048 2048 0 0.0
.strtab 380421 380469 48 0.0
.symtab 257408 257408 0 0.0
.text 537488 537712 224 0.0

@andy31415
Copy link
Contributor Author

fast track: java only change without generally affecting other library functionality

@andy31415 andy31415 merged commit 31e31cc into project-chip:master Mar 22, 2023
lecndav pushed a commit to lecndav/connectedhomeip that referenced this pull request Mar 22, 2023
…t-chip#25773)

* Some initial changes

* Parser support for timed writes

* Small doc update

* Matter idl support for timed write

* Fix indent and codegen all

* Remove extra line from readme

* Some fixes

* Fix conditional in requires_timed_write

* Most codegen looks ok. Java boxing logic is suspect still

* More updates, output idential EXCEPT types for boxing

* Increase 1000 to 10000 to match original template

* Fix byte count comparison when long starts to take effect

* Fix length of underlying bitmap type sizing

* Fixed files, they are IDENTICAL

* Integrate java-jni and java-class since build rules are different between cpp and java files

* Fix python syntax

* Switch default to not have underscore

* Add java codegen via jinja to zap_regen_all

* Restyle, fix restyle logic

* Fix duplicated generation target

* Do not attempt to zap-generate Cluster write mapping

* Add golden image unit test for java codegen

* Add prettyfy for java output ... makes the input/output files more obviously identical

* Remove unused variable

* Fix upper/lowercase of acronyms, to be fully backwards compatible

* Add license blurb since we checkin generated file (and maybe jinja files should also have licenses

* Restyle

* Fix unit test

---------

Co-authored-by: Andrei Litvin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants