Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"sys.uptime" diagnostic data #1393

Merged
merged 6 commits into from
Oct 5, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions hal/inc/hal_dynalib.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ DYNALIB_FN(BASE_IDX + 18, hal,HAL_EEPROM_Has_Pending_Erase, bool(void))
DYNALIB_FN(BASE_IDX + 19, hal,HAL_EEPROM_Perform_Pending_Erase, void(void))
DYNALIB_FN(BASE_IDX + 20, hal, HAL_RTC_Time_Is_Valid, uint8_t(void*))

DYNALIB_FN(BASE_IDX + 21, hal, hal_timer_millis, uint64_t(void*))

DYNALIB_END(hal)

#undef BASE_IDX
11 changes: 8 additions & 3 deletions hal/inc/timer_hal.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,23 @@

/* Exported macros -----------------------------------------------------------*/

#define HAL_Timer_Microseconds HAL_Timer_Get_Micro_Seconds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unsure about moving the #define since it will change the actual exported function name. I realize the chances are small, but someone could be using the HAL_Timer_Get_Micro_Seconds function. What is the value gained to moving the #define - if it's minimal, I'd prefer not to change it unless there is a known gain.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, but these macros do the opposite: they define aliases for already exported functions, rather than define their exported names. I don't see a possibility to break anything with this change, but, for sure, it doesn't bring any value either :)

#define HAL_Timer_Milliseconds HAL_Timer_Get_Milli_Seconds

/* Exported functions --------------------------------------------------------*/

#ifdef __cplusplus
extern "C" {
#endif


system_tick_t HAL_Timer_Get_Micro_Seconds(void);
system_tick_t HAL_Timer_Get_Milli_Seconds(void);

#define HAL_Timer_Microseconds HAL_Timer_Get_Micro_Seconds
#define HAL_Timer_Milliseconds HAL_Timer_Get_Milli_Seconds
/**
* Returns the number of milliseconds passed since the device was last reset. This function is
* similar to HAL_Timer_Get_Milli_Seconds() but returns a 64-bit value.
*/
uint64_t hal_timer_millis(void* reserved);

#ifdef __cplusplus
}
Expand Down
5 changes: 5 additions & 0 deletions hal/src/core/timer_hal.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,8 @@ system_tick_t HAL_Timer_Get_Milli_Seconds(void)
{
return GetSystem1MsTick();
}

uint64_t hal_timer_millis(void* reserved)
{
return GetSystem1MsTick();
}
18 changes: 11 additions & 7 deletions hal/src/gcc/timer_hal.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@

#include "timer_hal.h"

#include <boost/date_time/posix_time/posix_time.hpp>

auto start = boost::posix_time::microsec_clock::universal_time();
namespace {

const auto start = boost::posix_time::microsec_clock::universal_time();

} // namespace

system_tick_t HAL_Timer_Get_Micro_Seconds(void)
{
Expand All @@ -14,10 +17,11 @@ system_tick_t HAL_Timer_Get_Micro_Seconds(void)

system_tick_t HAL_Timer_Get_Milli_Seconds(void)
{
auto now = boost::posix_time::microsec_clock::universal_time();
auto diff = now - start;
return diff.total_milliseconds();
return hal_timer_millis(nullptr);
}



uint64_t hal_timer_millis(void* reserved)
{
const auto now = boost::posix_time::microsec_clock::universal_time();
return (now - start).total_milliseconds();
}
5 changes: 5 additions & 0 deletions hal/src/stm32f2xx/timer_hal.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,8 @@ system_tick_t HAL_Timer_Get_Milli_Seconds(void)
// review - wiced_time_get_time()) ??
return GetSystem1MsTick();
}

uint64_t hal_timer_millis(void* reserved)
{
return GetSystem1MsTick();
}
5 changes: 5 additions & 0 deletions hal/src/template/timer_hal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,8 @@ system_tick_t HAL_Timer_Get_Milli_Seconds(void)
{
return 0;
}

uint64_t hal_timer_millis(void* reserved)
{
return 0;
}
4 changes: 2 additions & 2 deletions platform/MCU/newhal-mcu/inc/hw_ticks.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ void System1UsTick(void);
/**
* Fetch the current milliseconds count.
* @return the number of milliseconds since the device was powered on or woken up from
* sleep. Automatically wraps around when above UINT_MAX;
* sleep.
*/
system_tick_t GetSystem1MsTick();
uint64_t GetSystem1MsTick();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an open question - do we break compatibility of these low-level functions? My instinct is to say no and move the 64-bit version to GetSystem1MsTick64 or similar, but happy to hear arguments to the contrary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


/**
* Fetch the current microseconds count.
Expand Down
18 changes: 8 additions & 10 deletions platform/MCU/shared/STM32/inc/hw_ticks.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,16 @@

#pragma once

#ifdef __cplusplus
extern "C" {
#endif


#include "platform_config.h"
#include "system_tick_hal.h"

#define SYSTEM_US_TICKS (SystemCoreClock / 1000000)//cycles per microsecond
#define SYSTEM_US_TICKS (SystemCoreClock / 1000000)//cycles per microsecond
#define SYSTEM_TICK_COUNTER (DWT->CYCCNT)

#ifdef __cplusplus
extern "C" {
#endif

/**
* Increment the millisecond tick counter.
*/
Expand All @@ -45,9 +44,9 @@ void System1UsTick(void);
/**
* Fetch the current milliseconds count.
* @return the number of milliseconds since the device was powered on or woken up from
* sleep. Automatically wraps around when above UINT_MAX;
* sleep.
*/
system_tick_t GetSystem1MsTick();
uint64_t GetSystem1MsTick();

