[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);