Moving zap templates away from zap helpers which use State (eg. ensureClusters, etc) and other cleanup #23410
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
brdandu
commented
Oct 31, 2022
| attribute TargetStruct binding[] = 0; | ||
| readonly attribute command_id generatedCommandList[] = 65528; | ||
| readonly attribute command_id acceptedCommandList[] = 65529; | ||
| readonly attribute attrib_id attributeList[] = 65531; |
There was a problem hiding this comment.
Were removed because as per the zap file. The binding server cluster was not enabled and therefore this should be be generated here as well.
brdandu
commented
Oct 31, 2022
|
PR #23410: Size comparison from eeb4cdc to 7907644 Increases (4 builds for bl702, cc13x2_26x2, telink)
Decreases (4 builds for cc13x2_26x2, nrfconnect, psoc6)
Full report (49 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
zzz_generated/placeholder/app1/zap-generated/CHIPClientCallbacks.h
Outdated
Show resolved
Hide resolved
brdandu
commented
Nov 1, 2022
| readonly attribute command_id acceptedCommandList[] = 65529; | ||
| readonly attribute attrib_id attributeList[] = 65531; | ||
| readonly attribute bitmap32 featureMap = 65532; | ||
| readonly attribute int16u clusterRevision = 65533; |
There was a problem hiding this comment.
These are the server side attributes of binding cluster which are not enabled and therefore removed because the binding cluster on the server side is not enabled.
|
PR #23410: Size comparison from eeb4cdc to 9ee296b Increases (2 builds for k32w, telink)
Decreases (3 builds for bl602, telink)
Full report (15 builds for bl602, bl702, k32w, linux, mbed, nrfconnect, telink)
|
brdandu
commented
Nov 1, 2022
| typedef void (*BindingAcceptedCommandListListAttributeCallback)(void * context, | ||
| const chip::app::DataModel::DecodableList<chip::CommandId> & data); | ||
| typedef void (*BindingAttributeListListAttributeCallback)(void * context, | ||
| const chip::app::DataModel::DecodableList<chip::AttributeId> & data); |
There was a problem hiding this comment.
Binding's server side cluster is not enabled for the app. Therefore the server side attributes are not generated either.
|
PR #23410: Size comparison from eeb4cdc to 65e5537 Increases (8 builds for bl602, cc13x2_26x2, esp32, k32w, psoc6, telink)
Decreases (4 builds for cc13x2_26x2, esp32, psoc6)
Full report (49 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
I have not done any generation analysis yet but we should be able to parallelize the generation when generating all the apps. |
brdandu
commented
Nov 2, 2022
| @@ -15,11 +15,11 @@ | |||
|
|
|||
| // List specific responses | |||
| {{#chip_client_clusters}} | |||
| {{#chip_server_cluster_attributes}} | |||
| {{#all_user_cluster_attributes_irrespective_of_manufatucuring_specification name 'server'}} | |||
b29f574 to
592b0fc
Compare
|
PR #23410: Size comparison from 0c00923 to 655e276 Increases (4 builds for cyw30739, esp32, psoc6)
Decreases (9 builds for bl602, psoc6, telink)
Full report (40 builds for bl602, bl702, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
…eClusters, etc) and other cleanup - Creating get_underlying_language_specific_zcl_type which can act as a common helper to get different kinds of data types for an underlying zcl type based on different languages - Switching from chip_server_cluster_attributes to zcl_attributes_server and all_user_cluster_attributes_irrespective_of_manufatucuring_specification in order to have stateless helpers - Updating the zap repo for the above changes. - .matter and CHIPClientCallbacks have a regen diff due to newer helpers. Need to see if that is valid or not. Committing these changes for now - Github: ZAP#682
…atterIDL.zapt Updating the zap repo Github: ZAP#682
…ers in CHIPClientCallbacks.zapt Github: ZAP#682
…eClusters, etc) and other cleanup
- Switching to zcl_attributes_server from chip_server_cluster_attributes
- Using if_unsupported_attribute_type instead of unless (isStrEqual chipCallback.name Unsupported)
- Using if_attribute_complex instead of if_basic_global_response
- Updating the zap repo
- Logic behind the diff in the applications: Some templates have the following hierarchy
{{#chip_client_clusters}}
{{#chip_server_cluster_attributes}}
In here we are going through the enabled client clusters and then the enabled server attributes for that cluster. However chip_server_cluster_attributes does not check if the server side cluster is actually enabled or not. The way zap is designed is to store user settings so a user may enable server cluster, enable server attribute and then disable server cluster. The backend saves user's enabled server attributes such that if in the future the user enables the server side cluster again then they get their old selections back. However that does not mean the server side attribute is enabled.
Switching to the following:
{#chip_client_clusters}}
{{#zcl_attributes_server}}
Gets all available server side attributes for the cluster. I believe this will give a better and consistent representation and ease of use for a user building the app. App developers do not need to worry if they enabled the server side cluster and attribute or not. Also they do not have to enable the server side attribute if not needed.
- Github: ZAP#682
- Also regening after rebase - Github: ZAP#682
Github: ZAP#682
655e276 to
6b91d8a
Compare
Github: ZAP#682
|
PR #23410: Size comparison from 1a8cf55 to f1c9b4f Increases (2 builds for bl602, telink)
Decreases (5 builds for bl602, psoc6, qpg, telink)
Full report (33 builds for bl602, bl702, cc13x2_26x2, cyw30739, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
…eClusters, etc) and other cleanup - Using all_user_clusters with side instead of chip_client_clusters - Using zcl_commands instead of chip_cluster_commands instead. May need an additional if condition to check based on source(client) of commands - Using zcl_command_arguments instead of chip_cluster_command_arguments - Using commandArgCount instead of hasArguments inside command block helpers - Using all_incoming_commands_for_cluster instead of chip_cluster_responses to get command responses - In asJavaType helper switching to null for chipType since it calls the underlying type function when that is null - Creating if_command_has_required_argument for commandHasRequiredField which existed in chip_cluster_commands - Switching naming from get_underlying_language_specific_zcl_type to as_underlying_language_specific_zcl_type - Correctly generating the underlying java type for a zcl type in all the java generated files - Updating zp repo - Github: ZAP#682
Github: ZAP#682
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
…cleanup - Using if_has_user_clusters helper instead of chip_has_client_clusters - Using zcl_command_arguments, if_is_struct and zcl_struct_items_by_struct_name instead of chip_cluster_command_arguments_with_structs_expanded - Github: ZAP#682
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
This stale pull request has been automatically closed. Thank you for your contributions. |
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.
Fixes: ZAP#682