Skip to content

Commit

Permalink
Type-safe System::Clock (#10589)
Browse files Browse the repository at this point in the history
* Type-safe System::Clock

#### Problem

Issue #10062 _Some operations on System::Clock types are not safe_

#### Change overview

Introduce safe types based on `std::chrono::duration` to `System::Clock`.

Future changes will use these types, starting with system timers
and proceeding upward through the stack.

#### Testing

Updated `src/system/tests/TestSystemClock.cpp`

* review
  • Loading branch information
kpschoedel authored and pull[bot] committed Nov 23, 2021
1 parent 837c045 commit 1055483
Show file tree
Hide file tree
Showing 10 changed files with 232 additions and 89 deletions.
4 changes: 2 additions & 2 deletions src/lib/dnssd/minimal_mdns/ActiveResolveAttempts.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class ActiveResolveAttempts
static constexpr size_t kRetryQueueSize = 4;
static constexpr uint32_t kMaxRetryDelaySec = 16;

ActiveResolveAttempts(chip::System::ClockBase * clock) : mClock(clock) { Reset(); }
ActiveResolveAttempts(chip::System::Clock::ClockBase * clock) : mClock(clock) { Reset(); }

/// Clear out the internal queue
void Reset();
Expand Down Expand Up @@ -92,7 +92,7 @@ class ActiveResolveAttempts
uint32_t nextRetryDelaySec = 1;
};

chip::System::ClockBase * mClock;
chip::System::Clock::ClockBase * mClock;
RetryEntry mRetryQueue[kRetryQueueSize];
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ namespace {

using namespace chip;

class MockClock : public System::ClockBase
class MockClock : public System::Clock::ClockBase
{
public:
MonotonicMicroseconds GetMonotonicMicroseconds() override { return mUsec; }
MonotonicMilliseconds GetMonotonicMilliseconds() override { return mUsec / 1000; }
System::Clock::Microseconds64 GetMonotonicMicroseconds64() override { return System::Clock::Microseconds64(mUsec); }
System::Clock::Milliseconds64 GetMonotonicMilliseconds64() override { return System::Clock::Milliseconds64(mUsec / 1000); }

void AdvanceMs(MonotonicMilliseconds ms) { mUsec += ms * 1000L; }
void AdvanceSec(uint32_t s) { AdvanceMs(s * 1000); }
Expand Down
10 changes: 6 additions & 4 deletions src/platform/ESP32/SystemTimeSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,22 @@

namespace chip {
namespace System {
namespace Clock {

namespace Internal {
ClockImpl gClockImpl;
} // namespace Internal

Clock::MonotonicMicroseconds ClockImpl::GetMonotonicMicroseconds(void)
Microseconds64 ClockImpl::GetMonotonicMicroseconds64(void)
{
return (Clock::MonotonicMicroseconds)::esp_timer_get_time();
return Clock::Microseconds64(::esp_timer_get_time());
}

Clock::MonotonicMilliseconds GetMonotonicMilliseconds(void)
Milliseconds64 ClockImpl::GetMonotonicMilliseconds64(void)
{
return (Clock::MonotonicMilliseconds)::esp_timer_get_time() / kMicrosecondsPerMillisecond;
return std::chrono::duration_cast<Milliseconds64>(GetMonotonicMicroseconds64());
}

} // namespace Clock
} // namespace System
} // namespace chip
10 changes: 6 additions & 4 deletions src/platform/FreeRTOS/SystemTimeSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

namespace chip {
namespace System {
namespace Clock {

namespace Internal {
ClockImpl gClockImpl;
Expand Down Expand Up @@ -89,15 +90,16 @@ uint64_t FreeRTOSTicksSinceBoot(void)
return static_cast<uint64_t>(timeOut.xTimeOnEntering) + (static_cast<uint64_t>(timeOut.xOverflowCount) << kTicksOverflowShift);
}

Clock::MonotonicMicroseconds ClockImpl::GetMonotonicMicroseconds(void)
Clock::Microseconds64 ClockImpl::GetMonotonicMicroseconds64(void)
{
return (FreeRTOSTicksSinceBoot() * kMicrosecondsPerSecond) / configTICK_RATE_HZ;
return Clock::Microseconds64((FreeRTOSTicksSinceBoot() * kMicrosecondsPerSecond) / configTICK_RATE_HZ);
}

Clock::MonotonicMilliseconds ClockImpl::GetMonotonicMilliseconds(void)
Clock::Milliseconds64 ClockImpl::GetMonotonicMilliseconds64(void)
{
return (FreeRTOSTicksSinceBoot() * kMillisecondsPerSecond) / configTICK_RATE_HZ;
return Clock::Milliseconds64((FreeRTOSTicksSinceBoot() * kMillisecondsPerSecond) / configTICK_RATE_HZ);
}

} // namespace Clock
} // namespace System
} // namespace chip
16 changes: 6 additions & 10 deletions src/platform/Linux/SystemTimeSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,26 +34,22 @@

namespace chip {
namespace System {
namespace Clock {

namespace Internal {
ClockImpl gClockImpl;
} // namespace Internal

Clock::MonotonicMicroseconds ClockImpl::GetMonotonicMicroseconds()
Microseconds64 ClockImpl::GetMonotonicMicroseconds64()
{
std::chrono::microseconds epoch =
std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::steady_clock::now().time_since_epoch());
// count() is nominally signed, but for a monotonic clock it cannot be negative.
return static_cast<uint64_t>(epoch.count());
return std::chrono::duration_cast<Microseconds64>(std::chrono::steady_clock::now().time_since_epoch());
}

Clock::MonotonicMilliseconds ClockImpl::GetMonotonicMilliseconds()
Milliseconds64 ClockImpl::GetMonotonicMilliseconds64()
{
std::chrono::milliseconds epoch =
std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now().time_since_epoch());
// count() is nominally signed, but for a monotonic clock it cannot be negative.
return static_cast<uint64_t>(epoch.count());
return std::chrono::duration_cast<Milliseconds64>(std::chrono::steady_clock::now().time_since_epoch());
}

} // namespace Clock
} // namespace System
} // namespace chip
10 changes: 6 additions & 4 deletions src/platform/Zephyr/SystemTimeSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,23 @@

