From 30e4169875b24cb6333e25f45523fd1be84c645f Mon Sep 17 00:00:00 2001
From: lpbeliveau-silabs <112982107+lpbeliveau-silabs@users.noreply.github.com>
Date: Thu, 17 Aug 2023 16:16:57 -0400
Subject: [PATCH] Replaced the Sync timestamp in the report scheduler with a
 CanBeSynced flag and combined it with the IsEngineRunScheduled flag. Modified
 the logic to preserve the same synching behaviour (#28733)

---
 src/app/reporting/ReportScheduler.h           |  40 +++----
 src/app/reporting/ReportSchedulerImpl.cpp     |   1 +
 .../SynchronizedReportSchedulerImpl.cpp       |  23 ++--
 src/app/tests/TestReportScheduler.cpp         | 102 +++++++++++-------
 4 files changed, 93 insertions(+), 73 deletions(-)

diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h
index b6abbdb478e506..ae657fa51d0404 100644
--- a/src/app/reporting/ReportScheduler.h
+++ b/src/app/reporting/ReportScheduler.h
@@ -63,6 +63,15 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
     class ReadHandlerNode : public TimerContext
     {
     public:
+        enum class ReadHandlerNodeFlags : uint8_t
+        {
+            // Flag to indicate if the engine run is already scheduled so the scheduler can ignore
+            // it when calculating the next run time
+            EngineRunScheduled = (1 << 0),
+            // Flag to allow the read handler to be synced with other handlers that have an earlier max timestamp
+            CanBeSynced = (1 << 1),
+        };
+
         ReadHandlerNode(ReadHandler * aReadHandler, ReportScheduler * aScheduler, const Timestamp & now) : mScheduler(aScheduler)
         {
             VerifyOrDie(aReadHandler != nullptr);
@@ -80,11 +89,16 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
         bool IsReportableNow(const Timestamp & now) const
         {
             return (mReadHandler->CanStartReporting() &&
-                    (now >= mMinTimestamp && (mReadHandler->IsDirty() || now >= mMaxTimestamp || now >= mSyncTimestamp)));
+                    (now >= mMinTimestamp && (mReadHandler->IsDirty() || now >= mMaxTimestamp || CanBeSynced())));
         }
 
-        bool IsEngineRunScheduled() const { return mEngineRunScheduled; }
-        void SetEngineRunScheduled(bool aEngineRunScheduled) { mEngineRunScheduled = aEngineRunScheduled; }
+        bool IsEngineRunScheduled() const { return mFlags.Has(ReadHandlerNodeFlags::EngineRunScheduled); }
+        void SetEngineRunScheduled(bool aEngineRunScheduled)
+        {
+            mFlags.Set(ReadHandlerNodeFlags::EngineRunScheduled, aEngineRunScheduled);
+        }
+        bool CanBeSynced() const { return mFlags.Has(ReadHandlerNodeFlags::CanBeSynced); }
+        void SetCanBeSynced(bool aCanBeSynced) { mFlags.Set(ReadHandlerNodeFlags::CanBeSynced, aCanBeSynced); }
 
         /// @brief Set the interval timestamps for the node based on the read handler reporting intervals
         /// @param aReadHandler read handler to get the intervals from
@@ -94,9 +108,8 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
         {
             uint16_t minInterval, maxInterval;
             aReadHandler->GetReportingIntervals(minInterval, maxInterval);
-            mMinTimestamp  = now + System::Clock::Seconds16(minInterval);
-            mMaxTimestamp  = now + System::Clock::Seconds16(maxInterval);
-            mSyncTimestamp = mMaxTimestamp;
+            mMinTimestamp = now + System::Clock::Seconds16(minInterval);
+            mMaxTimestamp = now + System::Clock::Seconds16(maxInterval);
         }
 
         void TimerFired() override
@@ -105,27 +118,16 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
             SetEngineRunScheduled(true);
         }
 
-        void SetSyncTimestamp(System::Clock::Timestamp aSyncTimestamp)
-        {
-            // Prevents the sync timestamp being set to a value lower than the min timestamp to prevent it to appear as reportable
-            // on the next timeout calculation and cause the scheduler to run the engine too early
-            VerifyOrReturn(aSyncTimestamp >= mMinTimestamp);
-            mSyncTimestamp = aSyncTimestamp;
-        }
-
         System::Clock::Timestamp GetMinTimestamp() const { return mMinTimestamp; }
         System::Clock::Timestamp GetMaxTimestamp() const { return mMaxTimestamp; }
-        System::Clock::Timestamp GetSyncTimestamp() const { return mSyncTimestamp; }
 
     private:
         ReadHandler * mReadHandler;
         ReportScheduler * mScheduler;
         Timestamp mMinTimestamp;
         Timestamp mMaxTimestamp;
-        Timestamp mSyncTimestamp; // Timestamp at which the read handler will be allowed to emit a report so it can be synced with
-                                  // other handlers that have an earlier max timestamp
-        bool mEngineRunScheduled = false; // Flag to indicate if the engine run is already scheduled so the scheduler can ignore
-                                          // it when calculating the next run time
+
+        BitFlags<ReadHandlerNodeFlags> mFlags;
     };
 
     ReportScheduler(TimerDelegate * aTimerDelegate) : mTimerDelegate(aTimerDelegate) {}
diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp
index 632d2f456990cd..41d80cc7114ae6 100644
--- a/src/app/reporting/ReportSchedulerImpl.cpp
+++ b/src/app/reporting/ReportSchedulerImpl.cpp
@@ -95,6 +95,7 @@ void ReportSchedulerImpl::OnSubscriptionReportSent(ReadHandler * aReadHandler)
 
     Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp();
 
+    node->SetCanBeSynced(false);
     node->SetIntervalTimeStamps(aReadHandler, now);
     Milliseconds32 newTimeout;
     CalculateNextReportTimeout(newTimeout, node, now);
diff --git a/src/app/reporting/SynchronizedReportSchedulerImpl.cpp b/src/app/reporting/SynchronizedReportSchedulerImpl.cpp
index d9a410f555e506..ddb8bb7b1c4149 100644
--- a/src/app/reporting/SynchronizedReportSchedulerImpl.cpp
+++ b/src/app/reporting/SynchronizedReportSchedulerImpl.cpp
@@ -50,7 +50,7 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::ScheduleReport(Timeout timeout, Read
     mTimerDelegate->CancelTimer(this);
     if (timeout == Milliseconds32(0))
     {
-        ReportTimerCallback();
+        TimerFired();
         return CHIP_NO_ERROR;
     }
     ReturnErrorOnFailure(mTimerDelegate->StartTimer(this, timeout));
@@ -160,18 +160,6 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout &
         timeout = mNextMaxTimestamp - now;
     }
 
-    // Updates the synching time of each handler
-    mNodesPool.ForEachActiveObject([now, timeout](ReadHandlerNode * node) {
-        // Prevent modifying the sync if the handler is currently reportable, sync's purpose is to allow handler to become
-        // reportable earlier than their max interval
-        if (!node->IsReportableNow(now))
-        {
-            node->SetSyncTimestamp(Milliseconds64(now + timeout));
-        }
-
-        return Loop::Continue;
-    });
-
     return CHIP_NO_ERROR;
 }
 
@@ -184,11 +172,16 @@ void SynchronizedReportSchedulerImpl::TimerFired()
     InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun();
 
     mNodesPool.ForEachActiveObject([now](ReadHandlerNode * node) {
+        if (node->GetMinTimestamp() <= now)
+        {
+            node->SetCanBeSynced(true);
+        }
+
         if (node->IsReportableNow(now))
         {
             node->SetEngineRunScheduled(true);
-            ChipLogProgress(DataManagement, "Handler: %p with min: %" PRIu64 " and max: %" PRIu64 " and sync: %" PRIu64, (node),
-                            node->GetMinTimestamp().count(), node->GetMaxTimestamp().count(), node->GetSyncTimestamp().count());
+            ChipLogProgress(DataManagement, "Handler: %p with min: %" PRIu64 " and max: %" PRIu64 "", (node),
+                            node->GetMinTimestamp().count(), node->GetMaxTimestamp().count());
         }
 
         return Loop::Continue;
diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp
index 5f8fd3ffd36949..c626202f82f629 100644
--- a/src/app/tests/TestReportScheduler.cpp
+++ b/src/app/tests/TestReportScheduler.cpp
@@ -250,8 +250,8 @@ class TestReportScheduler
     /// 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)
+    static CHIP_ERROR MockReadHandlerSubscriptionTransaction(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));
@@ -364,18 +364,18 @@ class TestReportScheduler
         // Test OnReadHandler created
         ReadHandler * readHandler1 =
             readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler);
-        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler1, &sScheduler, 1, 2));
+        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransaction(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 == MockReadHandlerSubscriptionTransation(readHandler2, &sScheduler, 0, 3));
+        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransaction(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 == MockReadHandlerSubscriptionTransation(readHandler3, &sScheduler, 0, 3));
+        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransaction(readHandler3, &sScheduler, 0, 3));
 
         // Confirms that none of the ReadHandlers are currently reportable
         NL_TEST_ASSERT(aSuite, !sScheduler.IsReportableNow(readHandler1));
@@ -429,7 +429,7 @@ class TestReportScheduler
 
         ReadHandler * readHandler =
             readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler);
-        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler, &sScheduler, 1, 2));
+        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransaction(readHandler, &sScheduler, 1, 2));
 
         // Verifies OnSubscriptionEstablished registered the ReadHandler in the scheduler
         NL_TEST_ASSERT(aSuite, nullptr != sScheduler.FindReadHandlerNode(readHandler));
@@ -505,12 +505,12 @@ class TestReportScheduler
 
         ReadHandler * readHandler1 =
             readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler);
-        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler1, &syncScheduler, 0, 2));
+        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransaction(readHandler1, &syncScheduler, 0, 2));
         ReadHandlerNode * node1 = syncScheduler.FindReadHandlerNode(readHandler1);
 
         ReadHandler * readHandler2 =
             readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler);
-        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler2, &syncScheduler, 1, 3));
+        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransaction(readHandler2, &syncScheduler, 1, 3));
         ReadHandlerNode * node2 = syncScheduler.FindReadHandlerNode(readHandler2);
 
         // Confirm all handler are currently registered in the scheduler
@@ -558,12 +558,10 @@ class TestReportScheduler
         NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler2));
         NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler1));
 
-        // confirm report scheduled now
-        NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node2->GetMinTimestamp());
-        NL_TEST_ASSERT(aSuite, node1->GetSyncTimestamp() == node2->GetMinTimestamp());
-
-        // Confirm that the next report emission is scheduled on the min timestamp of readHandler2 as it is the highest reportable
+        // Confirm that the next report emission is scheduled on the min timestamp of readHandler2 (now) as it is the highest
+        // reportable
         NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node2->GetMinTimestamp());
+        NL_TEST_ASSERT(aSuite, node1->CanBeSynced() == true);
 
         // Simulate a report emission for readHandler1
         readHandler1->mObserver->OnSubscriptionReportSent(readHandler1);
@@ -575,11 +573,36 @@ class TestReportScheduler
         readHandler2->mObserver->OnSubscriptionReportSent(readHandler2);
         NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2));
 
+        // Simulate ReadHandler 1 becoming dirty after ReadHandler 2 past min will trigger a report emission for both
+
+        // Wait past ReadHandler 2 min
+        sTestTimerSynchronizedDelegate.IncrementMockTimestamp(System::Clock::Milliseconds64(1100));
+        // No handler should be reportable yet
+        NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2));
+        NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1));
+        readHandler1->ForceDirtyState();
+
+        // Both read handlers should now be reportable since the ForceDirty should immediately trigger the timer expiration callback
+        NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler2));
+        NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler1));
+
+        // Simulate a report emission for readHandler1
+        readHandler1->ClearForceDirtyFlag();
+        readHandler1->mObserver->OnSubscriptionReportSent(readHandler1);
+        NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1));
+
+        readHandler2->mObserver->OnSubscriptionReportSent(readHandler2);
+        NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2));
+
+        // Confirm both handlers are not reportable anymore
+        NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2));
+        NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1));
+
         // Validate next report scheduled on the max timestamp of readHandler1
         NL_TEST_ASSERT(aSuite, node1->GetMaxTimestamp() > sTestTimerSynchronizedDelegate.GetCurrentMonotonicTimestamp());
         NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node1->GetMaxTimestamp());
 
-        // Simulate readHandler1 becoming dirty after less than 1 seconds, since it is reportable now, this will Schedule an Engin
+        // Simulate readHandler1 becoming dirty after less than 1 seconds, since it is reportable now, this will Schedule an Engine
         // run immediately
         sTestTimerSynchronizedDelegate.IncrementMockTimestamp(System::Clock::Milliseconds64(900));
         readHandler1->ForceDirtyState();
@@ -592,11 +615,13 @@ class TestReportScheduler
         NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1));
         NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2));
 
-        // The next report should be scheduler on the max timestamp of readHandler1 and readHandler2 should be synced
+        // The next report should be scheduler on the max timestamp of readHandler1
         NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node1->GetMaxTimestamp());
-        NL_TEST_ASSERT(aSuite, node2->GetSyncTimestamp() == node1->GetMaxTimestamp());
 
         sTestTimerSynchronizedDelegate.IncrementMockTimestamp(System::Clock::Milliseconds64(2000));
+        // Confirm node 2 can now be synced since the scheduler timer has fired on the max timestamp of readHandler1
+        NL_TEST_ASSERT(aSuite, node2->CanBeSynced() == true);
+
         NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler1));
         NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler2));
         readHandler1->mObserver->OnSubscriptionReportSent(readHandler1);
@@ -612,7 +637,7 @@ class TestReportScheduler
 
         ReadHandler * readHandler3 =
             readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler);
-        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler3, &syncScheduler, 2, 3));
+        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransaction(readHandler3, &syncScheduler, 2, 3));
         ReadHandlerNode * node3 = syncScheduler.FindReadHandlerNode(readHandler3);
 
         // Confirm all handler are currently registered in the scheduler
@@ -641,10 +666,11 @@ class TestReportScheduler
 
         // Confirm that next report is scheduled on the max timestamp of readHandler3 and other 2 readHandlers are synced
         NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node3->GetMaxTimestamp());
-        NL_TEST_ASSERT(aSuite, node1->GetSyncTimestamp() == node3->GetMaxTimestamp());
-        NL_TEST_ASSERT(aSuite, node2->GetSyncTimestamp() == node3->GetMaxTimestamp());
 
         sTestTimerSynchronizedDelegate.IncrementMockTimestamp(System::Clock::Milliseconds64(2000));
+        // Confirm nodes 1 and 2 can now be synced since the scheduler timer has fired on the max timestamp of readHandler1
+        NL_TEST_ASSERT(aSuite, node1->CanBeSynced() == true);
+        NL_TEST_ASSERT(aSuite, node2->CanBeSynced() == true);
 
         NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler1));
         NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler2));
@@ -660,27 +686,25 @@ class TestReportScheduler
         NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2));
         NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler3));
         NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node1->GetMaxTimestamp());
-        NL_TEST_ASSERT(aSuite, node1->GetSyncTimestamp() == node1->GetMaxTimestamp());
-        NL_TEST_ASSERT(aSuite, node2->GetSyncTimestamp() == node1->GetMaxTimestamp());
 
         // 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 == MockReadHandlerSubscriptionTransation(readHandler4, &syncScheduler, 0, 1));
+        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransaction(readHandler4, &syncScheduler, 0, 1));
         ReadHandlerNode * node4 = syncScheduler.FindReadHandlerNode(readHandler4);
 
         // Confirm all handler are currently registered in the scheduler
         NL_TEST_ASSERT(aSuite, syncScheduler.GetNumReadHandlers() == 4);
 
-        // Confirm next report is scheduled on the max timestamp of readHandler4 and other handlers 1 and 2 are synced
+        // Confirm next report is scheduled on the max timestamp of readHandler4
         NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node4->GetMaxTimestamp());
-        NL_TEST_ASSERT(aSuite, node1->GetSyncTimestamp() == node4->GetMaxTimestamp());
-        NL_TEST_ASSERT(aSuite, node2->GetSyncTimestamp() == node4->GetMaxTimestamp());
-
-        // Confirm handler 3 is synched on a later timestamp since its min is higher than the max of readHandler4
-        NL_TEST_ASSERT(aSuite, node3->GetSyncTimestamp() == node1->GetMaxTimestamp());
 
         sTestTimerSynchronizedDelegate.IncrementMockTimestamp(System::Clock::Milliseconds64(1100));
+        // Confirm node 1 and 2 can now be synced since the scheduler timer has fired on the max timestamp of readHandler4
+        NL_TEST_ASSERT(aSuite, node1->CanBeSynced() == true);
+        NL_TEST_ASSERT(aSuite, node2->CanBeSynced() == true);
+        // Confirm handler 3 cannot be synched on a later timestamp since its min is higher than the max of readHandler4
+        NL_TEST_ASSERT(aSuite, node3->CanBeSynced() == false);
 
         // Confirm readHandler1, 2 and 4 are reportable
         NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler1));
