[Report Scheduler] Documentation of Scheduling Logic (#31134)

* Documented the ReportScheduler base class and its implementations

* Apply suggestions from code review

Co-authored-by: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com>

---------

Co-authored-by: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com>
diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h
index b98832b..d823c2f 100644
--- a/src/app/reporting/ReportScheduler.h
+++ b/src/app/reporting/ReportScheduler.h
@@ -37,6 +37,25 @@
     virtual void TimerFired() = 0;
 };
 
+/**
+ * @class ReportScheduler
+ *
+ * @brief This class is responsible for scheduling Engine runs based on the reporting intervals of the ReadHandlers.
+ *
+ *
+ * This class holds a pool of ReadHandlerNodes that are used to keep track of the minimum and maximum timestamps for a report to be
+ * emitted based on the reporting intervals of the ReadHandlers associated with the node.
+ *
+ * The ReportScheduler also holds a TimerDelegate pointer that is used to start and cancel timers for the ReadHandlers depending
+ * on the reporting logic of the Scheduler.
+ *
+ * It inherits the ReadHandler::Observer class to be notified of reportability changes in the ReadHandlers.
+ * It inherits the ICDStateObserver class to allow the implementation to generate reports based on the changes in ICD devices state,
+ * such as going from idle to active and vice-versa.
+ *
+ * @note The logic for how and when to schedule reports is implemented in the subclasses of ReportScheduler, such as
+ * ReportSchedulerImpl and SyncronizedReportSchedulerImpl.
+ */
 class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
 {
 public:
@@ -60,6 +79,29 @@
         virtual Timestamp GetCurrentMonotonicTimestamp()   = 0;
     };
 
+    /**
+     * @class ReadHandlerNode
+     *
+     * @brief This class is in charge of determining when a ReadHandler is reportable depending on the monotonic timestamp of the
+     * system and the intervals of the ReadHandler. It inherits the TimerContext class to allow it to be used as a context for a
+     * TimerDelegate so the TimerDelegate can call the TimerFired method when the timer expires.
+     *
+     *  The Logic to determine if a ReadHandler is reportable at a precise timestamp is as follows:
+     *  1: The ReadHandler is in the CanStartReporting state
+     *  2: The minimal interval since last report has elapsed
+     *  3: The maximal interval since last report has elapsed or the ReadHandler is dirty
+     *  If the three conditions are met, the ReadHandler is reportable.
+     *
+     *  Additionnal flags have been provided for specific use cases:
+     *
+     *  CanbeSynced: Mechanism to allow the ReadHandler to emit a report if another readHandler is ReportableNow.
+     *  This flag can substitute the maximal interval condition or the dirty condition. It is currently only used by the
+     *  SynchronizedReportScheduler.
+     *
+     *  EngineRunScheduled: Mechanism to ensure that the reporting engine will see the ReadHandler as reportable if a timer fires.
+     *  This flag can substitute the minimal interval condition or the maximal interval condition. The goal is to allow for
+     *  reporting when timers fire earlier than the minimal timestamp du to mechanism such as NTP clock adjustments.
+     */
     class ReadHandlerNode : public TimerContext
     {
     public:
diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp
index 1655fbc..a984cdf 100644
--- a/src/app/reporting/ReportSchedulerImpl.cpp
+++ b/src/app/reporting/ReportSchedulerImpl.cpp
@@ -38,8 +38,6 @@
     VerifyOrDie(nullptr != mTimerDelegate);
 }
 
-void ReportSchedulerImpl::OnTransitionToIdle() {}
-
 /// @brief Method that triggers a report emission on each ReadHandler that is not blocked on its min interval.
 ///        Each read handler that is not blocked is immediately marked dirty so that it will report as soon as possible.
 void ReportSchedulerImpl::OnEnterActiveMode()
@@ -57,9 +55,6 @@
 #endif
 }
 
-/// @brief When a ReadHandler is added, register it in the scheduler node pool. Scheduling the report here is un-necessary since the
-/// ReadHandler will call MoveToState(HandlerState::CanStartReporting);, which will call OnBecameReportable() and schedule the
-/// report.
 void ReportSchedulerImpl::OnSubscriptionEstablished(ReadHandler * aReadHandler)
 {
     ReadHandlerNode * newNode = FindReadHandlerNode(aReadHandler);
@@ -78,7 +73,6 @@
                     ChipLogValueX64(newNode->GetMinTimestamp().count()), ChipLogValueX64(newNode->GetMaxTimestamp().count()));
 }
 
