[ReportScheduler] Fixed chunked reporting for Synchronized Report Sch…

…eduler (#30608) * Fixed bug where chunked reports would fail on Synchronized report scheduler and added tests for Syncrhonized report scheduler interaction Also fixed behavior of Synchronized Report scheduler on Early Reports cases * Update src/app/reporting/SynchronizedReportSchedulerImpl.cpp Co-authored-by: Junior Martinez <[email protected]> * Clarified comment * Removed check on unused parameter in CalculateNextReportTiemout for Sync Scheduler --------- Co-authored-by: Junior Martinez <[email protected]>
project-chip · Feb 9, 2024 · 2298968 · 2298968
1 parent 2afa857
commit 2298968
Show file tree

Hide file tree

Showing 7 changed files with 351 additions and 228 deletions.
diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h
@@ -96,6 +96,7 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
                      IsEngineRunScheduled()));
         }
 
+        bool IsChunkedReport() const { return mReadHandler->IsChunkedReport(); }
         bool IsEngineRunScheduled() const { return mFlags.Has(ReadHandlerNodeFlags::EngineRunScheduled); }
         void SetEngineRunScheduled(bool aEngineRunScheduled)
         {
@@ -153,7 +154,10 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
     }
 
     /// @brief Check if a ReadHandler is reportable without considering the timing
-    bool IsReadHandlerReportable(ReadHandler * aReadHandler) const { return aReadHandler->ShouldStartReporting(); }
+    bool IsReadHandlerReportable(ReadHandler * aReadHandler) const
+    {
+        return (nullptr != aReadHandler) ? aReadHandler->ShouldStartReporting() : false;
+    }
     /// @brief Sets the ForceDirty flag of a ReadHandler
     void HandlerForceDirtyState(ReadHandler * aReadHandler) { aReadHandler->ForceDirtyState(); }
 

diff --git a/src/app/reporting/ReportSchedulerImpl.h b/src/app/reporting/ReportSchedulerImpl.h
@@ -44,7 +44,7 @@ class ReportSchedulerImpl : public ReportScheduler
     void OnSubscriptionReportSent(ReadHandler * aReadHandler) final;
     void OnReadHandlerDestroyed(ReadHandler * aReadHandler) override;
 
-    bool IsReportScheduled(ReadHandler * aReadHandler);
+    virtual bool IsReportScheduled(ReadHandler * aReadHandler);
 
     void ReportTimerCallback() override;
 

diff --git a/src/app/reporting/SynchronizedReportSchedulerImpl.cpp b/src/app/reporting/SynchronizedReportSchedulerImpl.cpp
@@ -79,7 +79,7 @@ void SynchronizedReportSchedulerImpl::CancelReport()
 }
 
 /// @brief Checks if the timer is active for the ReportScheduler
-bool SynchronizedReportSchedulerImpl::IsReportScheduled()
+bool SynchronizedReportSchedulerImpl::IsReportScheduled(ReadHandler * ReadHandler)
 {
     return mTimerDelegate->IsTimerActive(this);
 }
@@ -133,14 +133,13 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMinInterval(const Timestamp
 CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode,
                                                                        const Timestamp & now)
 {
-    VerifyOrReturnError(nullptr != FindReadHandlerNode(aNode->GetReadHandler()), CHIP_ERROR_INVALID_ARGUMENT);
     ReturnErrorOnFailure(FindNextMaxInterval(now));
     ReturnErrorOnFailure(FindNextMinInterval(now));
     bool reportableNow   = false;
     bool reportableAtMin = false;
 
     mNodesPool.ForEachActiveObject([&reportableNow, &reportableAtMin, this, now](ReadHandlerNode * node) {
-        if (!node->IsEngineRunScheduled())
+        if (!node->IsEngineRunScheduled() || node->IsChunkedReport())
         {
             if (node->IsReportableNow(now))
             {
@@ -180,25 +179,39 @@ 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();
+    Timestamp now   = mTimerDelegate->GetCurrentMonotonicTimestamp();
+    bool firedEarly = true;
 
-    InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun();
-
-    mNodesPool.ForEachActiveObject([now](ReadHandlerNode * node) {
+    mNodesPool.ForEachActiveObject([now, &firedEarly](ReadHandlerNode * node) {
         if (node->GetMinTimestamp() <= now)
         {
             node->SetCanBeSynced(true);
         }
 
         if (node->IsReportableNow(now))
         {
+            // We set firedEarly false here because we assume we fired the timer early if no handler is reportable at the moment,
+            // which becomes false if we find a handler that is reportable
+            firedEarly = false;
             node->SetEngineRunScheduled(true);
             ChipLogProgress(DataManagement, "Handler: %p with min: 0x" ChipLogFormatX64 " and max: 0x" ChipLogFormatX64 "", (node),
                             ChipLogValueX64(node->GetMinTimestamp().count()), ChipLogValueX64(node->GetMaxTimestamp().count()));
         }
 
         return Loop::Continue;
     });
+
+    // If there are no handlers registers, no need to schedule the next report
+    if (mNodesPool.Allocated() && firedEarly)
+    {
+        Timeout timeout = Milliseconds32(0);
+        ReturnOnFailure(CalculateNextReportTimeout(timeout, nullptr, now));
+        ScheduleReport(timeout, nullptr, now);
+    }
+    else
+    {
+        InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun();
+    }
 }
 
 } // namespace reporting

diff --git a/src/app/reporting/SynchronizedReportSchedulerImpl.h b/src/app/reporting/SynchronizedReportSchedulerImpl.h
@@ -40,7 +40,7 @@ class SynchronizedReportSchedulerImpl : public ReportSchedulerImpl, public Timer
 
     void OnTransitionToIdle() override;
 
-    bool IsReportScheduled();
+    bool IsReportScheduled(ReadHandler * ReadHandler) override;
 
     void TimerFired() override;
 

diff --git a/src/app/reporting/tests/MockReportScheduler.cpp b/src/app/reporting/tests/MockReportScheduler.cpp
@@ -29,7 +29,7 @@ namespace reporting {
 
 static chip::app::DefaultTimerDelegate sTimerDelegate;
 static ReportSchedulerImpl sTestDefaultReportScheduler(&sTimerDelegate);
-static SynchronizedReportSchedulerImpl sTestReportScheduler(&sTimerDelegate);
+static SynchronizedReportSchedulerImpl sTestSyncReportScheduler(&sTimerDelegate);
 
 ReportSchedulerImpl * GetDefaultReportScheduler()
 {
@@ -38,7 +38,7 @@ ReportSchedulerImpl * GetDefaultReportScheduler()
 
 SynchronizedReportSchedulerImpl * GetSynchronizedReportScheduler()
 {
-    return &sTestReportScheduler;
+    return &sTestSyncReportScheduler;
 }
 
 } // namespace reporting