From db6cc120279e679d2913af9bd79c4f9a6772953c Mon Sep 17 00:00:00 2001 From: graham sanderson Date: Wed, 17 Feb 2021 14:15:00 -0600 Subject: [PATCH] fix the represntation of at_the_end_of_time to be 63 one bits rather than 32 --- src/common/pico_base/include/pico/types.h | 15 ++++++++++++++- src/common/pico_time/include/pico/time.h | 4 +++- src/common/pico_time/time.c | 3 +-- src/host/hardware_timer/timer.c | 1 - test/pico_time_test/pico_time_test.c | 9 +++++++++ 5 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/common/pico_base/include/pico/types.h b/src/common/pico_base/include/pico/types.h index 40856bd..de63bcb 100644 --- a/src/common/pico_base/include/pico/types.h +++ b/src/common/pico_base/include/pico/types.h @@ -37,7 +37,8 @@ static inline uint64_t to_us_since_boot(absolute_time_t t) { /*! fn update_us_since_boot * \brief update an absolute_time_t value to represent a given number of microseconds since boot * \param t the absolute time value to update - * \param us_since_boot the number of microseconds since boot to represent + * \param us_since_boot the number of microseconds since boot to represent. Note this should be representable + * as a signed 64 bit integer */ static inline void update_us_since_boot(absolute_time_t *t, uint64_t us_since_boot) { *t = us_since_boot; @@ -49,11 +50,23 @@ typedef struct { uint64_t _private_us_since_boot; } absolute_time_t; +/*! fn to_us_since_boot + * \brief convert an absolute_time_t into a number of microseconds since boot. + * \param t the number of microseconds since boot + * \return an absolute_time_t value equivalent to t + */ static inline uint64_t to_us_since_boot(absolute_time_t t) { return t._private_us_since_boot; } +/*! fn update_us_since_boot + * \brief update an absolute_time_t value to represent a given number of microseconds since boot + * \param t the absolute time value to update + * \param us_since_boot the number of microseconds since boot to represent. Note this should be representable + * as a signed 64 bit integer + */ static inline void update_us_since_boot(absolute_time_t *t, uint64_t us_since_boot) { + assert(us_since_boot <= INT64_MAX); t->_private_us_since_boot = us_since_boot; } #define ABSOLUTE_TIME_INITIALIZED_VAR(name, value) name = {value} diff --git a/src/common/pico_time/include/pico/time.h b/src/common/pico_time/include/pico/time.h index bacddf3..6bed135 100644 --- a/src/common/pico_time/include/pico/time.h +++ b/src/common/pico_time/include/pico/time.h @@ -155,7 +155,9 @@ static inline int64_t absolute_time_diff_us(absolute_time_t from, absolute_time_ return (int64_t)(to_us_since_boot(to) - to_us_since_boot(from)); } -/*! \brief The timestamp representing the end of time; no timestamp is after this +/*! \brief The timestamp representing the end of time; this is actually not the maximum possible + * timestamp, but is set to 0x7fffffff_ffffffff microseconds to avoid sign overflows with time + * arithmetic. This is still over 7 million years, so should be sufficient. * \ingroup timestamp */ extern const absolute_time_t at_the_end_of_time; diff --git a/src/common/pico_time/time.c b/src/common/pico_time/time.c index 0cc7efd..0c94e12 100644 --- a/src/common/pico_time/time.c +++ b/src/common/pico_time/time.c @@ -11,10 +11,9 @@ #include "pico/time.h" #include "pico/util/pheap.h" #include "hardware/sync.h" -#include "hardware/gpio.h" const absolute_time_t ABSOLUTE_TIME_INITIALIZED_VAR(nil_time, 0); -const absolute_time_t ABSOLUTE_TIME_INITIALIZED_VAR(at_the_end_of_time, ULONG_MAX); +const absolute_time_t ABSOLUTE_TIME_INITIALIZED_VAR(at_the_end_of_time, INT64_MAX); typedef struct alarm_pool_entry { absolute_time_t target; diff --git a/src/host/hardware_timer/timer.c b/src/host/hardware_timer/timer.c index d6dfa1c..1bab845 100644 --- a/src/host/hardware_timer/timer.c +++ b/src/host/hardware_timer/timer.c @@ -51,7 +51,6 @@ uint32_t PICO_WEAK_FUNCTION_IMPL_NAME(timer_us_32)() { PICO_WEAK_FUNCTION_DEF(time_reached) bool PICO_WEAK_FUNCTION_IMPL_NAME(time_reached)(absolute_time_t t) { uint64_t target = to_us_since_boot(t); - if (target > 0xffffffffu) return false; return time_us_64() >= target; } diff --git a/test/pico_time_test/pico_time_test.c b/test/pico_time_test/pico_time_test.c index d9795e2..1093dd6 100644 --- a/test/pico_time_test/pico_time_test.c +++ b/test/pico_time_test/pico_time_test.c @@ -206,6 +206,15 @@ int main() { PICOTEST_END_SECTION(); + PICOTEST_START_SECTION("end of time"); + PICOTEST_CHECK(absolute_time_diff_us(at_the_end_of_time, get_absolute_time()) < 0, "now should be before the end of time") + PICOTEST_CHECK(absolute_time_diff_us(get_absolute_time(), at_the_end_of_time) > 0, "the end of time should be after now") + PICOTEST_CHECK(absolute_time_diff_us(at_the_end_of_time, at_the_end_of_time) == 0, "the end of time should equal itself") + absolute_time_t near_the_end_of_time; + update_us_since_boot(&near_the_end_of_time, 0x7ffffeffffffffff); + PICOTEST_CHECK(absolute_time_diff_us(near_the_end_of_time, at_the_end_of_time) > 0, "near the end of time should be before the end of time") + PICOTEST_END_SECTION(); + PICOTEST_END_TEST(); }