/**
* Fetch the current microseconds count.
Expand All @@ -56,11 +55,10 @@ system_tick_t GetSystem1MsTick();
*/
system_tick_t GetSystem1UsTick();


/**
* Testing method that simulates advancing the time forward.
*/
void __advance_system1MsTick(system_tick_t millis, system_tick_t micros_from_rollover);
void __advance_system1MsTick(uint64_t millis, system_tick_t micros_from_rollover);

void SysTick_Disable();

Expand Down
27 changes: 18 additions & 9 deletions platform/MCU/shared/STM32/src/hw_ticks.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
/**
* The current millisecond counter value.
*/
static volatile system_tick_t system_millis = 0;
static volatile uint64_t system_millis = 0;

/**
* The system clock value at last time system_millis was updated. This is updated
Expand All @@ -42,7 +42,7 @@ void System1MsTick(void)
int is = __get_PRIMASK();
__disable_irq();

system_millis++;
++system_millis;
system_millis_clock = DWT->CYCCNT;

if ((is & 1) == 0) {
Expand All @@ -54,16 +54,19 @@ void System1MsTick(void)
* Fetches the current millisecond tick counter.
* @return
*/
system_tick_t GetSystem1MsTick()
uint64_t GetSystem1MsTick()
{
uint64_t millis = 0;

int is = __get_PRIMASK();
__disable_irq();

system_tick_t millis = system_millis + (DWT->CYCCNT-system_millis_clock) / SYSTEM_US_TICKS / 1000;
millis = system_millis + (DWT->CYCCNT - system_millis_clock) / SYSTEM_US_TICKS / 1000;

if ((is & 1) == 0) {
__enable_irq();
}

return millis;
}

Expand All @@ -81,7 +84,7 @@ system_tick_t GetSystem1UsTick()

base_millis = system_millis;
base_clock = system_millis_clock;

system_tick_t elapsed_since_millis = ((DWT->CYCCNT-base_clock) / SYSTEM_US_TICKS);

if ((is & 1) == 0) {
Expand All @@ -93,14 +96,20 @@ system_tick_t GetSystem1UsTick()
/**
* Testing method that simulates advancing the time forward.
*/
void __advance_system1MsTick(system_tick_t millis, system_tick_t micros_from_rollover)
void __advance_system1MsTick(uint64_t millis, system_tick_t micros_from_rollover)
{
DWT->CYCCNT = UINT_MAX-(micros_from_rollover*SYSTEM_US_TICKS); // 10 seconds before rollover
const int is = __get_PRIMASK();
__disable_irq();

DWT->CYCCNT = UINT_MAX - (micros_from_rollover * SYSTEM_US_TICKS);
system_millis_clock = DWT->CYCCNT;
system_millis = millis;

if ((is & 1) == 0) {
__enable_irq();
}
}

void SysTick_Disable() {
SysTick->CTRL = SysTick->CTRL & ~SysTick_CTRL_ENABLE_Msk;
}


17 changes: 16 additions & 1 deletion system/src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include "spark_wiring_cellular.h"
#include "spark_wiring_cellular_printable.h"
#include "spark_wiring_led.h"
#include "spark_wiring_diagnostics.h"

#if PLATFORM_ID == 3
// Application loop uses std::this_thread::sleep_for() to workaround 100% CPU usage on the GCC platform
Expand Down Expand Up @@ -626,9 +627,23 @@ class HALEventHandler {
}
};

class UptimeDiagnosticData: public AbstractIntegerDiagnosticData {
public:
UptimeDiagnosticData() :
AbstractIntegerDiagnosticData(DIAG_ID_SYSTEM_UPTIME, "sys.uptime") {
}

virtual int get(IntType& val) override {
val = System.uptime();
return 0; // OK
}
};

// Certain HAL events can be generated before app_setup_and_loop() is called. Using constructor of a
// global variable allows to register a handler for HAL events early
HALEventHandler halEventHandler;
HALEventHandler g_halEventHandler;

UptimeDiagnosticData g_uptimeDiagData;

} // namespace

Expand Down
13 changes: 12 additions & 1 deletion user/tests/wiring/api/system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -241,4 +241,15 @@ test(system_mode_button)

API_COMPILE(System.disableButtonMirror());
API_COMPILE(System.disableButtonMirror(false));
}
}

test(system_uptime_millis) {
uint64_t u64 = 0;
unsigned u = 0;

API_COMPILE(u64 = System.millis());
API_COMPILE(u = System.uptime());

(void)u64;
(void)u;
}
4 changes: 2 additions & 2 deletions user/tests/wiring/no_fixture_long_running/pwm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

static const uint32_t maxPulseSamples = 100;
static const uint32_t minimumFrequency = 100;

uint8_t pwm_pins[] = {

#if defined(STM32F2XX)
Expand All @@ -22,7 +22,7 @@ uint8_t pwm_pins[] = {
#endif
};

static pin_t pin = pwm_pins[0];
// static pin_t pin = pwm_pins[0];

template <typename F> void for_all_pwm_pins(F callback)
{
Expand Down
Loading