diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 56b412b2655bfe..068ec5b577bca0 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -280,11 +280,10 @@ CHIP_ERROR InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeCon return CHIP_NO_ERROR; } -CHIP_ERROR InteractionModelEngine::OnReadInitialRequest(Messaging::ExchangeContext * apExchangeContext, - const PayloadHeader & aPayloadHeader, - System::PacketBufferHandle && aPayload, - ReadHandler::InteractionType aInteractionType, - Protocols::InteractionModel::Status & aStatus) +Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest(Messaging::ExchangeContext * apExchangeContext, + const PayloadHeader & aPayloadHeader, + System::PacketBufferHandle && aPayload, + ReadHandler::InteractionType aInteractionType) { ChipLogDetail(InteractionModel, "Received %s request", aInteractionType == ReadHandler::InteractionType::Subscribe ? "Subscribe" : "Read"); @@ -301,55 +300,68 @@ CHIP_ERROR InteractionModelEngine::OnReadInitialRequest(Messaging::ExchangeConte if (apExchangeContext->GetSessionHandle()->GetFabricIndex() == kUndefinedFabricIndex) { // Subscriptions must be associated to a fabric. - aStatus = Protocols::InteractionModel::Status::UnsupportedAccess; - return CHIP_NO_ERROR; + return Status::UnsupportedAccess; } reader.Init(aPayload.Retain()); SubscribeRequestMessage::Parser subscribeRequestParser; - ReturnErrorOnFailure(subscribeRequestParser.Init(reader)); + CHIP_ERROR err = subscribeRequestParser.Init(reader); + if (err != CHIP_NO_ERROR) + { + return Status::InvalidAction; + } { size_t requestedAttributePathCount = 0; size_t requestedEventPathCount = 0; AttributePathIBs::Parser attributePathListParser; - CHIP_ERROR err = subscribeRequestParser.GetAttributeRequests(&attributePathListParser); + err = subscribeRequestParser.GetAttributeRequests(&attributePathListParser); if (err == CHIP_NO_ERROR) { TLV::TLVReader pathReader; attributePathListParser.GetReader(&pathReader); - TLV::Utilities::Count(pathReader, requestedAttributePathCount, false); + err = TLV::Utilities::Count(pathReader, requestedAttributePathCount, false); } - else if (err != CHIP_ERROR_END_OF_TLV) + else if (err == CHIP_ERROR_END_OF_TLV) { - aStatus = Protocols::InteractionModel::Status::InvalidAction; - return CHIP_NO_ERROR; + err = CHIP_NO_ERROR; } + if (err != CHIP_NO_ERROR) + { + return Status::InvalidAction; + } + EventPathIBs::Parser eventpathListParser; err = subscribeRequestParser.GetEventRequests(&eventpathListParser); if (err == CHIP_NO_ERROR) { TLV::TLVReader pathReader; eventpathListParser.GetReader(&pathReader); - TLV::Utilities::Count(pathReader, requestedEventPathCount, false); + err = TLV::Utilities::Count(pathReader, requestedEventPathCount, false); } - else if (err != CHIP_ERROR_END_OF_TLV) + else if (err == CHIP_ERROR_END_OF_TLV) { - aStatus = Protocols::InteractionModel::Status::InvalidAction; - return CHIP_NO_ERROR; + err = CHIP_NO_ERROR; + } + if (err != CHIP_NO_ERROR) + { + return Status::InvalidAction; } // The following cast is safe, since we can only hold a few tens of paths in one request. if (!EnsureResourceForSubscription(apExchangeContext->GetSessionHandle()->GetFabricIndex(), requestedAttributePathCount, requestedEventPathCount)) { - aStatus = Protocols::InteractionModel::Status::PathsExhausted; - return CHIP_NO_ERROR; + return Status::PathsExhausted; } } - ReturnErrorOnFailure(subscribeRequestParser.GetKeepSubscriptions(&keepExistingSubscriptions)); + err = subscribeRequestParser.GetKeepSubscriptions(&keepExistingSubscriptions); + if (err != CHIP_NO_ERROR) + { + return Status::InvalidAction; + } if (!keepExistingSubscriptions) { @@ -381,33 +393,29 @@ CHIP_ERROR InteractionModelEngine::OnReadInitialRequest(Messaging::ExchangeConte if ((handlerPoolCapacity - GetNumActiveReadHandlers()) == 0) { - aStatus = Protocols::InteractionModel::Status::ResourceExhausted; - return CHIP_NO_ERROR; + return Status::ResourceExhausted; } #endif // We have already reserved enough resources for read requests, and have granted enough resources for current subscriptions, so // we should be able to allocate resources requested by this request. ReadHandler * handler = mReadHandlers.CreateObject(*this, apExchangeContext, aInteractionType); - if (handler) + if (handler == nullptr) { - CHIP_ERROR err = handler->OnInitialRequest(std::move(aPayload)); - if (err == CHIP_ERROR_NO_MEMORY) - { - aStatus = Protocols::InteractionModel::Status::ResourceExhausted; - } - else - { - aStatus = StatusIB(err).mStatus; - } - return err; + ChipLogProgress(InteractionModel, "no resource for %s interaction", + aInteractionType == ReadHandler::InteractionType::Subscribe ? "Subscribe" : "Read"); + return Status::ResourceExhausted; } - ChipLogProgress(InteractionModel, "no resource for %s interaction", - aInteractionType == ReadHandler::InteractionType::Subscribe ? "Subscribe" : "Read"); - aStatus = Protocols::InteractionModel::Status::ResourceExhausted; + CHIP_ERROR err = handler->OnInitialRequest(std::move(aPayload)); + if (err == CHIP_ERROR_NO_MEMORY) + { + return Status::ResourceExhausted; + } - return CHIP_NO_ERROR; + // TODO: Should probably map various TLV errors into InvalidAction, here + // or inside the read handler. + return StatusIB(err).mStatus; } Protocols::InteractionModel::Status InteractionModelEngine::OnWriteRequest(Messaging::ExchangeContext * apExchangeContext, @@ -494,7 +502,6 @@ CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext { using namespace Protocols::InteractionModel; - CHIP_ERROR err = CHIP_NO_ERROR; Protocols::InteractionModel::Status status = Protocols::InteractionModel::Status::Failure; // Group Message can only be an InvokeCommandRequest or WriteRequest @@ -503,18 +510,16 @@ CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext !aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::WriteRequest)) { ChipLogProgress(InteractionModel, "Msg type %d not supported for group message", aPayloadHeader.GetMessageType()); - return err; + return CHIP_NO_ERROR; } if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::InvokeCommandRequest)) { - SuccessOrExit( - OnInvokeCommandRequest(apExchangeContext, aPayloadHeader, std::move(aPayload), /* aIsTimedInvoke = */ false, status)); + OnInvokeCommandRequest(apExchangeContext, aPayloadHeader, std::move(aPayload), /* aIsTimedInvoke = */ false, status); } else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::ReadRequest)) { - SuccessOrExit(OnReadInitialRequest(apExchangeContext, aPayloadHeader, std::move(aPayload), - ReadHandler::InteractionType::Read, status)); + status = OnReadInitialRequest(apExchangeContext, aPayloadHeader, std::move(aPayload), ReadHandler::InteractionType::Read); } else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::WriteRequest)) { @@ -522,8 +527,8 @@ CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext } else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::SubscribeRequest)) { - SuccessOrExit(OnReadInitialRequest(apExchangeContext, aPayloadHeader, std::move(aPayload), - ReadHandler::InteractionType::Subscribe, status)); + status = + OnReadInitialRequest(apExchangeContext, aPayloadHeader, std::move(aPayload), ReadHandler::InteractionType::Subscribe); } else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::ReportData)) { @@ -532,19 +537,19 @@ CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext } else if (aPayloadHeader.HasMessageType(MsgType::TimedRequest)) { - SuccessOrExit(OnTimedRequest(apExchangeContext, aPayloadHeader, std::move(aPayload), status)); + OnTimedRequest(apExchangeContext, aPayloadHeader, std::move(aPayload), status); } else { ChipLogProgress(InteractionModel, "Msg type %d not supported", aPayloadHeader.GetMessageType()); } -exit: if (status != Protocols::InteractionModel::Status::Success && !apExchangeContext->IsGroupExchangeContext()) { - err = StatusResponse::Send(status, apExchangeContext, false /*aExpectResponse*/); + return StatusResponse::Send(status, apExchangeContext, false /*aExpectResponse*/); } - return err; + + return CHIP_NO_ERROR; } void InteractionModelEngine::OnResponseTimeout(Messaging::ExchangeContext * ec) diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index 59966967bf5c40..9dd82052b6eed9 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -377,13 +377,12 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, /** * Called when Interaction Model receives a Read Request message. Errors processing - * the Read Request are handled entirely within this function. The caller pre-sets status to failure and the callee is - * expected to set it to success if it does not want an automatic status response message to be sent. + * the Read Request are handled entirely within this function. If the + * status returned is not Status::Success, the caller will send a status + * response message with that status. */ - - CHIP_ERROR OnReadInitialRequest(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, - System::PacketBufferHandle && aPayload, ReadHandler::InteractionType aInteractionType, - Protocols::InteractionModel::Status & aStatus); + Status OnReadInitialRequest(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader, + System::PacketBufferHandle && aPayload, ReadHandler::InteractionType aInteractionType); /** * Called when Interaction Model receives a Write Request message. Errors processing