From 382b8184963c5510b9dcbc9af78c4587e496f910 Mon Sep 17 00:00:00 2001 From: Kevin Schoedel <67607049+kpschoedel@users.noreply.github.com> Date: Fri, 5 Nov 2021 11:04:49 -0400 Subject: [PATCH] Remove Microseconds from System::Clock (#11450) #### Problem `System::Clock` has microsecond-resolution code, but nothing actually uses it. #### Change overview Remove it. #### Testing CI; no change to functionality. Microsecond unit tests removed. --- .../tests/TestActiveResolveAttempts.cpp | 1 - src/platform/ESP32/SystemTimeSupport.cpp | 7 +-- src/platform/FreeRTOS/SystemTimeSupport.cpp | 5 -- src/platform/Linux/SystemTimeSupport.cpp | 5 -- src/platform/Zephyr/SystemTimeSupport.cpp | 5 -- src/platform/mbed/SystemTimeSupport.cpp | 10 +--- src/platform/tests/TestPlatformTime.cpp | 33 ------------- src/system/SystemClock.cpp | 35 +++---------- src/system/SystemClock.h | 49 ++----------------- src/system/tests/TestSystemClock.cpp | 14 +----- 10 files changed, 17 insertions(+), 147 deletions(-) diff --git a/src/lib/dnssd/minimal_mdns/tests/TestActiveResolveAttempts.cpp b/src/lib/dnssd/minimal_mdns/tests/TestActiveResolveAttempts.cpp index ea202c89d0bac0..d165e25a8bd537 100644 --- a/src/lib/dnssd/minimal_mdns/tests/TestActiveResolveAttempts.cpp +++ b/src/lib/dnssd/minimal_mdns/tests/TestActiveResolveAttempts.cpp @@ -29,7 +29,6 @@ using chip::System::Clock::Timeout; class MockClock : public System::Clock::ClockBase { public: - System::Clock::Microseconds64 GetMonotonicMicroseconds64() override { return mMsec; } System::Clock::Milliseconds64 GetMonotonicMilliseconds64() override { return mMsec; } void Advance(System::Clock::Milliseconds32 ms) { mMsec += ms; } diff --git a/src/platform/ESP32/SystemTimeSupport.cpp b/src/platform/ESP32/SystemTimeSupport.cpp index a0b1d734d1bc1e..7ef8f336ce82d4 100644 --- a/src/platform/ESP32/SystemTimeSupport.cpp +++ b/src/platform/ESP32/SystemTimeSupport.cpp @@ -38,14 +38,9 @@ namespace Internal { ClockImpl gClockImpl; } // namespace Internal -Microseconds64 ClockImpl::GetMonotonicMicroseconds64(void) -{ - return Clock::Microseconds64(::esp_timer_get_time()); -} - Milliseconds64 ClockImpl::GetMonotonicMilliseconds64(void) { - return std::chrono::duration_cast(GetMonotonicMicroseconds64()); + return std::chrono::duration_cast(std::chrono::duration(::esp_timer_get_time())); } } // namespace Clock diff --git a/src/platform/FreeRTOS/SystemTimeSupport.cpp b/src/platform/FreeRTOS/SystemTimeSupport.cpp index 712879ee4ef854..b0ae204856f3ed 100644 --- a/src/platform/FreeRTOS/SystemTimeSupport.cpp +++ b/src/platform/FreeRTOS/SystemTimeSupport.cpp @@ -90,11 +90,6 @@ uint64_t FreeRTOSTicksSinceBoot(void) return static_cast(timeOut.xTimeOnEntering) + (static_cast(timeOut.xOverflowCount) << kTicksOverflowShift); } -Clock::Microseconds64 ClockImpl::GetMonotonicMicroseconds64(void) -{ - return Clock::Microseconds64((FreeRTOSTicksSinceBoot() * kMicrosecondsPerSecond) / configTICK_RATE_HZ); -} - Clock::Milliseconds64 ClockImpl::GetMonotonicMilliseconds64(void) { return Clock::Milliseconds64((FreeRTOSTicksSinceBoot() * kMillisecondsPerSecond) / configTICK_RATE_HZ); diff --git a/src/platform/Linux/SystemTimeSupport.cpp b/src/platform/Linux/SystemTimeSupport.cpp index 3397bd185a0dd4..3d229a4f23a9a2 100644 --- a/src/platform/Linux/SystemTimeSupport.cpp +++ b/src/platform/Linux/SystemTimeSupport.cpp @@ -40,11 +40,6 @@ namespace Internal { ClockImpl gClockImpl; } // namespace Internal -Microseconds64 ClockImpl::GetMonotonicMicroseconds64() -{ - return std::chrono::duration_cast(std::chrono::steady_clock::now().time_since_epoch()); -} - Milliseconds64 ClockImpl::GetMonotonicMilliseconds64() { return std::chrono::duration_cast(std::chrono::steady_clock::now().time_since_epoch()); diff --git a/src/platform/Zephyr/SystemTimeSupport.cpp b/src/platform/Zephyr/SystemTimeSupport.cpp index 0eacdb7b2ce364..e9c2c724c55f9d 100644 --- a/src/platform/Zephyr/SystemTimeSupport.cpp +++ b/src/platform/Zephyr/SystemTimeSupport.cpp @@ -38,11 +38,6 @@ namespace Internal { ClockImpl gClockImpl; } // namespace Internal -Microseconds64 ClockImpl::GetMonotonicMicroseconds64(void) -{ - return Microseconds64(k_ticks_to_us_floor64(k_uptime_ticks())); -} - Milliseconds64 ClockImpl::GetMonotonicMilliseconds64(void) { return Milliseconds64(k_uptime_get()); diff --git a/src/platform/mbed/SystemTimeSupport.cpp b/src/platform/mbed/SystemTimeSupport.cpp index 1f4311eaebf74e..3992dfaa05c587 100644 --- a/src/platform/mbed/SystemTimeSupport.cpp +++ b/src/platform/mbed/SystemTimeSupport.cpp @@ -52,23 +52,17 @@ mbed::LowPowerTimer timer() } } // namespace +// Returns count of microseconds extern "C" uint64_t get_clock_monotonic() { return timer().elapsed_time().count(); } -// Platform-specific function for getting monotonic system time in microseconds. -// Returns elapsed time in microseconds since an arbitrary, platform-defined epoch. -Microseconds64 ClockImpl::GetMonotonicMicroseconds64() -{ - return Microseconds64(get_clock_monotonic()); -} - // Platform-specific function for getting monotonic system time in milliseconds. // Return elapsed time in milliseconds since an arbitrary, platform-defined epoch. Milliseconds64 ClockImpl::GetMonotonicMilliseconds64() { - return std::chrono::duration_cast(GetMonotonicMicroseconds64()); + return std::chrono::duration_cast(std::chrono::duration(get_clock_monotonic())); } } // namespace Clock diff --git a/src/platform/tests/TestPlatformTime.cpp b/src/platform/tests/TestPlatformTime.cpp index 95698f0d696f15..5acc494efe36df 100644 --- a/src/platform/tests/TestPlatformTime.cpp +++ b/src/platform/tests/TestPlatformTime.cpp @@ -42,43 +42,11 @@ using namespace chip::System; using namespace chip::System::Clock::Literals; constexpr Clock::Milliseconds64 kTestTimeMarginMs = 2_ms64; -constexpr Clock::Microseconds64 kTestTimeMarginUs = 500_us64; // ================================= // Unit tests // ================================= -static void TestDevice_GetMonotonicMicroseconds(nlTestSuite * inSuite, void * inContext) -{ - static const Clock::Microseconds64 kTestVectorSystemTimeUs[] = { - 600_us64, - 900_us64, - 1500_us64, - }; - int numOfTestsRan = 0; - constexpr Clock::Microseconds64 margin = kTestTimeMarginUs; - - for (const Clock::Microseconds64 & Tdelay : kTestVectorSystemTimeUs) - { - const Clock::Microseconds64 Tstart = System::SystemClock().GetMonotonicMicroseconds64(); - - chip::test_utils::SleepMicros(Tdelay.count()); - - const Clock::Microseconds64 Tend = System::SystemClock().GetMonotonicMicroseconds64(); - const Clock::Microseconds64 Tdelta = Tend - Tstart; - - ChipLogProgress(DeviceLayer, "Start=%" PRIu64 " End=%" PRIu64 " Delta=%" PRIu64 " Expected=%" PRIu64, Tstart.count(), - Tend.count(), Tdelta.count(), Tdelay.count()); - - // verify that timers don't fire early - NL_TEST_ASSERT(inSuite, Tdelta > (Tdelay - margin)); - // verify they're not too late - // NL_TEST_ASSERT(inSuite, Tdelta < (Tdelay + margin)); - numOfTestsRan++; - } - NL_TEST_ASSERT(inSuite, numOfTestsRan > 0); -} - static void TestDevice_GetMonotonicMilliseconds(nlTestSuite * inSuite, void * inContext) { static const System::Clock::Milliseconds64 kTestVectorSystemTimeMs[] = { @@ -115,7 +83,6 @@ static void TestDevice_GetMonotonicMilliseconds(nlTestSuite * inSuite, void * in */ static const nlTest sTests[] = { - NL_TEST_DEF("Test DeviceLayer::GetMonotonicMicroseconds", TestDevice_GetMonotonicMicroseconds), NL_TEST_DEF("Test DeviceLayer::GetMonotonicMilliseconds", TestDevice_GetMonotonicMilliseconds), NL_TEST_SENTINEL() diff --git a/src/system/SystemClock.cpp b/src/system/SystemClock.cpp index 1ec09aa65ef73a..791a687769a131 100644 --- a/src/system/SystemClock.cpp +++ b/src/system/SystemClock.cpp @@ -82,35 +82,25 @@ ClockBase * gClockBase = &gClockImpl; #define MONOTONIC_CLOCK_ID CLOCK_MONOTONIC #endif -Microseconds64 ClockImpl::GetMonotonicMicroseconds64() +Milliseconds64 ClockImpl::GetMonotonicMilliseconds64() { struct timespec ts; int res = clock_gettime(MONOTONIC_CLOCK_ID, &ts); VerifyOrDie(res == 0); return Seconds64(ts.tv_sec) + - std::chrono::duration_cast(std::chrono::duration(ts.tv_nsec)); -} - -Milliseconds64 ClockImpl::GetMonotonicMilliseconds64() -{ - return std::chrono::duration_cast(GetMonotonicMicroseconds64()); + std::chrono::duration_cast(std::chrono::duration(ts.tv_nsec)); } #endif // HAVE_CLOCK_GETTIME #if HAVE_GETTIMEOFDAY -Microseconds64 ClockImpl::GetMonotonicMicroseconds64() +Milliseconds64 ClockImpl::GetMonotonicMilliseconds64() { struct timeval tv; int res = gettimeofday(&tv, NULL); VerifyOrDie(res == 0); - return TimevalToMicroseconds(tv); -} - -Milliseconds64 ClockImpl::GetMonotonicMilliseconds64() -{ - return std::chrono::duration_cast(GetMonotonicMicroseconds64()); + return TimevalToMilliseconds(tv); } #endif // HAVE_GETTIMEOFDAY @@ -121,11 +111,6 @@ Milliseconds64 ClockImpl::GetMonotonicMilliseconds64() // -------------------- Default Get/SetClock Functions for LwIP Systems -------------------- -Microseconds64 ClockImpl::GetMonotonicMicroseconds64(void) -{ - return GetMonotonicMilliseconds64(); -} - Milliseconds64 ClockImpl::GetMonotonicMilliseconds64(void) { static volatile uint64_t overflow = 0; @@ -172,31 +157,27 @@ Milliseconds64 ClockImpl::GetMonotonicMilliseconds64(void) #if CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS || CHIP_SYSTEM_CONFIG_USE_SOCKETS -Microseconds64 TimevalToMicroseconds(const timeval & tv) +Milliseconds64 TimevalToMilliseconds(const timeval & tv) { - return Seconds64(tv.tv_sec) + Microseconds64(tv.tv_usec); + return Seconds64(tv.tv_sec) + Milliseconds64(tv.tv_usec); } -void ToTimeval(Microseconds64 in, timeval & out) +void ToTimeval(Milliseconds64 in, timeval & out) { Seconds32 seconds = std::chrono::duration_cast(in); in -= seconds; out.tv_sec = static_cast(seconds.count()); - out.tv_usec = static_cast(in.count()); + out.tv_usec = static_cast(1000 * in.count()); } #endif // CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS || CHIP_SYSTEM_CONFIG_USE_SOCKETS -static_assert(std::numeric_limits::is_integer, "Microseconds64 must be an integer type"); -static_assert(std::numeric_limits::is_integer, "Microseconds32 must be an integer type"); static_assert(std::numeric_limits::is_integer, "Milliseconds64 must be an integer type"); static_assert(std::numeric_limits::is_integer, "Milliseconds32 must be an integer type"); static_assert(std::numeric_limits::is_integer, "Seconds64 must be an integer type"); static_assert(std::numeric_limits::is_integer, "Seconds32 must be an integer type"); static_assert(std::numeric_limits::is_integer, "Seconds16 must be an integer type"); -static_assert(std::numeric_limits::digits >= 64, "Microseconds64 must be at least 64 bits"); -static_assert(std::numeric_limits::digits >= 32, "Microseconds32 must be at least 32 bits"); static_assert(std::numeric_limits::digits >= 64, "Milliseconds64 must be at least 64 bits"); static_assert(std::numeric_limits::digits >= 32, "Milliseconds32 must be at least 32 bits"); static_assert(std::numeric_limits::digits >= 64, "Seconds64 must be at least 64 bits"); diff --git a/src/system/SystemClock.h b/src/system/SystemClock.h index fc74a9601cca02..f7633c5162de2c 100644 --- a/src/system/SystemClock.h +++ b/src/system/SystemClock.h @@ -53,9 +53,6 @@ namespace Clock { * `std::chrono::duration_cast<>()`. */ -using Microseconds64 = std::chrono::duration; -using Microseconds32 = std::chrono::duration; - using Milliseconds64 = std::chrono::duration; using Milliseconds32 = std::chrono::duration; @@ -67,19 +64,6 @@ constexpr Seconds16 kZero{ 0 }; namespace Literals { -constexpr Microseconds64 operator""_us(unsigned long long int us) -{ - return Microseconds64(us); -} -constexpr Microseconds64 operator""_us64(unsigned long long int us) -{ - return Microseconds64(us); -} -constexpr Microseconds32 operator""_us32(unsigned long long int us) -{ - return Microseconds32(us); -} - constexpr Milliseconds64 operator""_ms(unsigned long long int ms) { return Milliseconds64(ms); @@ -143,29 +127,6 @@ class ClockBase */ Timestamp GetMonotonicTimestamp() { return GetMonotonicMilliseconds64(); } - /** - * Returns a monotonic system time in units of microseconds. - * - * This function returns an elapsed time in microseconds since an arbitrary, platform-defined epoch. - * The value returned is guaranteed to be ever-increasing (i.e. never wrapping or decreasing) between - * reboots of the system. Additionally, the underlying time source is guaranteed to tick - * continuously during any system sleep modes that do not entail a restart upon wake. - * - * Although some platforms may choose to return a value that measures the time since boot for the - * system, applications must *not* rely on this. - * - * Applications must not rely on the time returned by GetMonotonicMicroseconds64() actually having - * granularity finer than milliseconds. - * - * Platform implementations *must* use the same epoch for GetMonotonicMicroseconds64() and GetMonotonicMilliseconds64(). - * - * Platforms *must* use an epoch such that the upper bit of a value returned by GetMonotonicMicroseconds64() is zero - * for the expected operational life of the system. - * - * @returns Elapsed time in microseconds since an arbitrary, platform-defined epoch. - */ - virtual Microseconds64 GetMonotonicMicroseconds64() = 0; - /** * Returns a monotonic system time in units of milliseconds. * @@ -177,9 +138,8 @@ class ClockBase * Although some platforms may choose to return a value that measures the time since boot for the * system, applications must *not* rely on this. * - * Platform implementations *must* use the same epoch for GetMonotonicMicroseconds64() and GetMonotonicMilliseconds64(). - * (As a consequence of this, and the requirement for GetMonotonicMicroseconds64() to return high bit zero, values - * returned by GetMonotonicMilliseconds64() will have the high ten bits zero.) + * Platforms *must* use an epoch such that the upper bit of a value returned by GetMonotonicMilliseconds64() is zero + * for the expected operational life of the system. * * @returns Elapsed time in milliseconds since an arbitrary, platform-defined epoch. */ @@ -191,7 +151,6 @@ class ClockImpl : public ClockBase { public: ~ClockImpl() = default; - Microseconds64 GetMonotonicMicroseconds64() override; Milliseconds64 GetMonotonicMilliseconds64() override; }; @@ -208,8 +167,8 @@ inline void SetSystemClockForTesting(Clock::ClockBase * clock) } // namespace Internal #if CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS || CHIP_SYSTEM_CONFIG_USE_SOCKETS -Microseconds64 TimevalToMicroseconds(const timeval & in); -void ToTimeval(Microseconds64 in, timeval & out); +Milliseconds64 TimevalToMilliseconds(const timeval & in); +void ToTimeval(Milliseconds64 in, timeval & out); #endif // CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS || CHIP_SYSTEM_CONFIG_USE_SOCKETS } // namespace Clock diff --git a/src/system/tests/TestSystemClock.cpp b/src/system/tests/TestSystemClock.cpp index 3b0fc0977cbb18..99f38b65e220d8 100644 --- a/src/system/tests/TestSystemClock.cpp +++ b/src/system/tests/TestSystemClock.cpp @@ -51,12 +51,8 @@ void TestRealClock(nlTestSuite * inSuite, void * inContext) Clock::Milliseconds64 newMilli = SystemClock().GetMonotonicMilliseconds64(); NL_TEST_ASSERT(inSuite, newMilli >= oldMilli); - Clock::Microseconds64 oldMicro = SystemClock().GetMonotonicMicroseconds64(); - Clock::Microseconds64 newMicro = SystemClock().GetMonotonicMicroseconds64(); - NL_TEST_ASSERT(inSuite, newMicro >= oldMicro); - - Clock::Microseconds64::rep microseconds = newMicro.count(); - NL_TEST_ASSERT(inSuite, (microseconds & 0x8000'0000'0000'0000) == 0); + Clock::Milliseconds64::rep milliseconds = newMilli.count(); + NL_TEST_ASSERT(inSuite, (milliseconds & 0x8000'0000'0000'0000) == 0); #if !CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME && \ (CHIP_SYSTEM_CONFIG_USE_LWIP_MONOTONIC_TIME || CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS) @@ -76,9 +72,6 @@ void TestRealClock(nlTestSuite * inSuite, void * inContext) newMilli = SystemClock().GetMonotonicMilliseconds64(); NL_TEST_ASSERT(inSuite, newMilli > oldMilli); - newMicro = SystemClock().GetMonotonicMicroseconds64(); - NL_TEST_ASSERT(inSuite, newMicro > oldMicro); - #endif // !CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME && (CHIP_SYSTEM_CONFIG_USE_LWIP_MONOTONIC_TIME || // CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS) } @@ -88,7 +81,6 @@ void TestMockClock(nlTestSuite * inSuite, void * inContext) class MockClock : public Clock::ClockBase { public: - Clock::Microseconds64 GetMonotonicMicroseconds64() override { return mTime; } Clock::Milliseconds64 GetMonotonicMilliseconds64() override { return mTime; } Clock::Milliseconds64 mTime = Clock::kZero; }; @@ -98,12 +90,10 @@ void TestMockClock(nlTestSuite * inSuite, void * inContext) Clock::Internal::SetSystemClockForTesting(&clock); NL_TEST_ASSERT(inSuite, SystemClock().GetMonotonicMilliseconds64() == Clock::kZero); - NL_TEST_ASSERT(inSuite, SystemClock().GetMonotonicMicroseconds64() == Clock::kZero); constexpr Clock::Milliseconds64 k1234 = Clock::Milliseconds64(1234); clock.mTime = k1234; NL_TEST_ASSERT(inSuite, SystemClock().GetMonotonicMilliseconds64() == k1234); - NL_TEST_ASSERT(inSuite, SystemClock().GetMonotonicMicroseconds64() == k1234); Clock::Internal::SetSystemClockForTesting(savedRealClock); }