Add get/settimeofday retargets

[kegilbert]

Description

In reference to: #6988

---

Move the Time function implementation to get/settimeofday to match the stdlib calls and have the Time API call into the new functions.

Questions/Comments:

1. Should the retarget get/settimeofday functions live in mbed_retarget or are they fine here?
2. Updated tests pending once this PR gets feedback

Test code:

struct timeval tv;
time_t s = time(NULL);

wait(3.0f);

s = time(NULL);
int err = gettimeofday(&tv, NULL);

printf("Time: %d, gettimeofday: %d [Err: %d]\r\n", s, tv.tv_sec, err);

set_time(12345);

s = time(NULL);
err = gettimeofday(&tv, NULL);
printf("Time: %d, gettimeofday: %d [Err: %d]\r\n", s, tv.tv_sec, err);

wait(1.0f);
s = time(NULL);
err = gettimeofday(&tv, NULL);
printf("Time: %d, gettimeofday: %d [Err: %d]\r\n", s, tv.tv_sec, err);

const struct timeval tv_set = { 54321, 0 };
int seterr = settimeofday(&tv_set, NULL);
s = time(NULL);
err = gettimeofday(&tv, NULL);
printf("Time: %d, gettimeofday: %d [Err: %d] SetErr: %d\r\n", s, tv.tv_sec, err, seterr);

wait(1.0f);
s = time(NULL);
err = gettimeofday(&tv, NULL);
printf("Time: %d, gettimeofday: %d [Err: %d]\r\n", s, tv.tv_sec, err);

Time: 3, gettimeofday: 3 [Err: 0]
Time: 12345, gettimeofday: 12345 [Err: 0]
Time: 12346, gettimeofday: 12346 [Err: 0]
Time: 54321, gettimeofday: 54321 [Err: 0] SetErr: 0
Time: 54322, gettimeofday: 54322 [Err: 0]

Pull request type

[ ] Fix
[X] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

[cmonr]

@kegilbert Please take a look at the doxygen issues.

[kegilbert]

Likely forgot to mark the return type in the function declaration, will update tomorrow morning.

[kjbracey]

Seems a bit sad to be implementing this when it adds no new functionality - just another name for the second-resolution time().

Anyone who is happy with seconds could have trivially changed their code to use time, and anyone who was hoping for better (as I believe #6988 did) is going to be disappointed by this implementation.

Still, there's scope to improve the implementation later once the API is in (as long as tv_usec always being 0 isn't deemed API).

[kegilbert]

@kjbracey-arm Thanks for the review! I agree that this doesn't really add a whole lot. I proposed this idea of get/settimeofday essentially acting as a time call with a slightly different format for users expecting get/settimeofday here as it was a rework of Marcus's old implementation using our new timer calls in Mbed OS 5.

Usec being 0 is more a byproduct of the RTC implementations at the moment, that can always be updated.

[kjbracey]

I'm happy to add the API with second resolution like this, but you will get immediate pushback from users about the low quality of implementation. We will should commit to improve it at some point.

In the first pass, could be done without a change to HALs by making the default implementation always use a LPTicker/RTOS count offset added to an initial HAL RTC read, rather than performing an RTC read every time.

Have a new attach_rtc_hires using struct timeval, and a piece of glue for any legacy attach_rtc users with the time_t<->struct timeval conversion.

I'd favour that approach as I really want time and gettimeofday to work pretty well on all platforms, regardless of hardware, so we don't have a situation where stuff with real HW RTC gets second resolution but non-RTC HW gets millisecond LPTicker/RTOS resolution.

Second stage you could add DEVICE_RTC_HIRES for an extended HAL API, for which you go back to calling directly.

[cmonr]

Fyi, this didn't pass the cloud-client-test check.

