From 111616448ee4b8eab0c8b8074d0882deb963cceb Mon Sep 17 00:00:00 2001
From: lpbeliveau-silabs <112982107+lpbeliveau-silabs@users.noreply.github.com>
Date: Tue, 29 Aug 2023 18:37:00 -0400
Subject: [PATCH] [ReadHandler] Handler Reportable if timer fired (#28949)

* Now using the EngineRunScheduled to assess reportability in the scheduler

* Added note to IsReportableNow to explain the use of IsEngineRunScheduled flag
---
 src/app/reporting/ReportScheduler.h       | 10 +++++++---
 src/app/reporting/ReportSchedulerImpl.cpp |  3 ++-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h
index ae657fa51d0404..54b0bb5062286f 100644
--- a/src/app/reporting/ReportScheduler.h
+++ b/src/app/reporting/ReportScheduler.h
@@ -84,12 +84,16 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
 
         /// @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
+        /// report has elapsed.
+        /// @note If a handler has been flaged as scheduled for engine run, it will be reported regardless of the timestamps. This
+        /// is done to guarantee that the reporting engine will see the handler as reportable if a timer fires, even if it fires
+        /// early.
         /// @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
         {
             return (mReadHandler->CanStartReporting() &&
-                    (now >= mMinTimestamp && (mReadHandler->IsDirty() || now >= mMaxTimestamp || CanBeSynced())));
+                    ((now >= mMinTimestamp && (mReadHandler->IsDirty() || now >= mMaxTimestamp || CanBeSynced())) ||
+                     IsEngineRunScheduled()));
         }
 
         bool IsEngineRunScheduled() const { return mFlags.Has(ReadHandlerNodeFlags::EngineRunScheduled); }
@@ -114,8 +118,8 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
 
         void TimerFired() override
         {
-            mScheduler->ReportTimerCallback();
             SetEngineRunScheduled(true);
+            mScheduler->ReportTimerCallback();
         }
 
         System::Clock::Timestamp GetMinTimestamp() const { return mMinTimestamp; }
diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp
index 6c24dd0f5d7dc7..7e9b1838641a96 100644
--- a/src/app/reporting/ReportSchedulerImpl.cpp
+++ b/src/app/reporting/ReportSchedulerImpl.cpp
@@ -100,9 +100,10 @@ void ReportSchedulerImpl::OnSubscriptionReportSent(ReadHandler * aReadHandler)
     node->SetCanBeSynced(false);
     node->SetIntervalTimeStamps(aReadHandler, now);
     Milliseconds32 newTimeout;
+    // Reset the EngineRunScheduled flag so that the next report is scheduled correctly
+    node->SetEngineRunScheduled(false);
     CalculateNextReportTimeout(newTimeout, node, now);
     ScheduleReport(newTimeout, node, now);
-    node->SetEngineRunScheduled(false);
 }
 
 /// @brief When a ReadHandler is removed, unregister it, which will cancel any scheduled report