diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp index 803204dadaebcb..7e074df054ea6a 100644 --- a/src/app/CommandSender.cpp +++ b/src/app/CommandSender.cpp @@ -120,6 +120,8 @@ CHIP_ERROR CommandSender::SendInvokeRequest() CHIP_ERROR CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) { + using namespace Protocols::InteractionModel; + if (mState == State::CommandSent) { MoveToState(State::ResponseReceived); @@ -130,21 +132,27 @@ CHIP_ERROR CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExcha if (mState == State::AwaitingTimedStatus) { - err = HandleTimedStatus(aPayloadHeader, std::move(aPayload)); + VerifyOrExit(aPayloadHeader.HasMessageType(MsgType::StatusResponse), err = CHIP_ERROR_INVALID_MESSAGE_TYPE); + CHIP_ERROR statusError = CHIP_NO_ERROR; + SuccessOrExit(err = StatusResponse::ProcessStatusResponse(std::move(aPayload), statusError)); + SuccessOrExit(err = statusError); + err = SendInvokeRequest(); // Skip all other processing here (which is for the response to the // invoke request), no matter whether err is success or not. goto exit; } - if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::InvokeCommandResponse)) + if (aPayloadHeader.HasMessageType(MsgType::InvokeCommandResponse)) { err = ProcessInvokeResponse(std::move(aPayload)); SuccessOrExit(err); } - else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::StatusResponse)) + else if (aPayloadHeader.HasMessageType(MsgType::StatusResponse)) { - err = StatusResponse::ProcessStatusResponse(std::move(aPayload)); - SuccessOrExit(err); + CHIP_ERROR statusError = CHIP_NO_ERROR; + SuccessOrExit(err = StatusResponse::ProcessStatusResponse(std::move(aPayload), statusError)); + SuccessOrExit(err = statusError); + err = CHIP_ERROR_INVALID_MESSAGE_TYPE; } else { @@ -369,13 +377,6 @@ TLV::TLVWriter * CommandSender::GetCommandDataIBTLVWriter() return mInvokeRequestBuilder.GetInvokeRequests().GetCommandData().GetWriter(); } -CHIP_ERROR CommandSender::HandleTimedStatus(const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) -{ - ReturnErrorOnFailure(TimedRequest::HandleResponse(aPayloadHeader, std::move(aPayload))); - - return SendInvokeRequest(); -} - CHIP_ERROR CommandSender::FinishCommand(const Optional & aTimedInvokeTimeoutMs) { ReturnErrorOnFailure(FinishCommand(/* aEndDataStruct = */ false)); diff --git a/src/app/CommandSender.h b/src/app/CommandSender.h index e0ad257d5722fb..ab02fe9529517c 100644 --- a/src/app/CommandSender.h +++ b/src/app/CommandSender.h @@ -259,14 +259,6 @@ class CommandSender final : public Messaging::ExchangeDelegate CHIP_ERROR ProcessInvokeResponse(System::PacketBufferHandle && payload); CHIP_ERROR ProcessInvokeResponseIB(InvokeResponseIB::Parser & aInvokeResponse); - // Handle a message received when we are expecting a status response to a - // Timed Request. The caller is assumed to have already checked that our - // exchange context member is the one the message came in on. - // - // If the server returned an error status, that will be returned as an error - // value of CHIP_ERROR. - CHIP_ERROR HandleTimedStatus(const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload); - // Send our queued-up Invoke Request message. Assumes the exchange is ready // and mPendingInvokeData is populated. CHIP_ERROR SendInvokeRequest(); diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 8d581db39bf319..9c6df9be9dcbb2 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -424,8 +424,10 @@ CHIP_ERROR ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchange else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::StatusResponse)) { VerifyOrExit(apExchangeContext == mExchange.Get(), err = CHIP_ERROR_INCORRECT_STATE); - err = StatusResponse::ProcessStatusResponse(std::move(aPayload)); - SuccessOrExit(err); + CHIP_ERROR statusError = CHIP_NO_ERROR; + SuccessOrExit(err = StatusResponse::ProcessStatusResponse(std::move(aPayload), statusError)); + SuccessOrExit(err = statusError); + err = CHIP_ERROR_INVALID_MESSAGE_TYPE; } else { diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index e40d8bf007213f..ff04308c06f269 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -116,9 +116,10 @@ CHIP_ERROR ReadHandler::OnInitialRequest(System::PacketBufferHandle && aPayload) CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload) { - CHIP_ERROR err = CHIP_NO_ERROR; - err = StatusResponse::ProcessStatusResponse(std::move(aPayload)); - SuccessOrExit(err); + CHIP_ERROR err = CHIP_NO_ERROR; + CHIP_ERROR statusError = CHIP_NO_ERROR; + SuccessOrExit(err = StatusResponse::ProcessStatusResponse(std::move(aPayload), statusError)); + SuccessOrExit(err = statusError); switch (mState) { case HandlerState::AwaitingReportResponse: diff --git a/src/app/StatusResponse.cpp b/src/app/StatusResponse.cpp index e9e8af70a5f656..c6ed1cbfaf57b4 100644 --- a/src/app/StatusResponse.cpp +++ b/src/app/StatusResponse.cpp @@ -44,7 +44,7 @@ CHIP_ERROR StatusResponse::Send(Protocols::InteractionModel::Status aStatus, Mes return CHIP_NO_ERROR; } -CHIP_ERROR StatusResponse::ProcessStatusResponse(System::PacketBufferHandle && aPayload) +CHIP_ERROR StatusResponse::ProcessStatusResponse(System::PacketBufferHandle && aPayload, CHIP_ERROR & aStatusError) { StatusResponseMessage::Parser response; System::PacketBufferTLVReader reader; @@ -59,12 +59,8 @@ CHIP_ERROR StatusResponse::ProcessStatusResponse(System::PacketBufferHandle && a ChipLogValueIMStatus(status.mStatus)); ReturnErrorOnFailure(response.ExitContainer()); - if (status.mStatus == Protocols::InteractionModel::Status::Success) - { - return CHIP_NO_ERROR; - } - - return status.ToChipError(); + aStatusError = status.ToChipError(); + return CHIP_NO_ERROR; } } // namespace app } // namespace chip diff --git a/src/app/StatusResponse.h b/src/app/StatusResponse.h index 583cbb3dae8120..7021e3868bcb5a 100644 --- a/src/app/StatusResponse.h +++ b/src/app/StatusResponse.h @@ -32,7 +32,10 @@ class StatusResponse public: static CHIP_ERROR Send(Protocols::InteractionModel::Status aStatus, Messaging::ExchangeContext * apExchangeContext, bool aExpectResponse); - static CHIP_ERROR ProcessStatusResponse(System::PacketBufferHandle && aPayload); + + // The return value indicates whether the StatusResponse was parsed properly, and if it is CHIP_NO_ERROR + // then aStatus has been set to the actual status, which might be success or failure. + static CHIP_ERROR ProcessStatusResponse(System::PacketBufferHandle && aPayload, CHIP_ERROR & aStatus); }; } // namespace app } // namespace chip diff --git a/src/app/TimedRequest.cpp b/src/app/TimedRequest.cpp index 1c325b62a19187..59921ce17cbc11 100644 --- a/src/app/TimedRequest.cpp +++ b/src/app/TimedRequest.cpp @@ -53,12 +53,5 @@ CHIP_ERROR TimedRequest::Send(ExchangeContext * aExchangeContext, uint16_t aTime return aExchangeContext->SendMessage(MsgType::TimedRequest, std::move(payload), SendMessageFlags::kExpectResponse); } -CHIP_ERROR TimedRequest::HandleResponse(const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) -{ - VerifyOrReturnError(aPayloadHeader.HasMessageType(MsgType::StatusResponse), CHIP_ERROR_INVALID_MESSAGE_TYPE); - - return StatusResponse::ProcessStatusResponse(std::move(aPayload)); -} - } // namespace app } // namespace chip diff --git a/src/app/WriteClient.cpp b/src/app/WriteClient.cpp index a56f1d4960f9cf..85af809681aedc 100644 --- a/src/app/WriteClient.cpp +++ b/src/app/WriteClient.cpp @@ -414,6 +414,8 @@ CHIP_ERROR WriteClient::SendWriteRequest() CHIP_ERROR WriteClient::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) { + using namespace Protocols::InteractionModel; + if (mState == State::AwaitingResponse && // We had sent the last chunk of data, and received all responses mChunks.IsNull()) @@ -431,13 +433,18 @@ CHIP_ERROR WriteClient::OnMessageReceived(Messaging::ExchangeContext * apExchang if (mState == State::AwaitingTimedStatus) { - err = HandleTimedStatus(aPayloadHeader, std::move(aPayload)); + VerifyOrExit(aPayloadHeader.HasMessageType(MsgType::StatusResponse), err = CHIP_ERROR_INVALID_MESSAGE_TYPE); + CHIP_ERROR statusError = CHIP_NO_ERROR; + SuccessOrExit(err = StatusResponse::ProcessStatusResponse(std::move(aPayload), statusError)); + SuccessOrExit(err = statusError); + err = SendWriteRequest(); + // Skip all other processing here (which is for the response to the // write request), no matter whether err is success or not. goto exit; } - if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::WriteResponse)) + if (aPayloadHeader.HasMessageType(MsgType::WriteResponse)) { err = ProcessWriteResponseMessage(std::move(aPayload)); SuccessOrExit(err); @@ -447,10 +454,12 @@ CHIP_ERROR WriteClient::OnMessageReceived(Messaging::ExchangeContext * apExchang SuccessOrExit(SendWriteRequest()); } } - else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::StatusResponse)) + else if (aPayloadHeader.HasMessageType(MsgType::StatusResponse)) { - err = StatusResponse::ProcessStatusResponse(std::move(aPayload)); - SuccessOrExit(err); + CHIP_ERROR statusError = CHIP_NO_ERROR; + SuccessOrExit(err = StatusResponse::ProcessStatusResponse(std::move(aPayload), statusError)); + SuccessOrExit(err = statusError); + err = CHIP_ERROR_INVALID_MESSAGE_TYPE; } else { @@ -522,12 +531,5 @@ CHIP_ERROR WriteClient::ProcessAttributeStatusIB(AttributeStatusIB::Parser & aAt return err; } -CHIP_ERROR WriteClient::HandleTimedStatus(const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload) -{ - ReturnErrorOnFailure(TimedRequest::HandleResponse(aPayloadHeader, std::move(aPayload))); - - return SendWriteRequest(); -} - } // namespace app } // namespace chip diff --git a/src/app/WriteClient.h b/src/app/WriteClient.h index 395809dc6cb750..3afe6ff9179ff6 100644 --- a/src/app/WriteClient.h +++ b/src/app/WriteClient.h @@ -340,15 +340,6 @@ class WriteClient : public Messaging::ExchangeDelegate */ void Abort(); - // Handle a message received when we are expecting a status response to a - // Timed Request. The caller is assumed to have already checked that our - // exchange context member is the one the message came in on. - // - // If the server returned an error status response its status will be - // encapsulated in the CHIP_ERROR this returns. In that case, - // StatusIB::InitFromChipError can be used to extract the status. - CHIP_ERROR HandleTimedStatus(const PayloadHeader & aPayloadHeader, System::PacketBufferHandle && aPayload); - // Send our queued-up Write Request message. Assumes the exchange is ready // and mPendingWriteData is populated. CHIP_ERROR SendWriteRequest(); diff --git a/src/app/tests/TestTimedHandler.cpp b/src/app/tests/TestTimedHandler.cpp index e460d10d4a4dfe..6c081e27428a94 100644 --- a/src/app/tests/TestTimedHandler.cpp +++ b/src/app/tests/TestTimedHandler.cpp @@ -68,7 +68,12 @@ class TestExchangeDelegate : public Messaging::ExchangeDelegate mLastMessageWasStatus = aPayloadHeader.HasMessageType(MsgType::StatusResponse); if (mLastMessageWasStatus) { - mError = StatusResponse::ProcessStatusResponse(std::move(aPayload)); + CHIP_ERROR statusError = CHIP_NO_ERROR; + mError = StatusResponse::ProcessStatusResponse(std::move(aPayload), statusError); + if (mError == CHIP_NO_ERROR) + { + mError = statusError; + } } if (mKeepExchangeOpen) {