-/// @brief When a ReadHandler becomes reportable, schedule, recalculate and reschedule the report.
 void ReportSchedulerImpl::OnBecameReportable(ReadHandler * aReadHandler)
 {
     ReadHandlerNode * node = FindReadHandlerNode(aReadHandler);
@@ -107,7 +101,6 @@
     ScheduleReport(newTimeout, node, now);
 }
 
-/// @brief When a ReadHandler is removed, unregister it, which will cancel any scheduled report
 void ReportSchedulerImpl::OnReadHandlerDestroyed(ReadHandler * aReadHandler)
 {
     CancelReport(aReadHandler);
diff --git a/src/app/reporting/ReportSchedulerImpl.h b/src/app/reporting/ReportSchedulerImpl.h
index 0f0399a..ef1d314 100644
--- a/src/app/reporting/ReportSchedulerImpl.h
+++ b/src/app/reporting/ReportSchedulerImpl.h
@@ -24,6 +24,26 @@
 namespace app {
 namespace reporting {
 
+/**
+ * @class ReportSchedulerImpl
+ *
+ * @brief This class extends ReportScheduler and provides a scheduling logic for the CHIP Interaction Model Reporting Engine.
+ *
+ * It is reponsible for implementing the ReadHandler and ICD observers callbacks to the Scheduler can take actions whenever a
+ * ReadHandler event occurs or the ICD changes modes.
+ *
+ * All ReadHandlers Observers callbacks rely on the node pool to create or find the node associated to the ReadHandler that
+ * triggered the callback and will use the FindReadHandlerNode() method to do so.
+ *
+ * ## Scheduling Logic
+ *
+ * This class implements a scheduling logic that calculates the next report timeout based on the current system timestamp, the state
+ * of the ReadHandlers associated with the scheduler nodes and the min and max intervals of the ReadHandlers.
+ *
+ * @note This class mimics the original scheduling in which the ReadHandlers would  schedule themselves. The key difference is that
+ * this implementation only relies on a single timer from the scheduling moment rather than having a timer expiring on the min
+ * interval that would trigger the start of a second timer expiring on the max interval.
+ */
 class ReportSchedulerImpl : public ReportScheduler
 {
 public:
@@ -33,15 +53,57 @@
     ~ReportSchedulerImpl() override { UnregisterAllHandlers(); }
 
     // ICDStateObserver
+
+    /**
+     * @brief When the ICD changes to Idle, no action is taken in this implementation.
+     */
+    void OnTransitionToIdle() override{};
+
+    /**
+     * @brief When the ICD changes to Active, this implementation will trigger a report emission on each ReadHandler that is not
+     * blocked on its min interval.
+     *
+     * @note Most action triggering a change to the Active mode already trigger a report emission, so this method is optionnal as it
+     * might be redundant.
+     */
     void OnEnterActiveMode() override;
-    void OnTransitionToIdle() override;
-    // No action is needed by the ReportScheduler on ICD operation Mode changes
+
+    /**
+     * @brief When the ICD changes operation mode, no action is taken in this implementation.
+     */
     void OnICDModeChange() override{};
 
     // ReadHandlerObserver
+
+    /**
+     * @brief When a ReadHandler is added, adds a node and register it in the scheduler node pool. Scheduling the report here is
+     * un-necessary since the ReadHandler will call MoveToState(HandlerState::CanStartReporting);, which will call
+     * OnBecameReportable() and schedule the report.
+     *
+     * @note This method sets a now Timestamp that is used to calculate the next report timeout.
+     */
     void OnSubscriptionEstablished(ReadHandler * aReadHandler) final;
+
+    /**
+     * @brief When a ReadHandler becomes reportable, recalculate and reschedule the report.
+     *
+     * @note This method sets a now Timestamp that is used to calculate the next report timeout.
+     */
     void OnBecameReportable(ReadHandler * aReadHandler) final;
+
+    /**
+     * @brief When a ReadHandler report is sent, recalculate and reschedule the report.
+     *
+     * @note This method is called after the report is sent, so the ReadHandler is no longer reportable, and thus CanBeSynced and
+     * EngineRunScheduled of the node associated to the ReadHandler are set to false in this method.
+     *
+     * @note This method sets a now Timestamp that is used to calculate the next report timeout.
+     */
     void OnSubscriptionReportSent(ReadHandler * aReadHandler) final;
+
+    /**
+     * @brief When a ReadHandler is destroyed, remove the node from the scheduler node pool and cancel the timer associated to it.
+     */
     void OnReadHandlerDestroyed(ReadHandler * aReadHandler) override;
 
     virtual bool IsReportScheduled(ReadHandler * aReadHandler);
@@ -49,6 +111,18 @@
     void ReportTimerCallback() override;
 
 protected:
+    /**
+     * @brief Schedule a report for the ReadHandler associated to the node.
+     *
+     *  If a report is already scheduled for the ReadHandler, cancel it and schedule a new one.
+     *  If the timeout is 0, directly calls the TimerFired() method of the node instead of scheduling a report.
+     *
+     * @param[in] timeout The timeout to schedule the report.
+     * @param[in] node The node associated to the ReadHandler.
+     * @param[in] now The current system timestamp.
+     *
+     * @return CHIP_ERROR CHIP_NO_ERROR on success, timer related error code otherwise (This can only fail on starting the timer)
+     */
     virtual CHIP_ERROR ScheduleReport(Timeout timeout, ReadHandlerNode * node, const Timestamp & now);
     void CancelReport(ReadHandler * aReadHandler);
     virtual void UnregisterAllHandlers();
@@ -56,6 +130,21 @@
 private:
     friend class chip::app::reporting::TestReportScheduler;
 
+    /**
+     * @brief Find the next timer when a report should be scheduled for a ReadHandler.
+     *
+     * @param[out] timeout The timeout to calculate.
+     * @param[in] aNode The node associated to the ReadHandler.
+     * @param[in] now The current system timestamp.
+     *
+     * @return CHIP_ERROR CHIP_NO_ERROR on success or CHIP_ERROR_INVALID_ARGUMENT if aNode is not in the pool.
+     *
+     * The logic is as follows:
+     * - If the ReadHandler is reportable now, the timeout is 0.
+     * - If the ReadHandler is reportable, but the current timestamp is earlier thant the next min interval's timestamp, the timeout
+     * is the delta between the next min interval and now.
+     * - If the ReadHandler is not reportable, the timeout is the difference between the next max interval and now.
+     */
     virtual CHIP_ERROR CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode, const Timestamp & now);
 };
 
diff --git a/src/app/reporting/SynchronizedReportSchedulerImpl.cpp b/src/app/reporting/SynchronizedReportSchedulerImpl.cpp
index a406630..1620d9d 100644
--- a/src/app/reporting/SynchronizedReportSchedulerImpl.cpp
+++ b/src/app/reporting/SynchronizedReportSchedulerImpl.cpp
@@ -84,8 +84,6 @@
     return mTimerDelegate->IsTimerActive(this);
 }
 
-/// @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(const Timestamp & now)
 {
     VerifyOrReturnError(mNodesPool.Allocated(), CHIP_ERROR_INVALID_LIST_LENGTH);
@@ -105,16 +103,13 @@
     return CHIP_NO_ERROR;
 }
 
-/// @brief Find the highest minimum timestamp possible that still respects the lowest max timestamp and sets it as the common
-/// 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(const Timestamp & now)
 {
     VerifyOrReturnError(mNodesPool.Allocated(), CHIP_ERROR_INVALID_LIST_LENGTH);
     System::Clock::Timestamp latest = now;
 
     mNodesPool.ForEachActiveObject([&latest, this](ReadHandlerNode * node) {
+        // We only consider the min interval if the handler is reportable to prevent holding the reports
         if (node->GetMinTimestamp() > latest && this->IsReadHandlerReportable(node->GetReadHandler()) &&
             node->GetMinTimestamp() <= this->mNextMaxTimestamp)
         {
@@ -138,7 +133,10 @@
     bool reportableNow   = false;
     bool reportableAtMin = false;
 
+    // Find out if any handler is reportable now or at the next min interval
     mNodesPool.ForEachActiveObject([&reportableNow, &reportableAtMin, this, now](ReadHandlerNode * node) {
+        // If a node is already scheduled, we don't need to check if it is reportable now, unless a chunked report is in progress
+        // in which case we need to keep scheduling engine runs until the report is complete
         if (!node->IsEngineRunScheduled() || node->IsChunkedReport())
         {
             if (node->IsReportableNow(now))
@@ -156,8 +154,6 @@
         return Loop::Continue;
     });
 
-    // Find out if any handler is reportable now
-
     if (reportableNow)
     {
         timeout = Milliseconds32(0);
@@ -175,8 +171,6 @@
     return CHIP_NO_ERROR;
 }
 
-/// @brief Callback called when the report timer expires to schedule an engine run regardless of the state of the ReadHandlers, as
-/// the engine already verifies that read handlers are reportable before sending a report
 void SynchronizedReportSchedulerImpl::TimerFired()
 {
     Timestamp now   = mTimerDelegate->GetCurrentMonotonicTimestamp();
@@ -188,13 +182,14 @@
     mNodesPool.ForEachActiveObject([now, &firedEarly](ReadHandlerNode * node) {
         if (node->GetMinTimestamp() <= now)
         {
+            // Mark the handler as CanBeSynced if the min interval has elapsed so it will emit a report on the next engine run
             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
+            // 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),
@@ -206,12 +201,14 @@
 
     if (firedEarly)
     {
+        // If we fired the timer early, we need to recalculate the next report timeout and reschedule the report
         Timeout timeout = Milliseconds32(0);
         ReturnOnFailure(CalculateNextReportTimeout(timeout, nullptr, now));
         ScheduleReport(timeout, nullptr, now);
     }
     else
     {
+        // If we did not fire the timer early, we can schedule an engine run
         InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun();
     }
 }
diff --git a/src/app/reporting/SynchronizedReportSchedulerImpl.h b/src/app/reporting/SynchronizedReportSchedulerImpl.h
index 59a675f..bc8573b 100644
--- a/src/app/reporting/SynchronizedReportSchedulerImpl.h
+++ b/src/app/reporting/SynchronizedReportSchedulerImpl.h
@@ -30,6 +30,48 @@
 using ReadHandlerNode = ReportScheduler::ReadHandlerNode;
 using TimerDelegate   = ReportScheduler::TimerDelegate;
 
+/**
+ * @class Synchronized ReportSchedulerImpl
+ *
+ * @brief This class extends ReportSchedulerImpl and overrides it's scheduling logic.
+ *
+ * It only overrides Observers method where the scheduling logic make it necessary, the others are kept as is.
+ *
+ * It inherits from TimerContext so that it can be used as a TimerDelegate instead on relying on the nodes to schedule themselves.
+ *
+ * ## Scheduling Logic
+ *
+ * This class implements a scheduling logic that aims to make all ReadHandlers report at the same time when possible.
+ * The goal is to minimize the different times a device wakes up to report, and thus this aims to schedule all reports at the latest
+ * possible time while ensuring that all reports get sent before their max interval.
+ *
+ * The logic also aims to minimize the impact on the responsivity of the device.
+ *
+ * The scheduling logic is as follows:
+ * - The CalculateNextReportTimeout is called by the same ReadHandler Observer callbacks than the non-synchronized implementation:
+ *      * OnSubscriptionEstablished,
+ *      * OnBecameReportable,
+ *      * OnSubscriptionReportSent
+ *
+ * - The Synchronized Scheduler keeps track of the next min and max interval timestamps. It updates in CalculateNextReportTimeout
+ *
+ * - The next max interval is calculated as the earliest max interval of all the registered ReadHandlersNodes.
+ *
+ * - The next min interval is calculated as the latest min interval of the registered ReadHandlersNodes that:
+ *     * Have a min timestamp greater than the current time
+ *     * Are Reportable (this prevents a ReadHandler that is not reportable to hold the report of all the others)
+ *       TODO: Assess if we want to keep this behavior or simply let the min interval be the earliest min interval to prevent cases
+ *             where a ReadHandler with a dirty path but a very high min interval blocks all reports
+ * - If no ReadHandlerNode matches min interval  the criteria, the next min interval is set to current timestamp.
+ *
+ * - The next report timeout is calculated in CalculatedNextReportTimeout based on the next min and max interval timestamps, as well
+ * as the status of each ReadHandlerNode in the pool.
+ *
+ * @note Unlike the non-synchronized implementation, the Synchronized Scheduler will reschedule itself in the event where a timer
+ * fires before a reportable timestamp is reached.
+ *
+ * @note In this implementation, nodes still keep track of their own min and max interval timestamps.
+ */
 class SynchronizedReportSchedulerImpl : public ReportSchedulerImpl, public TimerContext
 {
 public:
@@ -42,17 +84,80 @@
 
     bool IsReportScheduled(ReadHandler * ReadHandler) override;
 
+    /** @brief Callback called when the report timer expires to schedule an engine run regardless of the state of the ReadHandlers,
+     *
+     * It loops through all handlers and sets their CanBeSynced flag to true if the current timstamp is greater than
+     * their respective minimal timestamps.
+     *
+     * While looping, it checks if any handler is reportable now. If not, we recalculate the next report timeout and reschedule the
+     * report.
+     *
+     * If a Readhangler is reportable now, an engine run is scheduled.
+     *
+     * If the timer expires after all nodes were unregistered, no action is taken.
+     */
     void TimerFired() override;
 
 protected:
+    /**
+     * @brief Schedule a report for the Scheduler.
+     *
+     * If a report is already scheduled, cancel it and schedule a new one.
+     *
+     * @param[in] timeout The timeout to schedule the report.
+     * @param[in] node The node associated to the ReadHandler.
+     * @param[in] now The current system timestamp.
+     *
+     * @return CHIP_ERROR CHIP_NO_ERROR on success, timer related error code otherwise (This can only fail on starting the timer)
+     */
     CHIP_ERROR ScheduleReport(System::Clock::Timeout timeout, ReadHandlerNode * node, const Timestamp & now) override;
     void CancelReport();
 
 private:
     friend class chip::app::reporting::TestReportScheduler;
 
+    /**
+     * @brief Find the highest minimum timestamp possible that still respects the lowest max timestamp and sets it as the common
+     * 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.
+     *
+     * @param[in] now The current system timestamp, set by the event that triggered the call of this method.
+     *
+     * @return CHIP_ERROR on success or CHIP_ERROR_INVALID_LIST_LENGTH if the list is empty
+     */
     CHIP_ERROR FindNextMinInterval(const Timestamp & now);
+
+    /**
+     * @brief Find the smallest maximum interval possible and set it as the common maximum
+     *
+     * @param[in] now The current system timestamp, set by the event that triggered the call of this method.
+     *
+     * @return CHIP_ERROR on success or CHIP_ERROR_INVALID_LIST_LENGTH if the list is empty
+     */
     CHIP_ERROR FindNextMaxInterval(const Timestamp & now);
+
+    /**
+     *  @brief Calculate the next report timeout for all ReadHandlerNodes
+     *
+     * @param[out] timeout The timeout to calculate.
+     * @param[in] aReadHandlerNode unused, kept to preserve the signature of the base class
+     * @param[in] now The current system timestamp when the event leading to the call of this method happened.
+     *
+     *  The next report timeout is calculated by looping through all the ReadHandlerNodes and finding if any are reportable now
+     *      or at min.
+     *   * If a ReadHandlerNode is reportable now, the timeout is set to 0.
+     *   * If a ReadHandlerNode is reportable at min, the timeout is set to the difference between the Scheduler's  min timestamp
+     *      and the current time.
+     *   * If no ReadHandlerNode is reportable, the timeout is set to the difference between the Scheduler's max timestamp and the
+     *      current time.
+     *
+     * @note Since this method is called after the OnSubscriptionReportSent callback, to avoid an endless reporting loop, Nodes with
+     * the IsEngineRunScheduled flag set are ignored when finding if the Scheduler should report at min, max or now.
+     *
+     * @note If a ReadHandler's report is Chunked, the IsEngineRunScheduled is ignored since we do want to keep rescheduling the
+     *       report to the now timestamp until it is fully sent. IsChunkedReport is used to prevent starting a chunked report and
+     * then waiting on the max interval after the first chunk is sent.
+     */
     CHIP_ERROR CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aReadHandlerNode, const Timestamp & now) override;
 
     Timestamp mNextMaxTimestamp = Milliseconds64(0);