@@ -719,15 +743,15 @@ class TestReportScheduler
         NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler3));
         NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler4));
 
-        // Next emission should be scheduled on the max timestamp of readHandler4 as it is the most restrictive, and handlers 1 and
-        // 2 should be synced to handler 4
+        // Next emission should be scheduled on the max timestamp of readHandler4 as it is the most restrictive
         NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node4->GetMaxTimestamp());
-        NL_TEST_ASSERT(aSuite, node1->GetSyncTimestamp() == node4->GetMaxTimestamp());
-        NL_TEST_ASSERT(aSuite, node2->GetSyncTimestamp() == node4->GetMaxTimestamp());
-        // handler 3 should have a sync on a different point as its min is higher, in this case it is the max timestamp of handler 1
-        NL_TEST_ASSERT(aSuite, node3->GetSyncTimestamp() == node1->GetMaxTimestamp());
 
         sTestTimerSynchronizedDelegate.IncrementMockTimestamp(System::Clock::Milliseconds64(1000));
+        // Confirm node 1 and 2 can now be synced since the scheduler timer has fired on the max timestamp of readHandler4
+        NL_TEST_ASSERT(aSuite, node1->CanBeSynced() == true);
+        NL_TEST_ASSERT(aSuite, node2->CanBeSynced() == true);
+        // Confirm node 3 still cannot sync
+        NL_TEST_ASSERT(aSuite, node3->CanBeSynced() == false);
 
         // Confirm  readHandler 1-2-4 are reportable
         NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler1));
@@ -747,14 +771,14 @@ class TestReportScheduler
         NL_TEST_ASSERT(aSuite, syncScheduler.GetNumReadHandlers() == 0);
 
         readHandler1->MoveToState(ReadHandler::HandlerState::Idle);
-        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransation(readHandler1, &syncScheduler, 0, 2));
+        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransaction(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 == MockReadHandlerSubscriptionTransation(readHandler2, &syncScheduler, 3, 4));
+        NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == MockReadHandlerSubscriptionTransaction(readHandler2, &syncScheduler, 3, 4));
         readHandler2->ForceDirtyState();
         NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2));
 
@@ -769,7 +793,7 @@ class TestReportScheduler
         // Confirm next report is scheduled on the max timestamp of readHandler1 and readhandler2 is not synced
         NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node1->GetMaxTimestamp());
         // Node 2's sync timestamp should have remained unaffected since its min is higher
-        NL_TEST_ASSERT(aSuite, node2->GetSyncTimestamp() == node2->GetMaxTimestamp());
+        NL_TEST_ASSERT(aSuite, node2->CanBeSynced() == false);
 
         sTestTimerSynchronizedDelegate.IncrementMockTimestamp(System::Clock::Milliseconds64(2000));
         // Verify handler 1 became reportable
@@ -782,9 +806,9 @@ class TestReportScheduler
         NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler1));
         NL_TEST_ASSERT(aSuite, !syncScheduler.IsReportableNow(readHandler2));
         NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node2->GetMinTimestamp());
-        NL_TEST_ASSERT(aSuite, node1->GetSyncTimestamp() == node2->GetMinTimestamp());
 
         sTestTimerSynchronizedDelegate.IncrementMockTimestamp(System::Clock::Milliseconds64(1000));
+        NL_TEST_ASSERT(aSuite, node1->CanBeSynced() == true);
         NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler1));
         NL_TEST_ASSERT(aSuite, syncScheduler.IsReportableNow(readHandler2));
 
@@ -793,7 +817,7 @@ class TestReportScheduler
         syncScheduler.OnSubscriptionReportSent(readHandler2);
 
         NL_TEST_ASSERT(aSuite, syncScheduler.mTestNextReportTimestamp == node1->GetMaxTimestamp());
-        NL_TEST_ASSERT(aSuite, node2->GetSyncTimestamp() == node2->GetMaxTimestamp());
+        NL_TEST_ASSERT(aSuite, node2->CanBeSynced() == false);
 
         syncScheduler.UnregisterAllHandlers();
         readHandlerPool.ReleaseAll();