Fix for the non-determinism issue on Command handler and Outgoing Com…

…mands (#13871) * - 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 Co-authored-by: Boris Zbarsky <[email protected]>
project-chip · Jul 21, 2023 · 1608484 · 1608484
1 parent cfda29d
commit 1608484
Show file tree

Hide file tree

Showing 12 changed files with 350 additions and 1,731 deletions.
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
@@ -1647,23 +1647,23 @@ server cluster IasZone = 1280 {
   readonly attribute int8u zoneId = 17;
   readonly global attribute int16u clusterRevision = 65533;
 
-  request struct ZoneEnrollRequestRequest {
+  request struct ZoneEnrollResponseRequest {
+    IasEnrollResponseCode enrollResponseCode = 0;
+    INT8U zoneId = 1;
+  }
+
+  response struct ZoneEnrollRequest {
     IasZoneType zoneType = 0;
     INT16U manufacturerCode = 1;
   }
 
-  request struct ZoneStatusChangeNotificationRequest {
+  response struct ZoneStatusChangeNotification {
     IasZoneStatus zoneStatus = 0;
     BITMAP8 extendedStatus = 1;
     INT8U zoneId = 2;
     INT16U delay = 3;
   }
 
-  response struct ZoneEnrollResponse {
-    IasEnrollResponseCode enrollResponseCode = 0;
-    INT8U zoneId = 1;
-  }
-
   command ZoneEnrollRequest(ZoneEnrollRequestRequest): ZoneEnrollResponse = 1;
   command ZoneStatusChangeNotification(ZoneStatusChangeNotificationRequest): DefaultSuccess = 0;
 }

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
@@ -617,13 +617,6 @@ 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 {
@@ -993,10 +986,6 @@ 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 {
@@ -1203,14 +1192,6 @@ 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
@@ -1043,6 +1043,17 @@ client cluster OnOff = 6 {
   attribute enum8 startUpOnOff = 16387;
   readonly global attribute bitmap32 featureMap = 65532;
   readonly global attribute int16u clusterRevision = 65533;
+
+  request struct OffWithEffectRequest {
+    OnOffEffectIdentifier effectId = 0;
+    OnOffDelayedAllOffEffectVariant effectVariant = 1;
+  }
+
+  request struct OnWithTimedOffRequest {
+    OnOffControl onOffControl = 0;
+    int16u onTime = 1;
+    int16u offWaitTime = 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
@@ -1471,23 +1471,23 @@ server cluster IasZone = 1280 {
   readonly attribute int8u zoneId = 17;
   readonly global attribute int16u clusterRevision = 65533;
 
-  request struct ZoneEnrollRequestRequest {
+  request struct ZoneEnrollResponseRequest {
+    IasEnrollResponseCode enrollResponseCode = 0;
+    INT8U zoneId = 1;
+  }
+
+  response struct ZoneEnrollRequest {
     IasZoneType zoneType = 0;
     INT16U manufacturerCode = 1;
   }
 
-  request struct ZoneStatusChangeNotificationRequest {
+  response struct ZoneStatusChangeNotification {
     IasZoneStatus zoneStatus = 0;
     BITMAP8 extendedStatus = 1;
     INT8U zoneId = 2;
     INT16U delay = 3;
   }
 
-  response struct ZoneEnrollResponse {
-    IasEnrollResponseCode enrollResponseCode = 0;
-    INT8U zoneId = 1;
-  }
-
   command ZoneEnrollRequest(ZoneEnrollRequestRequest): ZoneEnrollResponse = 1;
   command ZoneStatusChangeNotification(ZoneStatusChangeNotificationRequest): DefaultSuccess = 0;
 }

diff --git a/src/app/zap-templates/partials/im_command_handler_cluster_commands.zapt b/src/app/zap-templates/partials/im_command_handler_cluster_commands.zapt
@@ -1,14 +1,14 @@
-{{#if (isServer parent.side)}}
+{{#if (isServer parent.clusterSide)}}
 {{#if mustUseTimedInvoke}}
 if (!apCommandObj->IsTimedInvoke()) {
   apCommandObj->AddStatus(aCommandPath, Protocols::InteractionModel::Status::NeedsTimedInteraction);
   return;
 }
 {{/if}}
-Commands::{{asUpperCamelCase name}}::DecodableType commandData;
+Commands::{{asUpperCamelCase commandName}}::DecodableType commandData;
 TLVError = DataModel::Decode(aDataTlv, commandData);
 if (TLVError == CHIP_NO_ERROR) {
-wasHandled = emberAf{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback(apCommandObj, aCommandPath, commandData);
+wasHandled = emberAf{{asUpperCamelCase parent.clusterName}}Cluster{{asUpperCamelCase commandName}}Callback(apCommandObj, aCommandPath, commandData);
 }
 {{else}}
 {{#if (zcl_command_arguments_count this.id)}}
@@ -18,7 +18,7 @@ expectArgumentCount = {{ zcl_command_arguments_count this.id }};
     {{asUnderlyingZclType type}} {{asSymbol label}};
   {{else}}
     {{#if_is_struct type}}
-      {{zapTypeToDecodableClusterObjectType type ns=parent.parent.name}} {{asSymbol label}};
+      {{zapTypeToDecodableClusterObjectType type ns=parent.parent.clusterName}} {{asSymbol label}};
     {{else}}
       {{asUnderlyingZclType type}} {{asSymbol label}};
     {{/if_is_struct}}
@@ -90,7 +90,7 @@ if (CHIP_END_OF_TLV == TLVError)
 if (CHIP_NO_ERROR == TLVError && CHIP_NO_ERROR == TLVUnpackError && {{zcl_command_arguments_count this.id}} == validArgumentCount)
 {
 {{/if}}
-wasHandled = emberAf{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback(aCommandPath.mEndpointId, apCommandObj{{#zcl_command_arguments}}, {{asSymbol label}}{{/zcl_command_arguments}});
+wasHandled = emberAf{{asUpperCamelCase parent.clusterName}}Cluster{{asUpperCamelCase commandName}}Callback(aCommandPath.mEndpointId, apCommandObj{{#zcl_command_arguments}}, {{asSymbol label}}{{/zcl_command_arguments}});
 {{#if (zcl_command_arguments_count this.id)}}
 }
 {{/if}}

diff --git a/src/app/zap-templates/templates/app/MatterIDL.zapt b/src/app/zap-templates/templates/app/MatterIDL.zapt
@@ -61,26 +61,66 @@
     {{/unless}} {{asLowerCamelCase name~}}
     {{~#if isArray~}} [] {{~/if}} = {{code}};
   {{/chip_server_cluster_attributes}}
-  {{#chip_cluster_commands}}
-  {{#if arguments}}
+  {{#if (is_server side)}}
+  {{#all_incoming_commands_for_cluster name 'server'}}
+  {{#zcl_command_arguments}}
 
-  request struct {{asUpperCamelCase name}}Request {
-    {{#chip_cluster_command_arguments}}
+{{#first}}
+  request struct {{asUpperCamelCase parent.commandName}}Request {
+{{/first}}    
     {{> idl_structure_member}}
+{{#last}}
 
-    {{/chip_cluster_command_arguments}}
   }
+{{/last}}
+  {{/zcl_command_arguments}}
+  {{/all_incoming_commands_for_cluster}}
   {{/if}}
-  {{/chip_cluster_commands}}
-  {{#chip_cluster_responses}}
+  {{#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}}
+  {{/if}}
+{{#if (is_client side)}}
+  {{#all_incoming_commands_for_cluster name 'client'}}
+  {{#zcl_command_arguments}}
 
-  response struct {{asUpperCamelCase name}} {
-    {{#chip_cluster_response_arguments}}
+{{#first}}
+  response struct {{asUpperCamelCase parent.commandName}} {
+{{/first}}    
     {{> idl_structure_member}}
+{{#last}}
 
-    {{/chip_cluster_response_arguments}}
   }
-  {{/chip_cluster_responses}}
+{{/last}}
+  {{/zcl_command_arguments}}
+  {{/all_incoming_commands_for_cluster}}
+  {{/if}}
+  {{#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}}
+  {{/if}}
   {{#chip_cluster_commands}}
   {{#first}}
 

diff --git a/src/app/zap-templates/templates/app/im-cluster-command-handler.zapt b/src/app/zap-templates/templates/app/im-cluster-command-handler.zapt
@@ -24,22 +24,21 @@ namespace app {
 
 namespace Clusters {
 
-{{#all_user_clusters}}
-{{#if (isServer side)}}
-{{#if (user_cluster_has_enabled_command name side)}}
-namespace {{asUpperCamelCase name}} {
+{{#all_user_clusters_with_incoming_commands}}
+{{#if (isServer clusterSide)}}
+namespace {{asUpperCamelCase clusterName}} {
 
 void DispatchServerCommand(CommandHandler * apCommandObj, const ConcreteCommandPath & aCommandPath, TLV::TLVReader & aDataTlv)
 {
-    {{#chip_available_cluster_commands}}
+    {{#all_incoming_commands_for_cluster clusterName clusterSide}}
     {{#first}}
     CHIP_ERROR TLVError = CHIP_NO_ERROR;
     bool wasHandled = false;
     {
         switch (aCommandPath.mCommandId)
         {
     {{/first}}
-        case Commands::{{asUpperCamelCase name}}::Id: {
+        case Commands::{{asUpperCamelCase commandName}}::Id: {
         {{> im_command_handler_cluster_commands}}
             break;
         }
@@ -59,17 +58,13 @@ void DispatchServerCommand(CommandHandler * apCommandObj, const ConcreteCommandP
       ChipLogProgress(Zcl, "Failed to dispatch command, TLVError=%" CHIP_ERROR_FORMAT, TLVError.Format());
     }
     {{/last}}
-    {{else}}
-    apCommandObj->AddStatus(aCommandPath, Protocols::InteractionModel::Status::UnsupportedCommand);
-    ChipLogError(Zcl, "Unknown command " ChipLogFormatMEI " for cluster " ChipLogFormatMEI, ChipLogValueMEI(aCommandPath.mCommandId), ChipLogValueMEI(aCommandPath.mClusterId));
-    {{/chip_available_cluster_commands}}
+    {{/all_incoming_commands_for_cluster}}
 }
 
 }
 
 {{/if}}
-{{/if}}
-{{/all_user_clusters}}
+{{/all_user_clusters_with_incoming_commands}}
 
 } // namespace Clusters
 

diff --git a/src/app/zap-templates/templates/chip/helper.js b/src/app/zap-templates/templates/chip/helper.js
@@ -275,11 +275,14 @@ function chip_cluster_command_arguments(options)
 function chip_cluster_command_arguments_with_structs_expanded(options)
 {
   const commandId = checkIsInsideCommandBlock(this, 'chip_cluster_command_arguments');
-  const commands  = getCommands.call(this.parent, 'chip_cluster_commands_argments_with_structs_expanded');
+  const commands  = getCommands.call(this.parent, 'chip_cluster_command_arguments_with_structs_expanded');
 
   const filter = command => command.id == commandId;
   return asBlocks.call(this, commands.then(items => {
     const item = items.find(filter);
+    if (item === undefined) {
+      return [];
+    }
     return item.expandedArguments || item.arguments;
   }),
       options);

diff --git a/src/controller/CommandSenderAllocator.h b/src/controller/CommandSenderAllocator.h
@@ -31,8 +31,5 @@ class CommandSenderPlatformDeleter
 public:
     void operator()(app::CommandSender * commandSender) const { chip::Platform::Delete(commandSender); }
 };
-
-using CommandSenderHandle = std::unique_ptr<app::CommandSender, CommandSenderPlatformDeleter>;
-
 } // namespace Controller
 } // namespace chip