Fix status report ec nullout and suppress report during read (#11955)

src/app/InteractionModelEngine.cpp

@@ -376,7 +376,7 @@ CHIP_ERROR InteractionModelEngine::OnUnsolicitedReportData(Messaging::ExchangeCo
 
  for (auto & readClient : mReadClients)
  {
- if (!readClient.IsSubscriptionTypeIdle())
+ if (!readClient.IsSubscriptionIdle())
  {
  continue;
  }

src/app/ReadClient.cpp

@@ -291,7 +291,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);
@@ -306,7 +305,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;
@@ -326,7 +325,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);
 
@@ -399,12 +399,6 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload)
  mpCallback->OnReportEnd(this);
  }
 
- if (!suppressResponse)
- {
- // TODO: Add status report support and correspond handler in ReadHandler, particular for situation when there
- // are multiple reports
- }
-
 exit:
  if (IsSubscriptionType())
  {
@@ -418,14 +412,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;
 }
@@ -449,10 +448,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);
 

src/app/ReadClient.h

@@ -250,7 +250,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; }
 

src/app/ReadHandler.cpp

@@ -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())

src/app/reporting/Engine.cpp

@@ -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());
@@ -361,6 +365,10 @@ CHIP_ERROR Engine::BuildAndSendSingleReportData(ReadHandler * apReadHandler)
  {
  apReadHandler->Shutdown(ReadHandler::ShutdownOptions::AbortCurrentExchange);
  }
+ else if (apReadHandler->IsReadType() && !hasMoreChunks)
+ {
+ apReadHandler->Shutdown();
+ }
  return err;
 }
 

src/app/tests/TestReadInteraction.cpp

@@ -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;
@@ -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);
 
@@ -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);
 
@@ -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);
 
@@ -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);
 
@@ -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);
@@ -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);