Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix status report handling in read handler #21374

Merged
merged 8 commits into from
Aug 8, 2022
Merged

Fix status report handling in read handler #21374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
adf766c
Fix status report handling in read handler
yunhanw-google Jul 28, 2022
2f6d072
address comments
yunhanw-google Aug 5, 2022
63c4a60
Update src/app/tests/TestReadInteraction.cpp
yunhanw-google Aug 6, 2022
113aac1
Update src/app/tests/TestReadInteraction.cpp
yunhanw-google Aug 6, 2022
0473c1d
Update src/app/tests/TestReadInteraction.cpp
yunhanw-google Aug 6, 2022
c098a67
Update src/app/tests/TestReadInteraction.cpp
yunhanw-google Aug 6, 2022
ab11812
Fix
yunhanw-google Aug 8, 2022
4962d87
address comments
yunhanw-google Aug 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions src/app/InteractionModelEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest
reader.Init(aPayload.Retain());

ReadRequestMessage::Parser readRequestParser;
VerifyOrReturnError(readRequestParser.Init(reader) == CHIP_NO_ERROR, Status::Failure);
VerifyOrReturnError(readRequestParser.Init(reader) == CHIP_NO_ERROR, Status::InvalidAction);

{
size_t requestedAttributePathCount = 0;
Expand Down Expand Up @@ -458,15 +458,9 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest
return Status::ResourceExhausted;
}

CHIP_ERROR err = handler->OnInitialRequest(std::move(aPayload));
if (err == CHIP_ERROR_NO_MEMORY)
{
return Status::ResourceExhausted;
}
handler->OnInitialRequest(std::move(aPayload));

// TODO: Should probably map various TLV errors into InvalidAction, here
// or inside the read handler.
return StatusIB(err).mStatus;
return Status::Success;
}

Protocols::InteractionModel::Status InteractionModelEngine::OnWriteRequest(Messaging::ExchangeContext * apExchangeContext,
Expand Down
58 changes: 29 additions & 29 deletions src/app/ReadHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

namespace chip {
namespace app {
using Status = Protocols::InteractionModel::Status;

ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeContext * apExchangeContext,
InteractionType aInteractionType) :
Expand Down Expand Up @@ -87,7 +88,7 @@ void ReadHandler::Close()
mManagementCallback.OnDone(*this);
}

CHIP_ERROR ReadHandler::OnInitialRequest(System::PacketBufferHandle && aPayload)
void ReadHandler::OnInitialRequest(System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;
System::PacketBufferHandle response;
Expand All @@ -103,22 +104,30 @@ CHIP_ERROR ReadHandler::OnInitialRequest(System::PacketBufferHandle && aPayload)

if (err != CHIP_NO_ERROR)
{
Status status = Status::InvalidAction;
if (err.IsIMStatus())
{
status = StatusIB(err).mStatus;
}
StatusResponse::Send(status, mExchangeCtx.Get(), /* aExpectResponse = */ false);
Close();
}
else
{
// Force us to be in a dirty state so we get processed by the reporting
mFlags.Set(ReadHandlerFlags::ForceDirty);
}

return err;
}

CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload)
CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload,
bool & aSendStatusResponse)
{
CHIP_ERROR err = CHIP_NO_ERROR;
aSendStatusResponse = true;
CHIP_ERROR statusError = CHIP_NO_ERROR;
SuccessOrExit(err = StatusResponse::ProcessStatusResponse(std::move(aPayload), statusError));
// Since this is a valid Status Response message, we don't have to send a Status Response in reply to it.
aSendStatusResponse = false;
SuccessOrExit(err = statusError);
switch (mState)
{
Expand Down Expand Up @@ -162,11 +171,6 @@ CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchange
}

exit:
if (err != CHIP_NO_ERROR)
{
Close();
}

return err;
}

Expand Down Expand Up @@ -249,15 +253,26 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b
CHIP_ERROR ReadHandler::OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,
System::PacketBufferHandle && aPayload)
{
CHIP_ERROR err = CHIP_NO_ERROR;

CHIP_ERROR err = CHIP_NO_ERROR;
bool sendStatusResponse = true;
if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::StatusResponse))
{
err = OnStatusResponse(apExchangeContext, std::move(aPayload));
err = OnStatusResponse(apExchangeContext, std::move(aPayload), sendStatusResponse);
}
else
{
err = OnUnknownMsgType(apExchangeContext, aPayloadHeader, std::move(aPayload));
ChipLogDetail(DataManagement, "ReadHandler:: Msg type %d not supported", aPayloadHeader.GetMessageType());
err = CHIP_ERROR_INVALID_MESSAGE_TYPE;
}

if (sendStatusResponse)
{
StatusResponse::Send(Status::InvalidAction, apExchangeContext, false /*aExpectResponse*/);
}

if (err != CHIP_NO_ERROR)
{
Close();
}
return err;
}
Expand All @@ -269,14 +284,6 @@ bool ReadHandler::IsFromSubscriber(Messaging::ExchangeContext & apExchangeContex
GetAccessingFabricIndex() == apExchangeContext.GetSessionHandle()->GetFabricIndex());
}

CHIP_ERROR ReadHandler::OnUnknownMsgType(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,
System::PacketBufferHandle && aPayload)
{
ChipLogDetail(DataManagement, "Msg type %d not supported", aPayloadHeader.GetMessageType());
Close();
return CHIP_ERROR_INVALID_MESSAGE_TYPE;
}

void ReadHandler::OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext)
{
ChipLogError(DataManagement, "Time out! failed to receive status response from Exchange: " ChipLogFormatExchange,
Expand All @@ -298,14 +305,7 @@ CHIP_ERROR ReadHandler::ProcessReadRequest(System::PacketBufferHandle && aPayloa

ReturnErrorOnFailure(readRequestParser.Init(reader));
#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
err = readRequestParser.CheckSchemaValidity();
if (err != CHIP_NO_ERROR)
{
// The actual error we want to return to our consumer is an IM "invalid
// action" error, not whatever internal error CheckSchemaValidity
// happens to come up with.
return CHIP_IM_GLOBAL_STATUS(InvalidAction);
}
ReturnErrorOnFailure(readRequestParser.CheckSchemaValidity());
#endif

err = readRequestParser.GetAttributeRequests(&attributePathListParser);
Expand Down
7 changes: 3 additions & 4 deletions src/app/ReadHandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ class ReadHandler : public Messaging::ExchangeDelegate
* @retval #CHIP_NO_ERROR On success.
*
*/
CHIP_ERROR OnInitialRequest(System::PacketBufferHandle && aPayload);
void OnInitialRequest(System::PacketBufferHandle && aPayload);

/**
* Send ReportData to initiator
Expand Down Expand Up @@ -369,12 +369,11 @@ class ReadHandler : public Messaging::ExchangeDelegate
CHIP_ERROR ProcessAttributePathList(AttributePathIBs::Parser & aAttributePathListParser);
CHIP_ERROR ProcessEventPaths(EventPathIBs::Parser & aEventPathsParser);
CHIP_ERROR ProcessEventFilters(EventFilterIBs::Parser & aEventFiltersParser);
CHIP_ERROR OnStatusResponse(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload);
CHIP_ERROR OnStatusResponse(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload,
bool & aSendStatusResponse);
CHIP_ERROR OnMessageReceived(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,
System::PacketBufferHandle && aPayload) override;
void OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext) override;
CHIP_ERROR OnUnknownMsgType(Messaging::ExchangeContext * apExchangeContext, const PayloadHeader & aPayloadHeader,
System::PacketBufferHandle && aPayload);
void MoveToState(const HandlerState aTargetState);

const char * GetStateStr() const;
Expand Down
Loading