From d925b47a6c5fa04c19313235bc524cbb4731a9fd Mon Sep 17 00:00:00 2001 From: yunhanw-google Date: Tue, 30 Mar 2021 15:12:21 -0700 Subject: [PATCH] Add CommandResponseStatus, CommandResponseProtocolError, and CommandResponseError Callback in IM delegate (#5477) Problems: Consumer SDK need to get to know the status code when invoke command response has status code inside, and also need to know error when sender fails to process command element and response timeout Summary of Changes: -- After receive status code from invoke command reponse, command sender notify this status code via CommandStatus callback to Consumer SDK. -- When response timeout happens, notify the timeout error to consumer sdk. -- When command element fails to process, notify the protocol error to consuder sdk --- src/app/Command.cpp | 10 +++-- src/app/Command.h | 6 ++- src/app/CommandSender.cpp | 41 ++++++++++++------ src/app/InteractionModelDelegate.h | 43 +++++++++++++++++++ src/app/InteractionModelEngine.cpp | 4 +- .../tests/integration/chip_im_initiator.cpp | 22 ++++++++++ 6 files changed, 108 insertions(+), 18 deletions(-) diff --git a/src/app/Command.cpp b/src/app/Command.cpp index f068ebbbd82dc6..d044c711e6c7f8 100644 --- a/src/app/Command.cpp +++ b/src/app/Command.cpp @@ -31,7 +31,7 @@ namespace chip { namespace app { -CHIP_ERROR Command::Init(Messaging::ExchangeManager * apExchangeMgr) +CHIP_ERROR Command::Init(Messaging::ExchangeManager * apExchangeMgr, InteractionModelDelegate * apDelegate) { CHIP_ERROR err = CHIP_NO_ERROR; // Error if already initialized. @@ -40,8 +40,8 @@ CHIP_ERROR Command::Init(Messaging::ExchangeManager * apExchangeMgr) VerifyOrExit(mpExchangeCtx == nullptr, err = CHIP_ERROR_INCORRECT_STATE); mpExchangeMgr = apExchangeMgr; - - err = Reset(); + mpDelegate = apDelegate; + err = Reset(); SuccessOrExit(err); exit: @@ -69,6 +69,8 @@ CHIP_ERROR Command::Reset() mInvokeCommandBuilder.CreateCommandListBuilder(); MoveToState(CommandState::Initialized); + mCommandIndex = 0; + exit: ChipLogFunctError(err); @@ -134,8 +136,10 @@ void Command::Shutdown() ClearExistingExchangeContext(); mpExchangeMgr = nullptr; + mpDelegate = nullptr; MoveToState(CommandState::Uninitialized); + mCommandIndex = 0; exit: return; } diff --git a/src/app/Command.h b/src/app/Command.h index caf05692e60438..5a92b7e0df2b50 100644 --- a/src/app/Command.h +++ b/src/app/Command.h @@ -36,6 +36,7 @@ #include #include +#include #include #include #include @@ -92,13 +93,14 @@ class Command * instance. * * @param[in] apExchangeMgr A pointer to the ExchangeManager object. + * @param[in] apDelegate InteractionModelDelegate set by application. * * @retval #CHIP_ERROR_INCORRECT_STATE If the state is not equal to * CommandState::NotInitialized. * @retval #CHIP_NO_ERROR On success. * */ - CHIP_ERROR Init(Messaging::ExchangeManager * apExchangeMgr); + CHIP_ERROR Init(Messaging::ExchangeManager * apExchangeMgr, InteractionModelDelegate * apDelegate); /** * Shutdown the CommandSender. This terminates this instance @@ -147,7 +149,9 @@ class Command Messaging::ExchangeManager * mpExchangeMgr = nullptr; Messaging::ExchangeContext * mpExchangeCtx = nullptr; + InteractionModelDelegate * mpDelegate = nullptr; chip::System::PacketBufferHandle mCommandMessageBuf; + uint8_t mCommandIndex = 0; private: chip::System::PacketBufferHandle mpBufHandle; diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp index b19401f9f24cfe..87d26c8852bad8 100644 --- a/src/app/CommandSender.cpp +++ b/src/app/CommandSender.cpp @@ -103,6 +103,11 @@ void CommandSender::OnResponseTimeout(Messaging::ExchangeContext * apExchangeCon ChipLogProgress(DataManagement, "Time out! failed to receive invoke command response from Exchange: %d", apExchangeContext->GetExchangeId()); Reset(); + + if (mpDelegate != nullptr) + { + mpDelegate->CommandResponseTimeout(this); + } } CHIP_ERROR CommandSender::ProcessCommandDataElement(CommandDataElement::Parser & aCommandElement) @@ -118,6 +123,20 @@ CHIP_ERROR CommandSender::ProcessCommandDataElement(CommandDataElement::Parser & uint16_t protocolCode = 0; StatusElement::Parser statusElementParser; + mCommandIndex++; + + err = aCommandElement.GetCommandPath(&commandPath); + SuccessOrExit(err); + + err = commandPath.GetClusterId(&clusterId); + SuccessOrExit(err); + + err = commandPath.GetCommandId(&commandId); + SuccessOrExit(err); + + err = commandPath.GetEndpointId(&endpointId); + SuccessOrExit(err); + err = aCommandElement.GetStatusElement(&statusElementParser); if (CHIP_NO_ERROR == err) { @@ -127,21 +146,14 @@ CHIP_ERROR CommandSender::ProcessCommandDataElement(CommandDataElement::Parser & err = statusElementParser.DecodeStatusElement(&generalCode, &protocolId, &protocolCode, &clusterId); SuccessOrExit(err); + if (mpDelegate != nullptr) + { + mpDelegate->CommandResponseStatus(this, generalCode, protocolId, protocolCode, endpointId, clusterId, commandId, + mCommandIndex); + } } else if (CHIP_END_OF_TLV == err) { - err = aCommandElement.GetCommandPath(&commandPath); - SuccessOrExit(err); - - err = commandPath.GetClusterId(&clusterId); - SuccessOrExit(err); - - err = commandPath.GetCommandId(&commandId); - SuccessOrExit(err); - - err = commandPath.GetEndpointId(&endpointId); - SuccessOrExit(err); - err = aCommandElement.GetData(&commandDataReader); if (CHIP_END_OF_TLV == err) { @@ -155,6 +167,11 @@ CHIP_ERROR CommandSender::ProcessCommandDataElement(CommandDataElement::Parser & } exit: + ChipLogFunctError(err); + if (err != CHIP_NO_ERROR && mpDelegate != nullptr) + { + mpDelegate->CommandResponseProtocolError(this, mCommandIndex); + } return err; } diff --git a/src/app/InteractionModelDelegate.h b/src/app/InteractionModelDelegate.h index da2af3db1eef91..2e2a48d90aa8b2 100644 --- a/src/app/InteractionModelDelegate.h +++ b/src/app/InteractionModelDelegate.h @@ -30,6 +30,7 @@ namespace chip { namespace app { class ReadClient; +class CommandSender; struct EventPathParams; /** @@ -75,6 +76,48 @@ class InteractionModelDelegate */ virtual CHIP_ERROR ReportError(const ReadClient * apReadClient, CHIP_ERROR aError) { return CHIP_ERROR_NOT_IMPLEMENTED; } + /** + * Notification that a Command Send has received an Invoke Command Response containing a status code. + * @param[in] apCommandSender A current command sender which can identify the command sender to the consumer, particularly + * during multiple command interactions + * @param[in] aGeneralCode Status code defined by the standard + * @param[in] aProtocolId Protocol Id + * @param[in] aProtocolCode Detailed error information, protocol-specific. + * @param[in] aEndpointId Endpoint identifier + * @param[in] aClusterId Cluster identifier + * @param[in] aCommandId Command identifier + * @param[in] aCommandIndex Current processing command index which can identify command if there exists multiple commands with + * same command Id + * @retval # CHIP_ERROR_NOT_IMPLEMENTED if not implemented + */ + virtual CHIP_ERROR CommandResponseStatus(const CommandSender * apCommandSender, const uint16_t aGeneralCode, + const uint32_t aProtocolId, const uint16_t aProtocolCode, chip::EndpointId aEndpointId, + const chip::ClusterId aClusterId, chip::CommandId aCommandId, uint8_t aCommandIndex) + { + return CHIP_ERROR_NOT_IMPLEMENTED; + } + + /** + * Notification that a Command Send has received an Invoke Command Response and fails to process a command data element in that + * command response + * @param[in] apCommandSender A current command sender which can identify the command sender to the consumer, particularly + * during multiple command interactions + * @param[in] aCommandIndex Current processing command index which can identify failed command + * @retval # CHIP_ERROR_NOT_IMPLEMENTED if not implemented + */ + virtual CHIP_ERROR CommandResponseProtocolError(const CommandSender * apCommandSender, uint8_t aCommandIndex) + { + return CHIP_ERROR_NOT_IMPLEMENTED; + } + + /** + * Notification that a command sender encountered an asynchronous failure. + * @param[in] apCommandSender A current command sender which can identify the command sender to the consumer, particularly + * during multiple command interactions + * @retval # CHIP_ERROR_NOT_IMPLEMENTED if not implemented + */ + virtual CHIP_ERROR CommandResponseTimeout(const CommandSender * apCommandSender) { return CHIP_ERROR_NOT_IMPLEMENTED; } + virtual ~InteractionModelDelegate() = default; }; diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 3226c8dc554adb..7fbb790fefe759 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -87,7 +87,7 @@ CHIP_ERROR InteractionModelEngine::NewCommandSender(CommandSender ** const apCom if (commandSender.IsFree()) { *apCommandSender = &commandSender; - err = commandSender.Init(mpExchangeMgr); + err = commandSender.Init(mpExchangeMgr, mpDelegate); if (CHIP_NO_ERROR != err) { *apCommandSender = nullptr; @@ -155,7 +155,7 @@ void InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext * { if (commandHandler.IsFree()) { - err = commandHandler.Init(mpExchangeMgr); + err = commandHandler.Init(mpExchangeMgr, mpDelegate); SuccessOrExit(err); commandHandler.OnMessageReceived(apExchangeContext, aPacketHeader, aPayloadHeader, std::move(aPayload)); apExchangeContext = nullptr; diff --git a/src/app/tests/integration/chip_im_initiator.cpp b/src/app/tests/integration/chip_im_initiator.cpp index 59e59737799b4b..7f88ed30f46bf6 100644 --- a/src/app/tests/integration/chip_im_initiator.cpp +++ b/src/app/tests/integration/chip_im_initiator.cpp @@ -229,6 +229,28 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate printf("ReportError with err %d", aError); return CHIP_NO_ERROR; } + CHIP_ERROR CommandResponseStatus(const chip::app::CommandSender * apCommandSender, const uint16_t aGeneralCode, + const uint32_t aProtocolId, const uint16_t aProtocolCode, const chip::EndpointId aEndpointId, + const chip::ClusterId aClusterId, const chip::CommandId aCommandId, + uint8_t aCommandIndex) override + { + printf("CommandResponseStatus with GeneralCode %d, ProtocolId %d, ProtocolCode %d, EndpointId %d, ClusterId %d, CommandId " + "%d, CommandIndex %d", + aGeneralCode, aProtocolId, aProtocolCode, aEndpointId, aClusterId, aCommandId, aCommandIndex); + return CHIP_NO_ERROR; + } + + CHIP_ERROR CommandResponseProtocolError(const chip::app::CommandSender * apCommandSender, uint8_t aCommandIndex) override + { + printf("CommandResponseProtocolError happens with CommandIndex %d", aCommandIndex); + return CHIP_NO_ERROR; + } + + CHIP_ERROR CommandResponseTimeout(const chip::app::CommandSender * apCommandSender) override + { + printf("CommandResponseTimeout happens"); + return CHIP_NO_ERROR; + } }; } // namespace