namespace chip {
namespace System {
namespace Clock {

namespace Internal {
ClockImpl gClockImpl;
} // namespace Internal

Clock::MonotonicMicroseconds ClockImpl::GetMonotonicMicroseconds(void)
Microseconds64 ClockImpl::GetMonotonicMicroseconds64(void)
{
return k_ticks_to_us_floor64(k_uptime_ticks());
return Microseconds64(k_ticks_to_us_floor64(k_uptime_ticks()));
}

Clock::MonotonicMilliseconds ClockImpl::GetMonotonicMilliseconds(void)
Milliseconds64 ClockImpl::GetMonotonicMilliseconds64(void)
{
return k_uptime_get();
return Milliseconds64(k_uptime_get());
}

} // namespace Clock
} // namespace System
} // namespace chip

Expand Down
10 changes: 6 additions & 4 deletions src/platform/mbed/SystemTimeSupport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

namespace chip {
namespace System {
namespace Clock {

namespace Internal {
ClockImpl gClockImpl;
Expand All @@ -58,17 +59,18 @@ extern "C" uint64_t get_clock_monotonic()

// Platform-specific function for getting monotonic system time in microseconds.
// Returns elapsed time in microseconds since an arbitrary, platform-defined epoch.
Clock::MonotonicMicroseconds ClockImpl::GetMonotonicMicroseconds()
Microseconds64 ClockImpl::GetMonotonicMicroseconds64()
{
return get_clock_monotonic();
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.
Clock::MonotonicMilliseconds ClockImpl::GetMonotonicMilliseconds()
Milliseconds64 ClockImpl::GetMonotonicMilliseconds64()
{
return get_clock_monotonic() / kMicrosecondsPerMillisecond;
return std::chrono::duration_cast<Milliseconds64>(GetMonotonicMicroseconds64());
}

} // namespace Clock
} // namespace System
} // namespace chip
58 changes: 39 additions & 19 deletions src/system/SystemClock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <lib/support/TimeUtils.h>
#include <system/SystemError.h>

#include <limits>
#include <stdint.h>
#include <stdlib.h>

Expand All @@ -46,6 +47,7 @@

namespace chip {
namespace System {
namespace Clock {

namespace Internal {

Expand Down Expand Up @@ -80,35 +82,35 @@ ClockBase * gClockBase = &gClockImpl;
#define MONOTONIC_CLOCK_ID CLOCK_MONOTONIC
#endif

Clock::MonotonicMicroseconds ClockImpl::GetMonotonicMicroseconds()
Microseconds64 ClockImpl::GetMonotonicMicroseconds64()
{
struct timespec ts;
int res = clock_gettime(MONOTONIC_CLOCK_ID, &ts);
VerifyOrDie(res == 0);
return (static_cast<uint64_t>(ts.tv_sec) * kMicrosecondsPerSecond) +
(static_cast<uint64_t>(ts.tv_nsec) / kNanosecondsPerMicrosecond);
return Seconds64(ts.tv_sec) +
std::chrono::duration_cast<Microseconds64>(std::chrono::duration<uint64_t, std::nano>(ts.tv_nsec));
}

Clock::MonotonicMilliseconds ClockImpl::GetMonotonicMilliseconds()
Milliseconds64 ClockImpl::GetMonotonicMilliseconds64()
{
return GetMonotonicMicroseconds() / kMicrosecondsPerMillisecond;
return std::chrono::duration_cast<Milliseconds64>(GetMonotonicMicroseconds64());
}

#endif // HAVE_CLOCK_GETTIME

#if HAVE_GETTIMEOFDAY

Clock::MonotonicMicroseconds ClockImpl::GetMonotonicMicroseconds()
Microseconds64 ClockImpl::GetMonotonicMicroseconds64()
{
struct timeval tv;
int res = gettimeofday(&tv, NULL);
VerifyOrDie(res == 0);
return (tv.tv_sec * kMicrosecondsPerSecond) + tv.tv_usec;
return TimevalToMicroseconds(tv);
}

Clock::MonotonicMilliseconds ClockImpl::GetMonotonicMilliseconds()
Milliseconds64 ClockImpl::GetMonotonicMilliseconds64()
{
return GetMonotonicMicroseconds() / kMicrosecondsPerMillisecond;
return std::chrono::duration_cast<Milliseconds64>(GetMonotonicMicroseconds64());
}

#endif // HAVE_GETTIMEOFDAY
Expand All @@ -119,12 +121,12 @@ Clock::MonotonicMilliseconds ClockImpl::GetMonotonicMilliseconds()

// -------------------- Default Get/SetClock Functions for LwIP Systems --------------------

Clock::MonotonicMilliseconds ClockImpl::GetMonotonicMicroseconds(void)
Microseconds64 ClockImpl::GetMonotonicMicroseconds64(void)
{
return GetMonotonicMilliseconds() * kMicrosecondsPerMillisecond;
return GetMonotonicMilliseconds64();
}

Clock::MonotonicMilliseconds ClockImpl::GetMonotonicMilliseconds(void)
Milliseconds64 ClockImpl::GetMonotonicMilliseconds64(void)
{
static volatile uint64_t overflow = 0;
static volatile u32_t lastSample = 0;
Expand Down Expand Up @@ -161,15 +163,13 @@ Clock::MonotonicMilliseconds ClockImpl::GetMonotonicMilliseconds(void)
sample = sys_now();
}

return static_cast<uint64_t>(overflowSample | static_cast<uint64_t>(sample));
return Milliseconds64(overflowSample | static_cast<uint64_t>(sample));
}

#endif // CHIP_SYSTEM_CONFIG_USE_LWIP_MONOTONIC_TIME

#endif // CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME

namespace Clock {

static_assert(std::is_unsigned<ClockBase::Tick>::value, "ClockBase::Tick must be unsigned");
constexpr ClockBase::Tick kMaxTick = static_cast<ClockBase::Tick>(0) - static_cast<ClockBase::Tick>(1);
constexpr ClockBase::Tick kHalfMaxTick = static_cast<ClockBase::Tick>(kMaxTick / 2);
Expand All @@ -190,20 +190,40 @@ ClockBase::Tick AddOffset(const ClockBase::Tick & base, const ClockBase::Tick &

#if CHIP_SYSTEM_CONFIG_USE_POSIX_TIME_FUNCTS || CHIP_SYSTEM_CONFIG_USE_SOCKETS

Clock::MonotonicMilliseconds TimevalToMilliseconds(const timeval & in)
Microseconds64 TimevalToMicroseconds(const timeval & tv)
{
return static_cast<Clock::MonotonicMilliseconds>(in.tv_sec) * 1000 +
static_cast<Clock::MonotonicMilliseconds>(in.tv_usec / 1000);
return Seconds64(tv.tv_sec) + Microseconds64(tv.tv_usec);
}

void MillisecondsToTimeval(Clock::MonotonicMilliseconds in, timeval & out)
MonotonicMilliseconds TimevalToMilliseconds(const timeval & in)
{
return static_cast<MonotonicMilliseconds>(in.tv_sec) * 1000 + static_cast<MonotonicMilliseconds>(in.tv_usec / 1000);
}

void MillisecondsToTimeval(MonotonicMilliseconds in, timeval & out)
{
out.tv_sec = static_cast<time_t>(in / 1000);
out.tv_usec = static_cast<suseconds_t>((in % 1000) * 1000);
}

#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");
static_assert(std::numeric_limits<Seconds32::rep>::digits >= 32, "Seconds32 must be at least 32 bits");
static_assert(std::numeric_limits<Seconds16::rep>::digits >= 16, "Seconds16 must be at least 16 bits");

} // namespace Clock
} // namespace System
} // namespace chip
Loading

0 comments on commit 1055483

Please sign in to comment.