Skip to content

Commit

Permalink
[Group] Add GroupDataProvider to Group command processing (#13833)
Browse files Browse the repository at this point in the history 
* Add group data provider to group command processing

update group messaging test

* Add group data provider to write attribute

* Add Group Data provider to Server

* update test file

* update lighting app zap file

* Group messagins bypass

* update fabric

* multi/uni cast fix for ember command type

* Replaced incorrect define

* fix log

* Fix TestWriteInteraction hardfault

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <[email protected]>
Co-authored-by: Martin Turon <[email protected]>

* PR comments changes

* server comments

* fabric fix

* restyle

* add group-server to cmake files

* esp build fix

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <[email protected]>

* PR comments

* generated files

* add issue number to comment

Co-authored-by: Boris Zbarsky <[email protected]>
Co-authored-by: Martin Turon <[email protected]>
  • Loading branch information
@mkardous-silabs @bzbarsky-apple @turon
3 people authored Jan 25, 2022
1 parent e8d8906 commit de95567
Show file tree
Hide file tree
Showing 27 changed files with 844 additions and 174 deletions.
11 changes: 8 additions & 3 deletions examples/chip-tool/commands/common/CommandInvoker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ class CommandInvoker final : public ResponseReceiver<typename RequestType::Respo
return CHIP_NO_ERROR;
}

CHIP_ERROR InvokeGroupCommand(Messaging::ExchangeManager * exchangeManager, GroupId groupId, const RequestType & aRequestData)
CHIP_ERROR InvokeGroupCommand(Messaging::ExchangeManager * exchangeManager, FabricIndex fabric, GroupId groupId,
const RequestType & aRequestData)
{
app::CommandPathParams commandPath = { 0 /* endpoint */, groupId, RequestType::GetClusterId(), RequestType::GetCommandId(),
(app::CommandPathFlags::kGroupIdValid) };
Expand All @@ -116,7 +117,7 @@ class CommandInvoker final : public ResponseReceiver<typename RequestType::Respo

ReturnErrorOnFailure(commandSender->AddRequestData(commandPath, aRequestData));

Optional<SessionHandle> session = exchangeManager->GetSessionManager()->CreateGroupSession(groupId);
Optional<SessionHandle> session = exchangeManager->GetSessionManager()->CreateGroupSession(groupId, fabric);
if (!session.HasValue())
{
return CHIP_ERROR_NO_MEMORY;
Expand Down Expand Up @@ -231,7 +232,11 @@ CHIP_ERROR InvokeGroupCommand(DeviceProxy * aDevice, void * aContext,

// invoker will be deleted by the onDone call before the return of InvokeGroupCommand
// invoker should not be used after the InvokeGroupCommand call
ReturnErrorOnFailure(invoker->InvokeGroupCommand(aDevice->GetExchangeManager(), groupId, aRequestData));
//
// We assume the aDevice already has a Case session which is way we can use he established Secure Session
ReturnErrorOnFailure(invoker->InvokeGroupCommand(aDevice->GetExchangeManager(),
aDevice->GetSecureSession().Value()->AsSecureSession()->GetFabricIndex(),
groupId, aRequestData));

// invoker is already deleted and is not to be used
invoker.release();
Expand Down
1 change: 1 addition & 0 deletions examples/lighting-app/esp32/main/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ idf_component_register(PRIV_INCLUDE_DIRS
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/user-label-server"
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/wifi-network-diagnostics-server"
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/ota-requestor"
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/groups-server"
PRIV_REQUIRES chip QRCode bt led_strip app_update)

set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 17)
Expand Down
57 changes: 57 additions & 0 deletions examples/lighting-app/lighting-common/lighting-app.matter
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,61 @@ server cluster GeneralDiagnostics = 51 {
readonly global attribute int16u clusterRevision = 65533;
}

server cluster Groups = 4 {
readonly attribute bitmap8 nameSupport = 0;
readonly global attribute int16u clusterRevision = 65533;

request struct AddGroupRequest {
INT16U groupId = 0;
CHAR_STRING groupName = 1;
}

request struct AddGroupIfIdentifyingRequest {
INT16U groupId = 0;
CHAR_STRING groupName = 1;
}

request struct GetGroupMembershipRequest {
INT16U groupList[] = 0;
}

request struct RemoveGroupRequest {
INT16U groupId = 0;
}

request struct ViewGroupRequest {
INT16U groupId = 0;
}

response struct AddGroupResponse {
ENUM8 status = 0;
INT16U groupId = 1;
}

response struct GetGroupMembershipResponse {
INT8U capacity = 0;
INT16U groupList[] = 1;
}

response struct RemoveGroupResponse {
ENUM8 status = 0;
INT16U groupId = 1;
}

response struct ViewGroupResponse {
ENUM8 status = 0;
INT16U groupId = 1;
CHAR_STRING groupName = 2;
}

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;
}

server cluster Identify = 3 {
enum IdentifyEffectIdentifier : ENUM8 {
kBlink = 0;
Expand Down Expand Up @@ -1424,6 +1479,7 @@ endpoint 0 {
server cluster FixedLabel;
server cluster GeneralCommissioning;
server cluster GeneralDiagnostics;
server cluster Groups;
server cluster LocalizationConfiguration;
server cluster NetworkCommissioning;
binding cluster OtaSoftwareUpdateProvider;
Expand All @@ -1440,6 +1496,7 @@ endpoint 0 {
endpoint 1 {
server cluster ColorControl;
server cluster Descriptor;
server cluster Groups;
server cluster Identify;
server cluster LevelControl;
server cluster OccupancySensing;
Expand Down
24 changes: 22 additions & 2 deletions examples/lighting-app/lighting-common/lighting-app.zap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@
"mfgCode": null,
"define": "GROUPS_CLUSTER",
"side": "server",
"enabled": 0,
"enabled": 1,
"commands": [
{
"name": "AddGroupResponse",
Expand Down Expand Up @@ -4653,7 +4653,7 @@
"mfgCode": null,
"define": "GROUPS_CLUSTER",
"side": "server",
"enabled": 0,
"enabled": 1,
"commands": [
{
"name": "AddGroupResponse",
Expand Down Expand Up @@ -6734,6 +6734,26 @@
}
]
},
{
"name": "Groups",
"code": 4,
"mfgCode": null,
"define": "GROUPS_CLUSTER",
"side": "client",
"enabled": 0,
"commands": [],
"attributes": []
},
{
"name": "Groups",
"code": 4,
"mfgCode": null,
"define": "GROUPS_CLUSTER",
"side": "server",
"enabled": 0,
"commands": [],
"attributes": []
},
{
"name": "On/Off",
"code": 6,
Expand Down
1 change: 1 addition & 0 deletions examples/lighting-app/mbed/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ target_sources(${APP_TARGET} PRIVATE
${CHIP_ROOT}/src/app/clusters/ota-requestor/ota-requestor-server.cpp
${CHIP_ROOT}/src/app/clusters/ota-requestor/BDXDownloader.cpp
${CHIP_ROOT}/src/app/clusters/ota-requestor/OTARequestor.cpp
${CHIP_ROOT}/src/app/clusters/groups-server/groups-server.cpp
)

target_link_libraries(${APP_TARGET} mbed-os-posix-socket mbed-os mbed-ble mbed-events mbed-netsocket mbed-storage mbed-storage-kv-global-api mbed-mbedtls mbed-emac chip)
Expand Down
1 change: 1 addition & 0 deletions examples/lighting-app/telink/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,5 @@ target_sources(app PRIVATE
${CHIP_ROOT}/src/app/clusters/ota-requestor/ota-requestor-server.cpp
${CHIP_ROOT}/src/app/clusters/ota-requestor/BDXDownloader.cpp
${CHIP_ROOT}/src/app/clusters/ota-requestor/OTARequestor.cpp
${CHIP_ROOT}/src/app/clusters/groups-server/groups-server.cpp
)
143 changes: 120 additions & 23 deletions src/app/CommandHandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

#include <access/AccessControl.h>
#include <app/util/MatterCallbacks.h>
#include <credentials/GroupDataProvider.h>
#include <lib/support/TypeTraits.h>
#include <protocols/secure_channel/Constants.h>

Expand Down Expand Up @@ -127,7 +128,15 @@ CHIP_ERROR CommandHandler::ProcessInvokeRequest(System::PacketBufferHandle && pa
VerifyOrReturnError(TLV::AnonymousTag() == invokeRequestsReader.GetTag(), CHIP_ERROR_INVALID_TLV_TAG);
CommandDataIB::Parser commandData;
ReturnErrorOnFailure(commandData.Init(invokeRequestsReader));
ReturnErrorOnFailure(ProcessCommandDataIB(commandData));

if (mpExchangeCtx->IsGroupExchangeContext())
{
ReturnErrorOnFailure(ProcessGroupCommandDataIB(commandData));
}
else
{
ReturnErrorOnFailure(ProcessCommandDataIB(commandData));
}
}

// if we have exhausted this container
Expand Down Expand Up @@ -181,21 +190,24 @@ void CommandHandler::DecrementHoldOff()
return;
}

CHIP_ERROR err = CHIP_NO_ERROR;
if (!mpExchangeCtx->IsGroupExchangeContext())
if (mpExchangeCtx->IsGroupExchangeContext())
{
err = SendCommandResponse();
mpExchangeCtx->Close();
}

if (err != CHIP_NO_ERROR)
else
{
ChipLogError(DataManagement, "Failed to send command response: %" CHIP_ERROR_FORMAT, err.Format());
// We marked the exchange as "WillSendMessage", need to shutdown the exchange manually to avoid leaking exchanges.
if (mpExchangeCtx != nullptr)
CHIP_ERROR err = SendCommandResponse();
if (err != CHIP_NO_ERROR)
{
mpExchangeCtx->Close();
ChipLogError(DataManagement, "Failed to send command response: %" CHIP_ERROR_FORMAT, err.Format());
// We marked the exchange as "WillSendMessage", need to shutdown the exchange manually to avoid leaking exchanges.
if (mpExchangeCtx != nullptr)
{
mpExchangeCtx->Close();
}
}
}

Close();
}

Expand Down Expand Up @@ -236,19 +248,7 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand
err = commandPath.GetCommandId(&concretePath.mCommandId);
SuccessOrExit(err);

if (mpExchangeCtx != nullptr && mpExchangeCtx->IsGroupExchangeContext())
{
// TODO retrieve Endpoint ID with GroupDataProvider using GroupId and FabricId
// Issue 11075

// Using endpoint 1 for test purposes
concretePath.mEndpointId = 1;
err = CHIP_NO_ERROR;
}
else
{
err = commandPath.GetEndpointId(&concretePath.mEndpointId);
}
err = commandPath.GetEndpointId(&concretePath.mEndpointId);
SuccessOrExit(err);

VerifyOrExit(mpCallback->CommandExists(concretePath), err = CHIP_ERROR_INVALID_PROFILE_ID);
Expand Down Expand Up @@ -312,6 +312,103 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand
return CHIP_NO_ERROR;
}

CHIP_ERROR CommandHandler::ProcessGroupCommandDataIB(CommandDataIB::Parser & aCommandElement)
{
CHIP_ERROR err = CHIP_NO_ERROR;
CommandPathIB::Parser commandPath;
TLV::TLVReader commandDataReader;
ClusterId clusterId;
CommandId commandId;
GroupId groupId;
FabricIndex fabric;

Credentials::GroupDataProvider::GroupEndpoint mapping;
Credentials::GroupDataProvider * groupDataProvider = Credentials::GetGroupDataProvider();
Credentials::GroupDataProvider::EndpointIterator * iterator;

err = aCommandElement.GetPath(&commandPath);
SuccessOrExit(err);

err = commandPath.GetClusterId(&clusterId);
SuccessOrExit(err);

err = commandPath.GetCommandId(&commandId);
SuccessOrExit(err);

groupId = mpExchangeCtx->GetSessionHandle()->AsGroupSession()->GetGroupId();
fabric = GetAccessingFabricIndex();

ChipLogDetail(DataManagement,
"Received group command for Group=%" PRIu16 " Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI, groupId,
ChipLogValueMEI(clusterId), ChipLogValueMEI(commandId));

err = aCommandElement.GetData(&commandDataReader);
if (CHIP_END_OF_TLV == err)
{
ChipLogDetail(DataManagement,
"Received command without data for Group=%" PRIu16 " Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI,
groupId, ChipLogValueMEI(clusterId), ChipLogValueMEI(commandId));
err = CHIP_NO_ERROR;
}
SuccessOrExit(err);

iterator = groupDataProvider->IterateEndpoints(fabric);
VerifyOrExit(iterator != nullptr, err = CHIP_ERROR_NO_MEMORY);

while (iterator->Next(mapping))
{
if (groupId != mapping.group_id)
{
continue;
}

ChipLogDetail(DataManagement,
"Processing group command for Endpoint=%" PRIu16 " Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI,
mapping.endpoint_id, ChipLogValueMEI(clusterId), ChipLogValueMEI(commandId));

const ConcreteCommandPath concretePath(mapping.endpoint_id, clusterId, commandId);

if (!mpCallback->CommandExists(concretePath))
{
ChipLogError(DataManagement, "No Cluster " ChipLogFormatMEI " on Endpoint 0x%" PRIx16, ChipLogValueMEI(clusterId),
mapping.endpoint_id);

continue;
}

{
Access::SubjectDescriptor subjectDescriptor = mpExchangeCtx->GetSessionHandle()->GetSubjectDescriptor();
Access::RequestPath requestPath{ .cluster = concretePath.mClusterId, .endpoint = concretePath.mEndpointId };
Access::Privilege requestPrivilege = Access::Privilege::kOperate; // TODO: get actual request privilege
err = Access::GetAccessControl().Check(subjectDescriptor, requestPath, requestPrivilege);
err = CHIP_NO_ERROR; // TODO: remove override
if (err != CHIP_NO_ERROR)
{
continue;
}
}

if ((err = MatterPreCommandReceivedCallback(concretePath)) == CHIP_NO_ERROR)
{
TLV::TLVReader dataReader(commandDataReader);
mpCallback->DispatchCommand(*this, concretePath, dataReader);
MatterPostCommandReceivedCallback(concretePath);
}
else
{
ChipLogError(DataManagement,
"Error when calling MatterPreCommandReceivedCallback for Endpoint=%" PRIu16 " Cluster=" ChipLogFormatMEI
" Command=" ChipLogFormatMEI " : %" CHIP_ERROR_FORMAT,
mapping.endpoint_id, ChipLogValueMEI(clusterId), ChipLogValueMEI(commandId), err.Format());
continue;
}
}
iterator->Release();

exit:
return CHIP_NO_ERROR;
}

CHIP_ERROR CommandHandler::AddStatusInternal(const ConcreteCommandPath & aCommandPath,
const Protocols::InteractionModel::Status aStatus,
const Optional<ClusterStatus> & aClusterStatus)
Expand Down
10 changes: 10 additions & 0 deletions src/app/CommandHandler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,17 @@ class CommandHandler
*/
void Close();

/**
* ProcessCommandDataIB is only called when a unicast invoke command request is received
* It requires the endpointId in its command path to be able to dispatch the command
*/
CHIP_ERROR ProcessCommandDataIB(CommandDataIB::Parser & aCommandElement);

/**
* ProcessGroupCommandDataIB is only called when a group invoke command request is received
* It doesn't need the endpointId in it's command path since it uses the GroupId in message metadata to find it
*/
CHIP_ERROR ProcessGroupCommandDataIB(CommandDataIB::Parser & aCommandElement);
CHIP_ERROR SendCommandResponse();
CHIP_ERROR AddStatusInternal(const ConcreteCommandPath & aCommandPath, const Protocols::InteractionModel::Status aStatus,
const Optional<ClusterStatus> & aClusterStatus);
Expand Down
Loading

0 comments on commit de95567

Please sign in to comment.