From cb6e2744f6a9a68f51496c98bd13fce069e39c3e Mon Sep 17 00:00:00 2001 From: martinwork Date: Fri, 4 Sep 2020 14:15:17 +0100 Subject: [PATCH] Fix drift of system_timer_current_time Add "tick64" to yotta config.json: { "microbit-dal":{ "tick64":1 } } --- inc/core/MicroBitConfig.h | 6 +++ inc/core/MicroBitTick64.h | 81 +++++++++++++++++++++++++++++ inc/platform/yotta_cfg_mappings.h | 4 ++ source/CMakeLists.txt | 1 + source/core/MicroBitSystemTimer.cpp | 40 ++++++++++++++ source/core/MicroBitTick64.cpp | 37 +++++++++++++ 6 files changed, 169 insertions(+) create mode 100644 inc/core/MicroBitTick64.h create mode 100644 source/core/MicroBitTick64.cpp diff --git a/inc/core/MicroBitConfig.h b/inc/core/MicroBitConfig.h index 9f5e3855..c99f1b1e 100644 --- a/inc/core/MicroBitConfig.h +++ b/inc/core/MicroBitConfig.h @@ -189,6 +189,12 @@ extern uint32_t __etext; #define MICROBIT_IDLE_COMPONENTS 6 #endif +// Use MicroBitTick64 to fix slow running of system_timer_current_time +// Default is zero because existing code may have workarounds that would be broken by this fix +#ifndef MICROBIT_TICK64 +#define MICROBIT_TICK64 0 +#endif + // // BLE options // diff --git a/inc/core/MicroBitTick64.h b/inc/core/MicroBitTick64.h new file mode 100644 index 00000000..8852e010 --- /dev/null +++ b/inc/core/MicroBitTick64.h @@ -0,0 +1,81 @@ +/* +The MIT License (MIT) + +Copyright (c) 2020 Insight Resources + +Permission is hereby granted, free of charge, to any person obtaining a +copy of this software and associated documentation files (the "Software"), +to deal in the Software without restriction, including without limitation +the rights to use, copy, modify, merge, publish, distribute, sublicense, +and/or sell copies of the Software, and to permit persons to whom the +Software is furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +DEALINGS IN THE SOFTWARE. +*/ + +#ifndef MICROBIT_TICK64_H +#define MICROBIT_TICK64_H + +#include "mbed.h" +#include "MicroBitConfig.h" + + +typedef struct microbit_tick64_t +{ + uint32_t low0; // microseconds + uint32_t high; // microbit_tick64_highunit microseconds +} microbit_tick64_t; + + +extern microbit_tick64_t microbit_tick640; +extern microbit_tick64_t microbit_tick641; +extern microbit_tick64_t *microbit_tick64; +extern const uint32_t microbit_tick64_highunit; + + +inline void microbit_tick64_initialise() +{ + microbit_tick64 = µbit_tick640; + microbit_tick64->low0 = us_ticker_read(); + microbit_tick64->high = 0; +} + + +/** + * Check the microsecond ticker and swap microbit_tick64_t instances + * Not reentrant. Call from one place. e.g. in a ticker. + * Assumes the microsecond ticker difference is less than + * 2 * microbit_tick64_highunit and calculates new values when + * the difference is between microbit_tick64_highunit and 0xFFFFFFF. + * With microbit_tick64_highunit = 0x80000000, the check is needed + * every 30mins and rollover is after 250,000 years +*/ +inline void microbit_tick64_update() +{ + if ( us_ticker_read() - microbit_tick64->low0 > microbit_tick64_highunit) + { + microbit_tick64_t *other = microbit_tick64 == µbit_tick640 ? µbit_tick641 : µbit_tick640; + other->low0 = microbit_tick64->low0 + microbit_tick64_highunit; + other->high = microbit_tick64->high + 1; + microbit_tick64 = other; + } +} + + +inline uint64_t microbit_tick64_microseconds() +{ + microbit_tick64_t *p = microbit_tick64; // guard against IRQ changing microbit_tick64 + return microbit_tick64_highunit * ( uint64_t) p->high + ( us_ticker_read() - p->low0); +} + + +#endif diff --git a/inc/platform/yotta_cfg_mappings.h b/inc/platform/yotta_cfg_mappings.h index 08ef57fc..628c9e96 100644 --- a/inc/platform/yotta_cfg_mappings.h +++ b/inc/platform/yotta_cfg_mappings.h @@ -35,6 +35,10 @@ #define MICROBIT_IDLE_COMPONENTS YOTTA_CFG_MICROBIT_DAL_IDLE_COMPONENTS #endif +#ifdef YOTTA_CFG_MICROBIT_DAL_TICK64 + #define MICROBIT_TICK64 YOTTA_CFG_MICROBIT_DAL_TICK64 +#endif + #ifdef YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_ENABLED #define MICROBIT_BLE_ENABLED YOTTA_CFG_MICROBIT_DAL_BLUETOOTH_ENABLED #endif diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt index f98a2ec9..efda830d 100755 --- a/source/CMakeLists.txt +++ b/source/CMakeLists.txt @@ -14,6 +14,7 @@ set(YOTTA_AUTO_MICROBIT-DAL_CPP_FILES "core/MicroBitHeapAllocator.cpp" "core/MicroBitListener.cpp" "core/MicroBitSystemTimer.cpp" + "core/MicroBitTick64.cpp" "core/MicroBitUtil.cpp" "types/CoordinateSystem.cpp" diff --git a/source/core/MicroBitSystemTimer.cpp b/source/core/MicroBitSystemTimer.cpp index f745c783..d46e82dd 100644 --- a/source/core/MicroBitSystemTimer.cpp +++ b/source/core/MicroBitSystemTimer.cpp @@ -38,11 +38,21 @@ DEALINGS IN THE SOFTWARE. #include "MicroBitSystemTimer.h" #include "ErrorNo.h" + +#if CONFIG_ENABLED( MICROBIT_TICK64) + +#include "MicroBitTick64.h" + +#else // CONFIG_ENABLED( MICROBIT_TICK64) + /* * Time since power on. Measured in milliseconds. * When stored as an unsigned long, this gives us approx 50 days between rollover, which is ample. :-) */ static uint64_t time_us = 0; + +#endif //CONFIG_ENABLED( MICROBIT_TICK64) + static unsigned int tick_period = 0; // Array of components which are iterated during a system tick @@ -51,9 +61,13 @@ static MicroBitComponent* systemTickComponents[MICROBIT_SYSTEM_COMPONENTS]; // Periodic callback interrupt static Ticker *ticker = NULL; +#if CONFIG_ENABLED( MICROBIT_TICK64) +#else // CONFIG_ENABLED( MICROBIT_TICK64) + // System timer. static Timer *timer = NULL; +#endif //CONFIG_ENABLED( MICROBIT_TICK64) /** * Initialises a system wide timer, used to drive the various components used in the runtime. @@ -66,6 +80,13 @@ static Timer *timer = NULL; */ int system_timer_init(int period) { +#if CONFIG_ENABLED( MICROBIT_TICK64) + if ( ticker == NULL) + { + ticker = new Ticker(); + microbit_tick64_initialise(); + } +#else if (ticker == NULL) ticker = new Ticker(); @@ -74,6 +95,7 @@ int system_timer_init(int period) timer = new Timer(); timer->start(); } +#endif return system_timer_set_period(period); } @@ -119,12 +141,19 @@ int system_timer_get_period() */ void update_time() { +#if CONFIG_ENABLED( MICROBIT_TICK64) + if ( ticker == NULL) + system_timer_init(SYSTEM_TICK_PERIOD_MS); + else + microbit_tick64_update(); +#else // If we haven't been initialized, bring up the timer with the default period. if (timer == NULL || ticker == NULL) system_timer_init(SYSTEM_TICK_PERIOD_MS); time_us += timer->read_us(); timer->reset(); +#endif } /** @@ -144,8 +173,15 @@ uint64_t system_timer_current_time() */ uint64_t system_timer_current_time_us() { +#if CONFIG_ENABLED( MICROBIT_TICK64) + if ( ticker == NULL) + system_timer_init(SYSTEM_TICK_PERIOD_MS); + + return microbit_tick64_microseconds(); +#else update_time(); return time_us; +#endif } /** @@ -179,7 +215,11 @@ int system_timer_add_component(MicroBitComponent *component) int i = 0; // If we haven't been initialized, bring up the timer with the default period. +#if CONFIG_ENABLED( MICROBIT_TICK64) + if ( ticker == NULL) +#else if (timer == NULL || ticker == NULL) +#endif system_timer_init(SYSTEM_TICK_PERIOD_MS); while(systemTickComponents[i] != NULL && i < MICROBIT_SYSTEM_COMPONENTS) diff --git a/source/core/MicroBitTick64.cpp b/source/core/MicroBitTick64.cpp new file mode 100644 index 00000000..7b57a8ed --- /dev/null +++ b/source/core/MicroBitTick64.cpp @@ -0,0 +1,37 @@ +/* +The MIT License (MIT) + +Copyright (c) 2020 Insight Resources + +Permission is hereby granted, free of charge, to any person obtaining a +copy of this software and associated documentation files (the "Software"), +to deal in the Software without restriction, including without limitation +the rights to use, copy, modify, merge, publish, distribute, sublicense, +and/or sell copies of the Software, and to permit persons to whom the +Software is furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +DEALINGS IN THE SOFTWARE. +*/ + +#include "MicroBitTick64.h" + + +microbit_tick64_t microbit_tick640; +microbit_tick64_t microbit_tick641; +microbit_tick64_t *microbit_tick64; + + +//#if CONFIG_ENABLED(MICROBIT_DBG) +//const uint32_t microbit_tick64_highunit = 0x10000; +//#else +const uint32_t microbit_tick64_highunit = 0x80000000ul; +//#endif