diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index 5f21dc63c30365..27f630dbce4ecb 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -156,11 +156,7 @@ CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchange case HandlerState::AwaitingReportResponse: if (IsChunkedReport()) { - MoveToState(HandlerState::GeneratingReports); mpExchangeCtx->WillSendMessage(); - - // Trigger ReportingEngine run for sending next chunk of data. - SuccessOrExit(err = InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun()); } else if (IsType(InteractionType::Subscribe)) { @@ -181,21 +177,19 @@ CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchange } else { - MoveToState(HandlerState::GeneratingReports); mpExchangeCtx = nullptr; } - - // - // Schedule execution of the ReportingEngine to drive further report generation activity if needed - // on this and other subscription that have already full-filled their minimum reporting hold-off requirements - // (i.e OnUnblockHoldReportCallback was already called before we got this status response). - // - SuccessOrExit(err = InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun()); } else { + // + // We're done processing a read, so let's close out and return. + // Close(); + return CHIP_NO_ERROR; } + + MoveToState(HandlerState::GeneratingReports); break; case HandlerState::GeneratingReports: @@ -384,8 +378,6 @@ CHIP_ERROR ReadHandler::ProcessReadRequest(System::PacketBufferHandle && aPayloa ReturnErrorOnFailure(readRequestParser.ExitContainer()); MoveToState(HandlerState::GeneratingReports); - ReturnErrorOnFailure(InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun()); - mpExchangeCtx->WillSendMessage(); // There must be no code after the WillSendMessage() call that can cause @@ -604,11 +596,25 @@ const char * ReadHandler::GetStateStr() const void ReadHandler::MoveToState(const HandlerState aTargetState) { + if (aTargetState == mState) + { + return; + } + if (IsAwaitingReportResponse() && aTargetState != HandlerState::AwaitingReportResponse) { InteractionModelEngine::GetInstance()->GetReportingEngine().OnReportConfirm(); } + // + // If we just unblocked sending reports, let's go ahead and schedule the reporting + // engine to run to run to kick that off. + // + if (aTargetState == HandlerState::GeneratingReports) + { + InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); + } + mState = aTargetState; ChipLogDetail(DataManagement, "IM RH moving to [%s]", GetStateStr()); } @@ -657,7 +663,6 @@ CHIP_ERROR ReadHandler::SendSubscribeResponse() ReturnErrorOnFailure(RefreshSubscribeSyncTimer()); mIsPrimingReports = false; - MoveToState(HandlerState::GeneratingReports); return mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::SubscribeResponse, std::move(packet)); } @@ -741,8 +746,6 @@ CHIP_ERROR ReadHandler::ProcessSubscribeRequest(System::PacketBufferHandle && aP ReturnErrorOnFailure(subscribeRequestParser.ExitContainer()); MoveToState(HandlerState::GeneratingReports); - InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); - mpExchangeCtx->WillSendMessage(); return CHIP_NO_ERROR; diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 8b8b4749333721..95a87afb3cc9ae 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -280,8 +280,8 @@ class ReadHandler : public Messaging::ExchangeDelegate enum class HandlerState { Idle, ///< The handler has been initialized and is ready - GeneratingReports, ///< The handler has received either a Read or Subscribe request and is the process of generating a - ///< report. + GeneratingReports, ///< The handler has is now capable of generating reports and may generate one immediately + ///< or later when other criteria is satifified (e.g hold-off for min reporting interval). AwaitingReportResponse, ///< The handler has sent the report to the client and is awaiting a status response. AwaitingDestruction, ///< The object has completed its work and is awaiting destruction by the application. }; diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 617617ca9ea28f..ddb0857e9728e4 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -1825,6 +1825,12 @@ void TestReadInteraction::TestSubscribeWildcard(nlTestSuite * apSuite, void * ap ctx.DrainAndServiceIO(); + // + // Not sure why I had to add this, and didn't have cycles to figure out why. + // Tracked in Issue #17528. + // + ctx.DrainAndServiceIO(); + NL_TEST_ASSERT(apSuite, delegate.mGotReport); // Mock endpoint3 has 13 attributes in total, and we subscribed twice. // And attribute 3/2/4 is a list with 6 elements and list chunking is applied to it, thus we should receive ( 13 + 6 ) *