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

Conversation

sergeuz
Copy link
Member

@sergeuz sergeuz commented Sep 22, 2017

Problem

The system should report its uptime in seconds.

Solution

  • Add HAL_Timer_Get_Seconds() function returning the number of seconds passed since the device was powered on or woken up from sleep.
  • Add hal_timer_millis() function returning the number of milliseconds passed since the device was last reset as a 64-bit value.
  • Add System.uptime() and System.millis() methods returning the system uptime in seconds and milliseconds respectively.
  • Add sys.uptime diagnostic data source.

Steps to Test

Run wiring/no_fixture_long_running test.

Example App

#include "application.h"

SYSTEM_MODE(MANUAL)

namespace {

const Serial1LogHandler logHandler(115200, LOG_LEVEL_WARN, {
    { "app", LOG_LEVEL_ALL }
});

bool logDiagData(void* appender, const uint8_t* data, size_t size) {
    Log.write(LOG_LEVEL_INFO, (const char*)data, size);
    return true;
}

} // namespace

void setup() {
    delay(3000);
    uint16_t id = DIAG_ID_SYSTEM_UPTIME;
    system_format_diag_data(&id, 1, 0, logDiagData, nullptr, nullptr);
    Log.print("\r\n");
}

void loop() {
}

References


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@sergeuz sergeuz added this to the 0.8.0 milestone Sep 22, 2017
@m-mcgowan
Copy link
Contributor

Any reason to not use a 64-bit type and keep the precision to milliseconds?

@sergeuz
Copy link
Member Author

sergeuz commented Sep 22, 2017

I just decided not to use any types which are non-native for the target platforms. We also don't seem to have a good use case for such extended milliseconds counter.

@sergeuz
Copy link
Member Author

sergeuz commented Sep 25, 2017

Updated the milliseconds counter to use uint64_t.

@@ -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 :)

*/
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.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants