Skip to content

gettimeofday incorrect value for tv_usec #3814

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

Closed
BrandonLWhite opened this issue Nov 9, 2017 · 3 comments · Fixed by #3830
Closed

gettimeofday incorrect value for tv_usec #3814

BrandonLWhite opened this issue Nov 9, 2017 · 3 comments · Fixed by #3830

Comments

@BrandonLWhite
Copy link
Contributor

The current implementation _gettimeofday_r is incorrectly setting the tv_usec field.

https://github.com/esp8266/Arduino/blob/master/cores/esp8266/time.c#L104

According to http://www.gnu.org/software/libc/manual/html_node/Elapsed-Time.html

tv_usec should be "This is the rest of the elapsed time (a fraction of a second), represented as the number of microseconds. It is always less than one million."

The current code is simply doing tp->tv_usec = micros();. This leads to the wall clock time advancing forward too quickly. I am certainly aware of the caveats of tracking wallclock (ie CLOCK_REALTIME) in ESP8266, but this is just making it far worse.

I have some ideas on how to square this away once and for all (crossing my fingers). I will try to follow-up with additional info or possibly a pull request.

@5chufti
Copy link
Contributor

5chufti commented Nov 9, 2017

so basically it boils down to micros() - millis()*1000
or simply micros() % 1000000 if tv_sec is micros()/1000000

@BrandonLWhite
Copy link
Contributor Author

BrandonLWhite commented Nov 9, 2017

I think you have the right idea, but the problem with using micros directly like that is that it is only 32-bit. This could all be gravy if we had a uint64_t micros64() version.

Something like this is a step in the right direction:

uint64_t elapsed_ms = s_bootTime * 1000 + millis();
tp->tv_sec  = elapsed_ms / 1000;
tp->tv_usec = (elapsed_ms % 1000) * 1000;

I think that would work in a pinch. Unfortunately this would still be bugged over the long haul because s_bootTime is never updated and millis() overflows after 2^32 ms.

If we had the micros64(), and I think it can be done effectively, then it could be:

uint64_t elapsed_us = s_bootTime*1000000 + micros64();
tp->tv_sec  = elapsed_us / 1000000;
tp->tv_usec = elapsed_ms % 1000000;

That doesn't solve the long term inherent drift people encounter on the device, but that is another problem to tackle. I think the best strategy there is to periodically get a fresh NTP stamp to re-establish the base. In other words, s_bootTime needs to get updated from an NTP source every now and then. Probably need to rename it too!

@5chufti
Copy link
Contributor

5chufti commented Nov 10, 2017

@d-a-v : I think this is a topic along #1679 ...

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