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

Jump to bottom

Fix status report ec nullout and suppress status response during read #11955

Merged
merged 1 commit into from
Nov 20, 2021
+49 −33
Merged

Fix status report ec nullout and suppress status response during read #11955

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/app/InteractionModelEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ CHIP_ERROR InteractionModelEngine::OnUnsolicitedReportData(Messaging::ExchangeCo

for (auto & readClient : mReadClients)
{
if (!readClient.IsSubscriptionTypeIdle())
if (!readClient.IsSubscriptionIdle())
{
continue;
}
Expand Down
31 changes: 14 additions & 17 deletions src/app/ReadClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ CHIP_ERROR ReadClient::OnUnsolicitedReportData(Messaging::ExchangeContext * apEx
{
mpExchangeCtx = apExchangeContext;
CHIP_ERROR err = ProcessReportData(std::move(aPayload));
mpExchangeCtx = nullptr;
if (err != CHIP_NO_ERROR)
{
ShutdownInternal(err);
Expand All @@ -305,7 +304,7 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload)

bool isEventReportsPresent = false;
bool isAttributeReportIBsPresent = false;
bool suppressResponse = false;
bool suppressResponse = true;
uint64_t subscriptionId = 0;
EventReports::Parser EventReports;
AttributeReportIBs::Parser attributeReportIBs;
Expand All @@ -325,7 +324,8 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload)
err = report.GetSuppressResponse(&suppressResponse);
if (CHIP_END_OF_TLV == err)
{
err = CHIP_NO_ERROR;
suppressResponse = false;
err = CHIP_NO_ERROR;
}
SuccessOrExit(err);

Expand Down Expand Up @@ -393,12 +393,6 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload)
SuccessOrExit(err);
}

if (!suppressResponse)
{
// TODO: Add status report support and correspond handler in ReadHandler, particular for situation when there
// are multiple reports
}

exit:
if (IsSubscriptionType())
{
Expand All @@ -412,14 +406,19 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload)
}
}

StatusResponse::SendStatusResponse(err == CHIP_NO_ERROR ? Protocols::InteractionModel::Status::Success
: Protocols::InteractionModel::Status::InvalidSubscription,
mpExchangeCtx, IsAwaitingSubscribeResponse() || mPendingMoreChunks);

if (!mInitialReport && !mPendingMoreChunks)
if (!suppressResponse)
{
mpExchangeCtx = nullptr;
bool noResponseExpected = IsSubscriptionIdle() && !mPendingMoreChunks;
err = StatusResponse::SendStatusResponse(err == CHIP_NO_ERROR ? Protocols::InteractionModel::Status::Success
: Protocols::InteractionModel::Status::InvalidSubscription,
mpExchangeCtx, !noResponseExpected);

if (noResponseExpected || (err != CHIP_NO_ERROR))
{
mpExchangeCtx = nullptr;
}
}

mInitialReport = false;
return err;
}
Expand All @@ -443,10 +442,8 @@ CHIP_ERROR ReadClient::ProcessAttributePath(AttributePathIB::Parser & aAttribute
// The ReportData must contain a concrete attribute path
err = aAttributePath.GetEndpoint(&(aClusterInfo.mEndpointId));
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

err = aAttributePath.GetCluster(&(aClusterInfo.mClusterId));
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

err = aAttributePath.GetAttribute(&(aClusterInfo.mAttributeId));
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

Expand Down
2 changes: 1 addition & 1 deletion src/app/ReadClient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ class ReadClient : public Messaging::ExchangeDelegate
*
*/
bool IsFree() const { return mState == ClientState::Uninitialized; }
bool IsSubscriptionTypeIdle() const { return mState == ClientState::SubscriptionActive; }
bool IsSubscriptionIdle() const { return mState == ClientState::SubscriptionActive; }
bool IsAwaitingInitialReport() const { return mState == ClientState::AwaitingInitialReport; }
bool IsAwaitingSubscribeResponse() const { return mState == ClientState::AwaitingSubscribeResponse; }

Expand Down
20 changes: 16 additions & 4 deletions src/app/ReadHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,22 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b
mpExchangeCtx->SetResponseTimeout(kImMessageTimeout);
}
VerifyOrReturnLogError(mpExchangeCtx != nullptr, CHIP_ERROR_INCORRECT_STATE);
mIsChunkedReport = aMoreChunks;
MoveToState(HandlerState::AwaitingReportResponse);
CHIP_ERROR err = mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::ReportData, std::move(aPayload),
Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse));
mIsChunkedReport = aMoreChunks;
bool noResponseExpected = IsReadType() && !mIsChunkedReport;
if (!noResponseExpected)
{
MoveToState(HandlerState::AwaitingReportResponse);
}
CHIP_ERROR err =
mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::ReportData, std::move(aPayload),
Messaging::SendFlags(noResponseExpected ? Messaging::SendMessageFlags::kNone
: Messaging::SendMessageFlags::kExpectResponse));
if (err == CHIP_NO_ERROR && noResponseExpected)
{
mpExchangeCtx = nullptr;
InteractionModelEngine::GetInstance()->GetReportingEngine().OnReportConfirm();
}

