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

[rtl872x] Fix millis() rollover #2501

Merged
merged 1 commit into from
Aug 4, 2022
Merged

Conversation

scott-brust
Copy link
Member

Problem

HAL_Timer_Get_Milli_Seconds() / HAL_Timer_Get_Micro_Seconds() Eventually reset after ~1 hour of execution. This results in millis() rolling over to zero.

Solution

It appears that the problem is due to integer promotion (in this case, demotion?) when calculating elapsed micro seconds:

constexpr int US_PER_OVERFLOW = 65536;                 // 65536us
...
return usSystemTimeMicrosBase_ + state.overflowCounter * US_PER_OVERFLOW + ticksToTime(state.curTimerTicks);

Making US_PER_OVERFLOW a uint64_t seems to solve the problem.

Steps to Test

Modify timer_hal.cpp init() and start() functions to start the overflowCounter_ at a value close to 65536

        overflowCounter_ = 65000;

Run the app and observe the uptime millis incrementing beyond 0004294659

Example App

tinker or any app with timestamp logging

References

Links to the Community, Docs, Other Issues, etc..


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)

@scott-brust scott-brust requested a review from avtolstoy August 4, 2022 20:51
Copy link
Member

@avtolstoy avtolstoy left a comment

Choose a reason for hiding this comment

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

🎉

@scott-brust scott-brust merged commit 3e380c8 into develop Aug 4, 2022
@scott-brust scott-brust deleted the fix/sc-107615/millis-rollover branch August 4, 2022 21:50
@technobly technobly added the bug label Aug 15, 2022
@technobly technobly added this to the 5.0.0 milestone Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants