From 14393892619819513338353cedaa71714379c28b Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs <112982107+lpbeliveau-silabs@users.noreply.github.com> Date: Tue, 22 Aug 2023 12:58:20 -0400 Subject: [PATCH] [ICD] Report before idle mode (#28714) * Added a check before going to idle mode to emit a report if it would avoid the device waking up earlier * Added check to prevent looping when emiting a report before leaving active mode, Changed OnActiveModeAlmostDone to OnTransitionToIdle, added check to prevent wrap around when computing OnTransitionToIdle timeout * Moved boolean set to true to proper place * Set flag before calling observer method * Added stub in observer test function --- src/app/icd/ICDManager.cpp | 26 +++++++++++++++++++ src/app/icd/ICDManager.h | 8 ++++++ src/app/icd/ICDStateObserver.h | 11 +++++++- src/app/reporting/ReportSchedulerImpl.cpp | 2 ++ src/app/reporting/ReportSchedulerImpl.h | 1 + .../SynchronizedReportSchedulerImpl.cpp | 13 ++++++++++ .../SynchronizedReportSchedulerImpl.h | 2 ++ src/app/tests/TestICDManager.cpp | 1 + 8 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/app/icd/ICDManager.cpp b/src/app/icd/ICDManager.cpp index 287d0dd11a385a..6601877793f8e0 100644 --- a/src/app/icd/ICDManager.cpp +++ b/src/app/icd/ICDManager.cpp @@ -67,6 +67,7 @@ void ICDManager::Shutdown() // cancel any running timer of the icd DeviceLayer::SystemLayer().CancelTimer(OnIdleModeDone, this); DeviceLayer::SystemLayer().CancelTimer(OnActiveModeDone, this); + DeviceLayer::SystemLayer().CancelTimer(OnTransitionToIdle, this); mICDMode = ICDMode::SIT; mOperationalState = OperationalState::IdleMode; mStorage = nullptr; @@ -157,6 +158,9 @@ void ICDManager::UpdateOperationState(OperationalState state) mOperationalState = OperationalState::ActiveMode; uint32_t activeModeInterval = IcdManagementServer::GetInstance().GetActiveModeInterval(); DeviceLayer::SystemLayer().StartTimer(System::Clock::Timeout(activeModeInterval), OnActiveModeDone, this); + uint32_t activeModeJitterInterval = + (activeModeInterval >= ICD_ACTIVE_TIME_JITTER_MS) ? activeModeInterval - ICD_ACTIVE_TIME_JITTER_MS : 0; + DeviceLayer::SystemLayer().StartTimer(System::Clock::Timeout(activeModeJitterInterval), OnTransitionToIdle, this); CHIP_ERROR err = DeviceLayer::ConnectivityMgr().SetPollingInterval(GetFastPollingInterval()); if (err != CHIP_NO_ERROR) @@ -170,6 +174,13 @@ void ICDManager::UpdateOperationState(OperationalState state) { uint16_t activeModeThreshold = IcdManagementServer::GetInstance().GetActiveModeThreshold(); DeviceLayer::SystemLayer().ExtendTimerTo(System::Clock::Timeout(activeModeThreshold), OnActiveModeDone, this); + uint16_t activeModeJitterThreshold = + (activeModeThreshold >= ICD_ACTIVE_TIME_JITTER_MS) ? activeModeThreshold - ICD_ACTIVE_TIME_JITTER_MS : 0; + if (!mTransitionToIdleCalled) + { + DeviceLayer::SystemLayer().ExtendTimerTo(System::Clock::Timeout(activeModeJitterThreshold), OnTransitionToIdle, + this); + } } } } @@ -195,6 +206,10 @@ void ICDManager::OnIdleModeDone(System::Layer * aLayer, void * appState) { ICDManager * pIcdManager = reinterpret_cast(appState); pIcdManager->UpdateOperationState(OperationalState::ActiveMode); + + // We only reset this flag when idle mode is complete to avoid re-triggering the check when an event brings us back to active, + // which could cause a loop. + pIcdManager->mTransitionToIdleCalled = false; } void ICDManager::OnActiveModeDone(System::Layer * aLayer, void * appState) @@ -207,5 +222,16 @@ void ICDManager::OnActiveModeDone(System::Layer * aLayer, void * appState) pIcdManager->UpdateOperationState(OperationalState::IdleMode); } } + +void ICDManager::OnTransitionToIdle(System::Layer * aLayer, void * appState) +{ + ICDManager * pIcdManager = reinterpret_cast(appState); + + // OnTransitionToIdle will trigger a report message if reporting is needed, which should extend the active mode until the + // ack for the report is received. + pIcdManager->mTransitionToIdleCalled = true; + pIcdManager->mStateObserver->OnTransitionToIdle(); +} + } // namespace app } // namespace chip diff --git a/src/app/icd/ICDManager.h b/src/app/icd/ICDManager.h index 6e9890240c5eee..7a84a1a3c4dfee 100644 --- a/src/app/icd/ICDManager.h +++ b/src/app/icd/ICDManager.h @@ -75,6 +75,13 @@ class ICDManager static void OnIdleModeDone(System::Layer * aLayer, void * appState); static void OnActiveModeDone(System::Layer * aLayer, void * appState); + /** + * @brief Callback function called shortly before the device enters idle mode to allow checks to be made. This is currently only + * called once to prevent entering in a loop if some events re-trigger this check (for instance if a check for subscription + * before entering idle mode leads to emiting a report, we will re-enter UpdateOperationState and check again for subscription, + * etc.) + */ + static void OnTransitionToIdle(System::Layer * aLayer, void * appState); private: // SIT ICDs should have a SlowPollingThreshold shorter than or equal to 15s (spec 9.16.1.5) @@ -95,6 +102,7 @@ class ICDManager PersistentStorageDelegate * mStorage = nullptr; FabricTable * mFabricTable = nullptr; ICDStateObserver * mStateObserver = nullptr; + bool mTransitionToIdleCalled = false; }; } // namespace app diff --git a/src/app/icd/ICDStateObserver.h b/src/app/icd/ICDStateObserver.h index aa088a7b87f81a..7c71d4b3c5b463 100644 --- a/src/app/icd/ICDStateObserver.h +++ b/src/app/icd/ICDStateObserver.h @@ -16,6 +16,14 @@ */ #pragma once +#ifndef ICD_SLEEP_TIME_JITTER_MS +#define ICD_SLEEP_TIME_JITTER_MS (CHIP_CONFIG_ICD_IDLE_MODE_INTERVAL * 0.75) +#endif + +#ifndef ICD_ACTIVE_TIME_JITTER_MS +#define ICD_ACTIVE_TIME_JITTER_MS 300 +#endif + namespace chip { namespace app { @@ -23,7 +31,8 @@ class ICDStateObserver { public: virtual ~ICDStateObserver() {} - virtual void OnEnterActiveMode() = 0; + virtual void OnEnterActiveMode() = 0; + virtual void OnTransitionToIdle() = 0; }; } // namespace app diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index 41d80cc7114ae6..6c24dd0f5d7dc7 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -37,6 +37,8 @@ ReportSchedulerImpl::ReportSchedulerImpl(TimerDelegate * aTimerDelegate) : Repor 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() diff --git a/src/app/reporting/ReportSchedulerImpl.h b/src/app/reporting/ReportSchedulerImpl.h index f3870192c4b1f6..33c70d39f53f21 100644 --- a/src/app/reporting/ReportSchedulerImpl.h +++ b/src/app/reporting/ReportSchedulerImpl.h @@ -34,6 +34,7 @@ class ReportSchedulerImpl : public ReportScheduler // ICDStateObserver void OnEnterActiveMode() override; + void OnTransitionToIdle() override; // ReadHandlerObserver void OnSubscriptionEstablished(ReadHandler * aReadHandler) final; diff --git a/src/app/reporting/SynchronizedReportSchedulerImpl.cpp b/src/app/reporting/SynchronizedReportSchedulerImpl.cpp index ddb8bb7b1c4149..55992e35febd24 100644 --- a/src/app/reporting/SynchronizedReportSchedulerImpl.cpp +++ b/src/app/reporting/SynchronizedReportSchedulerImpl.cpp @@ -44,6 +44,19 @@ void SynchronizedReportSchedulerImpl::OnReadHandlerDestroyed(ReadHandler * aRead } } +void SynchronizedReportSchedulerImpl::OnTransitionToIdle() +{ + Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); + uint32_t targetIdleInterval = static_cast(ICD_SLEEP_TIME_JITTER_MS); + VerifyOrReturn(now >= mTestNextReportTimestamp); + if (((mTestNextReportTimestamp - now) < Seconds16(targetIdleInterval)) && (now > mNextMinTimestamp)) + { + // If the next report is due in less than the idle mode interval and we are past the min interval, we can just send it now + CancelReport(); + TimerFired(); + } +} + CHIP_ERROR SynchronizedReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandlerNode * node, const Timestamp & now) { // Cancel Report if it is currently scheduled diff --git a/src/app/reporting/SynchronizedReportSchedulerImpl.h b/src/app/reporting/SynchronizedReportSchedulerImpl.h index 9e3689d144194a..69f9649b3a575f 100644 --- a/src/app/reporting/SynchronizedReportSchedulerImpl.h +++ b/src/app/reporting/SynchronizedReportSchedulerImpl.h @@ -38,6 +38,8 @@ class SynchronizedReportSchedulerImpl : public ReportSchedulerImpl, public Timer SynchronizedReportSchedulerImpl(TimerDelegate * aTimerDelegate) : ReportSchedulerImpl(aTimerDelegate) {} ~SynchronizedReportSchedulerImpl() override { UnregisterAllHandlers(); } + void OnTransitionToIdle() override; + bool IsReportScheduled(); void TimerFired() override; diff --git a/src/app/tests/TestICDManager.cpp b/src/app/tests/TestICDManager.cpp index 81739aea43ceb4..2f5546f067c2e4 100644 --- a/src/app/tests/TestICDManager.cpp +++ b/src/app/tests/TestICDManager.cpp @@ -37,6 +37,7 @@ class TestICDStateObserver : public app::ICDStateObserver { public: void OnEnterActiveMode() {} + void OnTransitionToIdle() {} }; TestICDStateObserver mICDStateObserver;