if (err == CHIP_NO_ERROR)
{
if (IsSubscriptionType() && !IsInitialReport())
Expand Down
8 changes: 8 additions & 0 deletions src/app/reporting/Engine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,10 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
{
reportDataBuilder.MoreChunkedMessages(hasMoreChunks);
}
else if (apReadHandler->IsReadType())
{
reportDataBuilder.SuppressResponse(true);
}

reportDataBuilder.EndOfReportDataMessage();
SuccessOrExit(err = reportDataBuilder.GetError());
Expand Down Expand Up @@ -361,6 +365,10 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
{
apReadHandler->Shutdown(ReadHandler::ShutdownOptions::AbortCurrentExchange);
}
else if (apReadHandler->IsReadType() && !hasMoreChunks)
{
apReadHandler->Shutdown();
}
return err;
}

Expand Down
19 changes: 9 additions & 10 deletions src/app/tests/TestReadInteraction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,11 @@ class TestReadInteraction

private:
static void GenerateReportData(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload,
bool aNeedInvalidReport = false);
bool aNeedInvalidReport, bool aSuppressResponse);
};

void TestReadInteraction::GenerateReportData(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload,
bool aNeedInvalidReport)
bool aNeedInvalidReport, bool aSuppressResponse)
{
CHIP_ERROR err = CHIP_NO_ERROR;
System::PacketBufferTLVWriter writer;
Expand All @@ -294,6 +294,9 @@ void TestReadInteraction::GenerateReportData(nlTestSuite * apSuite, void * apCon
err = reportDataMessageBuilder.Init(&writer);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

reportDataMessageBuilder.SuppressResponse(aSuppressResponse);
NL_TEST_ASSERT(apSuite, reportDataMessageBuilder.GetError() == CHIP_NO_ERROR);

AttributeReportIBs::Builder attributeReportIBsBuilder = reportDataMessageBuilder.CreateAttributeReportIBs();
NL_TEST_ASSERT(apSuite, reportDataMessageBuilder.GetError() == CHIP_NO_ERROR);

Expand Down Expand Up @@ -346,9 +349,6 @@ void TestReadInteraction::GenerateReportData(nlTestSuite * apSuite, void * apCon
attributeReportIBsBuilder.EndOfAttributeReportIBs();
NL_TEST_ASSERT(apSuite, attributeReportIBsBuilder.GetError() == CHIP_NO_ERROR);

reportDataMessageBuilder.SuppressResponse(true);
NL_TEST_ASSERT(apSuite, reportDataMessageBuilder.GetError() == CHIP_NO_ERROR);

reportDataMessageBuilder.MoreChunkedMessages(false);
NL_TEST_ASSERT(apSuite, reportDataMessageBuilder.GetError() == CHIP_NO_ERROR);

Expand All @@ -372,8 +372,7 @@ void TestReadInteraction::TestReadClient(nlTestSuite * apSuite, void * apContext
err = readClient.SendReadRequest(readPrepareParams);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

GenerateReportData(apSuite, apContext, buf);

GenerateReportData(apSuite, apContext, buf, false /*aNeedInvalidReport*/, true /* aSuppressResponse*/);
err = readClient.ProcessReportData(std::move(buf));
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

Expand All @@ -397,7 +396,7 @@ void TestReadInteraction::TestReadHandler(nlTestSuite * apSuite, void * apContex
Messaging::ExchangeContext * exchangeCtx = ctx.NewExchangeToAlice(nullptr);
readHandler.Init(&ctx.GetExchangeManager(), nullptr, exchangeCtx, chip::app::ReadHandler::InteractionType::Read);

GenerateReportData(apSuite, apContext, reportDatabuf);
GenerateReportData(apSuite, apContext, reportDatabuf, false /*aNeedInvalidReport*/, false /* aSuppressResponse*/);
err = readHandler.SendReportData(std::move(reportDatabuf), false);
NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_INCORRECT_STATE);

Expand Down Expand Up @@ -500,7 +499,7 @@ void TestReadInteraction::TestReadClientInvalidReport(nlTestSuite * apSuite, voi
err = readClient.SendReadRequest(readPrepareParams);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

GenerateReportData(apSuite, apContext, buf, true /*aNeedInvalidReport*/);
GenerateReportData(apSuite, apContext, buf, true /*aNeedInvalidReport*/, true /* aSuppressResponse*/);

err = readClient.ProcessReportData(std::move(buf));
NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
Expand All @@ -526,7 +525,7 @@ void TestReadInteraction::TestReadHandlerInvalidAttributePath(nlTestSuite * apSu
Messaging::ExchangeContext * exchangeCtx = ctx.NewExchangeToAlice(nullptr);
readHandler.Init(&ctx.GetExchangeManager(), nullptr, exchangeCtx, chip::app::ReadHandler::InteractionType::Read);

GenerateReportData(apSuite, apContext, reportDatabuf);
GenerateReportData(apSuite, apContext, reportDatabuf, false /*aNeedInvalidReport*/, false /* aSuppressResponse*/);
err = readHandler.SendReportData(std::move(reportDatabuf), false);
NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_INCORRECT_STATE);

Expand Down