Add methods ExtendTimerTo and IsTimerActive to the SystemLayerTimer. … (

#27332) * Add methods ExtendTimerTo and IsTimerActive to the SystemLayerTimer. Add tests for those new methods * correct spelling * Update src/system/SystemLayer.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/system/SystemLayerImplSelect.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Apply comment, add a check in ExtendTimerTo. test that check in TestSystemTimer --------- Co-authored-by: Justin Wood <woody@apple.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
project-chip · Feb 13, 2024 · 3761782 · 3761782
1 parent 7ce20d2
commit 3761782
Show file tree

Hide file tree

Showing 8 changed files with 255 additions and 0 deletions.
diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h
@@ -116,6 +116,40 @@ class DLL_EXPORT Layer
      */
     virtual CHIP_ERROR StartTimer(Clock::Timeout aDelay, TimerCompleteCallback aComplete, void * aAppState) = 0;
 
+    /**
+     * @brief
+     *   This method extends the timer expiry to the provided aDelay. This method must be called while in the Matter context
+     *   (from the Matter event loop, or while holding the Matter stack lock).
+     *   aDelay is not added to the Remaining time of the timer. The finish line is pushed back to aDelay.
+     *
+     *   @note The goal of this method is that the timer remaining time cannot be shrunk and only extended to a new time
+     *         If the provided new Delay is smaller than the timer's remaining time, the timer is left untouched.
+     *         In the other case the method acts like StartTimer
+     *
+     *   @param[in]  aDelay             Time before this timer fires.
+     *   @param[in]  aComplete          A pointer to the function called when timer expires.
+     *   @param[in]  aAppState          A pointer to the application state object used when timer expires.
+     *
+     *   @return CHIP_NO_ERROR On success.
+     *   @return CHIP_ERROR_INVALID_ARGUMENT If the provided aDelay value is 0
+     *   @return CHIP_ERROR_NO_MEMORY If a timer cannot be allocated.
+     *   @return Other Value indicating timer failed to start.
+     */
+    virtual CHIP_ERROR ExtendTimerTo(Clock::Timeout aDelay, TimerCompleteCallback aComplete, void * aAppState) = 0;
+
+    /**
+     * @brief
+     *   This method searches for the timer matching the provided parameters.
+     *   and returns whether it is still "running" and waiting to trigger or not.
+     *
+     *   @param[in]  onComplete         A pointer to the function called when timer expires.
+     *   @param[in]  appState           A pointer to the application state object used when timer expires.
+     *
+     *   @return True if there is a current timer set to call, at some point in the future, the provided onComplete callback
+     *           with the corresponding appState context. False otherwise.
+     */
+    virtual bool IsTimerActive(TimerCompleteCallback onComplete, void * appState) = 0;
+
     /**
      * @brief This method cancels a one-shot timer, started earlier through @p StartTimer().  This method must
      *        be called while in the Matter context (from the Matter event loop, or while holding the Matter

diff --git a/src/system/SystemLayerImplFreeRTOS.cpp b/src/system/SystemLayerImplFreeRTOS.cpp
@@ -76,6 +76,26 @@ CHIP_ERROR LayerImplFreeRTOS::StartTimer(Clock::Timeout delay, TimerCompleteCall
     return CHIP_NO_ERROR;
 }
 
+CHIP_ERROR LayerImplFreeRTOS::ExtendTimerTo(Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState)
+{
+    VerifyOrReturnError(delay.count() > 0, CHIP_ERROR_INVALID_ARGUMENT);
+
+    assertChipStackLockedByCurrentThread();
+
+    Clock::Timeout remainingTime = mTimerList.GetRemainingTime(onComplete, appState);
+    if (remainingTime.count() < delay.count())
+    {
+        return StartTimer(delay, onComplete, appState);
+    }
+
+    return CHIP_NO_ERROR;
+}
+
+bool LayerImplFreeRTOS::IsTimerActive(TimerCompleteCallback onComplete, void * appState)
+{
+    return (mTimerList.GetRemainingTime(onComplete, appState) > Clock::kZero);
+}
+
 void LayerImplFreeRTOS::CancelTimer(TimerCompleteCallback onComplete, void * appState)
 {
     assertChipStackLockedByCurrentThread();

diff --git a/src/system/SystemLayerImplFreeRTOS.h b/src/system/SystemLayerImplFreeRTOS.h
@@ -40,6 +40,8 @@ class LayerImplFreeRTOS : public LayerFreeRTOS
     void Shutdown() override;
     bool IsInitialized() const override { return mLayerState.IsInitialized(); }
     CHIP_ERROR StartTimer(Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState) override;
+    CHIP_ERROR ExtendTimerTo(Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState) override;
+    bool IsTimerActive(TimerCompleteCallback onComplete, void * appState) override;
     void CancelTimer(TimerCompleteCallback onComplete, void * appState) override;
     CHIP_ERROR ScheduleWork(TimerCompleteCallback onComplete, void * appState) override;
 

diff --git a/src/system/SystemLayerImplSelect.cpp b/src/system/SystemLayerImplSelect.cpp
@@ -200,6 +200,46 @@ CHIP_ERROR LayerImplSelect::StartTimer(Clock::Timeout delay, TimerCompleteCallba
 #endif // !CHIP_SYSTEM_CONFIG_USE_LIBEV
 }
 
+CHIP_ERROR LayerImplSelect::ExtendTimerTo(Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState)
+{
+    VerifyOrReturnError(delay.count() > 0, CHIP_ERROR_INVALID_ARGUMENT);
+
+    assertChipStackLockedByCurrentThread();
+
+    Clock::Timeout remainingTime = mTimerList.GetRemainingTime(onComplete, appState);
+    if (remainingTime.count() < delay.count())
+    {
+        if (remainingTime == Clock::kZero)
+        {
+            // If remaining time is Clock::kZero, it might possible that our timer is in
+            // the mExpiredTimers list and about to be fired. Remove it from that list, since we are extending it.
+            mExpiredTimers.Remove(onComplete, appState);
+        }
+        return StartTimer(delay, onComplete, appState);
+    }
+
+    return CHIP_NO_ERROR;
+}
+
+bool LayerImplSelect::IsTimerActive(TimerCompleteCallback onComplete, void * appState)
+{
+    bool timerIsActive = (mTimerList.GetRemainingTime(onComplete, appState) > Clock::kZero);
+
+    if (!timerIsActive)
+    {
+        // check if the timer is in the mExpiredTimers list about to be fired.
+        for (TimerList::Node * timer = mExpiredTimers.Earliest(); timer != nullptr; timer = timer->mNextTimer)
+        {
+            if (timer->GetCallback().GetOnComplete() == onComplete && timer->GetCallback().GetAppState() == appState)
+            {
+                return true;
+            }
+        }
+    }
+
+    return timerIsActive;
+}
+
 void LayerImplSelect::CancelTimer(TimerCompleteCallback onComplete, void * appState)
 {
     assertChipStackLockedByCurrentThread();

diff --git a/src/system/SystemLayerImplSelect.h b/src/system/SystemLayerImplSelect.h
@@ -61,6 +61,8 @@ class LayerImplSelect : public LayerSocketsLoop
     void Shutdown() override;
     bool IsInitialized() const override { return mLayerState.IsInitialized(); }
     CHIP_ERROR StartTimer(Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState) override;
+    CHIP_ERROR ExtendTimerTo(Clock::Timeout delay, TimerCompleteCallback onComplete, void * appState) override;
+    bool IsTimerActive(TimerCompleteCallback onComplete, void * appState) override;
     void CancelTimer(TimerCompleteCallback onComplete, void * appState) override;
     CHIP_ERROR ScheduleWork(TimerCompleteCallback onComplete, void * appState) override;
 

diff --git a/src/system/SystemTimer.cpp b/src/system/SystemTimer.cpp
@@ -160,5 +160,23 @@ TimerList TimerList::ExtractEarlier(Clock::Timestamp t)
     return out;
 }
 
+Clock::Timeout TimerList::GetRemainingTime(TimerCompleteCallback aOnComplete, void * aAppState)
+{
+    for (TimerList::Node * timer = mEarliestTimer; timer != nullptr; timer = timer->mNextTimer)
+    {
+        if (timer->GetCallback().GetOnComplete() == aOnComplete && timer->GetCallback().GetAppState() == aAppState)
+        {
+            Clock::Timestamp currentTime = SystemClock().GetMonotonicTimestamp();
+
+            if (currentTime < timer->AwakenTime())
+            {
+                return Clock::Timeout(timer->AwakenTime() - currentTime);
+            }
+            return Clock::kZero;
+        }
+    }
+    return Clock::kZero;
+}
+
 } // namespace System
 } // namespace chip
diff --git a/src/system/SystemTimer.h b/src/system/SystemTimer.h
@@ -179,6 +179,13 @@ class TimerList
      */
     void Clear() { mEarliestTimer = nullptr; }
 
+    /**
+     * Find the timer with the given properties, if present, and return its remaining time
+     *
+     * @return The remaining time on this partifcular timer or 0 if not found.
+     */
+    Clock::Timeout GetRemainingTime(TimerCompleteCallback aOnComplete, void * aAppState);
+
 private:
     Node * mEarliestTimer;
 };

