[ReadHandler] Unification of serial "now" timestamp

src/app/reporting/ReportScheduler.h

@@ -30,8 +30,6 @@ namespace reporting {
 // Forward declaration of TestReportScheduler to allow it to be friend with ReportScheduler
 class TestReportScheduler;
 
-using Timestamp = System::Clock::Timestamp;
-
 class TimerContext
 {
 public:
@@ -42,6 +40,8 @@ class TimerContext
 class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
 {
 public:
+    using Timestamp = System::Clock::Timestamp;
+
     /// @brief This class acts as an interface between the report scheduler and the system timer to reduce dependencies on the
     /// system layer.
     class TimerDelegate
@@ -63,37 +63,37 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
     class ReadHandlerNode : public TimerContext
     {
     public:
-        ReadHandlerNode(ReadHandler * aReadHandler, TimerDelegate * aTimerDelegate, ReportScheduler * aScheduler) :
-            mTimerDelegate(aTimerDelegate), mScheduler(aScheduler)
+        ReadHandlerNode(ReadHandler * aReadHandler, ReportScheduler * aScheduler, const Timestamp & now) : mScheduler(aScheduler)
         {
             VerifyOrDie(aReadHandler != nullptr);
-            VerifyOrDie(aTimerDelegate != nullptr);
             VerifyOrDie(aScheduler != nullptr);
 
             mReadHandler = aReadHandler;
-            SetIntervalTimeStamps(aReadHandler);
+            SetIntervalTimeStamps(aReadHandler, now);
         }
         ReadHandler * GetReadHandler() const { return mReadHandler; }
 
         /// @brief Check if the Node is reportable now, meaning its readhandler was made reportable by attribute dirtying and
         /// handler state, and minimal time interval since last report has elapsed, or the maximal time interval since last
         /// report has elapsed
-        bool IsReportableNow() const
+        /// @param now current time to use for the check, user must ensure to provide a valid time for this to be reliable
+        bool IsReportableNow(const Timestamp & now) const
         {
-            Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp();
-
             return (mReadHandler->CanStartReporting() &&
                     (now >= mMinTimestamp && (mReadHandler->IsDirty() || now >= mMaxTimestamp || now >= mSyncTimestamp)));
         }
 
         bool IsEngineRunScheduled() const { return mEngineRunScheduled; }
         void SetEngineRunScheduled(bool aEngineRunScheduled) { mEngineRunScheduled = aEngineRunScheduled; }
 
-        void SetIntervalTimeStamps(ReadHandler * aReadHandler)
+        /// @brief Set the interval timestamps for the node based on the read handler reporting intervals
+        /// @param aReadHandler read handler to get the intervals from
+        /// @param now current time to calculate the mMin and mMax timestamps, user must ensure to provide a valid time for this to
+        /// be reliable
+        void SetIntervalTimeStamps(ReadHandler * aReadHandler, const Timestamp & now)
         {
             uint16_t minInterval, maxInterval;
             aReadHandler->GetReportingIntervals(minInterval, maxInterval);
-            Timestamp now  = mTimerDelegate->GetCurrentMonotonicTimestamp();
             mMinTimestamp  = now + System::Clock::Seconds16(minInterval);
             mMaxTimestamp  = now + System::Clock::Seconds16(maxInterval);
             mSyncTimestamp = mMaxTimestamp;
@@ -118,7 +118,6 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
         System::Clock::Timestamp GetSyncTimestamp() const { return mSyncTimestamp; }
 
     private:
-        TimerDelegate * mTimerDelegate;
         ReadHandler * mReadHandler;
         ReportScheduler * mScheduler;
         Timestamp mMinTimestamp;
@@ -141,9 +140,12 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
     /// @param aReadHandler read handler to check
     bool IsReportableNow(ReadHandler * aReadHandler)
     {
+        // Update the now timestamp to ensure external calls to IsReportableNow are always comparing to the current time
+        Timestamp now          = mTimerDelegate->GetCurrentMonotonicTimestamp();
         ReadHandlerNode * node = FindReadHandlerNode(aReadHandler);
-        return (nullptr != node) ? node->IsReportableNow() : false;
+        return (nullptr != node) ? node->IsReportableNow(now) : false;
     }
+
     /// @brief Check if a ReadHandler is reportable without considering the timing
     bool IsReadHandlerReportable(ReadHandler * aReadHandler) const { return aReadHandler->ShouldStartReporting(); }
     /// @brief Sets the ForceDirty flag of a ReadHandler

src/app/reporting/ReportSchedulerImpl.cpp

@@ -43,7 +43,7 @@ void ReportSchedulerImpl::OnEnterActiveMode()
 {
 #if ICD_REPORT_ON_ENTER_ACTIVE_MODE
     Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp();
-    mNodesPool.ForEachActiveObject([now, this](ReadHandlerNode * node) {
+    mNodesPool.ForEachActiveObject([now](ReadHandlerNode * node) {
         if (now >= node->GetMinTimestamp())
         {
             this->HandlerForceDirtyState(node->GetReadHandler());
@@ -62,9 +62,12 @@ void ReportSchedulerImpl::OnSubscriptionEstablished(ReadHandler * aReadHandler)
     ReadHandlerNode * newNode = FindReadHandlerNode(aReadHandler);
     // Handler must not be registered yet; it's just being constructed.
     VerifyOrDie(nullptr == newNode);
+
+    Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp();
+
     // The NodePool is the same size as the ReadHandler pool from the IM Engine, so we don't need a check for size here since if a
     // ReadHandler was created, space should be available.
-    newNode = mNodesPool.CreateObject(aReadHandler, mTimerDelegate, this);
+    newNode = mNodesPool.CreateObject(aReadHandler, this, now);
 
     ChipLogProgress(DataManagement,
                     "Registered a ReadHandler that will schedule a report between system Timestamp: %" PRIu64
@@ -77,19 +80,25 @@ void ReportSchedulerImpl::OnBecameReportable(ReadHandler * aReadHandler)
 {
     ReadHandlerNode * node = FindReadHandlerNode(aReadHandler);
     VerifyOrReturn(nullptr != node);
+
+    Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp();
+
     Milliseconds32 newTimeout;
-    CalculateNextReportTimeout(newTimeout, node);
-    ScheduleReport(newTimeout, node);
+    CalculateNextReportTimeout(newTimeout, node, now);
+    ScheduleReport(newTimeout, node, now);
 }
 
 void ReportSchedulerImpl::OnSubscriptionReportSent(ReadHandler * aReadHandler)
 {
     ReadHandlerNode * node = FindReadHandlerNode(aReadHandler);
     VerifyOrReturn(nullptr != node);
-    node->SetIntervalTimeStamps(aReadHandler);
+
+    Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp();
+
+    node->SetIntervalTimeStamps(aReadHandler, now);
     Milliseconds32 newTimeout;
-    CalculateNextReportTimeout(newTimeout, node);
-    ScheduleReport(newTimeout, node);
+    CalculateNextReportTimeout(newTimeout, node, now);
+    ScheduleReport(newTimeout, node, now);
     node->SetEngineRunScheduled(false);
 }
 
@@ -105,7 +114,7 @@ void ReportSchedulerImpl::OnReadHandlerDestroyed(ReadHandler * aReadHandler)
     mNodesPool.ReleaseObject(removeNode);
 }
 
-CHIP_ERROR ReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandlerNode * node)
+CHIP_ERROR ReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandlerNode * node, const Timestamp & now)
 {
     // Cancel Report if it is currently scheduled
     mTimerDelegate->CancelTimer(node);
@@ -141,13 +150,12 @@ bool ReportSchedulerImpl::IsReportScheduled(ReadHandler * aReadHandler)
     return mTimerDelegate->IsTimerActive(node);
 }
 
-CHIP_ERROR ReportSchedulerImpl::CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode)
+CHIP_ERROR ReportSchedulerImpl::CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode, const Timestamp & now)
 {
     VerifyOrReturnError(nullptr != FindReadHandlerNode(aNode->GetReadHandler()), CHIP_ERROR_INVALID_ARGUMENT);
-    Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp();
 
     // If the handler is reportable now, just schedule a report immediately
-    if (aNode->IsReportableNow())
+    if (aNode->IsReportableNow(now))
     {
         // If the handler is reportable now, just schedule a report immediately
         timeout = Milliseconds32(0);

src/app/reporting/ReportSchedulerImpl.h

@@ -46,14 +46,14 @@ class ReportSchedulerImpl : public ReportScheduler
     void ReportTimerCallback() override;
 
 protected:
-    virtual CHIP_ERROR ScheduleReport(Timeout timeout, ReadHandlerNode * node);
+    virtual CHIP_ERROR ScheduleReport(Timeout timeout, ReadHandlerNode * node, const Timestamp & now);
     void CancelReport(ReadHandler * aReadHandler);
     virtual void UnregisterAllHandlers();
 
 private:
     friend class chip::app::reporting::TestReportScheduler;
 
-    virtual CHIP_ERROR CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode);
+    virtual CHIP_ERROR CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode, const Timestamp & now);
 };
 
 } // namespace reporting

src/app/reporting/SynchronizedReportSchedulerImpl.cpp

@@ -44,7 +44,7 @@ void SynchronizedReportSchedulerImpl::OnReadHandlerDestroyed(ReadHandler * aRead
     }
 }
 
-CHIP_ERROR SynchronizedReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandlerNode * node)
+CHIP_ERROR SynchronizedReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandlerNode * node, const Timestamp & now)
 {
     // Cancel Report if it is currently scheduled
     mTimerDelegate->CancelTimer(this);
@@ -54,7 +54,7 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::ScheduleReport(Timeout timeout, Read
         return CHIP_NO_ERROR;
     }
     ReturnErrorOnFailure(mTimerDelegate->StartTimer(this, timeout));
-    mTestNextReportTimestamp = mTimerDelegate->GetCurrentMonotonicTimestamp() + timeout;
+    mTestNextReportTimestamp = now + timeout;
 
     return CHIP_NO_ERROR;
 }
@@ -73,10 +73,9 @@ bool SynchronizedReportSchedulerImpl::IsReportScheduled()
 
 /// @brief Find the smallest maximum interval possible and set it as the common maximum
 /// @return NO_ERROR if the smallest maximum interval was found, error otherwise, INVALID LIST LENGTH if the list is empty
-CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMaxInterval()
+CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMaxInterval(const Timestamp & now)
 {
     VerifyOrReturnError(mNodesPool.Allocated(), CHIP_ERROR_INVALID_LIST_LENGTH);
-    System::Clock::Timestamp now      = mTimerDelegate->GetCurrentMonotonicTimestamp();
     System::Clock::Timestamp earliest = now + Seconds16::max();
 
     mNodesPool.ForEachActiveObject([&earliest, now](ReadHandlerNode * node) {
@@ -97,10 +96,10 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMaxInterval()
 /// minimum. If the max timestamp has not been updated and is in the past, or if no min timestamp is lower than the current max
 /// timestamp, this will set now as the common minimum timestamp, thus allowing the report to be sent immediately.
 /// @return NO_ERROR if the highest minimum timestamp was found, error otherwise, INVALID LIST LENGTH if the list is empty
-CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMinInterval()
+CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMinInterval(const Timestamp & now)
 {
     VerifyOrReturnError(mNodesPool.Allocated(), CHIP_ERROR_INVALID_LIST_LENGTH);
-    System::Clock::Timestamp latest = mTimerDelegate->GetCurrentMonotonicTimestamp();
+    System::Clock::Timestamp latest = now;
 
     mNodesPool.ForEachActiveObject([&latest, this](ReadHandlerNode * node) {
         if (node->GetMinTimestamp() > latest && this->IsReadHandlerReportable(node->GetReadHandler()) &&
@@ -118,20 +117,19 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMinInterval()
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode)
+CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode,
+                                                                       const Timestamp & now)
 {
     VerifyOrReturnError(nullptr != FindReadHandlerNode(aNode->GetReadHandler()), CHIP_ERROR_INVALID_ARGUMENT);
-    ReturnErrorOnFailure(FindNextMaxInterval());
-    ReturnErrorOnFailure(FindNextMinInterval());
+    ReturnErrorOnFailure(FindNextMaxInterval(now));
+    ReturnErrorOnFailure(FindNextMinInterval(now));
     bool reportableNow   = false;
     bool reportableAtMin = false;
 
-    Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp();
-
-    mNodesPool.ForEachActiveObject([&reportableNow, &reportableAtMin, this](ReadHandlerNode * node) {
+    mNodesPool.ForEachActiveObject([&reportableNow, &reportableAtMin, this, now](ReadHandlerNode * node) {
         if (!node->IsEngineRunScheduled())
         {
-            if (node->IsReportableNow())
+            if (node->IsReportableNow(now))
             {
                 reportableNow = true;
                 return Loop::Break;
@@ -166,7 +164,7 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout &
     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())
+        if (!node->IsReportableNow(now))
         {
             node->SetSyncTimestamp(Milliseconds64(now + timeout));
         }
@@ -181,10 +179,12 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout &
 /// the engine already verifies that read handlers are reportable before sending a report
 void SynchronizedReportSchedulerImpl::TimerFired()
 {
+    Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp();
+
     InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun();
 
-    mNodesPool.ForEachActiveObject([](ReadHandlerNode * node) {
-        if (node->IsReportableNow())
+    mNodesPool.ForEachActiveObject([now](ReadHandlerNode * node) {
+        if (node->IsReportableNow(now))
         {
             node->SetEngineRunScheduled(true);
             ChipLogProgress(DataManagement, "Handler: %p with min: %" PRIu64 " and max: %" PRIu64 " and sync: %" PRIu64, (node),

src/app/reporting/SynchronizedReportSchedulerImpl.h

@@ -43,15 +43,15 @@ class SynchronizedReportSchedulerImpl : public ReportSchedulerImpl, public Timer
     void TimerFired() override;
 
 protected:
-    CHIP_ERROR ScheduleReport(System::Clock::Timeout timeout, ReadHandlerNode * node) override;
+    CHIP_ERROR ScheduleReport(System::Clock::Timeout timeout, ReadHandlerNode * node, const Timestamp & now) override;
     void CancelReport();
 
 private:
     friend class chip::app::reporting::TestReportScheduler;
 
-    CHIP_ERROR FindNextMinInterval();
-    CHIP_ERROR FindNextMaxInterval();
-    CHIP_ERROR CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aReadHandlerNode) override;
+    CHIP_ERROR FindNextMinInterval(const Timestamp & now);
+    CHIP_ERROR FindNextMaxInterval(const Timestamp & now);
+    CHIP_ERROR CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aReadHandlerNode, const Timestamp & now) override;
 
     Timestamp mNextMaxTimestamp = Milliseconds64(0);
     Timestamp mNextMinTimestamp = Milliseconds64(0);