Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Junior Martinez <[email protected]>
  • Loading branch information
lpbeliveau-silabs and jmartinez-silabs committed Jan 3, 2024
1 parent 3d2acc2 commit 512ffa2
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 15 deletions.
4 changes: 2 additions & 2 deletions src/app/reporting/ReportScheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ class TimerContext
* 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 pointer a TimerDelegate that is used to start and cancel timers for the ReadHandlers depending
* 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 iddle to active and vice-versa.
* 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.
Expand Down
17 changes: 9 additions & 8 deletions src/app/reporting/ReportSchedulerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,17 @@ namespace reporting {
* 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 relie on the node pool to create or find the node associated to the ReadHandler that
* 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 Calculate the next report timeout based on the
* 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 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.
* @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
{
Expand Down Expand Up @@ -84,14 +85,14 @@ class ReportSchedulerImpl : public ReportScheduler
void OnSubscriptionEstablished(ReadHandler * aReadHandler) final;

/**
* @brief When a ReadHandler becomes reportable, schedule, recalculate and reschedule the report.
* @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, schedule, recalculate and reschedule the report.
* @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.
Expand Down Expand Up @@ -141,7 +142,7 @@ class ReportSchedulerImpl : public ReportScheduler
* 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 difference between the next min interval and now.
* 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);
Expand Down
10 changes: 5 additions & 5 deletions src/app/reporting/SynchronizedReportSchedulerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ using TimerDelegate = ReportScheduler::TimerDelegate;
*
* @brief This class extends ReportSchedulerImpl and overrides it's scheduling logic.
*
* It only overrides Observers method where thhe scheduling logic make it necessary, the others are kept as is.
* 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.
*
Expand All @@ -55,7 +55,7 @@ using TimerDelegate = ReportScheduler::TimerDelegate;
*
* - The Synchronized Scheduler keeps track of the next min and max interval timestamps. It updates in CalculateNextReportTimeout
*
* - The nex max interval is calculated as the earliest max interval of all the registered ReadHandlersNodes.
* - 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
Expand Down Expand Up @@ -92,7 +92,7 @@ class SynchronizedReportSchedulerImpl : public ReportSchedulerImpl, public Timer
* 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, we an engine run is scheduled.
* If a Readhangler is reportable now, an engine run is scheduled.
*
* If the timer expires after all nodes were unregistered, no action is taken.
*/
Expand Down Expand Up @@ -123,7 +123,7 @@ class SynchronizedReportSchedulerImpl : public ReportSchedulerImpl, public Timer
*
* @param[in] now The current system timestamp, set by the event that triggered the call of this method.
*
* @return CHIP_ERROR CHIP_NO_ERROR on success or CHIP_ERROR_INVALID_LIST_LENGTH if the list is empty
* @return CHIP_ERROR on success or CHIP_ERROR_INVALID_LIST_LENGTH if the list is empty
*/
CHIP_ERROR FindNextMinInterval(const Timestamp & now);

Expand All @@ -132,7 +132,7 @@ class SynchronizedReportSchedulerImpl : public ReportSchedulerImpl, public Timer
*
* @param[in] now The current system timestamp, set by the event that triggered the call of this method.
*
* @return CHIP_ERROR CHIP_NO_ERROR on success or CHIP_ERROR_INVALID_LIST_LENGTH if the list is empty
* @return CHIP_ERROR on success or CHIP_ERROR_INVALID_LIST_LENGTH if the list is empty
*/
CHIP_ERROR FindNextMaxInterval(const Timestamp & now);

Expand Down

0 comments on commit 512ffa2

Please sign in to comment.