Skip to content

sys/arduino: use 64bit usec to compute millis()#12275

Merged
maribu merged 1 commit into
RIOT-OS:masterfrom
keestux:fix-arduino-millis
Sep 19, 2019
Merged

sys/arduino: use 64bit usec to compute millis()#12275
maribu merged 1 commit into
RIOT-OS:masterfrom
keestux:fix-arduino-millis

Conversation

@keestux

@keestux keestux commented Sep 19, 2019

Copy link
Copy Markdown
Contributor

Contribution description

This is quick solution to avoid wrapping around after 4294967 milliseconds.
It uses xtimer_now_usec64 instead of xtimer_now_usec.

Notice that this is more expansive than the previous solution, especially
on AVR systems.

Testing procedure

Use the new upcoming #12180 which adds support for using Arduino libraries. The test program is a simple loop with TalkingLED, flashing leds. Without the fix in this PR the test program will hang roughly after 4295 seconds. I realize that this is way too long for a normal test program. An alternative is to create a special test program which would bump the "now" value to, say 4290 seconds, wait 10 seconds and look at the millis(). But I don't think it is worth the effort.

Issues/PRs references

See also #12180 and #12116

This is quick solution to avoid wrapping around after 4294967 milliseconds.
It uses xtimer_now_usec64 instead of xtimer_now_usec.

Notice that this is more expansive than the previous solution, especially
on AVR systems.
@keestux keestux requested a review from maribu September 19, 2019 18:06

@maribu maribu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK. (I am 100% sure that this PR works, even without testing.)

@maribu maribu added Area: arduino API Area: Arduino wrapper API CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Sep 19, 2019
@maribu maribu merged commit 8c94a16 into RIOT-OS:master Sep 19, 2019
@keestux keestux deleted the fix-arduino-millis branch September 19, 2019 19:58
@kb2ma kb2ma added this to the Release 2019.10 milestone Sep 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: arduino API Area: Arduino wrapper API CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants