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

[p2] Simplify burn in LED blink, work around System.millis() reset #2491

Merged
merged 2 commits into from
Jul 26, 2022

Conversation

scott-brust
Copy link
Member

Problem

The P2 burn in process is supposed to indicate how long it has run by blinking the LED every 1s. An alternating green/blue pattern indicates the tests are still running and passing.

For burn in hours X:
x >= 72h : 6 green blinks 4 blue
72h > x >= 48h : 7 green blinks 3 blue
48h > x >= 24h : 8 green blinks, 2 blue
x < 24h : 9 green blinks, 1 blue

The current LED blink logic is convoluted, and relies on the System.millis() value monotonically increasing. Unfortunately there is also a bug right now where it resets sometimes, which causes the LED logic to break and stop blinking all together.

Solution

Simplify LED blink logic. Just use delay() since LED loop has its own thread. No need to rely on System.millis()

Steps to Test

Build tinker, run burnin

Example App

Tinker

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)

Sorry, something went wrong.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@scott-brust scott-brust requested a review from avtolstoy July 25, 2022 19:24
}

// Blink LED / signal uptime / failure state
void BurninTest::ledLoop(void * arg) {
BurninTest* self = static_cast<BurninTest*>(arg);
uint32_t total_runtime_millis = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: system_tick_t (32-bit) or uint64_t

@scott-brust scott-brust merged commit 57b9643 into develop Jul 26, 2022
@scott-brust scott-brust deleted the fix/p2-burnin-led branch July 26, 2022 16:37
@technobly technobly added this to the 5.0.0 milestone Aug 18, 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.

None yet

3 participants