From 44c289595bf6a904d26443ff7dbd513275308462 Mon Sep 17 00:00:00 2001 From: Bharat Raju Date: Fri, 4 Feb 2022 09:12:16 +0530 Subject: [PATCH] Bug/non determinism commands/GitHub#342 (#14708) * - Adding changes to the zap templates such that the incoming and outgoing commands are generated with determinism. Using the upto date helpers in the *.zapt templates Currently pointing to a zap repo which has new helpers as well. These changes have not been merged into the github zap repo gihub#342 * - Generation for outgoing commands which originate from the client side only. - All commands originating on the server sides are response commands which do not need to be generated here Github#342 * - Reverting from zcl_command_arguments to chip_cluster_command_arguments_with_structs_expanded - Applying changes to chip_cluster_command_arguments_with_structs_expanded such that it can be used within all_outgoing_commands_for_cluster block helper instead of chip_cluster_command_arguments - Github#342 * - Cleaning the templates further such that we have lesser diff to what we generated before Github#342 * Making sure that outgoing commands are generated for both mfg and non-mfg specific clusters and commands Github#342 * Generating for clusters which do not have any outgoing commands because there is code which depends on this generated code Github#342 * Using chip_client_clusters instead of all_user_clusters_with_outgoing_commands in CHIPClusters-src.zapt just like we do in CHIPClusters.zapt to maintain consistency Also reverting the changes in helper.js since those are no longer required Github#342 * Cleaning up asBlock and regening Github#342 * Regening light switch app with the right content after rebasing from upstream master Github#342 * Cleaning up the typo in helper.js Github#342 * Restore the previous asBlocks behavior * Fixing the request structs for commands in .matter generated files such that request struct should be defined for all incoming commands from client to server and outgoing commands from client to server Github#342 * Fixing the response structs for commands in .matter generated files such that response struct should be defined for all incoming commands from server to client and outgoing commands from server to client Github#342 * Regening after rebasing master Github#342 * Remove unnecessary isMfgSpecific="false" and fix rebase problem in cluster-objects.h * Cleaning up the remaining non determinism in MatterIDL template Github#342 * Formatting the MatterIDL.zapt template Github#342 * Formatting the MatterIDL.zapt template Github#342 * Adding partials to remove duplication in the template and cleaning up Github#342 * minor cleanup Github#342 * Using responseRef instead of the complicated logic in the template Adding some endpoint config changes which came up after pulling the latest zap repo Github#342 * Regening after rebase Github#342 * Remove stray CommandSenderHandle bit that is not used. Co-authored-by: Boris Zbarsky --- .../all-clusters-app.matter | 3 +- .../light-switch-app.matter | 19 ++++ .../lighting-common/lighting-app.matter | 7 ++ .../tv-casting-common/tv-casting-app.matter | 4 +- src/app/zap-templates/app-templates.json | 12 +++ .../idl/command_request_response.zapt | 19 ++++ .../partials/idl/command_request_struct.zapt | 10 ++ .../partials/idl/command_response_struct.zapt | 10 ++ .../templates/app/MatterIDL.zapt | 102 ++++++------------ .../zap-templates/templates/chip/helper.js | 5 +- third_party/zap/repo | 2 +- 11 files changed, 115 insertions(+), 78 deletions(-) create mode 100644 src/app/zap-templates/partials/idl/command_request_response.zapt create mode 100644 src/app/zap-templates/partials/idl/command_request_struct.zapt create mode 100644 src/app/zap-templates/partials/idl/command_response_struct.zapt diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter index 5d421d2ad0a9f2..38b1804af0cd64 100644 --- a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter +++ b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter @@ -1678,8 +1678,7 @@ server cluster IasZone = 1280 { INT16U delay = 3; } - command ZoneEnrollRequest(ZoneEnrollRequestRequest): ZoneEnrollResponse = 1; - command ZoneStatusChangeNotification(ZoneStatusChangeNotificationRequest): DefaultSuccess = 0; + command ZoneEnrollResponse(ZoneEnrollResponseRequest): DefaultSuccess = 0; } server cluster Identify = 3 { diff --git a/examples/light-switch-app/light-switch-common/light-switch-app.matter b/examples/light-switch-app/light-switch-common/light-switch-app.matter index f4a37fff63c66b..c2cab7b5644e8f 100644 --- a/examples/light-switch-app/light-switch-common/light-switch-app.matter +++ b/examples/light-switch-app/light-switch-common/light-switch-app.matter @@ -640,6 +640,13 @@ client cluster Groups = 4 { request struct ViewGroupRequest { INT16U groupId = 0; } + + command AddGroup(AddGroupRequest): AddGroupResponse = 0; + command AddGroupIfIdentifying(AddGroupIfIdentifyingRequest): DefaultSuccess = 5; + command GetGroupMembership(GetGroupMembershipRequest): GetGroupMembershipResponse = 2; + command RemoveAllGroups(): DefaultSuccess = 4; + command RemoveGroup(RemoveGroupRequest): RemoveGroupResponse = 3; + command ViewGroup(ViewGroupRequest): ViewGroupResponse = 1; } client cluster Identify = 3 { @@ -1009,6 +1016,10 @@ client cluster OnOff = 6 { readonly global attribute attrib_id attributeList[] = 65531; readonly global attribute bitmap32 featureMap = 65532; readonly global attribute int16u clusterRevision = 65533; + + command Off(): DefaultSuccess = 0; + command On(): DefaultSuccess = 1; + command Toggle(): DefaultSuccess = 2; } server cluster OperationalCredentials = 62 { @@ -1215,6 +1226,14 @@ client cluster Scenes = 5 { CHAR_STRING sceneName = 4; SceneExtensionFieldSet extensionFieldSets[] = 5; } + + command AddScene(AddSceneRequest): AddSceneResponse = 0; + command GetSceneMembership(GetSceneMembershipRequest): GetSceneMembershipResponse = 6; + command RecallScene(RecallSceneRequest): DefaultSuccess = 5; + command RemoveAllScenes(RemoveAllScenesRequest): RemoveAllScenesResponse = 3; + command RemoveScene(RemoveSceneRequest): RemoveSceneResponse = 2; + command StoreScene(StoreSceneRequest): StoreSceneResponse = 4; + command ViewScene(ViewSceneRequest): ViewSceneResponse = 1; } server cluster SoftwareDiagnostics = 52 { diff --git a/examples/lighting-app/lighting-common/lighting-app.matter b/examples/lighting-app/lighting-common/lighting-app.matter index c50f9fb09caff7..d62f0d0cd8abbb 100644 --- a/examples/lighting-app/lighting-common/lighting-app.matter +++ b/examples/lighting-app/lighting-common/lighting-app.matter @@ -1116,6 +1116,13 @@ client cluster OnOff = 6 { int16u onTime = 1; int16u offWaitTime = 2; } + + command Off(): DefaultSuccess = 0; + command OffWithEffect(OffWithEffectRequest): DefaultSuccess = 64; + command On(): DefaultSuccess = 1; + command OnWithRecallGlobalScene(): DefaultSuccess = 65; + command OnWithTimedOff(OnWithTimedOffRequest): DefaultSuccess = 66; + command Toggle(): DefaultSuccess = 2; } server cluster OnOff = 6 { diff --git a/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter b/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter index 86a64e07333c15..c78f676567decb 100644 --- a/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter +++ b/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter @@ -1559,8 +1559,7 @@ server cluster IasZone = 1280 { INT16U delay = 3; } - command ZoneEnrollRequest(ZoneEnrollRequestRequest): ZoneEnrollResponse = 1; - command ZoneStatusChangeNotification(ZoneStatusChangeNotificationRequest): DefaultSuccess = 0; + command ZoneEnrollResponse(ZoneEnrollResponseRequest): DefaultSuccess = 0; } server cluster Identify = 3 { @@ -2497,7 +2496,6 @@ server cluster TestCluster = 1295 { command Test(): DefaultSuccess = 0; command TestNotHandled(): DefaultSuccess = 1; - command TestSpecific(): TestSpecificResponse = 2; } server cluster Thermostat = 513 { diff --git a/src/app/zap-templates/app-templates.json b/src/app/zap-templates/app-templates.json index 3a075c6e90c7b9..4381b76e02fbe8 100644 --- a/src/app/zap-templates/app-templates.json +++ b/src/app/zap-templates/app-templates.json @@ -34,6 +34,18 @@ { "name": "idl_structure_member", "path": "partials/idl/structure_member.zapt" + }, + { + "name": "idl_command_request_response", + "path": "partials/idl/command_request_response.zapt" + }, + { + "name": "idl_command_request_struct", + "path": "partials/idl/command_request_struct.zapt" + }, + { + "name": "idl_command_response_struct", + "path": "partials/idl/command_response_struct.zapt" } ], "templates": [ diff --git a/src/app/zap-templates/partials/idl/command_request_response.zapt b/src/app/zap-templates/partials/idl/command_request_response.zapt new file mode 100644 index 00000000000000..03b5133b0bdd44 --- /dev/null +++ b/src/app/zap-templates/partials/idl/command_request_response.zapt @@ -0,0 +1,19 @@ +{{#first}} + +{{/first}} +{{#if mustUseTimedInvoke}} + {{~indent 1~}}timed command +{{~else}} + {{~indent 1~}}command +{{~/if}} {{asUpperCamelCase commandName}}( +{{~#zcl_command_arguments~}} + {{~#first~}} + {{asUpperCamelCase parent.commandName}}Request + {{~/first~}} +{{~/zcl_command_arguments~}} +): {{#if responseRef~}} + {{getResponseCommandName responseRef}} +{{~else~}} + DefaultSuccess +{{~/if~}} +{{~indent 0}} = {{code}}; \ No newline at end of file diff --git a/src/app/zap-templates/partials/idl/command_request_struct.zapt b/src/app/zap-templates/partials/idl/command_request_struct.zapt new file mode 100644 index 00000000000000..da9352c61cc3a1 --- /dev/null +++ b/src/app/zap-templates/partials/idl/command_request_struct.zapt @@ -0,0 +1,10 @@ +{{#zcl_command_arguments}} + {{#first}} + {{~new_line 1~}}{{~indent 1~}}request struct {{asUpperCamelCase parent.commandName}}Request { + {{/first}} + {{~indent 2~}}{{> idl_structure_member}} + {{#last}} + + {{~indent 1~}}} + {{/last}} +{{/zcl_command_arguments}} \ No newline at end of file diff --git a/src/app/zap-templates/partials/idl/command_response_struct.zapt b/src/app/zap-templates/partials/idl/command_response_struct.zapt new file mode 100644 index 00000000000000..dda69e083dedd9 --- /dev/null +++ b/src/app/zap-templates/partials/idl/command_response_struct.zapt @@ -0,0 +1,10 @@ +{{#zcl_command_arguments}} + {{#first}} + {{~new_line 1~}}{{~indent 1~}}response struct {{asUpperCamelCase parent.commandName}} { + {{/first}} + {{~indent 2~}}{{> idl_structure_member}} + {{#last}} + + {{~indent 1~}}} + {{/last}} +{{/zcl_command_arguments}} \ No newline at end of file diff --git a/src/app/zap-templates/templates/app/MatterIDL.zapt b/src/app/zap-templates/templates/app/MatterIDL.zapt index 09775b363c43f0..c9ce37cb97af50 100644 --- a/src/app/zap-templates/templates/app/MatterIDL.zapt +++ b/src/app/zap-templates/templates/app/MatterIDL.zapt @@ -8,7 +8,7 @@ {{!TODO: consider #chip_clusters as iteration point as well. {{! Unsure about the differences}} {{#all_user_clusters}} -{{side}} cluster {{asUpperCamelCase name}} = {{code}} { + {{~side}} cluster {{asUpperCamelCase name}} = {{code}} { {{#zcl_enums}} enum {{asUpperCamelCase name}} : {{type}} { {{#zcl_enum_items}} @@ -26,7 +26,7 @@ {{/zcl_bitmaps}} {{#chip_cluster_specific_structs}} -{{>idl_structure_definition extraIndent=1}} + {{~>idl_structure_definition extraIndent=1}} {{/chip_cluster_specific_structs}} {{#zcl_events}} @@ -61,80 +61,44 @@ {{/unless}} {{asLowerCamelCase name~}} {{~#if isArray~}} [] {{~/if}} = {{code}}; {{/chip_server_cluster_attributes}} + {{~!--Open:Generating command request structs for all incoming commands into server side--~}} {{#if (is_server side)}} - {{#all_incoming_commands_for_cluster name 'server'}} - {{#zcl_command_arguments}} - -{{#first}} - request struct {{asUpperCamelCase parent.commandName}}Request { -{{/first}} - {{> idl_structure_member}} -{{#last}} - - } -{{/last}} - {{/zcl_command_arguments}} - {{/all_incoming_commands_for_cluster}} + {{#all_incoming_commands_for_cluster name side}} + {{~>idl_command_request_struct}} + {{/all_incoming_commands_for_cluster}} {{/if}} + {{~!--Close:Generating command request structs for all incoming commands into server side--~}} + {{~!--Open:Generating command request structs for all outgoing commands from client side--~}} {{#if (is_client side)}} - {{#all_outgoing_commands_for_cluster name 'client'}} - {{#zcl_command_arguments}} - -{{#first}} - request struct {{asUpperCamelCase parent.commandName}}Request { -{{/first}} - {{> idl_structure_member}} -{{#last}} - - } -{{/last}} - {{/zcl_command_arguments}} - {{/all_outgoing_commands_for_cluster}} + {{#all_outgoing_commands_for_cluster name side}} + {{~>idl_command_request_struct}} + {{/all_outgoing_commands_for_cluster}} {{/if}} -{{#if (is_client side)}} - {{#all_incoming_commands_for_cluster name 'client'}} - {{#zcl_command_arguments}} - -{{#first}} - response struct {{asUpperCamelCase parent.commandName}} { -{{/first}} - {{> idl_structure_member}} -{{#last}} - - } -{{/last}} - {{/zcl_command_arguments}} - {{/all_incoming_commands_for_cluster}} + {{~!--Close:Generating command request structs for all outgoing commands from client side--~}} + {{~!--Open:Generating command response structs for all incoming commands into client side--~}} + {{#if (is_client side)}} + {{#all_incoming_commands_for_cluster name side}} + {{~>idl_command_response_struct}} + {{/all_incoming_commands_for_cluster}} {{/if}} + {{~!--Close:Generating command response structs for all incoming commands into client side--~}} + {{~!--Open:Generating command response structs for all outgoing commands from server side--~}} {{#if (is_server side)}} - {{#all_outgoing_commands_for_cluster name 'server'}} - {{#zcl_command_arguments}} - -{{#first}} - response struct {{asUpperCamelCase parent.commandName}} { -{{/first}} - {{> idl_structure_member}} -{{#last}} - - } -{{/last}} - {{/zcl_command_arguments}} - {{/all_outgoing_commands_for_cluster}} + {{#all_outgoing_commands_for_cluster name side}} + {{~>idl_command_response_struct}} + {{/all_outgoing_commands_for_cluster}} + {{/if}} + {{~!--Close:Generating command response structs for all outgoing commands from server side--~}} + {{#if (is_server side)}} + {{#all_incoming_commands_for_cluster name side}} + {{~>idl_command_request_response}}{{~new_line 1~}} + {{/all_incoming_commands_for_cluster}} + {{/if}} + {{#if (is_client side)}} + {{#all_outgoing_commands_for_cluster name side}} + {{~>idl_command_request_response}}{{~new_line 1~}} + {{/all_outgoing_commands_for_cluster}} {{/if}} - {{#chip_cluster_commands}} - {{#first}} - - {{/first}} - {{#if mustUseTimedInvoke}} - timed command - {{~else}} - command - {{~/if}} {{asUpperCamelCase name}}( - {{~#if arguments~}} - {{asUpperCamelCase name}}Request - {{~/if~}} - ): {{asUpperCamelCase responseName}} = {{code}}; - {{/chip_cluster_commands}} } {{/all_user_clusters}} diff --git a/src/app/zap-templates/templates/chip/helper.js b/src/app/zap-templates/templates/chip/helper.js index 876c32bc6e651a..4309bfc77c5258 100644 --- a/src/app/zap-templates/templates/chip/helper.js +++ b/src/app/zap-templates/templates/chip/helper.js @@ -38,10 +38,9 @@ function throwErrorIfUndefined(item, errorMsg, conditions) function checkIsInsideClusterBlock(context, name) { - const clusterName = context.name; - const clusterSide = context.side; + const clusterName = context.name ? context.name : context.clusterName; + const clusterSide = context.side ? context.side : context.clusterSide; const errorMsg = name + ': Not inside a ({#chip_server_clusters}} block.'; - throwErrorIfUndefined(context, errorMsg, [ clusterName, clusterSide ]); return { clusterName, clusterSide }; diff --git a/third_party/zap/repo b/third_party/zap/repo index ebacff870b0b00..149d47c22621df 160000 --- a/third_party/zap/repo +++ b/third_party/zap/repo @@ -1 +1 @@ -Subproject commit ebacff870b0b0085ba48e058858f25f1b2fc063a +Subproject commit 149d47c22621df1a064be1e24fa2f1185b69dfef