Correct millis() drift, issue #3078

[mrwgx3]

This pull-request corrects the cumlative (296us / usec overflow) drift seen in the orignal 'millis()' function. Extensive comments are included.

[igrr]

Thanks for putting together this fix and the great explanation!
Would you mind adding the benchmarking code into the repository as a test? That would make it easier for others to replicate the result in the future, if needed. Adding a test is mostly equivalent to dropping a sketch into tests/device directory, with a few extra things described here: https://github.com/esp8266/Arduino/tree/master/tests#testing-on-device.

[devyte]

@igrr I think the exhaustive testing of the calculation accuracy was done host side, so it should be possible to do something similar for a test here.

[igrr]

I was referring to the performance benchmark (time spent in a call to millis), which i think can only be done on the device. For the accuracy/drift test, perhaps it can be done on the host.

[mrwgx3]

@igrr and @devyte
Would you mind adding the benchmarking code into the repository as a test?

After some study, I believe I can manage the device test. I have (2) questions:

1. Can I use printf statements inside a 'TEST_CASE' block? It's not obvious from the examples.
2. I'm assuming I need to generate a pull-request containing the test code when I'm done.

[igrr]

Yes, you should be able to use printf statements inside the test case.

You can simply push more commits to your mrwgx3_millis_corr branch, and they will be visible in this PR. You don't need to create a separate PR for the tests.

[tonton81]

you ran this test for the year!? wow :)

[mrwgx3]

@igrr and @devyte
Benchmarking code has been added to 'test/devices'

[devyte]

@mrwgx3 there seems to be a warning on compile. Could you please fix that? I'll merge then.

[mrwgx3]

Hi @devyte
On the Arduino side, I didn't have any compiler errors or warnings. If its the Travis build which is the problem, you'll have to provide some guidance on how to fix those. Please clarify.

[earlephilhower]

@mrwgx3 You'll need to enable all warnings in the ide to see them.
File->Preferences->Compiler Warnings->All

Here's the relevant snippet from the logs:

/home/travis/arduino_ide/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_wiring.c: In function 'millis':
/home/travis/arduino_ide/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_wiring.c:173:3: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   ((uint64_t *)(&a[0]))[0]  =
   ^
/home/travis/arduino_ide/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_wiring.c:176:3: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   ((uint64_t *)(&a[0]))[0] +=              // (b) Offset sum, low-acc
   ^
/home/travis/arduino_ide/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_wiring.c:179:3: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   ((uint64_t *)(&a[0]))[0] +=              // (c) Offset sum, low-acc
   ^
cc1: all warnings being treated as errors

[mrwgx3]

Thank you, @earlephilhower.
Can now see the warnings. Will see what I can do.

[mrwgx3]

Hi @devyte and @earlephilhower

Make requested corrections to eliminate 'punning' warnings to files
'cores/esp8266/core_esp8266_wiring.c' and
'tests/device/test_millis_mm/test_millis_mm.ino'

Hope this solves the problem

[earlephilhower]

@mrwgx3 Thanks for the fast fix, it compiled cleanly in CI! I've not been following and will leave it to @devyte to do the merge as he knows what's going on here.