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

Matter Cleanup for Chip Tool Repo #737

Merged

Conversation

brdandu
Copy link
Collaborator

@brdandu brdandu commented Sep 20, 2022

Adding generic helpers with less hardcoding such that they can be used across different kinds of templates

  • Adding selectStructsWithClusterAssociation which takes the logic src-electron/generator/matter/chip-tool/templates/helper.js#structs_with_cluster_name to the backend(sql query). structs_with_clusters helper uses this instead of exports.structs_with_cluster_name
  • Updating cleanseLabelAsKebabCase so that it can be used instead of asDelimitedCommand
  • Added as_underlying_zcl_type_size_bits which can be used to calculate the size of data types such as bitmaps, enums and numbers in bits or bytes. HOwever the implementation currently returns 0 for struct, stringg or anything else. May be this can be updated over time based on usage
  • Added if_is_underlying_zcl_type_signed which can be used to find whether a data type is signed or not
  • Similarly if_is_float_or_double_data_type is added to find out if a data type is float or double.
  • endpoint type was being treated as signed integer due to parsing logic in zcl-silabs-loader.js. Added a separate condition for this
  • Added generation tests for all the new helpers with an additional matter generation template
  • Github: ZAP#679

This PR will be associated with a CHIP repo PR project-chip/connectedhomeip#22762

src-electron/generator/helper-zcl.js Outdated Show resolved Hide resolved
src-electron/generator/helper-zcl.js Outdated Show resolved Hide resolved
src-electron/zcl/zcl-loader-silabs.js Outdated Show resolved Hide resolved
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Er, meant to request changes.

@brdandu brdandu force-pushed the task/cleanupChipToolMatterRepo/ZAP#679 branch from a40fa5b to e730753 Compare September 30, 2022 13:17
@vivien-apple
Copy link
Contributor

@brdandu I see 4 different changes here and each of them could have been its own dedicated PR. That would make the process easier and not all changes will be blocked on one particular changes:

  1. Changes to the structs helper
  2. Changes to the asTypeMinValue/asTypeMaxValue helpers (for those I wonder if a partial on the Matter repo PR side would not make it easier to accept)
  3. Changes to asDelimitedCommand which I believe would easily be accepted if it was a single dedicated PR
  4. endpoint type handling

@brdandu brdandu requested a review from bzbarsky-apple October 3, 2022 20:44
src-electron/generator/helper-zcl.js Show resolved Hide resolved
src-electron/generator/helper-zcl.js Outdated Show resolved Hide resolved
src-electron/generator/helper-zcl.js Outdated Show resolved Hide resolved
src-electron/zcl/zcl-loader-silabs.js Outdated Show resolved Hide resolved
src-electron/zcl/zcl-loader-silabs.js Show resolved Hide resolved
@brdandu brdandu requested a review from bzbarsky-apple October 4, 2022 20:48
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This generally looks pretty good, but I'm still not convinced on the max behavior. See #737 (comment)

…d across different kinds of templates

- Adding selectStructsWithClusterAssociation which takes the logic src-electron/generator/matter/chip-tool/templates/helper.js#structs_with_cluster_name to the backend(sql query). structs_with_clusters helper uses this instead of exports.structs_with_cluster_name
- Updating cleanseLabelAsKebabCase so that it can be used instead of asDelimitedCommand
- endpoint type was being treated as signed integer due to parsing logic in zcl-silabs-loader.js. Added a separate condition for this
- Added generation tests for all the new helpers with an additional matter generation template
- Adding the groupByStructName option to structs_with_clusters to avoid duplicate rows
- Cleaning up the loader
- Cleaning up the helpers in helper-zcl
- For zcl data types, taking into account the signed attribute in the xml for numbers. If not mentioned then accounting for known exceptions such as single and double data types
Throwing an error in as_type_max/min_value when language option is not specified
Github: ZAP#679
@brdandu brdandu force-pushed the task/cleanupChipToolMatterRepo/ZAP#679 branch from 8db1f24 to 8a3a89a Compare October 7, 2022 17:09
@brdandu brdandu merged commit 4f70b20 into project-chip:master Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cleanup zap/repo/src-electron/generator/matter/chip-tool
4 participants