diff --git a/src/system/tests/TestSystemTimer.cpp b/src/system/tests/TestSystemTimer.cpp
@@ -580,6 +580,136 @@ void chip::System::TestTimer::CheckTimerPool(nlTestSuite * inSuite, void * aCont
     NL_TEST_ASSERT(suite, SYSTEM_STATS_TEST_HIGH_WATER_MARK(Stats::kSystemLayer_NumTimers, 4));
 }
 
+static void ExtendTimerToTest(nlTestSuite * inSuite, void * aContext)
+{
+    if (!LayerEvents<LayerImpl>::HasServiceEvents())
+        return;
+
+    TestContext & testContext = *static_cast<TestContext *>(aContext);
+    Layer & systemLayer       = *testContext.mLayer;
+    nlTestSuite * const suite = testContext.mTestSuite;
+
+    struct TestState
+    {
+        void Record(char c)
+        {
+            size_t n = strlen(record);
+            if (n + 1 < sizeof(record))
+            {
+                record[n++] = c;
+                record[n]   = 0;
+            }
+        }
+        static void A(Layer * layer, void * state) { static_cast<TestState *>(state)->Record('A'); }
+        static void B(Layer * layer, void * state) { static_cast<TestState *>(state)->Record('B'); }
+        static void C(Layer * layer, void * state) { static_cast<TestState *>(state)->Record('C'); }
+        static void D(Layer * layer, void * state) { static_cast<TestState *>(state)->Record('D'); }
+        char record[5] = { 0 };
+    };
+    TestState testState;
+    NL_TEST_ASSERT(suite, testState.record[0] == 0);
+
+    Clock::ClockBase * const savedClock = &SystemClock();
+    Clock::Internal::MockClock mockClock;
+    Clock::Internal::SetSystemClockForTesting(&mockClock);
+
+    using namespace Clock::Literals;
+    systemLayer.StartTimer(150_ms, TestState::B, &testState);
+    systemLayer.StartTimer(200_ms, TestState::C, &testState);
+    systemLayer.StartTimer(150_ms, TestState::D, &testState);
+
+    // Timer wasn't started before. ExtendTimerTo will start it.
+    systemLayer.ExtendTimerTo(100_ms, TestState::A, &testState);
+    mockClock.AdvanceMonotonic(100_ms);
+    LayerEvents<LayerImpl>::ServiceEvents(systemLayer);
+    NL_TEST_ASSERT(suite, strcmp(testState.record, "A") == 0);
+
+    // Timer B as 50ms remaining. ExtendTimerTo 25 should have no effect
+    // Timer C as 100ms remaining. ExtendTimerTo 75ms should have no effect
+    // Timer D as 50ms remaining. Timer should be extend to a duration of 75ms
+    systemLayer.ExtendTimerTo(25_ms, TestState::B, &testState);
+    systemLayer.ExtendTimerTo(75_ms, TestState::D, &testState);
+    systemLayer.ExtendTimerTo(75_ms, TestState::D, &testState);
+
+    mockClock.AdvanceMonotonic(25_ms);
+    LayerEvents<LayerImpl>::ServiceEvents(systemLayer);
+    NL_TEST_ASSERT(suite, strcmp(testState.record, "A") == 0);
+
+    mockClock.AdvanceMonotonic(25_ms);
+    LayerEvents<LayerImpl>::ServiceEvents(systemLayer);
+    NL_TEST_ASSERT(suite, strcmp(testState.record, "AB") == 0);
+
+    // Timer D as 25ms remaining. Timer should be extend to a duration of 75ms
+    systemLayer.ExtendTimerTo(75_ms, TestState::D, &testState);
+    mockClock.AdvanceMonotonic(100_ms);
+    LayerEvents<LayerImpl>::ServiceEvents(systemLayer);
+    NL_TEST_ASSERT(suite, strcmp(testState.record, "ABCD") == 0);
+
+    Clock::Internal::SetSystemClockForTesting(savedClock);
+
+    // Extending a timer by 0 ms permitted
+    NL_TEST_ASSERT(suite, systemLayer.ExtendTimerTo(0_ms, TestState::A, &testState) == CHIP_ERROR_INVALID_ARGUMENT);
+}
+
+static void IsTimerActiveTest(nlTestSuite * inSuite, void * aContext)
+{
+    if (!LayerEvents<LayerImpl>::HasServiceEvents())
+        return;
+
+    TestContext & testContext = *static_cast<TestContext *>(aContext);
+    Layer & systemLayer       = *testContext.mLayer;
+    nlTestSuite * const suite = testContext.mTestSuite;
+
+    struct TestState
+    {
+        void Record(char c)
+        {
+            size_t n = strlen(record);
+            if (n + 1 < sizeof(record))
+            {
+                record[n++] = c;
+                record[n]   = 0;
+            }
+        }
+        static void A(Layer * layer, void * state) { static_cast<TestState *>(state)->Record('A'); }
+        static void B(Layer * layer, void * state) { static_cast<TestState *>(state)->Record('B'); }
+        static void C(Layer * layer, void * state) { static_cast<TestState *>(state)->Record('C'); }
+        char record[4] = { 0 };
+    };
+    TestState testState;
+    NL_TEST_ASSERT(suite, testState.record[0] == 0);
+
+    Clock::ClockBase * const savedClock = &SystemClock();
+    Clock::Internal::MockClock mockClock;
+    Clock::Internal::SetSystemClockForTesting(&mockClock);
+
+    using namespace Clock::Literals;
+    systemLayer.StartTimer(100_ms, TestState::A, &testState);
+    systemLayer.StartTimer(200_ms, TestState::B, &testState);
+    systemLayer.StartTimer(300_ms, TestState::C, &testState);
+
+    NL_TEST_ASSERT(suite, systemLayer.IsTimerActive(TestState::A, &testState));
+    NL_TEST_ASSERT(suite, systemLayer.IsTimerActive(TestState::B, &testState));
+    NL_TEST_ASSERT(suite, systemLayer.IsTimerActive(TestState::C, &testState));
+
+    mockClock.AdvanceMonotonic(100_ms);
+    LayerEvents<LayerImpl>::ServiceEvents(systemLayer);
+    NL_TEST_ASSERT(suite, systemLayer.IsTimerActive(TestState::A, &testState) == false);
+    NL_TEST_ASSERT(suite, systemLayer.IsTimerActive(TestState::B, &testState));
+    NL_TEST_ASSERT(suite, systemLayer.IsTimerActive(TestState::C, &testState));
+
+    mockClock.AdvanceMonotonic(100_ms);
+    LayerEvents<LayerImpl>::ServiceEvents(systemLayer);
+    NL_TEST_ASSERT(suite, systemLayer.IsTimerActive(TestState::B, &testState) == false);
+    NL_TEST_ASSERT(suite, systemLayer.IsTimerActive(TestState::C, &testState));
+
+    mockClock.AdvanceMonotonic(100_ms);
+    LayerEvents<LayerImpl>::ServiceEvents(systemLayer);
+    NL_TEST_ASSERT(suite, systemLayer.IsTimerActive(TestState::C, &testState) == false);
+
+    Clock::Internal::SetSystemClockForTesting(savedClock);
+}
+
 // Test Suite
 
 /**
@@ -594,6 +724,8 @@ static const nlTest sTests[] =
     NL_TEST_DEF("Timer::TestTimerCancellation",    CheckCancellation),
     NL_TEST_DEF("Timer::TestTimerPool",            chip::System::TestTimer::CheckTimerPool),
     NL_TEST_DEF("Timer::TestCancelTimer",          CancelTimerTest::Test),
+    NL_TEST_DEF("Timer::ExtendTimerTo",            ExtendTimerToTest),
+    NL_TEST_DEF("Timer::TestIsTimerActive",        IsTimerActiveTest),
     NL_TEST_SENTINEL()
 };
 // clang-format on