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

Force global atrributes to be declared for every cluster in MatterIDL #24951

Merged
merged 15 commits into from
Feb 10, 2023

Conversation

andy31415
Copy link
Contributor

@andy31415 andy31415 commented Feb 9, 2023

Spect section 7.13 Global Elements describes a set of elements that are mandatory for every cluster to have.

Fixes #24947 for the matter IDL bits (although java may still depend on zap, where handling seems to need specialization)

It also turns out that both JAVA and PYTHON rely on zap templating to have global attributes enabled. As a result I updated controller_clusters.zap to enable mandatory attributes (a lot of event list adding, some adding/enabling for a few other clusters). This resulted in java and python generated code updates.

@github-actions
Copy link

github-actions bot commented Feb 9, 2023

PR #24951: Size comparison from 11c0cdf to ea10d7e

Full report (1 build for cc32xx)
platform target config section 11c0cdf ea10d7e change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 640233 640233 0 0.0
(read/write) 204084 204084 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197488 197488 0 0.0
.comment 194 194 0 0.0
.data 1476 1476 0 0.0
.debug_abbrev 928439 928439 0 0.0
.debug_aranges 87352 87352 0 0.0
.debug_frame 299840 299840 0 0.0
.debug_info 20194395 20194395 0 0.0
.debug_line 2649797 2649797 0 0.0
.debug_loc 2785922 2785922 0 0.0
.debug_ranges 280720 280720 0 0.0
.debug_str 3001474 3001474 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105585 105585 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 375840 375840 0 0.0
.symtab 255856 255856 0 0.0
.text 532524 532524 0 0.0

@github-actions
Copy link

PR #24951: Size comparison from 36735ed to a9d9294

Decreases (1 build for cc32xx)
platform target config section 36735ed a9d9294 change % change
cc32xx lock CC3235SF_LAUNCHXL .debug_info 20194395 20194394 -1 -0.0
Full report (1 build for cc32xx)
platform target config section 36735ed a9d9294 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 640233 640233 0 0.0
(read/write) 204084 204084 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197488 197488 0 0.0
.comment 194 194 0 0.0
.data 1476 1476 0 0.0
.debug_abbrev 928439 928439 0 0.0
.debug_aranges 87352 87352 0 0.0
.debug_frame 299840 299840 0 0.0
.debug_info 20194395 20194394 -1 -0.0
.debug_line 2649797 2649797 0 0.0
.debug_loc 2785922 2785922 0 0.0
.debug_ranges 280720 280720 0 0.0
.debug_str 3001474 3001474 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105585 105585 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 375840 375840 0 0.0
.symtab 255856 255856 0 0.0
.text 532524 532524 0 0.0

@andy31415
Copy link
Contributor Author

This does not seem to work - java read callback paths do not actually take into consideration that some things are just hard-coded and as a result we do not codegen the underlying types in CHIPReadCallbacks.h / CHIPReadCallbacks.cpp .

Hardcoding into matter files makes more things available to the python codegen and results in build errors. Commenting out the event list was not sufficient.

@andy31415
Copy link
Contributor Author

Updated PR: created a script to ensure all the mandatory attributes are enabled for all clusters.
This enabled and added attributes on a few clusters (User localization and such) and added the event list to ALL clusters (all were missing).

@github-actions
Copy link

PR #24951: Size comparison from 36735ed to 61f3e1d

Full report (2 builds for cc32xx, mbed)
platform target config section 36735ed 61f3e1d change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 640233 640233 0 0.0
(read/write) 204084 204084 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197488 197488 0 0.0
.comment 194 194 0 0.0
.data 1476 1476 0 0.0
.debug_abbrev 928439 928439 0 0.0
.debug_aranges 87352 87352 0 0.0
.debug_frame 299840 299840 0 0.0
.debug_info 20194395 20194395 0 0.0
.debug_line 2649797 2649797 0 0.0
.debug_loc 2785922 2785922 0 0.0
.debug_ranges 280720 280720 0 0.0
.debug_str 3001474 3001474 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 105585 105585 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 375840 375840 0 0.0
.symtab 255856 255856 0 0.0
.text 532524 532524 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2464576 2464576 0 0.0
.bss 215892 215892 0 0.0
.data 5880 5880 0 0.0
.text 1427220 1427220 0 0.0

@andy31415 andy31415 merged commit c6ae82c into project-chip:master Feb 10, 2023
lecndav pushed a commit to lecndav/connectedhomeip that referenced this pull request Mar 22, 2023
…project-chip#24951)

* Update IDL tempalte: force a fixed set of global attributes

* Ran zap regen

* Regen and fix some odd UFT8 wrong values

* Comment out eventList from matter as ZAP global resposes does NOT yet seem to return this list.

* Zap regen

* Ensure acceptedcommandlist is available in ALL clusters

* Zap regen

* Add generated command list as well

* Zap regen

* Marking AttributeList as included for several clusters

* Zap regen

* Add EventList to all clusters

* zap regen

* Zap regen: re-enable event list attribute

---------

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
5 participants