[Build K66F ARM] [ERROR] "./mbed-os/platform/mbed_rtc_time.cpp", line 91: Error:  #393: pointer to incomplete class type is not allowed
[Build K66F ARM] "./mbed-os/platform/mbed_rtc_time.cpp", line 116: Error:  #393: pointer to incomplete class type is not allowed
[Build K66F ARM] "./mbed-os/platform/mbed_rtc_time.cpp", line 117: Error:  #393: pointer to incomplete class type is not allowed
[Build K66F ARM] "./mbed-os/platform/mbed_rtc_time.cpp", line 130: Error:  #70: incomplete type is not allowed
[Build K66F ARM] "./mbed-os/platform/mbed_rtc_time.cpp", line 142: Error:  #70: incomplete type is not allowed
[Build K66F ARM] ./mbed-os/platform/mbed_rtc_time.cpp: 0 warnings, 5 errors
[Build K66F ARM] 
[Build K66F ARM] [mbed] ERROR: "/usr/bin/python" returned error.
[Build K66F ARM]        Code: 1
[Build K66F ARM]        Path: "/builds/ws/mbed-os-ci_cloud-client-test/mbed-cloud-client-example"
[Build K66F ARM]        Command: "/usr/bin/python -u /builds/ws/mbed-os-ci_cloud-client-test/mbed-cloud-client-example/mbed-os/tools/make.py -t ARM -m K66F --source . --build ./BUILD/K66F/ARM"
[Build K66F ARM]        Tip: You could retry the last command with "-v" flag for verbose output
[Build K66F ARM] ---
[Build K66F ARM] [mbed] Auto-installing missing Python modules...

[kegilbert]

@cmonr Resolved the test failure

[donatieng]

Good stuff @kegilbert - a few remarks

[kegilbert]

@kjbracey-arm @donatieng Thanks for the additional feedback! Updated the PR to match the request changes.

[0xc0170]

This is ready for another round of reviews!

[kegilbert]

Apologies for the delay, updated.
@kjbracey-arm
@donatieng

Changes addressed. Please re-review.

[cmonr]

Reviewers, when y'all have a chance.
@donatieng @kjbracey-arm @ARMmbed/mbed-os-core @marcuschangarm

[donatieng]

Looks good @kegilbert !

[bulislaw]

All the PRs need to be engineering ready (marked as "needs: CI") by the end of the day (Austin time). Otherwise it won't make 5.11 and will need to come in the next release (5.12 for features, 5.11.1 for fixes and new platforms).

[0xc0170]

Moved to 5.12, as there are still build failures. Please @kegilbert continue to work on this

[cmonr]

@kegilbert Any Progress?

[kegilbert]

Sorry for the delay! Got a bit caught up in release work.

@kjbracey-arm Running into an issue were some targets (Odin, Realtek it seems like, potentially others which are pulling in LWIP by default) are including sys/time.h which has a handle for get/settimeofday in the GCC_ARM toolchain in particular: https://github.com/ARMmbed/mbed-os/blob/master/features/lwipstack/lwipopts.h#L23-L26

Does this need to be <sys/time.h> or would <time.h> work if we just need it for some struct definitions (was under the impression time.h had the required ones but could be wrong). Also leads to the question on if we could use the toolchain get/setimeofday functions. I was unable to find usable handles in IAR or the Arm compiler but might've missed them hence why I went this route.

[cmonr]

@kjbracey-arm I think @kegilbert is waiting for feedback

[kjbracey]

time.h is the C header, which does not include gettimeofday, or shouldn't. sys/time.h is POSIX, and where it lives.

For other POSIX functions we have declared them locally in mbed_retarget.h, and we've made sure those definitions are compatible so that it doesn't matter if the compiler header is also included.

In this case, if there's type variation in the compiler headers, you may need to have toolchain-conditional - either to make sure you match, or to defer to the compiler header.

[cmonr]

@kegilbert Thoughts?

[kegilbert]

Sorry for the late response, still traveling so will be a bit before I can take a closer look but sounds good to me. I'll update this ASAP when I'm back at the beginning of January.

[kegilbert]

@cmonr Changed the function declaration to match the conflicting one. Seems to run alright now.

[0xc0170]

CI started

[mbed-ci]

Test run: FAILED

Summary: 1 of 7 test jobs failed
Build number : 9
Build artifacts

Failed test jobs:

• jenkins-ci/mbed-os-ci_build-ARM

[kegilbert]

Didn't realize ARMC6 doesn't set the __CC_ARM macro, this should work now.

[NirSonnenschein]

restart CI

[mbed-ci]

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 10
Build artifacts