From 75dc90b2a01b295b87e66577f59ffc5100d8bdf1 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 6 Jun 2023 11:25:29 -0400 Subject: [PATCH] Address review comment. --- src/app/ReadClient.cpp | 8 ++++---- src/app/ReadClient.h | 10 +++++++++- src/app/tests/TestReadInteraction.cpp | 6 +++--- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 791371176fe879..21eb83f65ad584 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -433,7 +433,7 @@ CHIP_ERROR ReadClient::OnMessageReceived(Messaging::ExchangeContext * apExchange if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::ReportData)) { - err = ProcessReportData(std::move(aPayload), /* aIsUnsolicitedReport = */ false); + err = ProcessReportData(std::move(aPayload), ReportType::kContinuingTransaction); } else if (aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::SubscribeResponse)) { @@ -487,7 +487,7 @@ void ReadClient::OnUnsolicitedReportData(Messaging::ExchangeContext * apExchange // mReadPrepareParams.mSessionHolder.Grab(mExchange->GetSessionHandle()); - CHIP_ERROR err = ProcessReportData(std::move(aPayload), /* aIsUnsolicitedReport = */ true); + CHIP_ERROR err = ProcessReportData(std::move(aPayload), ReportType::kUnsolicited); if (err != CHIP_NO_ERROR) { if (err == CHIP_ERROR_INVALID_SUBSCRIPTION) @@ -504,7 +504,7 @@ void ReadClient::OnUnsolicitedReportData(Messaging::ExchangeContext * apExchange } } -CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload, bool aIsUnsolicitedReport) +CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload, ReportType aReportType) { CHIP_ERROR err = CHIP_NO_ERROR; ReportDataMessage::Parser report; @@ -518,7 +518,7 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload, SuccessOrExit(err); #if CHIP_CONFIG_IM_PRETTY_PRINT - if (!aIsUnsolicitedReport) + if (aReportType != ReportType::kUnsolicited) { report.PrettyPrint(); } diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h index 1dfca8697da63c..f45b8b1448cc2e 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -447,6 +447,14 @@ class ReadClient : public Messaging::ExchangeDelegate SubscriptionActive, ///< The client is maintaining subscription }; + enum class ReportType + { + // kUnsolicited reports are the first message in an exchange. + kUnsolicited, + // kContinuingTransaction reports are responses to a message we sent. + kContinuingTransaction + }; + bool IsMatchingSubscriptionId(SubscriptionId aSubscriptionId) { return aSubscriptionId == mSubscriptionId && mInteractionType == InteractionType::Subscribe; @@ -486,7 +494,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, bool aIsUnsolicitedReport); + CHIP_ERROR ProcessReportData(System::PacketBufferHandle && aPayload, ReportType aReportType); const char * GetStateStr() const; /* diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index e5fcc0d6da55eb..f972125632a11d 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -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), false /* aIsUnsolicitedReport */); + err = readClient.ProcessReportData(std::move(buf), ReadClient::ReportType::kContinuingTransaction); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); } @@ -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), false /* aIsUnsolicitedReport */); + err = readClient.ProcessReportData(std::move(buf), ReadClient::ReportType::kContinuingTransaction); NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_INVALID_ARGUMENT); } @@ -637,7 +637,7 @@ void TestReadInteraction::TestReadClientInvalidReport(nlTestSuite * apSuite, voi GenerateReportData(apSuite, apContext, buf, true /*aNeedInvalidReport*/, true /* aSuppressResponse*/); - err = readClient.ProcessReportData(std::move(buf), false /* aIsUnsolicitedReport */); + err = readClient.ProcessReportData(std::move(buf), ReadClient::ReportType::kContinuingTransaction); NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH_IB); }