Skip to content

Commit 63a5c52

Browse files
Fix for the non-determinism issue on Command handler and Outgoing Commands (project-chip#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 <bzbarsky@apple.com>
1 parent 15ef474 commit 63a5c52

File tree

12 files changed

+350
-1731
lines changed

12 files changed

+350
-1731
lines changed

examples/all-clusters-app/all-clusters-common/all-clusters-app.matter

+7-7
Original file line numberDiff line numberDiff line change
@@ -1647,23 +1647,23 @@ server cluster IasZone = 1280 {
16471647
readonly attribute int8u zoneId = 17;
16481648
readonly global attribute int16u clusterRevision = 65533;
16491649

1650-
request struct ZoneEnrollRequestRequest {
1650+
request struct ZoneEnrollResponseRequest {
1651+
IasEnrollResponseCode enrollResponseCode = 0;
1652+
INT8U zoneId = 1;
1653+
}
1654+
1655+
response struct ZoneEnrollRequest {
16511656
IasZoneType zoneType = 0;
16521657
INT16U manufacturerCode = 1;
16531658
}
16541659

1655-
request struct ZoneStatusChangeNotificationRequest {
1660+
response struct ZoneStatusChangeNotification {
16561661
IasZoneStatus zoneStatus = 0;
16571662
BITMAP8 extendedStatus = 1;
16581663
INT8U zoneId = 2;
16591664
INT16U delay = 3;
16601665
}
16611666

1662-
response struct ZoneEnrollResponse {
1663-
IasEnrollResponseCode enrollResponseCode = 0;
1664-
INT8U zoneId = 1;
1665-
}
1666-
16671667
command ZoneEnrollRequest(ZoneEnrollRequestRequest): ZoneEnrollResponse = 1;
16681668
command ZoneStatusChangeNotification(ZoneStatusChangeNotificationRequest): DefaultSuccess = 0;
16691669
}

examples/light-switch-app/light-switch-common/light-switch-app.matter

-19
Original file line numberDiff line numberDiff line change
@@ -617,13 +617,6 @@ client cluster Groups = 4 {
617617
request struct ViewGroupRequest {
618618
INT16U groupId = 0;
619619
}
620-
621-
command AddGroup(AddGroupRequest): AddGroupResponse = 0;
622-
command AddGroupIfIdentifying(AddGroupIfIdentifyingRequest): DefaultSuccess = 5;
623-
command GetGroupMembership(GetGroupMembershipRequest): GetGroupMembershipResponse = 2;
624-
command RemoveAllGroups(): DefaultSuccess = 4;
625-
command RemoveGroup(RemoveGroupRequest): RemoveGroupResponse = 3;
626-
command ViewGroup(ViewGroupRequest): ViewGroupResponse = 1;
627620
}
628621

629622
client cluster Identify = 3 {
@@ -993,10 +986,6 @@ client cluster OnOff = 6 {
993986
readonly global attribute attrib_id attributeList[] = 65531;
994987
readonly global attribute bitmap32 featureMap = 65532;
995988
readonly global attribute int16u clusterRevision = 65533;
996-
997-
command Off(): DefaultSuccess = 0;
998-
command On(): DefaultSuccess = 1;
999-
command Toggle(): DefaultSuccess = 2;
1000989
}
1001990

1002991
server cluster OperationalCredentials = 62 {
@@ -1203,14 +1192,6 @@ client cluster Scenes = 5 {
12031192
CHAR_STRING sceneName = 4;
12041193
SceneExtensionFieldSet extensionFieldSets[] = 5;
12051194
}
1206-
1207-
command AddScene(AddSceneRequest): AddSceneResponse = 0;
1208-
command GetSceneMembership(GetSceneMembershipRequest): GetSceneMembershipResponse = 6;
1209-
command RecallScene(RecallSceneRequest): DefaultSuccess = 5;
1210-
command RemoveAllScenes(RemoveAllScenesRequest): RemoveAllScenesResponse = 3;
1211-
command RemoveScene(RemoveSceneRequest): RemoveSceneResponse = 2;
1212-
command StoreScene(StoreSceneRequest): StoreSceneResponse = 4;
1213-
command ViewScene(ViewSceneRequest): ViewSceneResponse = 1;
12141195
}
12151196

12161197
server cluster SoftwareDiagnostics = 52 {

examples/lighting-app/lighting-common/lighting-app.matter

+11
Original file line numberDiff line numberDiff line change
@@ -1043,6 +1043,17 @@ client cluster OnOff = 6 {
10431043
attribute enum8 startUpOnOff = 16387;
10441044
readonly global attribute bitmap32 featureMap = 65532;
10451045
readonly global attribute int16u clusterRevision = 65533;
1046+
1047+
request struct OffWithEffectRequest {
1048+
OnOffEffectIdentifier effectId = 0;
1049+
OnOffDelayedAllOffEffectVariant effectVariant = 1;
1050+
}
1051+
1052+
request struct OnWithTimedOffRequest {
1053+
OnOffControl onOffControl = 0;
1054+
int16u onTime = 1;
1055+
int16u offWaitTime = 2;
1056+
}
10461057
}
10471058

10481059
server cluster OnOff = 6 {

examples/tv-casting-app/tv-casting-common/tv-casting-app.matter

+7-7
Original file line numberDiff line numberDiff line change
@@ -1471,23 +1471,23 @@ server cluster IasZone = 1280 {
14711471
readonly attribute int8u zoneId = 17;
14721472
readonly global attribute int16u clusterRevision = 65533;
14731473

1474-
request struct ZoneEnrollRequestRequest {
1474+
request struct ZoneEnrollResponseRequest {
1475+
IasEnrollResponseCode enrollResponseCode = 0;
1476+
INT8U zoneId = 1;
1477+
}
1478+
1479+
response struct ZoneEnrollRequest {
14751480
IasZoneType zoneType = 0;
14761481
INT16U manufacturerCode = 1;
14771482
}
14781483

1479-
request struct ZoneStatusChangeNotificationRequest {
1484+
response struct ZoneStatusChangeNotification {
14801485
IasZoneStatus zoneStatus = 0;
14811486
BITMAP8 extendedStatus = 1;
14821487
INT8U zoneId = 2;
14831488
INT16U delay = 3;
14841489
}
14851490

1486-
response struct ZoneEnrollResponse {
1487-
IasEnrollResponseCode enrollResponseCode = 0;
1488-
INT8U zoneId = 1;
1489-
}
1490-
14911491
command ZoneEnrollRequest(ZoneEnrollRequestRequest): ZoneEnrollResponse = 1;
14921492
command ZoneStatusChangeNotification(ZoneStatusChangeNotificationRequest): DefaultSuccess = 0;
14931493
}

src/app/zap-templates/partials/im_command_handler_cluster_commands.zapt

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
1-
{{#if (isServer parent.side)}}
1+
{{#if (isServer parent.clusterSide)}}
22
{{#if mustUseTimedInvoke}}
33
if (!apCommandObj->IsTimedInvoke()) {
44
apCommandObj->AddStatus(aCommandPath, Protocols::InteractionModel::Status::NeedsTimedInteraction);
55
return;
66
}
77
{{/if}}
8-
Commands::{{asUpperCamelCase name}}::DecodableType commandData;
8+
Commands::{{asUpperCamelCase commandName}}::DecodableType commandData;
99
TLVError = DataModel::Decode(aDataTlv, commandData);
1010
if (TLVError == CHIP_NO_ERROR) {
11-
wasHandled = emberAf{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback(apCommandObj, aCommandPath, commandData);
11+
wasHandled = emberAf{{asUpperCamelCase parent.clusterName}}Cluster{{asUpperCamelCase commandName}}Callback(apCommandObj, aCommandPath, commandData);
1212
}
1313
{{else}}
1414
{{#if (zcl_command_arguments_count this.id)}}
@@ -18,7 +18,7 @@ expectArgumentCount = {{ zcl_command_arguments_count this.id }};
1818
{{asUnderlyingZclType type}} {{asSymbol label}};
1919
{{else}}
2020
{{#if_is_struct type}}
21-
{{zapTypeToDecodableClusterObjectType type ns=parent.parent.name}} {{asSymbol label}};
21+
{{zapTypeToDecodableClusterObjectType type ns=parent.parent.clusterName}} {{asSymbol label}};
2222
{{else}}
2323
{{asUnderlyingZclType type}} {{asSymbol label}};
2424
{{/if_is_struct}}
@@ -90,7 +90,7 @@ if (CHIP_END_OF_TLV == TLVError)
9090
if (CHIP_NO_ERROR == TLVError && CHIP_NO_ERROR == TLVUnpackError && {{zcl_command_arguments_count this.id}} == validArgumentCount)
9191
{
9292
{{/if}}
93-
wasHandled = emberAf{{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}Callback(aCommandPath.mEndpointId, apCommandObj{{#zcl_command_arguments}}, {{asSymbol label}}{{/zcl_command_arguments}});
93+
wasHandled = emberAf{{asUpperCamelCase parent.clusterName}}Cluster{{asUpperCamelCase commandName}}Callback(aCommandPath.mEndpointId, apCommandObj{{#zcl_command_arguments}}, {{asSymbol label}}{{/zcl_command_arguments}});
9494
{{#if (zcl_command_arguments_count this.id)}}
9595
}
9696
{{/if}}

src/app/zap-templates/templates/app/MatterIDL.zapt

+51-11
Original file line numberDiff line numberDiff line change
@@ -61,26 +61,66 @@
6161
{{/unless}} {{asLowerCamelCase name~}}
6262
{{~#if isArray~}} [] {{~/if}} = {{code}};
6363
{{/chip_server_cluster_attributes}}
64-
{{#chip_cluster_commands}}
65-
{{#if arguments}}
64+
{{#if (is_server side)}}
65+
{{#all_incoming_commands_for_cluster name 'server'}}
66+
{{#zcl_command_arguments}}
6667

67-
request struct {{asUpperCamelCase name}}Request {
68-
{{#chip_cluster_command_arguments}}
68+
{{#first}}
69+
request struct {{asUpperCamelCase parent.commandName}}Request {
70+
{{/first}}
6971
{{> idl_structure_member}}
72+
{{#last}}
7073

71-
{{/chip_cluster_command_arguments}}
7274
}
75+
{{/last}}
76+
{{/zcl_command_arguments}}
77+
{{/all_incoming_commands_for_cluster}}
7378
{{/if}}
74-
{{/chip_cluster_commands}}
75-
{{#chip_cluster_responses}}
79+
{{#if (is_client side)}}
80+
{{#all_outgoing_commands_for_cluster name 'client'}}
81+
{{#zcl_command_arguments}}
82+
83+
{{#first}}
84+
request struct {{asUpperCamelCase parent.commandName}}Request {
85+
{{/first}}
86+
{{> idl_structure_member}}
87+
{{#last}}
88+
89+
}
90+
{{/last}}
91+
{{/zcl_command_arguments}}
92+
{{/all_outgoing_commands_for_cluster}}
93+
{{/if}}
94+
{{#if (is_client side)}}
95+
{{#all_incoming_commands_for_cluster name 'client'}}
96+
{{#zcl_command_arguments}}
7697

77-
response struct {{asUpperCamelCase name}} {
78-
{{#chip_cluster_response_arguments}}
98+
{{#first}}
99+
response struct {{asUpperCamelCase parent.commandName}} {
100+
{{/first}}
79101
{{> idl_structure_member}}
102+
{{#last}}
80103

81-
{{/chip_cluster_response_arguments}}
82104
}
83-
{{/chip_cluster_responses}}
105+
{{/last}}
106+
{{/zcl_command_arguments}}
107+
{{/all_incoming_commands_for_cluster}}
108+
{{/if}}
109+
{{#if (is_server side)}}
110+
{{#all_outgoing_commands_for_cluster name 'server'}}
111+
{{#zcl_command_arguments}}
112+
113+
{{#first}}
114+
response struct {{asUpperCamelCase parent.commandName}} {
115+
{{/first}}
116+
{{> idl_structure_member}}
117+
{{#last}}
118+
119+
}
120+
{{/last}}
121+
{{/zcl_command_arguments}}
122+
{{/all_outgoing_commands_for_cluster}}
123+
{{/if}}
84124
{{#chip_cluster_commands}}
85125
{{#first}}
86126

src/app/zap-templates/templates/app/im-cluster-command-handler.zapt

+7-12
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,21 @@ namespace app {
2424

2525
namespace Clusters {
2626

27-
{{#all_user_clusters}}
28-
{{#if (isServer side)}}
29-
{{#if (user_cluster_has_enabled_command name side)}}
30-
namespace {{asUpperCamelCase name}} {
27+
{{#all_user_clusters_with_incoming_commands}}
28+
{{#if (isServer clusterSide)}}
29+
namespace {{asUpperCamelCase clusterName}} {
3130

3231
void DispatchServerCommand(CommandHandler * apCommandObj, const ConcreteCommandPath & aCommandPath, TLV::TLVReader & aDataTlv)
3332
{
34-
{{#chip_available_cluster_commands}}
33+
{{#all_incoming_commands_for_cluster clusterName clusterSide}}
3534
{{#first}}
3635
CHIP_ERROR TLVError = CHIP_NO_ERROR;
3736
bool wasHandled = false;
3837
{
3938
switch (aCommandPath.mCommandId)
4039
{
4140
{{/first}}
42-
case Commands::{{asUpperCamelCase name}}::Id: {
41+
case Commands::{{asUpperCamelCase commandName}}::Id: {
4342
{{> im_command_handler_cluster_commands}}
4443
break;
4544
}
@@ -59,17 +58,13 @@ void DispatchServerCommand(CommandHandler * apCommandObj, const ConcreteCommandP
5958
ChipLogProgress(Zcl, "Failed to dispatch command, TLVError=%" CHIP_ERROR_FORMAT, TLVError.Format());
6059
}
6160
{{/last}}
62-
{{else}}
63-
apCommandObj->AddStatus(aCommandPath, Protocols::InteractionModel::Status::UnsupportedCommand);
64-
ChipLogError(Zcl, "Unknown command " ChipLogFormatMEI " for cluster " ChipLogFormatMEI, ChipLogValueMEI(aCommandPath.mCommandId), ChipLogValueMEI(aCommandPath.mClusterId));
65-
{{/chip_available_cluster_commands}}
61+
{{/all_incoming_commands_for_cluster}}
6662
}
6763

6864
}
6965

7066
{{/if}}
71-
{{/if}}
72-
{{/all_user_clusters}}
67+
{{/all_user_clusters_with_incoming_commands}}
7368

7469
} // namespace Clusters
7570

src/app/zap-templates/templates/chip/helper.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -275,11 +275,14 @@ function chip_cluster_command_arguments(options)
275275
function chip_cluster_command_arguments_with_structs_expanded(options)
276276
{
277277
const commandId = checkIsInsideCommandBlock(this, 'chip_cluster_command_arguments');
278-
const commands = getCommands.call(this.parent, 'chip_cluster_commands_argments_with_structs_expanded');
278+
const commands = getCommands.call(this.parent, 'chip_cluster_command_arguments_with_structs_expanded');
279279

280280
const filter = command => command.id == commandId;
281281
return asBlocks.call(this, commands.then(items => {
282282
const item = items.find(filter);
283+
if (item === undefined) {
284+
return [];
285+
}
283286
return item.expandedArguments || item.arguments;
284287
}),
285288
options);

src/controller/CommandSenderAllocator.h

-3
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,5 @@ class CommandSenderPlatformDeleter
3131
public:
3232
void operator()(app::CommandSender * commandSender) const { chip::Platform::Delete(commandSender); }
3333
};
34-
35-
using CommandSenderHandle = std::unique_ptr<app::CommandSender, CommandSenderPlatformDeleter>;
36-
3734
} // namespace Controller
3835
} // namespace chip

0 commit comments

Comments
 (0)