Replaced the Sync timestamp in the report scheduler with a CanBeSynce…

…d flag and combined it with the IsEngineRunScheduled flag. Modified the logic to preserve the same synching behaviour (#28733)
project-chip · Aug 26, 2023 · 1032224 · 1032224
1 parent 5e1dfaa
commit 1032224
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 73 deletions.
diff --git 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
@@ -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
@@ -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;