Skip to content

Commit

Permalink
Fix for the non-determinism issue on Command handler and Outgoing Com…
Browse files Browse the repository at this point in the history
…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 <bzbarsky@apple.com>
  • Loading branch information
2 people authored and pull[bot] committed Feb 23, 2022
1 parent 467ef04 commit 4540934
Show file tree
Hide file tree
Showing 12 changed files with 350 additions and 1,731 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
11 changes: 11 additions & 0 deletions examples/lighting-app/lighting-common/lighting-app.matter
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 7 additions & 7 deletions examples/tv-casting-app/tv-casting-common/tv-casting-app.matter
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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)}}
Expand All @@ -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}}
Expand Down Expand Up @@ -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}}
Expand Down
62 changes: 51 additions & 11 deletions src/app/zap-templates/templates/app/MatterIDL.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -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}}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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

Expand Down
5 changes: 4 additions & 1 deletion src/app/zap-templates/templates/chip/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions src/controller/CommandSenderAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading

0 comments on commit 4540934

Please sign in to comment.