Skip to content

Commit

Permalink
Remove incorrect checks for unicast commands in groups cluster. (#19124)
Browse files Browse the repository at this point in the history
In the original Silicon Labs code, these checks happened _after_ the actual
action of the command had taken place, and only affected the sending of the
response.  But the groups cluster edits have now placed the actual work of the
command after these checks, which makes the resulting behavior not
spec-compliant.

Since we don't send a response on the CommandHandler level if the incoming
command was not unicast, we can just remove these checks altogether.  The
behavior they are meant to achieve is handled by CommandHandler.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Nov 13, 2023
1 parent 46d5e42 commit 2266496
Showing 1 changed file with 0 additions and 19 deletions.
19 changes: 0 additions & 19 deletions src/app/clusters/groups-server/groups-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,6 @@ bool emberAfGroupsClusterAddGroupCallback(app::CommandHandler * commandObj, cons
auto fabricIndex = commandObj->GetAccessingFabricIndex();
Groups::Commands::AddGroupResponse::Type response;

// For all networks, Add Group commands are only responded to when
// they are addressed to a single device.
if (emberAfCurrentCommand()->type != EMBER_INCOMING_UNICAST && emberAfCurrentCommand()->type != EMBER_INCOMING_UNICAST_REPLY)
{
return true;
}

response.groupId = commandData.groupId;
response.status = GroupAdd(fabricIndex, commandPath.mEndpointId, commandData.groupId, commandData.groupName);
commandObj->AddResponse(commandPath, response);
Expand All @@ -158,11 +151,6 @@ bool emberAfGroupsClusterViewGroupCallback(app::CommandHandler * commandObj, con
CHIP_ERROR err = CHIP_NO_ERROR;
EmberAfStatus status = EMBER_ZCL_STATUS_NOT_FOUND;

if (emberAfCurrentCommand()->type != EMBER_INCOMING_UNICAST && emberAfCurrentCommand()->type != EMBER_INCOMING_UNICAST_REPLY)
{
return true;
}

VerifyOrExit(IsFabricGroupId(groupId), status = EMBER_ZCL_STATUS_INVALID_VALUE);
VerifyOrExit(nullptr != provider, status = EMBER_ZCL_STATUS_FAILURE);
VerifyOrExit(provider->HasEndpoint(fabricIndex, groupId, commandPath.mEndpointId), status = EMBER_ZCL_STATUS_NOT_FOUND);
Expand Down Expand Up @@ -293,13 +281,6 @@ bool emberAfGroupsClusterRemoveGroupCallback(app::CommandHandler * commandObj, c
auto fabricIndex = commandObj->GetAccessingFabricIndex();
Groups::Commands::RemoveGroupResponse::Type response;

// For all networks, Remove Group commands are only responded to when
// they are addressed to a single device.
if (emberAfCurrentCommand()->type != EMBER_INCOMING_UNICAST && emberAfCurrentCommand()->type != EMBER_INCOMING_UNICAST_REPLY)
{
return true;
}

#ifdef EMBER_AF_PLUGIN_SCENES
// If a group is, removed the scenes associated with that group SHOULD be removed.
emberAfScenesClusterRemoveScenesInGroupCallback(commandPath.mEndpointId, commandData.groupId);
Expand Down

0 comments on commit 2266496

Please sign in to comment.