From 256715723d0493f626c95babbd95b2942a9703d6 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs <112982107+lpbeliveau-silabs@users.noreply.github.com> Date: Thu, 10 Aug 2023 15:19:21 -0400 Subject: [PATCH] [ReadHandler] Removed Scheduling of report from OnReadHandlerCreated (#28536) * Removed Scheduling of report from OnReadHandlerCreated since it caused immediate scheduling before the intervals are negotiated * Separated Read reports from Subscription reports and renamed flags and accessors for clarity * Apply suggestions from code review Co-authored-by: Boris Zbarsky * Put back rescheduling in the OnReadHandlerSubscribed, added a CanEmitReadReport() method for readhandler for read request. Fixed call order in TestReportScheduler tests * Removed redundancy by removing un-necessary OnSubscriptionAction(), added comment for OnReadHandlerSubscribed and modified reporting condition for Read reports to be sent independently from the report scheduler * Apply suggestions from code review Co-authored-by: Boris Zbarsky * Appplied change to method name and fixe condition in SetStateFlag * Update src/app/ReadHandler.h Co-authored-by: Boris Zbarsky * Removed un-necessary check in SetStateFlag --------- Co-authored-by: Boris Zbarsky --- src/app/ReadHandler.cpp | 42 +++---- src/app/ReadHandler.h | 44 +++++--- src/app/reporting/Engine.cpp | 4 +- src/app/reporting/ReportScheduler.h | 10 +- src/app/reporting/ReportSchedulerImpl.cpp | 13 +-- src/app/reporting/ReportSchedulerImpl.h | 4 +- src/app/tests/TestReadInteraction.cpp | 8 +- src/app/tests/TestReportScheduler.cpp | 131 ++++++++++------------ 8 files changed, 128 insertions(+), 128 deletions(-) diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index 4e9e8fccdf5646..1295cc6920c462 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -79,7 +79,6 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeCon VerifyOrDie(observer != nullptr); mObserver = observer; - mObserver->OnReadHandlerCreated(this); } #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS @@ -92,7 +91,6 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Observer * observer) : VerifyOrDie(observer != nullptr); mObserver = observer; - mObserver->OnReadHandlerCreated(this); } void ReadHandler::ResumeSubscription(CASESessionManager & caseSessionManager, @@ -235,6 +233,7 @@ CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchange { appCallback->OnSubscriptionEstablished(*this); } + mObserver->OnSubscriptionEstablished(this); } } else @@ -246,10 +245,10 @@ CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchange return CHIP_NO_ERROR; } - MoveToState(HandlerState::GeneratingReports); + MoveToState(HandlerState::CanStartReporting); break; - case HandlerState::GeneratingReports: + case HandlerState::CanStartReporting: case HandlerState::Idle: default: err = CHIP_ERROR_INCORRECT_STATE; @@ -262,7 +261,7 @@ CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchange CHIP_ERROR ReadHandler::SendStatusReport(Protocols::InteractionModel::Status aStatus) { - VerifyOrReturnLogError(mState == HandlerState::GeneratingReports, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnLogError(mState == HandlerState::CanStartReporting, CHIP_ERROR_INCORRECT_STATE); if (IsPriming() || IsChunkedReport()) { mSessionHandle.Grab(mExchangeCtx->GetSessionHandle()); @@ -286,7 +285,7 @@ CHIP_ERROR ReadHandler::SendStatusReport(Protocols::InteractionModel::Status aSt CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, bool aMoreChunks) { - VerifyOrReturnLogError(mState == HandlerState::GeneratingReports, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnLogError(mState == HandlerState::CanStartReporting, CHIP_ERROR_INCORRECT_STATE); VerifyOrDie(!IsAwaitingReportResponse()); // Should not be reportable! if (IsPriming() || IsChunkedReport()) { @@ -335,7 +334,7 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b // Priming reports are handled when we send a SubscribeResponse. if (IsType(InteractionType::Subscribe) && !IsPriming() && !IsChunkedReport()) { - mObserver->OnSubscriptionAction(this); + mObserver->OnSubscriptionReportSent(this); } } if (!aMoreChunks) @@ -456,7 +455,7 @@ CHIP_ERROR ReadHandler::ProcessReadRequest(System::PacketBufferHandle && aPayloa ReturnErrorOnFailure(readRequestParser.GetIsFabricFiltered(&isFabricFiltered)); SetStateFlag(ReadHandlerFlags::FabricFiltered, isFabricFiltered); ReturnErrorOnFailure(readRequestParser.ExitContainer()); - MoveToState(HandlerState::GeneratingReports); + MoveToState(HandlerState::CanStartReporting); mExchangeCtx->WillSendMessage(); @@ -574,8 +573,8 @@ const char * ReadHandler::GetStateStr() const return "Idle"; case HandlerState::AwaitingDestruction: return "AwaitingDestruction"; - case HandlerState::GeneratingReports: - return "GeneratingReports"; + case HandlerState::CanStartReporting: + return "CanStartReporting"; case HandlerState::AwaitingReportResponse: return "AwaitingReportResponse"; @@ -603,10 +602,17 @@ void ReadHandler::MoveToState(const HandlerState aTargetState) // If we just unblocked sending reports, let's go ahead and schedule the reporting // engine to run to kick that off. // - if (aTargetState == HandlerState::GeneratingReports) + if (aTargetState == HandlerState::CanStartReporting) { - // mObserver will take care of scheduling the report as soon as allowed - mObserver->OnBecameReportable(this); + if (ShouldReportUnscheduled()) + { + InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); + } + else + { + // If we became reportable, the scheduler will schedule a run as soon as allowed + mObserver->OnBecameReportable(this); + } } } @@ -647,8 +653,6 @@ CHIP_ERROR ReadHandler::SendSubscribeResponse() ReturnErrorOnFailure(writer.Finalize(&packet)); VerifyOrReturnLogError(mExchangeCtx, CHIP_ERROR_INCORRECT_STATE); - mObserver->OnSubscriptionAction(this); - ClearStateFlag(ReadHandlerFlags::PrimingReports); return mExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::SubscribeResponse, std::move(packet)); } @@ -785,7 +789,7 @@ CHIP_ERROR ReadHandler::ProcessSubscribeRequest(System::PacketBufferHandle && aP SetStateFlag(ReadHandlerFlags::FabricFiltered, isFabricFiltered); ReturnErrorOnFailure(Crypto::DRBG_get_bytes(reinterpret_cast(&mSubscriptionId), sizeof(mSubscriptionId))); ReturnErrorOnFailure(subscribeRequestParser.ExitContainer()); - MoveToState(HandlerState::GeneratingReports); + MoveToState(HandlerState::CanStartReporting); mExchangeCtx->WillSendMessage(); @@ -872,11 +876,11 @@ void ReadHandler::ForceDirtyState() void ReadHandler::SetStateFlag(ReadHandlerFlags aFlag, bool aValue) { - bool oldReportable = IsReportable(); + bool oldReportable = ShouldStartReporting(); mFlags.Set(aFlag, aValue); // If we became reportable, schedule a reporting run. - if (!oldReportable && IsReportable()) + if (!oldReportable && ShouldStartReporting()) { // If we became reportable, the scheduler will schedule a run as soon as allowed mObserver->OnBecameReportable(this); @@ -895,7 +899,7 @@ void ReadHandler::HandleDeviceConnected(void * context, Messaging::ExchangeManag _this->mSessionHandle.Grab(sessionHandle); - _this->MoveToState(HandlerState::GeneratingReports); + _this->MoveToState(HandlerState::CanStartReporting); ObjectList * attributePath = _this->mpAttributePathList; while (attributePath) diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index a87c121ffaf8b0..ecdc0c1c0696c1 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -166,20 +166,23 @@ class ReadHandler : public Messaging::ExchangeDelegate public: virtual ~Observer() = default; - /// @brief Callback invoked to notify a ReadHandler was created and can be registered - /// @param[in] apReadHandler ReadHandler getting added - virtual void OnReadHandlerCreated(ReadHandler * apReadHandler) = 0; - - /// @brief Callback invoked when a ReadHandler went from a non reportable state to a reportable state so a report can be - /// sent immediately if the minimal interval allows it. Otherwise the report should be rescheduled to the earliest time - /// allowed. - /// @param[in] apReadHandler ReadHandler that became dirty + /// @brief Callback invoked to notify a subscription was successfully established for the ReadHandler + /// @param[in] apReadHandler ReadHandler that completed its subscription + virtual void OnSubscriptionEstablished(ReadHandler * apReadHandler) = 0; + + /// @brief Callback invoked when a ReadHandler went from a non reportable state to a reportable state. Indicates to the + /// observer that a report should be emitted when the min interval allows it. + /// + /// This will only be invoked for subscribe-type ReadHandler objects, and only after + /// OnSubscriptionEstablished has been called. + /// + /// @param[in] apReadHandler ReadHandler that became dirty and in HandlerState::CanStartReporting state virtual void OnBecameReportable(ReadHandler * apReadHandler) = 0; /// @brief Callback invoked when the read handler needs to make sure to send a message to the subscriber within the next /// maxInterval time period. /// @param[in] apReadHandler ReadHandler that has generated a report - virtual void OnSubscriptionAction(ReadHandler * apReadHandler) = 0; + virtual void OnSubscriptionReportSent(ReadHandler * apReadHandler) = 0; /// @brief Callback invoked when a ReadHandler is getting removed so it can be unregistered /// @param[in] apReadHandler ReadHandler getting destroyed @@ -333,13 +336,22 @@ class ReadHandler : public Messaging::ExchangeDelegate bool IsIdle() const { return mState == HandlerState::Idle; } /// @brief Returns whether the ReadHandler is in a state where it can send a report and there is data to report. - bool IsReportable() const + bool ShouldStartReporting() const { - // Important: Anything that changes the state IsReportable must call mObserver->OnBecameReportable(this) for the scheduler - // to plan the next run accordingly. - return mState == HandlerState::GeneratingReports && IsDirty(); + // Important: Anything that changes ShouldStartReporting() from false to true + // (which can only happen for subscriptions) must call + // mObserver->OnBecameReportable(this). + return CanStartReporting() && (ShouldReportUnscheduled() || IsDirty()); + } + /// @brief CanStartReporting() is true if the ReadHandler is in a state where it could generate + /// a (possibly empty) report if someone asked it to. + bool CanStartReporting() const { return mState == HandlerState::CanStartReporting; } + /// @brief ShouldReportUnscheduled() is true if the ReadHandler should be asked to generate reports + /// without consulting the report scheduler. + bool ShouldReportUnscheduled() const + { + return CanStartReporting() && (IsType(ReadHandler::InteractionType::Read) || IsPriming()); } - bool IsGeneratingReports() const { return mState == HandlerState::GeneratingReports; } bool IsAwaitingReportResponse() const { return mState == HandlerState::AwaitingReportResponse; } // Resets the path iterator to the beginning of the whole report for generating a series of new reports. @@ -418,14 +430,14 @@ class ReadHandler : public Messaging::ExchangeDelegate friend class chip::app::reporting::Engine; friend class chip::app::InteractionModelEngine; - // The report scheduler needs to be able to access StateFlag private functions IsReportable(), IsGeneratingReports(), + // The report scheduler needs to be able to access StateFlag private functions ShouldStartReporting(), CanStartReporting(), // ForceDirtyState() and IsDirty() to know when to schedule a run so it is declared as a friend class. friend class chip::app::reporting::ReportScheduler; enum class HandlerState : uint8_t { Idle, ///< The handler has been initialized and is ready - GeneratingReports, ///< The handler has is now capable of generating reports and may generate one immediately + CanStartReporting, ///< The handler has is now capable of generating reports and may generate one immediately ///< or later when other criteria are satisfied (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/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index a1eae3abe0d1b0..0a3eedd4ad767c 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -638,7 +638,7 @@ void Engine::Run() ReadHandler * readHandler = imEngine->ActiveHandlerAt(mCurReadHandlerIdx % (uint32_t) imEngine->mReadHandlers.Allocated()); VerifyOrDie(readHandler != nullptr); - if (imEngine->GetReportScheduler()->IsReportableNow(readHandler)) + if (readHandler->ShouldReportUnscheduled() || imEngine->GetReportScheduler()->IsReportableNow(readHandler)) { mRunningReadHandler = readHandler; CHIP_ERROR err = BuildAndSendSingleReportData(readHandler); @@ -831,7 +831,7 @@ CHIP_ERROR Engine::SetDirty(AttributePathParams & aAttributePath) // We call AttributePathIsDirty for both read interactions and subscribe interactions, since we may send inconsistent // attribute data between two chunks. AttributePathIsDirty will not schedule a new run for read handlers which are // waiting for a response to the last message chunk for read interactions. - if (handler->IsGeneratingReports() || handler->IsAwaitingReportResponse()) + if (handler->CanStartReporting() || handler->IsAwaitingReportResponse()) { for (auto object = handler->GetAttributePathList(); object != nullptr; object = object->mpNext) { diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index 068237780b86a0..5b7b62709985bd 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -82,7 +82,7 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver { Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); - return (mReadHandler->IsGeneratingReports() && + return (mReadHandler->CanStartReporting() && (now >= mMinTimestamp && (mReadHandler->IsDirty() || now >= mMaxTimestamp || now >= mSyncTimestamp))); } @@ -139,9 +139,13 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver /// @brief Check whether a ReadHandler is reportable right now, taking into account its minimum and maximum intervals. /// @param aReadHandler read handler to check - bool IsReportableNow(ReadHandler * aReadHandler) { return FindReadHandlerNode(aReadHandler)->IsReportableNow(); } + bool IsReportableNow(ReadHandler * aReadHandler) + { + ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); + return (nullptr != node) ? node->IsReportableNow() : false; + } /// @brief Check if a ReadHandler is reportable without considering the timing - bool IsReadHandlerReportable(ReadHandler * aReadHandler) const { return aReadHandler->IsReportable(); } + bool IsReadHandlerReportable(ReadHandler * aReadHandler) const { return aReadHandler->ShouldStartReporting(); } /// @brief Sets the ForceDirty flag of a ReadHandler void HandlerForceDirtyState(ReadHandler * aReadHandler) { aReadHandler->ForceDirtyState(); } diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index 74f75a663277aa..44be647b921d9d 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -54,8 +54,10 @@ void ReportSchedulerImpl::OnEnterActiveMode() #endif } -/// @brief When a ReadHandler is added, register it, which will schedule an engine run -void ReportSchedulerImpl::OnReadHandlerCreated(ReadHandler * aReadHandler) +/// @brief When a ReadHandler is added, register it in the scheduler node pool. Scheduling the report here is un-necessary since the +/// ReadHandler will call MoveToState(HandlerState::CanStartReporting);, which will call OnBecameReportable() and schedule the +/// report. +void ReportSchedulerImpl::OnSubscriptionEstablished(ReadHandler * aReadHandler) { ReadHandlerNode * newNode = FindReadHandlerNode(aReadHandler); // Handler must not be registered yet; it's just being constructed. @@ -68,11 +70,6 @@ void ReportSchedulerImpl::OnReadHandlerCreated(ReadHandler * aReadHandler) "Registered a ReadHandler that will schedule a report between system Timestamp: %" PRIu64 " and system Timestamp %" PRIu64 ".", newNode->GetMinTimestamp().count(), newNode->GetMaxTimestamp().count()); - - Milliseconds32 newTimeout; - // No need to check for error here, since the node is already in the list otherwise we would have Died - CalculateNextReportTimeout(newTimeout, newNode); - ScheduleReport(newTimeout, newNode); } /// @brief When a ReadHandler becomes reportable, schedule, recalculate and reschedule the report. @@ -85,7 +82,7 @@ void ReportSchedulerImpl::OnBecameReportable(ReadHandler * aReadHandler) ScheduleReport(newTimeout, node); } -void ReportSchedulerImpl::OnSubscriptionAction(ReadHandler * aReadHandler) +void ReportSchedulerImpl::OnSubscriptionReportSent(ReadHandler * aReadHandler) { ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); VerifyOrReturn(nullptr != node); diff --git a/src/app/reporting/ReportSchedulerImpl.h b/src/app/reporting/ReportSchedulerImpl.h index 573184ada02d5a..003be7cb4637eb 100644 --- a/src/app/reporting/ReportSchedulerImpl.h +++ b/src/app/reporting/ReportSchedulerImpl.h @@ -36,9 +36,9 @@ class ReportSchedulerImpl : public ReportScheduler void OnEnterActiveMode() override; // ReadHandlerObserver - void OnReadHandlerCreated(ReadHandler * aReadHandler) final; + void OnSubscriptionEstablished(ReadHandler * aReadHandler) final; void OnBecameReportable(ReadHandler * aReadHandler) final; - void OnSubscriptionAction(ReadHandler * aReadHandler) final; + void OnSubscriptionReportSent(ReadHandler * aReadHandler) final; void OnReadHandlerDestroyed(ReadHandler * aReadHandler) override; bool IsReportScheduled(ReadHandler * aReadHandler); diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 0f907567ee44f4..ea464c3d9471b0 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -2234,7 +2234,7 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite reportScheduler->GetMinTimestampForHandler(delegate.mpReadHandler) < System::SystemClock().GetMonotonicTimestamp()); NL_TEST_ASSERT(apSuite, !delegate.mpReadHandler->IsDirty()); - NL_TEST_ASSERT(apSuite, !delegate.mpReadHandler->IsReportable()); + NL_TEST_ASSERT(apSuite, !delegate.mpReadHandler->ShouldStartReporting()); // And the non-urgent one should not have changed state either, since // it's waiting for the max-interval. @@ -2245,7 +2245,7 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite reportScheduler->GetMaxTimestampForHandler(nonUrgentDelegate.mpReadHandler) > System::SystemClock().GetMonotonicTimestamp()); NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->IsDirty()); - NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->IsReportable()); + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->ShouldStartReporting()); // There should be no reporting run scheduled. This is very important; // otherwise we can get a false-positive pass below because the run was @@ -2257,12 +2257,12 @@ void TestReadInteraction::TestSubscribeUrgentWildcardEvent(nlTestSuite * apSuite // Urgent read handler should now be dirty, and reportable. NL_TEST_ASSERT(apSuite, delegate.mpReadHandler->IsDirty()); - NL_TEST_ASSERT(apSuite, delegate.mpReadHandler->IsReportable()); + NL_TEST_ASSERT(apSuite, delegate.mpReadHandler->ShouldStartReporting()); NL_TEST_ASSERT(apSuite, reportScheduler->IsReadHandlerReportable(delegate.mpReadHandler)); // Non-urgent read handler should not be reportable. NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->IsDirty()); - NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->IsReportable()); + NL_TEST_ASSERT(apSuite, !nonUrgentDelegate.mpReadHandler->ShouldStartReporting()); // Still no reporting should have happened. NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index 9d19f85636e15e..5f8fd3ffd36949 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -135,9 +135,6 @@ class TestTimerDelegate : public ReportScheduler::TimerDelegate static void TimerCallbackInterface(System::Layer * aLayer, void * aAppState) { - // Normaly we would call the callback here, thus scheduling an engine run, but we don't need it for this test as we simulate - // all the callbacks related to report emissions. The actual callback should look like this: - // TimerContext * context = static_cast(aAppState); context->TimerFired(); ChipLogProgress(DataManagement, "Simluating engine run for Handler: %p", aAppState); @@ -249,6 +246,23 @@ SynchronizedReportSchedulerImpl syncScheduler(&sTestTimerSynchronizedDelegate); class TestReportScheduler { public: + /// @brief Mimicks the various operations that happen on a subscription transaction after a read handler was created so that + /// readhandlers are in the expected state for further tests. + /// @param readHandler + /// @param scheduler + static CHIP_ERROR MockReadHandlerSubscriptionTransation(ReadHandler * readHandler, ReportScheduler * scheduler, + uint8_t min_interval_seconds, uint8_t max_interval_seconds) + { + ReturnErrorOnFailure(readHandler->SetMaxReportingInterval(max_interval_seconds)); + ReturnErrorOnFailure(readHandler->SetMinReportingIntervalForTests(min_interval_seconds)); + readHandler->ClearStateFlag(ReadHandler::ReadHandlerFlags::PrimingReports); + readHandler->SetStateFlag(ReadHandler::ReadHandlerFlags::ActiveSubscription); + scheduler->OnSubscriptionEstablished(readHandler); + readHandler->MoveToState(ReadHandler::HandlerState::CanStartReporting); + + return CHIP_NO_ERROR; + } + static ReadHandler * GetReadHandlerFromPool(ReportScheduler * scheduler, uint32_t target) { uint32_t i = 0; @@ -285,6 +299,7 @@ class TestReportScheduler { ReadHandler * readHandler = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler); + sScheduler.OnSubscriptionEstablished(readHandler); NL_TEST_ASSERT(aSuite, nullptr != readHandler); VerifyOrReturn(nullptr != readHandler); NL_TEST_ASSERT(aSuite, nullptr != sScheduler.FindReadHandlerNode(readHandler)); @@ -349,31 +364,18 @@ class TestReportScheduler // Test OnReadHandler created ReadHandler * readHandler1 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler); - - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMaxReportingInterval(2)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMinReportingIntervalForTests(1)); - ReadHandlerNode * node = sScheduler.FindReadHandlerNode(readHandler1); - node->SetIntervalTimeStamps(readHandler1); - readHandler1->MoveToState(ReadHandler::HandlerState::GeneratingReports); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler1, &sScheduler, 1, 2)); readHandler1->ForceDirtyState(); // Clean read handler, will be triggered at max interval ReadHandler * readHandler2 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMaxReportingInterval(3)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMinReportingIntervalForTests(0)); - node = sScheduler.FindReadHandlerNode(readHandler2); - node->SetIntervalTimeStamps(readHandler2); - readHandler2->MoveToState(ReadHandler::HandlerState::GeneratingReports); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler2, &sScheduler, 0, 3)); // Clean read handler, will be triggered at max interval, but will be cancelled before ReadHandler * readHandler3 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMaxReportingInterval(3)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMinReportingIntervalForTests(0)); - node = sScheduler.FindReadHandlerNode(readHandler3); - node->SetIntervalTimeStamps(readHandler3); - readHandler3->MoveToState(ReadHandler::HandlerState::GeneratingReports); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler3, &sScheduler, 0, 3)); // Confirms that none of the ReadHandlers are currently reportable NL_TEST_ASSERT(aSuite, !sScheduler.IsReportableNow(readHandler1)); @@ -427,22 +429,20 @@ class TestReportScheduler ReadHandler * readHandler = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler); - // Test OnReadHandler created registered the ReadHandler in the scheduler + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler, &sScheduler, 1, 2)); + + // Verifies OnSubscriptionEstablished registered the ReadHandler in the scheduler NL_TEST_ASSERT(aSuite, nullptr != sScheduler.FindReadHandlerNode(readHandler)); // Should have registered the read handler in the scheduler and scheduled a report NL_TEST_ASSERT(aSuite, sScheduler.GetNumReadHandlers() == 1); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMaxReportingInterval(2)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMinReportingIntervalForTests(1)); + ReadHandlerNode * node = sScheduler.FindReadHandlerNode(readHandler); - node->SetIntervalTimeStamps(readHandler); // Test OnReportingIntervalsChanged modified the intervals and re-scheduled a report NL_TEST_ASSERT(aSuite, node->GetMinTimestamp().count() == 1000); NL_TEST_ASSERT(aSuite, node->GetMaxTimestamp().count() == 2000); - // Do those manually to avoid scheduling an engine run - readHandler->MoveToState(ReadHandler::HandlerState::GeneratingReports); NL_TEST_ASSERT(aSuite, sScheduler.IsReportScheduled(readHandler)); NL_TEST_ASSERT(aSuite, nullptr != node); @@ -459,9 +459,9 @@ class TestReportScheduler // Check that no report is scheduled since the min interval has expired, the timer should now be stopped NL_TEST_ASSERT(aSuite, !sScheduler.IsReportScheduled(readHandler)); - // Test OnSubscriptionAction + // Test OnSubscriptionReportSent readHandler->ClearForceDirtyFlag(); - readHandler->mObserver->OnSubscriptionAction(readHandler); + readHandler->mObserver->OnSubscriptionReportSent(readHandler); // Should have changed the scheduled timeout to the handlers max interval, to check, we wait for the min interval to // confirm it is not expired yet so the report should still be scheduled @@ -505,19 +505,13 @@ class TestReportScheduler ReadHandler * readHandler1 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMaxReportingInterval(2)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMinReportingIntervalForTests(0)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler1, &syncScheduler, 0, 2)); ReadHandlerNode * node1 = syncScheduler.FindReadHandlerNode(readHandler1); - node1->SetIntervalTimeStamps(readHandler1); - readHandler1->MoveToState(ReadHandler::HandlerState::GeneratingReports); ReadHandler * readHandler2 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMaxReportingInterval(3)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMinReportingIntervalForTests(1)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler2, &syncScheduler, 1, 3)); ReadHandlerNode * node2 = syncScheduler.FindReadHandlerNode(readHandler2); - node2->SetIntervalTimeStamps(readHandler2); - readHandler2->MoveToState(ReadHandler::HandlerState::GeneratingReports); // Confirm all handler are currently registered in the scheduler NL_TEST_ASSERT(aSuite, syncScheduler.GetNumReadHandlers() == 2); @@ -544,9 +538,9 @@ class TestReportScheduler NL_TEST_ASSERT(aSuite, !sScheduler.IsReportScheduled(readHandler2)); // Simulate a report emission for readHandler1 - readHandler1->mObserver->OnSubscriptionAction(readHandler1); + readHandler1->mObserver->OnSubscriptionReportSent(readHandler1); // Simulate a report emission for readHandler2 - readHandler2->mObserver->OnSubscriptionAction(readHandler2); + readHandler2->mObserver->OnSubscriptionReportSent(readHandler2); // Validate that the max timestamp for both readhandlers got updated and that the next report emission is scheduled on // the new max timestamp for readhandler1 @@ -572,13 +566,13 @@ class TestReportScheduler NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node2->GetMinTimestamp()); // Simulate a report emission for readHandler1 - readHandler1->mObserver->OnSubscriptionAction(readHandler1); + readHandler1->mObserver->OnSubscriptionReportSent(readHandler1); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1)); // ReadHandler 2 should still be reportable since it hasn't emitted a report yet NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler2)); readHandler2->ClearForceDirtyFlag(); - readHandler2->mObserver->OnSubscriptionAction(readHandler2); + readHandler2->mObserver->OnSubscriptionReportSent(readHandler2); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2)); // Validate next report scheduled on the max timestamp of readHandler1 @@ -594,7 +588,7 @@ class TestReportScheduler // Simulate a report emission for readHandler1 readHandler1->ClearForceDirtyFlag(); - readHandler1->mObserver->OnSubscriptionAction(readHandler1); + readHandler1->mObserver->OnSubscriptionReportSent(readHandler1); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1)); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2)); @@ -605,8 +599,8 @@ class TestReportScheduler sTestTimerSynchronizedDelegate.IncrementMockTimestamp(System::Clock::Milliseconds64(2000)); NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler1)); NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler2)); - readHandler1->mObserver->OnSubscriptionAction(readHandler1); - readHandler2->mObserver->OnSubscriptionAction(readHandler2); + readHandler1->mObserver->OnSubscriptionReportSent(readHandler1); + readHandler2->mObserver->OnSubscriptionReportSent(readHandler2); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1)); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2)); NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node1->GetMaxTimestamp()); @@ -618,11 +612,8 @@ class TestReportScheduler ReadHandler * readHandler3 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMaxReportingInterval(3)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMinReportingIntervalForTests(2)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler3, &syncScheduler, 2, 3)); ReadHandlerNode * node3 = syncScheduler.FindReadHandlerNode(readHandler3); - node3->SetIntervalTimeStamps(readHandler3); - readHandler3->MoveToState(ReadHandler::HandlerState::GeneratingReports); // Confirm all handler are currently registered in the scheduler NL_TEST_ASSERT(aSuite, syncScheduler.GetNumReadHandlers() == 3); @@ -643,8 +634,8 @@ class TestReportScheduler readHandler2->mObserver->OnBecameReportable(readHandler2); // Simulate a report emission for readHandler1 and readHandler2 - readHandler1->mObserver->OnSubscriptionAction(readHandler1); - readHandler1->mObserver->OnSubscriptionAction(readHandler2); + readHandler1->mObserver->OnSubscriptionReportSent(readHandler1); + readHandler1->mObserver->OnSubscriptionReportSent(readHandler2); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1)); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2)); @@ -662,9 +653,9 @@ class TestReportScheduler readHandler2->mObserver->OnBecameReportable(readHandler2); readHandler3->mObserver->OnBecameReportable(readHandler3); // Engine run should happen here and send all reports - readHandler1->mObserver->OnSubscriptionAction(readHandler1); - readHandler2->mObserver->OnSubscriptionAction(readHandler2); - readHandler3->mObserver->OnSubscriptionAction(readHandler3); + readHandler1->mObserver->OnSubscriptionReportSent(readHandler1); + readHandler2->mObserver->OnSubscriptionReportSent(readHandler2); + readHandler3->mObserver->OnSubscriptionReportSent(readHandler3); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1)); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2)); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler3)); @@ -675,11 +666,8 @@ class TestReportScheduler // Now simulate a new readHandler being added with a max forcing a conflict ReadHandler * readHandler4 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler4->SetMaxReportingInterval(1)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler4->SetMinReportingIntervalForTests(0)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler4, &syncScheduler, 0, 1)); ReadHandlerNode * node4 = syncScheduler.FindReadHandlerNode(readHandler4); - node4->SetIntervalTimeStamps(readHandler4); - readHandler4->MoveToState(ReadHandler::HandlerState::GeneratingReports); // Confirm all handler are currently registered in the scheduler NL_TEST_ASSERT(aSuite, syncScheduler.GetNumReadHandlers() == 4); @@ -704,9 +692,9 @@ class TestReportScheduler readHandler4->mObserver->OnBecameReportable(readHandler1); readHandler4->mObserver->OnBecameReportable(readHandler2); readHandler4->mObserver->OnBecameReportable(readHandler4); - readHandler4->mObserver->OnSubscriptionAction(readHandler1); - readHandler4->mObserver->OnSubscriptionAction(readHandler2); - readHandler4->mObserver->OnSubscriptionAction(readHandler4); + readHandler4->mObserver->OnSubscriptionReportSent(readHandler1); + readHandler4->mObserver->OnSubscriptionReportSent(readHandler2); + readHandler4->mObserver->OnSubscriptionReportSent(readHandler4); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1)); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2)); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler4)); @@ -722,10 +710,10 @@ class TestReportScheduler syncScheduler.OnBecameReportable(readHandler2); syncScheduler.OnBecameReportable(readHandler3); syncScheduler.OnBecameReportable(readHandler4); - syncScheduler.OnSubscriptionAction(readHandler1); - syncScheduler.OnSubscriptionAction(readHandler2); - syncScheduler.OnSubscriptionAction(readHandler3); - syncScheduler.OnSubscriptionAction(readHandler4); + syncScheduler.OnSubscriptionReportSent(readHandler1); + syncScheduler.OnSubscriptionReportSent(readHandler2); + syncScheduler.OnSubscriptionReportSent(readHandler3); + syncScheduler.OnSubscriptionReportSent(readHandler4); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1)); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2)); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler3)); @@ -759,19 +747,14 @@ class TestReportScheduler NL_TEST_ASSERT(aSuite, syncScheduler.GetNumReadHandlers() == 0); readHandler1->MoveToState(ReadHandler::HandlerState::Idle); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMaxReportingInterval(2)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMinReportingIntervalForTests(0)); - readHandler1->MoveToState(ReadHandler::HandlerState::GeneratingReports); - syncScheduler.OnReadHandlerCreated(readHandler1); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler1, &syncScheduler, 0, 2)); + // Forcing the dirty flag to make the scheduler call Engine::ScheduleRun() immediately readHandler1->ForceDirtyState(); NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler1)); readHandler2->MoveToState(ReadHandler::HandlerState::Idle); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMaxReportingInterval(4)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMinReportingIntervalForTests(3)); - readHandler2->MoveToState(ReadHandler::HandlerState::GeneratingReports); - syncScheduler.OnReadHandlerCreated(readHandler2); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler2, &syncScheduler, 3, 4)); readHandler2->ForceDirtyState(); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2)); @@ -779,7 +762,7 @@ class TestReportScheduler node2 = syncScheduler.FindReadHandlerNode(readHandler2); readHandler1->ClearForceDirtyFlag(); // report got emited so clear dirty flag - syncScheduler.OnSubscriptionAction(readHandler1); + syncScheduler.OnSubscriptionReportSent(readHandler1); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1)); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2)); @@ -795,7 +778,7 @@ class TestReportScheduler syncScheduler.OnBecameReportable(readHandler1); // simulate run with only readhandler1 reportable - syncScheduler.OnSubscriptionAction(readHandler1); + syncScheduler.OnSubscriptionReportSent(readHandler1); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1)); NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2)); NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node2->GetMinTimestamp()); @@ -806,8 +789,8 @@ class TestReportScheduler NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler2)); readHandler2->ClearForceDirtyFlag(); - syncScheduler.OnSubscriptionAction(readHandler1); - syncScheduler.OnSubscriptionAction(readHandler2); + syncScheduler.OnSubscriptionReportSent(readHandler1); + syncScheduler.OnSubscriptionReportSent(readHandler2); NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node1->GetMaxTimestamp()); NL_TEST_ASSERT(aSuite, node2->GetSyncTimestamp() == node2->GetMaxTimestamp());