diff --git a/examples/platform/nrfconnect/util/ICDUtil.cpp b/examples/platform/nrfconnect/util/ICDUtil.cpp index b1c2c4797adb80..b3dc9c80bbc9dd 100644 --- a/examples/platform/nrfconnect/util/ICDUtil.cpp +++ b/examples/platform/nrfconnect/util/ICDUtil.cpp @@ -36,5 +36,5 @@ CHIP_ERROR ICDUtil::OnSubscriptionRequested(chip::app::ReadHandler & aReadHandle agreedMaxInterval = kSubscriptionMaxIntervalPublisherLimit; } - return aReadHandler.SetMaxReportingIntervals(agreedMaxInterval); + return aReadHandler.SetMaxReportingInterval(agreedMaxInterval); } diff --git a/examples/platform/silabs/ICDSubscriptionCallback.cpp b/examples/platform/silabs/ICDSubscriptionCallback.cpp index 8551277fe19885..318300472ecfaa 100644 --- a/examples/platform/silabs/ICDSubscriptionCallback.cpp +++ b/examples/platform/silabs/ICDSubscriptionCallback.cpp @@ -61,5 +61,5 @@ CHIP_ERROR ICDSubscriptionCallback::OnSubscriptionRequested(chip::app::ReadHandl decidedMaxInterval = maximumMaxInterval; } - return aReadHandler.SetMaxReportingIntervals(decidedMaxInterval); + return aReadHandler.SetMaxReportingInterval(decidedMaxInterval); } diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 579c34880bfc4d..40b240180f7689 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -229,7 +229,8 @@ class ReadHandler : public Messaging::ExchangeDelegate { VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(aMinInterval <= mMaxInterval, CHIP_ERROR_INVALID_ARGUMENT); - mMinIntervalFloorSeconds = aMinInterval; + // Ensures the new min interval is higher than the subscriber established one. + mMinIntervalFloorSeconds = std::max(mMinIntervalFloorSeconds, aMinInterval); return CHIP_NO_ERROR; } diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index 36dbc3147cbba4..bbe10fb56e6155 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -35,14 +35,22 @@ using Timestamp = System::Clock::Timestamp; class ReportScheduler : public ReadHandler::Observer { public: + /// @brief This class acts as an interface between the report scheduler and the system timer to reduce dependencies on the + /// system layer. class TimerDelegate { public: virtual ~TimerDelegate() {} + /// @brief Start a timer for a given context, the report scheduler will always start by trying to cancel an existing timer + /// before starting a new one + /// @param context context to pass to the timer callback, in this case, a read handler node is passed + /// @param aTimeout time in miliseconds before the timer expires virtual CHIP_ERROR StartTimer(void * context, System::Clock::Timeout aTimeout) = 0; - virtual void CancelTimer(void * context) = 0; - virtual bool IsTimerActive(void * context) = 0; - virtual Timestamp GetCurrentMonotonicTimestamp() = 0; + /// @brief Cancel a timer for a given context + /// @param context used to identify the timer to cancel + virtual void CancelTimer(void * context) = 0; + virtual bool IsTimerActive(void * context) = 0; + virtual Timestamp GetCurrentMonotonicTimestamp() = 0; }; class ReadHandlerNode : public IntrusiveListNodeBase<> @@ -101,26 +109,6 @@ class ReportScheduler : public ReadHandler::Observer */ virtual ~ReportScheduler() = default; - /// @brief Register a ReadHandler to the scheduler, will schedule report - /// @param aReadHandler read handler to register - virtual CHIP_ERROR RegisterReadHandler(ReadHandler * aReadHandler) = 0; - /// @brief Schedule a report for a given ReadHandler - /// @param timeout time in seconds before the next engine run is scheduled - /// @param aReadHandler read handler to schedule a report for - /// @return CHIP_ERROR not found if the ReadHandler is not registered in the scheduler, specific timer error if the timer - /// couldn't be started - virtual CHIP_ERROR ScheduleReport(System::Clock::Timeout timeout, ReadHandler * aReadHandler) = 0; - /// @brief Cancel a scheduled report for a given ReadHandler - /// @param aReadHandler readhandler to look for in the ReadHandler nodes list. If found, the timer started for this report will - /// be cancelled. - virtual void CancelReport(ReadHandler * aReadHandler) = 0; - /// @brief Unregister a ReadHandler from the scheduler - /// @param aReadHandler read handler to unregister - /// @return CHIP_NO_ERROR if the ReadHandler was successfully unregistered or not found, specific error otherwise - virtual void UnregisterReadHandler(ReadHandler * aReadHandler) = 0; - /// @brief Unregister all ReadHandlers from the scheduler - /// @return CHIP_NO_ERROR if all ReadHandlers were successfully unregistered, specific error otherwise - virtual void UnregisterAllHandlers() = 0; /// @brief Check if a ReadHandler is scheduled for reporting virtual bool IsReportScheduled(ReadHandler * aReadHandler) = 0; /// @brief Check whether a ReadHandler is reportable right now, taking into account its minimum and maximum intervals. @@ -131,9 +119,6 @@ class ReportScheduler : public ReadHandler::Observer { return aReadHandler->IsGeneratingReports() && aReadHandler->IsDirty(); } - /// @brief Check the ReadHandler's ChunkedReport flag to prevent rescheduling if the Schedule is called when the engine is - /// processing a chunked report - bool IsChunkedReport(ReadHandler * aReadHandler) const { return aReadHandler->IsChunkedReport(); } /// @brief Get the number of ReadHandlers registered in the scheduler's node pool size_t GetNumReadHandlers() const { return mNodesPool.Allocated(); } @@ -144,13 +129,17 @@ class ReportScheduler : public ReadHandler::Observer /// @brief Find the ReadHandlerNode for a given ReadHandler pointer /// @param [in] aReadHandler ReadHandler pointer to look for in the ReadHandler nodes list /// @return Node Address if node was found, nullptr otherwise - virtual ReadHandlerNode * FindReadHandlerNode(const ReadHandler * aReadHandler) = 0; - /// @brief Start a timer for a given ReadHandlerNode, ensures that if a timer is already running for this node, it is cancelled - /// @param node Node of the ReadHandler list to start a timer for - /// @param aTimeout Delay before the timer expires - virtual CHIP_ERROR StartTimerForHandler(ReadHandlerNode * node, System::Clock::Timeout aTimeout) = 0; - virtual void CancelTimerForHandler(ReadHandlerNode * node) = 0; - virtual bool CheckTimerActiveForHandler(ReadHandlerNode * node) = 0; + ReadHandlerNode * FindReadHandlerNode(const ReadHandler * aReadHandler) + { + for (auto & iter : mReadHandlerList) + { + if (iter.GetReadHandler() == aReadHandler) + { + return &iter; + } + } + return nullptr; + } IntrusiveList mReadHandlerList; ObjectPool mNodesPool; diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index ec46a6c1c89a72..8b62ad49536791 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -62,10 +62,10 @@ void ReportSchedulerImpl::OnBecameReportable(ReadHandler * aReadHandler) else { // If the handler is not reportable now, schedule a report for the min interval - newTimeout = Milliseconds32(node->GetMinTimestamp().count() - mTimerDelegate->GetCurrentMonotonicTimestamp().count()); + newTimeout = node->GetMinTimestamp() - mTimerDelegate->GetCurrentMonotonicTimestamp(); } - ScheduleReport(newTimeout, aReadHandler); + ScheduleReport(newTimeout, node); } void ReportSchedulerImpl::OnSubscriptionAction(ReadHandler * apReadHandler) @@ -74,9 +74,8 @@ void ReportSchedulerImpl::OnSubscriptionAction(ReadHandler * apReadHandler) VerifyOrReturn(nullptr != node); // Schedule callback for max interval by computing the difference between the max timestamp and the current timestamp node->SetIntervalTimeStamps(apReadHandler); - Milliseconds32 newTimeout = - Milliseconds32(node->GetMaxTimestamp().count() - mTimerDelegate->GetCurrentMonotonicTimestamp().count()); - ScheduleReport(newTimeout, apReadHandler); + Milliseconds32 newTimeout = node->GetMaxTimestamp() - mTimerDelegate->GetCurrentMonotonicTimestamp(); + ScheduleReport(newTimeout, node); } /// @brief When a ReadHandler is removed, unregister it, which will cancel any scheduled report @@ -89,7 +88,9 @@ CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) { ReadHandlerNode * newNode = FindReadHandlerNode(aReadHandler); // If the handler is already registered, no need to register it again - VerifyOrReturnValue(nullptr == newNode, CHIP_NO_ERROR); + VerifyOrDie(nullptr == newNode); + // 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, ReportTimerCallback); mReadHandlerList.PushBack(newNode); @@ -110,33 +111,23 @@ CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) { // If the handler is reportable now, but the min interval is not elapsed, schedule a report for the moment the min interval // has elapsed - newTimeout = Milliseconds32(newNode->GetMinTimestamp().count() - now.count()); - - ReturnErrorOnFailure(ScheduleReport(newTimeout, aReadHandler)); + newTimeout = newNode->GetMinTimestamp() - now; } else { // If the handler is not reportable now, schedule a report for the max interval - newTimeout = Milliseconds32(newNode->GetMaxTimestamp().count() - now.count()); + newTimeout = newNode->GetMaxTimestamp() - now; } - ReturnErrorOnFailure(ScheduleReport(newTimeout, aReadHandler)); + ReturnErrorOnFailure(ScheduleReport(newTimeout, newNode)); return CHIP_NO_ERROR; } -CHIP_ERROR ReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandler * aReadHandler) +CHIP_ERROR ReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandlerNode * node) { - // Verify the handler is still registered - ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); - VerifyOrReturnError(nullptr != node, CHIP_ERROR_NOT_FOUND); - // Cancel Report if it is currently scheduled - CancelTimerForHandler(node); - - if (!IsChunkedReport(aReadHandler)) - { - StartTimerForHandler(node, timeout); - } + CancelSchedulerTimer(node); + StartSchedulerTimer(node, timeout); return CHIP_NO_ERROR; } @@ -145,13 +136,11 @@ void ReportSchedulerImpl::CancelReport(ReadHandler * aReadHandler) { ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); VerifyOrReturn(nullptr != node); - CancelTimerForHandler(node); + CancelSchedulerTimer(node); } void ReportSchedulerImpl::UnregisterReadHandler(ReadHandler * aReadHandler) { - // Verify list is populated and handler is not null - VerifyOrReturn((!mReadHandlerList.Empty() || (nullptr == aReadHandler))); CancelReport(aReadHandler); ReadHandlerNode * removeNode = FindReadHandlerNode(aReadHandler); @@ -175,33 +164,21 @@ bool ReportSchedulerImpl::IsReportScheduled(ReadHandler * aReadHandler) { ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); VerifyOrReturnValue(nullptr != node, false); - return CheckTimerActiveForHandler(node); -} - -ReportSchedulerImpl::ReadHandlerNode * ReportSchedulerImpl::FindReadHandlerNode(const ReadHandler * aReadHandler) -{ - for (auto & iter : mReadHandlerList) - { - if (iter.GetReadHandler() == aReadHandler) - { - return &iter; - } - } - return nullptr; + return CheckSchedulerTimerActive(node); } -CHIP_ERROR ReportSchedulerImpl::StartTimerForHandler(ReadHandlerNode * node, System::Clock::Timeout aTimeout) +CHIP_ERROR ReportSchedulerImpl::StartSchedulerTimer(ReadHandlerNode * node, System::Clock::Timeout aTimeout) { // Schedule Report return mTimerDelegate->StartTimer(node, aTimeout); } -void ReportSchedulerImpl::CancelTimerForHandler(ReadHandlerNode * node) +void ReportSchedulerImpl::CancelSchedulerTimer(ReadHandlerNode * node) { mTimerDelegate->CancelTimer(node); } -bool ReportSchedulerImpl::CheckTimerActiveForHandler(ReadHandlerNode * node) +bool ReportSchedulerImpl::CheckSchedulerTimerActive(ReadHandlerNode * node) { return mTimerDelegate->IsTimerActive(node); } diff --git a/src/app/reporting/ReportSchedulerImpl.h b/src/app/reporting/ReportSchedulerImpl.h index 60deca82bd40ce..849f9b797b5f93 100644 --- a/src/app/reporting/ReportSchedulerImpl.h +++ b/src/app/reporting/ReportSchedulerImpl.h @@ -28,33 +28,34 @@ class ReportSchedulerImpl : public ReportScheduler { public: ReportSchedulerImpl(TimerDelegate * aTimerDelegate); - virtual ~ReportSchedulerImpl() override { UnregisterAllHandlers(); } + ~ReportSchedulerImpl() override { UnregisterAllHandlers(); } // ReadHandlerObserver - virtual void OnReadHandlerCreated(ReadHandler * aReadHandler) override; - virtual void OnBecameReportable(ReadHandler * aReadHandler) override; - virtual void OnSubscriptionAction(ReadHandler * aReadHandler) override; - virtual void OnReadHandlerDestroyed(ReadHandler * aReadHandler) override; - - // ReportScheduler specific - virtual CHIP_ERROR RegisterReadHandler(ReadHandler * aReadHandler) override; - virtual CHIP_ERROR ScheduleReport(System::Clock::Timeout timeout, ReadHandler * aReadHandler) override; - virtual void CancelReport(ReadHandler * aReadHandler) override; - virtual void UnregisterReadHandler(ReadHandler * aReadHandler) override; - virtual void UnregisterAllHandlers() override; - virtual bool IsReportScheduled(ReadHandler * aReadHandler) override; + void OnReadHandlerCreated(ReadHandler * aReadHandler) override; + void OnBecameReportable(ReadHandler * aReadHandler) override; + void OnSubscriptionAction(ReadHandler * aReadHandler) override; + void OnReadHandlerDestroyed(ReadHandler * aReadHandler) override; protected: + virtual CHIP_ERROR RegisterReadHandler(ReadHandler * aReadHandler); + virtual CHIP_ERROR ScheduleReport(System::Clock::Timeout timeout, ReadHandlerNode * node); + virtual void CancelReport(ReadHandler * aReadHandler); + virtual void UnregisterReadHandler(ReadHandler * aReadHandler); + virtual void UnregisterAllHandlers(); + +private: friend class chip::app::reporting::TestReportScheduler; - /// @brief Find the ReadHandlerNode for a given ReadHandler pointer - /// @param [in] aReadHandler - /// @return Node Address if node was found, nullptr otherwise - virtual ReadHandlerNode * FindReadHandlerNode(const ReadHandler * aReadHandler) override; + bool IsReportScheduled(ReadHandler * aReadHandler) override; - virtual CHIP_ERROR StartTimerForHandler(ReadHandlerNode * node, System::Clock::Timeout aTimeout) override; - virtual void CancelTimerForHandler(ReadHandlerNode * node) override; - virtual bool CheckTimerActiveForHandler(ReadHandlerNode * node) override; + /// @brief Start a timer for a given ReadHandlerNode, ensures that if a timer is already running for this node, it is cancelled + /// @param node Node of the ReadHandler list to start a timer for + /// @param aTimeout Delay before the timer expires + virtual CHIP_ERROR StartSchedulerTimer(ReadHandlerNode * node, System::Clock::Timeout aTimeout); + /// @brief Cancel the timer for a given ReadHandlerNode + virtual void CancelSchedulerTimer(ReadHandlerNode * node); + /// @brief Check if the timer for a given ReadHandlerNode is active + virtual bool CheckSchedulerTimerActive(ReadHandlerNode * node); }; } // namespace reporting diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index 1c43ada639022d..308c2cea83eaae 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -25,11 +25,6 @@ namespace { -uint8_t gDebugEventBuffer[128]; -uint8_t gInfoEventBuffer[128]; -uint8_t gCritEventBuffer[128]; -chip::app::CircularEventBuffer gCircularEventBuffer[3]; - class TestContext : public chip::Test::AppContext { public: @@ -45,15 +40,6 @@ class TestContext : public chip::Test::AppContext return FAILURE; } - chip::app::LogStorageResources logStorageResources[] = { - { &gDebugEventBuffer[0], sizeof(gDebugEventBuffer), chip::app::PriorityLevel::Debug }, - { &gInfoEventBuffer[0], sizeof(gInfoEventBuffer), chip::app::PriorityLevel::Info }, - { &gCritEventBuffer[0], sizeof(gCritEventBuffer), chip::app::PriorityLevel::Critical }, - }; - - chip::app::EventManagement::CreateEventManagement(&ctx->GetExchangeManager(), ArraySize(logStorageResources), - gCircularEventBuffer, logStorageResources, &ctx->mEventCounter); - return SUCCESS; } diff --git a/src/platform/telink/ICDUtil.cpp b/src/platform/telink/ICDUtil.cpp index b1c2c4797adb80..b3dc9c80bbc9dd 100644 --- a/src/platform/telink/ICDUtil.cpp +++ b/src/platform/telink/ICDUtil.cpp @@ -36,5 +36,5 @@ CHIP_ERROR ICDUtil::OnSubscriptionRequested(chip::app::ReadHandler & aReadHandle agreedMaxInterval = kSubscriptionMaxIntervalPublisherLimit; } - return aReadHandler.SetMaxReportingIntervals(agreedMaxInterval); + return aReadHandler.SetMaxReportingInterval(agreedMaxInterval); }