From ae762f4b3741d12b98e59a26fd53c68149103545 Mon Sep 17 00:00:00 2001 From: Marc Lepage Date: Tue, 7 Dec 2021 11:13:23 -0500 Subject: [PATCH 1/2] Clean up concrete command path Streamline ConcreteCommandPath handling in CommandHandler::ProcessCommandDataIB. --- src/app/CommandHandler.cpp | 40 ++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index 66324e08e9b9f3..1622920f588759 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -215,18 +215,18 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand { CHIP_ERROR err = CHIP_NO_ERROR; CommandPathIB::Parser commandPath; + ConcreteCommandPath concreteCommandPath(0, 0, 0); TLV::TLVReader commandDataReader; - ClusterId clusterId; - CommandId commandId; - EndpointId endpointId; + + // NOTE: errors may occur before the concrete command path is even fully decoded. err = aCommandElement.GetPath(&commandPath); SuccessOrExit(err); - err = commandPath.GetClusterId(&clusterId); + err = commandPath.GetClusterId(&concreteCommandPath.mClusterId); SuccessOrExit(err); - err = commandPath.GetCommandId(&commandId); + err = commandPath.GetCommandId(&concreteCommandPath.mCommandId); SuccessOrExit(err); if (mpExchangeCtx != nullptr && mpExchangeCtx->IsGroupExchangeContext()) @@ -235,17 +235,16 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand // Issue 11075 // Using endpoint 1 for test purposes - endpointId = 1; - err = CHIP_NO_ERROR; + concreteCommandPath.mEndpointId = 1; + err = CHIP_NO_ERROR; } else { - err = commandPath.GetEndpointId(&endpointId); + err = commandPath.GetEndpointId(&concreteCommandPath.mEndpointId); } SuccessOrExit(err); - VerifyOrExit(mpCallback->CommandExists(ConcreteCommandPath(endpointId, clusterId, commandId)), - err = CHIP_ERROR_INVALID_PROFILE_ID); + VerifyOrExit(mpCallback->CommandExists(concreteCommandPath), err = CHIP_ERROR_INVALID_PROFILE_ID); err = aCommandElement.GetData(&commandDataReader); if (CHIP_END_OF_TLV == err) @@ -253,37 +252,36 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand ChipLogDetail(DataManagement, "Received command without data for Endpoint=%" PRIu16 " Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI, - endpointId, ChipLogValueMEI(clusterId), ChipLogValueMEI(commandId)); + concreteCommandPath.mEndpointId, ChipLogValueMEI(concreteCommandPath.mClusterId), + ChipLogValueMEI(concreteCommandPath.mCommandId)); err = CHIP_NO_ERROR; } if (CHIP_NO_ERROR == err) { ChipLogDetail(DataManagement, "Received command for Endpoint=%" PRIu16 " Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI, - endpointId, ChipLogValueMEI(clusterId), ChipLogValueMEI(commandId)); - const ConcreteCommandPath concretePath(endpointId, clusterId, commandId); - SuccessOrExit(MatterPreCommandReceivedCallback(concretePath)); - mpCallback->DispatchCommand(*this, ConcreteCommandPath(endpointId, clusterId, commandId), commandDataReader); - MatterPostCommandReceivedCallback(concretePath); + concreteCommandPath.mEndpointId, ChipLogValueMEI(concreteCommandPath.mClusterId), + ChipLogValueMEI(concreteCommandPath.mCommandId)); + SuccessOrExit(MatterPreCommandReceivedCallback(concreteCommandPath)); + mpCallback->DispatchCommand(*this, concreteCommandPath, commandDataReader); + MatterPostCommandReceivedCallback(concreteCommandPath); } exit: if (err != CHIP_NO_ERROR) { - ConcreteCommandPath path(endpointId, clusterId, commandId); - // The Path is the path in the request if there are any error occurred before we dispatch the command to clusters. // Currently, it could be failed to decode Path or failed to find cluster / command on desired endpoint. // TODO: The behavior when receiving a malformed message is not clear in the Spec. (Spec#3259) // TODO: The error code should be updated after #7072 added error codes required by IM. if (err == CHIP_ERROR_INVALID_PROFILE_ID) { - ChipLogDetail(DataManagement, "No Cluster " ChipLogFormatMEI " on Endpoint 0x%" PRIx16, ChipLogValueMEI(clusterId), - endpointId); + ChipLogDetail(DataManagement, "No Cluster " ChipLogFormatMEI " on Endpoint 0x%" PRIx16, + ChipLogValueMEI(concreteCommandPath.mClusterId), concreteCommandPath.mEndpointId); } // TODO:in particular different reasons for ServerClusterCommandExists to test false should result in different errors here - AddStatus(path, Protocols::InteractionModel::Status::InvalidCommand); + AddStatus(concreteCommandPath, Protocols::InteractionModel::Status::InvalidCommand); } // We have handled the error status above and put the error status in response, now return success status so we can process From 47d97ada257ddc4b1890dc24a545c120a81e25b2 Mon Sep 17 00:00:00 2001 From: Marc Lepage Date: Tue, 7 Dec 2021 12:14:47 -0500 Subject: [PATCH 2/2] Shorten variable name --- src/app/CommandHandler.cpp | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index 1622920f588759..913969adec9e1a 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -215,7 +215,7 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand { CHIP_ERROR err = CHIP_NO_ERROR; CommandPathIB::Parser commandPath; - ConcreteCommandPath concreteCommandPath(0, 0, 0); + ConcreteCommandPath concretePath(0, 0, 0); TLV::TLVReader commandDataReader; // NOTE: errors may occur before the concrete command path is even fully decoded. @@ -223,10 +223,10 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand err = aCommandElement.GetPath(&commandPath); SuccessOrExit(err); - err = commandPath.GetClusterId(&concreteCommandPath.mClusterId); + err = commandPath.GetClusterId(&concretePath.mClusterId); SuccessOrExit(err); - err = commandPath.GetCommandId(&concreteCommandPath.mCommandId); + err = commandPath.GetCommandId(&concretePath.mCommandId); SuccessOrExit(err); if (mpExchangeCtx != nullptr && mpExchangeCtx->IsGroupExchangeContext()) @@ -235,16 +235,16 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand // Issue 11075 // Using endpoint 1 for test purposes - concreteCommandPath.mEndpointId = 1; - err = CHIP_NO_ERROR; + concretePath.mEndpointId = 1; + err = CHIP_NO_ERROR; } else { - err = commandPath.GetEndpointId(&concreteCommandPath.mEndpointId); + err = commandPath.GetEndpointId(&concretePath.mEndpointId); } SuccessOrExit(err); - VerifyOrExit(mpCallback->CommandExists(concreteCommandPath), err = CHIP_ERROR_INVALID_PROFILE_ID); + VerifyOrExit(mpCallback->CommandExists(concretePath), err = CHIP_ERROR_INVALID_PROFILE_ID); err = aCommandElement.GetData(&commandDataReader); if (CHIP_END_OF_TLV == err) @@ -252,19 +252,17 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand ChipLogDetail(DataManagement, "Received command without data for Endpoint=%" PRIu16 " Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI, - concreteCommandPath.mEndpointId, ChipLogValueMEI(concreteCommandPath.mClusterId), - ChipLogValueMEI(concreteCommandPath.mCommandId)); + concretePath.mEndpointId, ChipLogValueMEI(concretePath.mClusterId), ChipLogValueMEI(concretePath.mCommandId)); err = CHIP_NO_ERROR; } if (CHIP_NO_ERROR == err) { ChipLogDetail(DataManagement, "Received command for Endpoint=%" PRIu16 " Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI, - concreteCommandPath.mEndpointId, ChipLogValueMEI(concreteCommandPath.mClusterId), - ChipLogValueMEI(concreteCommandPath.mCommandId)); - SuccessOrExit(MatterPreCommandReceivedCallback(concreteCommandPath)); - mpCallback->DispatchCommand(*this, concreteCommandPath, commandDataReader); - MatterPostCommandReceivedCallback(concreteCommandPath); + concretePath.mEndpointId, ChipLogValueMEI(concretePath.mClusterId), ChipLogValueMEI(concretePath.mCommandId)); + SuccessOrExit(MatterPreCommandReceivedCallback(concretePath)); + mpCallback->DispatchCommand(*this, concretePath, commandDataReader); + MatterPostCommandReceivedCallback(concretePath); } exit: @@ -277,11 +275,11 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand if (err == CHIP_ERROR_INVALID_PROFILE_ID) { ChipLogDetail(DataManagement, "No Cluster " ChipLogFormatMEI " on Endpoint 0x%" PRIx16, - ChipLogValueMEI(concreteCommandPath.mClusterId), concreteCommandPath.mEndpointId); + ChipLogValueMEI(concretePath.mClusterId), concretePath.mEndpointId); } // TODO:in particular different reasons for ServerClusterCommandExists to test false should result in different errors here - AddStatus(concreteCommandPath, Protocols::InteractionModel::Status::InvalidCommand); + AddStatus(concretePath, Protocols::InteractionModel::Status::InvalidCommand); } // We have handled the error status above and put the error status in response, now return success status so we can process