Skip to content

Commit

Permalink
Log payloads for unsolicited reports even when we fail to dispatch them.
Browse files Browse the repository at this point in the history
This makes it easier to see what the other side thinks it's doing.
  • Loading branch information
bzbarsky-apple committed Jun 6, 2023
1 parent 77a92d4 commit 395a392
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 8 deletions.
4 changes: 4 additions & 0 deletions src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,10 @@ Status InteractionModelEngine::OnUnsolicitedReportData(Messaging::ExchangeContex
ReportDataMessage::Parser report;
VerifyOrReturnError(report.Init(reader) == CHIP_NO_ERROR, Status::InvalidAction);

#if CHIP_CONFIG_IM_PRETTY_PRINT
report.PrettyPrint();
#endif

SubscriptionId subscriptionId = 0;
VerifyOrReturnError(report.GetSubscriptionId(&subscriptionId) == CHIP_NO_ERROR, Status::InvalidAction);
VerifyOrReturnError(report.ExitContainer() == CHIP_NO_ERROR, Status::InvalidAction);
Expand Down
11 changes: 7 additions & 4 deletions src/app/ReadClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ CHIP_ERROR ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchange

if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::ReportData))
{
err = ProcessReportData(std::move(aPayload));
err = ProcessReportData(std::move(aPayload), /* aIsUnsolicitedReport = */ false);
}
else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::SubscribeResponse))
{
Expand Down Expand Up @@ -487,7 +487,7 @@ void ReadClient::OnUnsolicitedReportData(Messaging::ExchangeContext * apExchange
//
mReadPrepareParams.mSessionHolder.Grab(mExchange->GetSessionHandle());

CHIP_ERROR err = ProcessReportData(std::move(aPayload));
CHIP_ERROR err = ProcessReportData(std::move(aPayload), /* aIsUnsolicitedReport = */ true);
if (err != CHIP_NO_ERROR)
{
if (err == CHIP_ERROR_INVALID_SUBSCRIPTION)
Expand All @@ -504,7 +504,7 @@ void ReadClient::OnUnsolicitedReportData(Messaging::ExchangeContext * apExchange
}
}

CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload)
CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload, bool aIsUnsolicitedReport)
{
CHIP_ERROR err = CHIP_NO_ERROR;
ReportDataMessage::Parser report;
Expand All @@ -518,7 +518,10 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload)
SuccessOrExit(err);

#if CHIP_CONFIG_IM_PRETTY_PRINT
report.PrettyPrint();
if (!aIsUnsolicitedReport)
{
report.PrettyPrint();
}
#endif

err = report.GetSuppressResponse(&suppressResponse);
Expand Down
2 changes: 1 addition & 1 deletion src/app/ReadClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ class ReadClient : public Messaging::ExchangeDelegate
void CancelResubscribeTimer();
void MoveToState(const ClientState aTargetState);
CHIP_ERROR ProcessAttributePath(AttributePathIB::Parser & aAttributePath, ConcreteDataAttributePath & aClusterInfo);
CHIP_ERROR ProcessReportData(System::PacketBufferHandle && aPayload);
CHIP_ERROR ProcessReportData(System::PacketBufferHandle && aPayload, bool aIsUnsolicitedReport);
const char * GetStateStr() const;

/*
Expand Down
6 changes: 3 additions & 3 deletions src/app/tests/TestReadInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ void TestReadInteraction::TestReadClient(nlTestSuite * apSuite, void * apContext
ctx.DrainAndServiceIO();

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

Expand All @@ -494,7 +494,7 @@ void TestReadInteraction::TestReadUnexpectedSubscriptionId(nlTestSuite * apSuite
// For read, we don't expect there is subscription id in report data.
GenerateReportData(apSuite, apContext, buf, false /*aNeedInvalidReport*/, true /* aSuppressResponse*/,
true /*aHasSubscriptionId*/);
err = readClient.ProcessReportData(std::move(buf));
err = readClient.ProcessReportData(std::move(buf), false /* aIsUnsolicitedReport */);
NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
}

Expand Down Expand Up @@ -637,7 +637,7 @@ void TestReadInteraction::TestReadClientInvalidReport(nlTestSuite * apSuite, voi

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

err = readClient.ProcessReportData(std::move(buf));
err = readClient.ProcessReportData(std::move(buf), false /* aIsUnsolicitedReport */);
NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH_IB);
}

Expand Down

0 comments on commit 395a392

Please sign in to comment.