From 376178276f60a569f40881b1263dcf5e82b28749 Mon Sep 17 00:00:00 2001 From: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com> Date: Wed, 21 Jun 2023 13:27:33 -0400 Subject: [PATCH] =?UTF-8?q?Add=20methods=20ExtendTimerTo=20and=20IsTimerAc?= =?UTF-8?q?tive=20to=20the=20SystemLayerTimer.=20=E2=80=A6=20(#27332)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * Update src/system/SystemLayerImplSelect.cpp Co-authored-by: Boris Zbarsky * Apply comment, add a check in ExtendTimerTo. test that check in TestSystemTimer --------- Co-authored-by: Justin Wood Co-authored-by: Boris Zbarsky --- src/system/SystemLayer.h | 34 +++++++ src/system/SystemLayerImplFreeRTOS.cpp | 20 ++++ src/system/SystemLayerImplFreeRTOS.h | 2 + src/system/SystemLayerImplSelect.cpp | 40 ++++++++ src/system/SystemLayerImplSelect.h | 2 + src/system/SystemTimer.cpp | 18 ++++ src/system/SystemTimer.h | 7 ++ src/system/tests/TestSystemTimer.cpp | 132 +++++++++++++++++++++++++ 8 files changed, 255 insertions(+) diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h index 279ac057b05ff7..566abb2a0a3378 100644 --- 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 index ecc8d9504b0f14..cfc8e091208f86 100644 --- 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 index 846e17b21854db..2e7401dfce03ce 100644 --- 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 index 513cfff4860f5a..9eb0b2ece2864b 100644 --- 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 index 552e8d9e40163d..c835b8a76720c5 100644 --- 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 index cb54d075ee23e1..3612b9c10461e2 100644 --- 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 index cd27fefcf11ef2..b8f727e566846e 100644 --- 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 index bc5d3bc6206870..1648c700f46e37 100644 --- 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::HasServiceEvents()) + return; + + TestContext & testContext = *static_cast(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(state)->Record('A'); } + static void B(Layer * layer, void * state) { static_cast(state)->Record('B'); } + static void C(Layer * layer, void * state) { static_cast(state)->Record('C'); } + static void D(Layer * layer, void * state) { static_cast(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::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::ServiceEvents(systemLayer); + NL_TEST_ASSERT(suite, strcmp(testState.record, "A") == 0); + + mockClock.AdvanceMonotonic(25_ms); + LayerEvents::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::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::HasServiceEvents()) + return; + + TestContext & testContext = *static_cast(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(state)->Record('A'); } + static void B(Layer * layer, void * state) { static_cast(state)->Record('B'); } + static void C(Layer * layer, void * state) { static_cast(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::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::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::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