Skip to content

Commit

Permalink
Remove Microseconds from System::Clock (#11450)
Browse files Browse the repository at this point in the history
#### 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.
  • Loading branch information
kpschoedel authored Nov 5, 2021
1 parent ea55863 commit 11c32e8
Show file tree
Hide file tree
Showing 10 changed files with 17 additions and 147 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
7 changes: 1 addition & 6 deletions src/platform/ESP32/SystemTimeSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Milliseconds64>(GetMonotonicMicroseconds64());
return std::chrono::duration_cast<Milliseconds64>(std::chrono::duration<uint64_t, std::micro>(::esp_timer_get_time()));
}

} // namespace Clock
Expand Down
5 changes: 0 additions & 5 deletions src/platform/FreeRTOS/SystemTimeSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,6 @@ uint64_t FreeRTOSTicksSinceBoot(void)
return static_cast<uint64_t>(timeOut.xTimeOnEntering) + (static_cast<uint64_t>(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);
Expand Down
5 changes: 0 additions & 5 deletions src/platform/Linux/SystemTimeSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,6 @@ namespace Internal {
ClockImpl gClockImpl;
} // namespace Internal

Microseconds64 ClockImpl::GetMonotonicMicroseconds64()
{
return std::chrono::duration_cast<Microseconds64>(std::chrono::steady_clock::now().time_since_epoch());
}

Milliseconds64 ClockImpl::GetMonotonicMilliseconds64()
{
return std::chrono::duration_cast<Milliseconds64>(std::chrono::steady_clock::now().time_since_epoch());
Expand Down
5 changes: 0 additions & 5 deletions src/platform/Zephyr/SystemTimeSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
10 changes: 2 additions & 8 deletions src/platform/mbed/SystemTimeSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Milliseconds64>(GetMonotonicMicroseconds64());
return std::chrono::duration_cast<Milliseconds64>(std::chrono::duration<uint64_t, std::micro>(get_clock_monotonic()));
}

} // namespace Clock
Expand Down
33 changes: 0 additions & 33 deletions src/platform/tests/TestPlatformTime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = {
Expand Down Expand Up @@ -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()
Expand Down
35 changes: 8 additions & 27 deletions src/system/SystemClock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Microseconds64>(std::chrono::duration<uint64_t, std::nano>(ts.tv_nsec));
}

Milliseconds64 ClockImpl::GetMonotonicMilliseconds64()
{
return std::chrono::duration_cast<Milliseconds64>(GetMonotonicMicroseconds64());
std::chrono::duration_cast<Milliseconds64>(std::chrono::duration<uint64_t, std::nano>(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<Milliseconds64>(GetMonotonicMicroseconds64());
return TimevalToMilliseconds(tv);
}

#endif // HAVE_GETTIMEOFDAY
Expand All @@ -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;
Expand Down Expand Up @@ -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<Seconds32>(in);
in -= seconds;
out.tv_sec = static_cast<time_t>(seconds.count());
out.tv_usec = static_cast<suseconds_t>(in.count());
out.tv_usec = static_cast<suseconds_t>(1000 * in.count());
}

#endif // CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS || CHIP_SYSTEM_CONFIG_USE_SOCKETS

static_assert(std::numeric_limits<Microseconds64::rep>::is_integer, "Microseconds64 must be an integer type");
static_assert(std::numeric_limits<Microseconds32::rep>::is_integer, "Microseconds32 must be an integer type");
static_assert(std::numeric_limits<Milliseconds64::rep>::is_integer, "Milliseconds64 must be an integer type");
static_assert(std::numeric_limits<Milliseconds32::rep>::is_integer, "Milliseconds32 must be an integer type");
static_assert(std::numeric_limits<Seconds64::rep>::is_integer, "Seconds64 must be an integer type");
static_assert(std::numeric_limits<Seconds32::rep>::is_integer, "Seconds32 must be an integer type");
static_assert(std::numeric_limits<Seconds16::rep>::is_integer, "Seconds16 must be an integer type");

static_assert(std::numeric_limits<Microseconds64::rep>::digits >= 64, "Microseconds64 must be at least 64 bits");
static_assert(std::numeric_limits<Microseconds32::rep>::digits >= 32, "Microseconds32 must be at least 32 bits");
static_assert(std::numeric_limits<Milliseconds64::rep>::digits >= 64, "Milliseconds64 must be at least 64 bits");
static_assert(std::numeric_limits<Milliseconds32::rep>::digits >= 32, "Milliseconds32 must be at least 32 bits");
static_assert(std::numeric_limits<Seconds64::rep>::digits >= 64, "Seconds64 must be at least 64 bits");
Expand Down
49 changes: 4 additions & 45 deletions src/system/SystemClock.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ namespace Clock {
* `std::chrono::duration_cast<>()`.
*/

using Microseconds64 = std::chrono::duration<uint64_t, std::micro>;
using Microseconds32 = std::chrono::duration<uint32_t, std::micro>;

using Milliseconds64 = std::chrono::duration<uint64_t, std::milli>;
using Milliseconds32 = std::chrono::duration<uint32_t, std::milli>;

Expand All @@ -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);
Expand Down Expand Up @@ -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.
*
Expand All @@ -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.
*/
Expand All @@ -191,7 +151,6 @@ class ClockImpl : public ClockBase
{
public:
~ClockImpl() = default;
Microseconds64 GetMonotonicMicroseconds64() override;
Milliseconds64 GetMonotonicMilliseconds64() override;
};

Expand All @@ -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
Expand Down
14 changes: 2 additions & 12 deletions src/system/tests/TestSystemClock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand All @@ -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;
};
Expand All @@ -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);
}
Expand Down

0 comments on commit 11c32e8

Please sign in to comment.