From c9f1284454bb6f8bd3f3b8117b4512a503cb543b Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Thu, 10 Aug 2023 16:57:26 -0400 Subject: [PATCH 1/4] Removed multiple calls to GetCurrentMonotonicTimestamp in series to ensure that the NOW value used remains the same along each checks --- src/app/reporting/ReportScheduler.h | 33 +++++++++++-------- src/app/reporting/ReportSchedulerImpl.cpp | 31 +++++++++++------ .../SynchronizedReportSchedulerImpl.cpp | 32 +++++++++--------- 3 files changed, 57 insertions(+), 39 deletions(-) diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index 5b7b62709985bd..daa5c4198817b2 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -30,8 +30,6 @@ namespace reporting { // Forward declaration of TestReportScheduler to allow it to be friend with ReportScheduler class TestReportScheduler; -using Timestamp = System::Clock::Timestamp; - class TimerContext { public: @@ -42,6 +40,8 @@ class TimerContext class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver { public: + using Timestamp = System::Clock::Timestamp; + /// @brief This class acts as an interface between the report scheduler and the system timer to reduce dependencies on the /// system layer. class TimerDelegate @@ -63,25 +63,22 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver class ReadHandlerNode : public TimerContext { public: - ReadHandlerNode(ReadHandler * aReadHandler, TimerDelegate * aTimerDelegate, ReportScheduler * aScheduler) : - mTimerDelegate(aTimerDelegate), mScheduler(aScheduler) + ReadHandlerNode(ReadHandler * aReadHandler, ReportScheduler * aScheduler, const Timestamp & now) : mScheduler(aScheduler) { VerifyOrDie(aReadHandler != nullptr); - VerifyOrDie(aTimerDelegate != nullptr); VerifyOrDie(aScheduler != nullptr); mReadHandler = aReadHandler; - SetIntervalTimeStamps(aReadHandler); + SetIntervalTimeStamps(aReadHandler, now); } ReadHandler * GetReadHandler() const { return mReadHandler; } /// @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 - bool IsReportableNow() const + /// @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 { - Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); - return (mReadHandler->CanStartReporting() && (now >= mMinTimestamp && (mReadHandler->IsDirty() || now >= mMaxTimestamp || now >= mSyncTimestamp))); } @@ -89,11 +86,14 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver bool IsEngineRunScheduled() const { return mEngineRunScheduled; } void SetEngineRunScheduled(bool aEngineRunScheduled) { mEngineRunScheduled = aEngineRunScheduled; } - void SetIntervalTimeStamps(ReadHandler * aReadHandler) + /// @brief Set the interval timestamps for the node based on the read handler reporting intervals + /// @param aReadHandler + /// @param now current time to calculate the mMin and mMax timestamps, user must ensure to provide a valid time for this to + /// be reliable + void SetIntervalTimeStamps(ReadHandler * aReadHandler, const Timestamp & now) { uint16_t minInterval, maxInterval; aReadHandler->GetReportingIntervals(minInterval, maxInterval); - Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); mMinTimestamp = now + System::Clock::Seconds16(minInterval); mMaxTimestamp = now + System::Clock::Seconds16(maxInterval); mSyncTimestamp = mMaxTimestamp; @@ -118,7 +118,6 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver System::Clock::Timestamp GetSyncTimestamp() const { return mSyncTimestamp; } private: - TimerDelegate * mTimerDelegate; ReadHandler * mReadHandler; ReportScheduler * mScheduler; Timestamp mMinTimestamp; @@ -141,9 +140,12 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver /// @param aReadHandler read handler to check bool IsReportableNow(ReadHandler * aReadHandler) { + // Update the now timestamp to ensure external calls to IsReportableNow are always comparing to the current time + mNow = mTimerDelegate->GetCurrentMonotonicTimestamp(); ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); - return (nullptr != node) ? node->IsReportableNow() : false; + return (nullptr != node) ? node->IsReportableNow(mNow) : false; } + /// @brief Check if a ReadHandler is reportable without considering the timing bool IsReadHandlerReportable(ReadHandler * aReadHandler) const { return aReadHandler->ShouldStartReporting(); } /// @brief Sets the ForceDirty flag of a ReadHandler @@ -188,6 +190,11 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver ObjectPool mNodesPool; TimerDelegate * mTimerDelegate; + + // This represents the current time to be used for scheduling the next report, or to compare against Timestamps to determine + // reportability based on time. This timestamp needs to be updated upon each callback that will do scheduling or check the + // reportability state. + Timestamp mNow = System::Clock::Milliseconds64(0); }; }; // namespace reporting }; // namespace app diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index 44be647b921d9d..7dbdda0872d525 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -42,9 +42,9 @@ ReportSchedulerImpl::ReportSchedulerImpl(TimerDelegate * aTimerDelegate) : Repor void ReportSchedulerImpl::OnEnterActiveMode() { #if ICD_REPORT_ON_ENTER_ACTIVE_MODE - Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); - mNodesPool.ForEachActiveObject([now, this](ReadHandlerNode * node) { - if (now >= node->GetMinTimestamp()) + mNow = mTimerDelegate->GetCurrentMonotonicTimestamp(); + mNodesPool.ForEachActiveObject([this](ReadHandlerNode * node) { + if (this->mNow >= node->GetMinTimestamp()) { this->HandlerForceDirtyState(node->GetReadHandler()); } @@ -62,9 +62,13 @@ void ReportSchedulerImpl::OnSubscriptionEstablished(ReadHandler * aReadHandler) ReadHandlerNode * newNode = FindReadHandlerNode(aReadHandler); // Handler must not be registered yet; it's just being constructed. VerifyOrDie(nullptr == newNode); + + // Update the now timestamp to the current time to determine the min and max timestamps + mNow = mTimerDelegate->GetCurrentMonotonicTimestamp(); + // The NodePool is the same size as the ReadHandler pool from the IM Engine, so we don't need a check for size here since if a // ReadHandler was created, space should be available. - newNode = mNodesPool.CreateObject(aReadHandler, mTimerDelegate, this); + newNode = mNodesPool.CreateObject(aReadHandler, this, mNow); ChipLogProgress(DataManagement, "Registered a ReadHandler that will schedule a report between system Timestamp: %" PRIu64 @@ -77,6 +81,10 @@ void ReportSchedulerImpl::OnBecameReportable(ReadHandler * aReadHandler) { ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); VerifyOrReturn(nullptr != node); + + // Update the now timestamp to the current time to calculate the next report timeout + mNow = mTimerDelegate->GetCurrentMonotonicTimestamp(); + Milliseconds32 newTimeout; CalculateNextReportTimeout(newTimeout, node); ScheduleReport(newTimeout, node); @@ -86,7 +94,11 @@ void ReportSchedulerImpl::OnSubscriptionReportSent(ReadHandler * aReadHandler) { ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); VerifyOrReturn(nullptr != node); - node->SetIntervalTimeStamps(aReadHandler); + + // Update the now timestamp to the current time to update the node's interval timestamps and calculate the next report timeout + mNow = mTimerDelegate->GetCurrentMonotonicTimestamp(); + + node->SetIntervalTimeStamps(aReadHandler, mNow); Milliseconds32 newTimeout; CalculateNextReportTimeout(newTimeout, node); ScheduleReport(newTimeout, node); @@ -144,24 +156,23 @@ bool ReportSchedulerImpl::IsReportScheduled(ReadHandler * aReadHandler) CHIP_ERROR ReportSchedulerImpl::CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode) { VerifyOrReturnError(nullptr != FindReadHandlerNode(aNode->GetReadHandler()), CHIP_ERROR_INVALID_ARGUMENT); - Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); // If the handler is reportable now, just schedule a report immediately - if (aNode->IsReportableNow()) + if (aNode->IsReportableNow(mNow)) { // If the handler is reportable now, just schedule a report immediately timeout = Milliseconds32(0); } - else if (IsReadHandlerReportable(aNode->GetReadHandler()) && (aNode->GetMinTimestamp() > now)) + else if (IsReadHandlerReportable(aNode->GetReadHandler()) && (aNode->GetMinTimestamp() > mNow)) { // If the handler is reportable now, but the min interval is not elapsed, schedule a report for the moment the min interval // has elapsed - timeout = aNode->GetMinTimestamp() - now; + timeout = aNode->GetMinTimestamp() - mNow; } else { // If the handler is not reportable now, schedule a report for the max interval - timeout = aNode->GetMaxTimestamp() - now; + timeout = aNode->GetMaxTimestamp() - mNow; } return CHIP_NO_ERROR; } diff --git a/src/app/reporting/SynchronizedReportSchedulerImpl.cpp b/src/app/reporting/SynchronizedReportSchedulerImpl.cpp index b3f700b8cbcc0f..0b089338949561 100644 --- a/src/app/reporting/SynchronizedReportSchedulerImpl.cpp +++ b/src/app/reporting/SynchronizedReportSchedulerImpl.cpp @@ -54,7 +54,7 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::ScheduleReport(Timeout timeout, Read return CHIP_NO_ERROR; } ReturnErrorOnFailure(mTimerDelegate->StartTimer(this, timeout)); - mTestNextReportTimestamp = mTimerDelegate->GetCurrentMonotonicTimestamp() + timeout; + mTestNextReportTimestamp = mNow + timeout; return CHIP_NO_ERROR; } @@ -76,11 +76,10 @@ bool SynchronizedReportSchedulerImpl::IsReportScheduled() CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMaxInterval() { VerifyOrReturnError(mNodesPool.Allocated(), CHIP_ERROR_INVALID_LIST_LENGTH); - System::Clock::Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); - System::Clock::Timestamp earliest = now + Seconds16::max(); + System::Clock::Timestamp earliest = mNow + Seconds16::max(); - mNodesPool.ForEachActiveObject([&earliest, now](ReadHandlerNode * node) { - if (node->GetMaxTimestamp() < earliest && node->GetMaxTimestamp() > now) + mNodesPool.ForEachActiveObject([&earliest, this](ReadHandlerNode * node) { + if (node->GetMaxTimestamp() < earliest && node->GetMaxTimestamp() > this->mNow) { earliest = node->GetMaxTimestamp(); } @@ -100,7 +99,7 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMaxInterval() CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMinInterval() { VerifyOrReturnError(mNodesPool.Allocated(), CHIP_ERROR_INVALID_LIST_LENGTH); - System::Clock::Timestamp latest = mTimerDelegate->GetCurrentMonotonicTimestamp(); + System::Clock::Timestamp latest = mNow; mNodesPool.ForEachActiveObject([&latest, this](ReadHandlerNode * node) { if (node->GetMinTimestamp() > latest && this->IsReadHandlerReportable(node->GetReadHandler()) && @@ -126,12 +125,10 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout & bool reportableNow = false; bool reportableAtMin = false; - Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); - mNodesPool.ForEachActiveObject([&reportableNow, &reportableAtMin, this](ReadHandlerNode * node) { if (!node->IsEngineRunScheduled()) { - if (node->IsReportableNow()) + if (node->IsReportableNow(mNow)) { reportableNow = true; return Loop::Break; @@ -154,21 +151,21 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout & } else if (reportableAtMin) { - timeout = mNextMinTimestamp - now; + timeout = mNextMinTimestamp - mNow; } else { // Schedule report at next max otherwise - timeout = mNextMaxTimestamp - now; + timeout = mNextMaxTimestamp - mNow; } // Updates the synching time of each handler - mNodesPool.ForEachActiveObject([now, timeout](ReadHandlerNode * node) { + mNodesPool.ForEachActiveObject([this, timeout](ReadHandlerNode * node) { // Prevent modifying the sync if the handler is currently reportable, sync's purpose is to allow handler to become // reportable earlier than their max interval - if (!node->IsReportableNow()) + if (!node->IsReportableNow(this->mNow)) { - node->SetSyncTimestamp(Milliseconds64(now + timeout)); + node->SetSyncTimestamp(Milliseconds64(this->mNow + timeout)); } return Loop::Continue; @@ -181,10 +178,13 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout & /// the engine already verifies that read handlers are reportable before sending a report void SynchronizedReportSchedulerImpl::TimerFired() { + // Update the current time to validate reportability upon current time + mNow = mTimerDelegate->GetCurrentMonotonicTimestamp(); + InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); - mNodesPool.ForEachActiveObject([](ReadHandlerNode * node) { - if (node->IsReportableNow()) + mNodesPool.ForEachActiveObject([this](ReadHandlerNode * node) { + if (node->IsReportableNow(this->mNow)) { node->SetEngineRunScheduled(true); ChipLogProgress(DataManagement, "Handler: %p with min: %" PRIu64 " and max: %" PRIu64 " and sync: %" PRIu64, (node), From 88dea90695647a28d5b4f13be6befdb05b8b38fb Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Fri, 11 Aug 2023 11:51:24 -0400 Subject: [PATCH 2/4] removed mNow, using stack now timestamps instead --- src/app/reporting/ReportScheduler.h | 9 +--- src/app/reporting/ReportSchedulerImpl.cpp | 36 ++++++++-------- src/app/reporting/ReportSchedulerImpl.h | 4 +- .../SynchronizedReportSchedulerImpl.cpp | 43 ++++++++++--------- .../SynchronizedReportSchedulerImpl.h | 8 ++-- 5 files changed, 48 insertions(+), 52 deletions(-) diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index daa5c4198817b2..5eb72f2712973f 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -141,9 +141,9 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver bool IsReportableNow(ReadHandler * aReadHandler) { // Update the now timestamp to ensure external calls to IsReportableNow are always comparing to the current time - mNow = mTimerDelegate->GetCurrentMonotonicTimestamp(); + Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); - return (nullptr != node) ? node->IsReportableNow(mNow) : false; + return (nullptr != node) ? node->IsReportableNow(now) : false; } /// @brief Check if a ReadHandler is reportable without considering the timing @@ -190,11 +190,6 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver ObjectPool mNodesPool; TimerDelegate * mTimerDelegate; - - // This represents the current time to be used for scheduling the next report, or to compare against Timestamps to determine - // reportability based on time. This timestamp needs to be updated upon each callback that will do scheduling or check the - // reportability state. - Timestamp mNow = System::Clock::Milliseconds64(0); }; }; // namespace reporting }; // namespace app diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index 7dbdda0872d525..bd4eac3b90bce9 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -42,9 +42,9 @@ ReportSchedulerImpl::ReportSchedulerImpl(TimerDelegate * aTimerDelegate) : Repor void ReportSchedulerImpl::OnEnterActiveMode() { #if ICD_REPORT_ON_ENTER_ACTIVE_MODE - mNow = mTimerDelegate->GetCurrentMonotonicTimestamp(); - mNodesPool.ForEachActiveObject([this](ReadHandlerNode * node) { - if (this->mNow >= node->GetMinTimestamp()) + Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); + mNodesPool.ForEachActiveObject([now](ReadHandlerNode * node) { + if (now >= node->GetMinTimestamp()) { this->HandlerForceDirtyState(node->GetReadHandler()); } @@ -64,11 +64,11 @@ void ReportSchedulerImpl::OnSubscriptionEstablished(ReadHandler * aReadHandler) VerifyOrDie(nullptr == newNode); // Update the now timestamp to the current time to determine the min and max timestamps - mNow = mTimerDelegate->GetCurrentMonotonicTimestamp(); + Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); // The NodePool is the same size as the ReadHandler pool from the IM Engine, so we don't need a check for size here since if a // ReadHandler was created, space should be available. - newNode = mNodesPool.CreateObject(aReadHandler, this, mNow); + newNode = mNodesPool.CreateObject(aReadHandler, this, now); ChipLogProgress(DataManagement, "Registered a ReadHandler that will schedule a report between system Timestamp: %" PRIu64 @@ -83,11 +83,11 @@ void ReportSchedulerImpl::OnBecameReportable(ReadHandler * aReadHandler) VerifyOrReturn(nullptr != node); // Update the now timestamp to the current time to calculate the next report timeout - mNow = mTimerDelegate->GetCurrentMonotonicTimestamp(); + Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); Milliseconds32 newTimeout; - CalculateNextReportTimeout(newTimeout, node); - ScheduleReport(newTimeout, node); + CalculateNextReportTimeout(newTimeout, node, now); + ScheduleReport(newTimeout, node, now); } void ReportSchedulerImpl::OnSubscriptionReportSent(ReadHandler * aReadHandler) @@ -96,12 +96,12 @@ void ReportSchedulerImpl::OnSubscriptionReportSent(ReadHandler * aReadHandler) VerifyOrReturn(nullptr != node); // Update the now timestamp to the current time to update the node's interval timestamps and calculate the next report timeout - mNow = mTimerDelegate->GetCurrentMonotonicTimestamp(); + Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); - node->SetIntervalTimeStamps(aReadHandler, mNow); + node->SetIntervalTimeStamps(aReadHandler, now); Milliseconds32 newTimeout; - CalculateNextReportTimeout(newTimeout, node); - ScheduleReport(newTimeout, node); + CalculateNextReportTimeout(newTimeout, node, now); + ScheduleReport(newTimeout, node, now); node->SetEngineRunScheduled(false); } @@ -117,7 +117,7 @@ void ReportSchedulerImpl::OnReadHandlerDestroyed(ReadHandler * aReadHandler) mNodesPool.ReleaseObject(removeNode); } -CHIP_ERROR ReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandlerNode * node) +CHIP_ERROR ReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandlerNode * node, const Timestamp & now) { // Cancel Report if it is currently scheduled mTimerDelegate->CancelTimer(node); @@ -153,26 +153,26 @@ bool ReportSchedulerImpl::IsReportScheduled(ReadHandler * aReadHandler) return mTimerDelegate->IsTimerActive(node); } -CHIP_ERROR ReportSchedulerImpl::CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode) +CHIP_ERROR ReportSchedulerImpl::CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode, const Timestamp & now) { VerifyOrReturnError(nullptr != FindReadHandlerNode(aNode->GetReadHandler()), CHIP_ERROR_INVALID_ARGUMENT); // If the handler is reportable now, just schedule a report immediately - if (aNode->IsReportableNow(mNow)) + if (aNode->IsReportableNow(now)) { // If the handler is reportable now, just schedule a report immediately timeout = Milliseconds32(0); } - else if (IsReadHandlerReportable(aNode->GetReadHandler()) && (aNode->GetMinTimestamp() > mNow)) + else if (IsReadHandlerReportable(aNode->GetReadHandler()) && (aNode->GetMinTimestamp() > now)) { // If the handler is reportable now, but the min interval is not elapsed, schedule a report for the moment the min interval // has elapsed - timeout = aNode->GetMinTimestamp() - mNow; + timeout = aNode->GetMinTimestamp() - now; } else { // If the handler is not reportable now, schedule a report for the max interval - timeout = aNode->GetMaxTimestamp() - mNow; + timeout = aNode->GetMaxTimestamp() - now; } return CHIP_NO_ERROR; } diff --git a/src/app/reporting/ReportSchedulerImpl.h b/src/app/reporting/ReportSchedulerImpl.h index 003be7cb4637eb..f3870192c4b1f6 100644 --- a/src/app/reporting/ReportSchedulerImpl.h +++ b/src/app/reporting/ReportSchedulerImpl.h @@ -46,14 +46,14 @@ class ReportSchedulerImpl : public ReportScheduler void ReportTimerCallback() override; protected: - virtual CHIP_ERROR ScheduleReport(Timeout timeout, ReadHandlerNode * node); + virtual CHIP_ERROR ScheduleReport(Timeout timeout, ReadHandlerNode * node, const Timestamp & now); void CancelReport(ReadHandler * aReadHandler); virtual void UnregisterAllHandlers(); private: friend class chip::app::reporting::TestReportScheduler; - virtual CHIP_ERROR CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode); + virtual CHIP_ERROR CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode, const Timestamp & now); }; } // namespace reporting diff --git a/src/app/reporting/SynchronizedReportSchedulerImpl.cpp b/src/app/reporting/SynchronizedReportSchedulerImpl.cpp index 0b089338949561..f0fed2aa8bbbda 100644 --- a/src/app/reporting/SynchronizedReportSchedulerImpl.cpp +++ b/src/app/reporting/SynchronizedReportSchedulerImpl.cpp @@ -44,7 +44,7 @@ void SynchronizedReportSchedulerImpl::OnReadHandlerDestroyed(ReadHandler * aRead } } -CHIP_ERROR SynchronizedReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandlerNode * node) +CHIP_ERROR SynchronizedReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandlerNode * node, const Timestamp & now) { // Cancel Report if it is currently scheduled mTimerDelegate->CancelTimer(this); @@ -54,7 +54,7 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::ScheduleReport(Timeout timeout, Read return CHIP_NO_ERROR; } ReturnErrorOnFailure(mTimerDelegate->StartTimer(this, timeout)); - mTestNextReportTimestamp = mNow + timeout; + mTestNextReportTimestamp = now + timeout; return CHIP_NO_ERROR; } @@ -73,13 +73,13 @@ bool SynchronizedReportSchedulerImpl::IsReportScheduled() /// @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() +CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMaxInterval(const Timestamp & now) { VerifyOrReturnError(mNodesPool.Allocated(), CHIP_ERROR_INVALID_LIST_LENGTH); - System::Clock::Timestamp earliest = mNow + Seconds16::max(); + System::Clock::Timestamp earliest = now + Seconds16::max(); - mNodesPool.ForEachActiveObject([&earliest, this](ReadHandlerNode * node) { - if (node->GetMaxTimestamp() < earliest && node->GetMaxTimestamp() > this->mNow) + mNodesPool.ForEachActiveObject([&earliest, now](ReadHandlerNode * node) { + if (node->GetMaxTimestamp() < earliest && node->GetMaxTimestamp() > now) { earliest = node->GetMaxTimestamp(); } @@ -96,10 +96,10 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMaxInterval() /// 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() +CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMinInterval(const Timestamp & now) { VerifyOrReturnError(mNodesPool.Allocated(), CHIP_ERROR_INVALID_LIST_LENGTH); - System::Clock::Timestamp latest = mNow; + System::Clock::Timestamp latest = now; mNodesPool.ForEachActiveObject([&latest, this](ReadHandlerNode * node) { if (node->GetMinTimestamp() > latest && this->IsReadHandlerReportable(node->GetReadHandler()) && @@ -117,18 +117,19 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::FindNextMinInterval() return CHIP_NO_ERROR; } -CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode) +CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aNode, + const Timestamp & now) { VerifyOrReturnError(nullptr != FindReadHandlerNode(aNode->GetReadHandler()), CHIP_ERROR_INVALID_ARGUMENT); - ReturnErrorOnFailure(FindNextMaxInterval()); - ReturnErrorOnFailure(FindNextMinInterval()); + ReturnErrorOnFailure(FindNextMaxInterval(now)); + ReturnErrorOnFailure(FindNextMinInterval(now)); bool reportableNow = false; bool reportableAtMin = false; - mNodesPool.ForEachActiveObject([&reportableNow, &reportableAtMin, this](ReadHandlerNode * node) { + mNodesPool.ForEachActiveObject([&reportableNow, &reportableAtMin, this, now](ReadHandlerNode * node) { if (!node->IsEngineRunScheduled()) { - if (node->IsReportableNow(mNow)) + if (node->IsReportableNow(now)) { reportableNow = true; return Loop::Break; @@ -151,21 +152,21 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout & } else if (reportableAtMin) { - timeout = mNextMinTimestamp - mNow; + timeout = mNextMinTimestamp - now; } else { // Schedule report at next max otherwise - timeout = mNextMaxTimestamp - mNow; + timeout = mNextMaxTimestamp - now; } // Updates the synching time of each handler - mNodesPool.ForEachActiveObject([this, timeout](ReadHandlerNode * node) { + mNodesPool.ForEachActiveObject([now, timeout](ReadHandlerNode * node) { // Prevent modifying the sync if the handler is currently reportable, sync's purpose is to allow handler to become // reportable earlier than their max interval - if (!node->IsReportableNow(this->mNow)) + if (!node->IsReportableNow(now)) { - node->SetSyncTimestamp(Milliseconds64(this->mNow + timeout)); + node->SetSyncTimestamp(Milliseconds64(now + timeout)); } return Loop::Continue; @@ -179,12 +180,12 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout & void SynchronizedReportSchedulerImpl::TimerFired() { // Update the current time to validate reportability upon current time - mNow = mTimerDelegate->GetCurrentMonotonicTimestamp(); + Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); - mNodesPool.ForEachActiveObject([this](ReadHandlerNode * node) { - if (node->IsReportableNow(this->mNow)) + mNodesPool.ForEachActiveObject([now](ReadHandlerNode * node) { + if (node->IsReportableNow(now)) { node->SetEngineRunScheduled(true); ChipLogProgress(DataManagement, "Handler: %p with min: %" PRIu64 " and max: %" PRIu64 " and sync: %" PRIu64, (node), diff --git a/src/app/reporting/SynchronizedReportSchedulerImpl.h b/src/app/reporting/SynchronizedReportSchedulerImpl.h index 9fd4a8ca0aa684..9e3689d144194a 100644 --- a/src/app/reporting/SynchronizedReportSchedulerImpl.h +++ b/src/app/reporting/SynchronizedReportSchedulerImpl.h @@ -43,15 +43,15 @@ class SynchronizedReportSchedulerImpl : public ReportSchedulerImpl, public Timer void TimerFired() override; protected: - CHIP_ERROR ScheduleReport(System::Clock::Timeout timeout, ReadHandlerNode * node) override; + CHIP_ERROR ScheduleReport(System::Clock::Timeout timeout, ReadHandlerNode * node, const Timestamp & now) override; void CancelReport(); private: friend class chip::app::reporting::TestReportScheduler; - CHIP_ERROR FindNextMinInterval(); - CHIP_ERROR FindNextMaxInterval(); - CHIP_ERROR CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aReadHandlerNode) override; + CHIP_ERROR FindNextMinInterval(const Timestamp & now); + CHIP_ERROR FindNextMaxInterval(const Timestamp & now); + CHIP_ERROR CalculateNextReportTimeout(Timeout & timeout, ReadHandlerNode * aReadHandlerNode, const Timestamp & now) override; Timestamp mNextMaxTimestamp = Milliseconds64(0); Timestamp mNextMinTimestamp = Milliseconds64(0); From e6dfd193acaad0dd4df66ac44f396635be2e804e Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Fri, 11 Aug 2023 14:13:40 -0400 Subject: [PATCH 3/4] Added missing @param description --- src/app/reporting/ReportScheduler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index 5eb72f2712973f..b6abbdb478e506 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -87,7 +87,7 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver void SetEngineRunScheduled(bool aEngineRunScheduled) { mEngineRunScheduled = aEngineRunScheduled; } /// @brief Set the interval timestamps for the node based on the read handler reporting intervals - /// @param aReadHandler + /// @param aReadHandler read handler to get the intervals from /// @param now current time to calculate the mMin and mMax timestamps, user must ensure to provide a valid time for this to /// be reliable void SetIntervalTimeStamps(ReadHandler * aReadHandler, const Timestamp & now) From e4d5c3dc118e6d4f94e565bc002b5ffa5cf10ad0 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Mon, 14 Aug 2023 15:54:46 -0400 Subject: [PATCH 4/4] Removed un-necessary comments --- src/app/reporting/ReportSchedulerImpl.cpp | 3 --- src/app/reporting/SynchronizedReportSchedulerImpl.cpp | 1 - 2 files changed, 4 deletions(-) diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index bd4eac3b90bce9..632d2f456990cd 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -63,7 +63,6 @@ void ReportSchedulerImpl::OnSubscriptionEstablished(ReadHandler * aReadHandler) // Handler must not be registered yet; it's just being constructed. VerifyOrDie(nullptr == newNode); - // Update the now timestamp to the current time to determine the min and max timestamps Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); // The NodePool is the same size as the ReadHandler pool from the IM Engine, so we don't need a check for size here since if a @@ -82,7 +81,6 @@ void ReportSchedulerImpl::OnBecameReportable(ReadHandler * aReadHandler) ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); VerifyOrReturn(nullptr != node); - // Update the now timestamp to the current time to calculate the next report timeout Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); Milliseconds32 newTimeout; @@ -95,7 +93,6 @@ void ReportSchedulerImpl::OnSubscriptionReportSent(ReadHandler * aReadHandler) ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); VerifyOrReturn(nullptr != node); - // Update the now timestamp to the current time to update the node's interval timestamps and calculate the next report timeout Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); node->SetIntervalTimeStamps(aReadHandler, now); diff --git a/src/app/reporting/SynchronizedReportSchedulerImpl.cpp b/src/app/reporting/SynchronizedReportSchedulerImpl.cpp index f0fed2aa8bbbda..d9a410f555e506 100644 --- a/src/app/reporting/SynchronizedReportSchedulerImpl.cpp +++ b/src/app/reporting/SynchronizedReportSchedulerImpl.cpp @@ -179,7 +179,6 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout & /// the engine already verifies that read handlers are reportable before sending a report void SynchronizedReportSchedulerImpl::TimerFired() { - // Update the current time to validate reportability upon current time Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun();