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] Poll micros instead of rom DelayUs function. Tweak 1ms test #2499

Merged
merged 2 commits into from
Aug 4, 2022

Conversation

scott-brust
Copy link
Member

Problem

DELAY_01_delay_1ms_is_within_tolerance fails occasionally on p2 no_fixture tests, ie the 1ms delay ends up being out of the 6% margin of error ( > 1060 us).

Solution

Changing the HAL_Delay_Microseconds() implementation to a simple poll of HAL_Timer_Get_Micro_Seconds() seems to be no worse than using the rom DelayUs() function.
Also tweaked the system_delay_pump() logic when doing a 1ms delay, which is what happens in the DELAY_01 test.

However, the 6% threshold is still too short in some situations. Doubling it seems to ensure tests pass a higher % of the time. I still see an occasional failure where the delay took >1120 us, but this happened once in 300 test iterations. Not sure if this is infrequent enough to ignore or if the threshold should be raised even higher.

Steps to Test

Run wiring/no_fixture tests

Example App

wiring/no_fixture

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 merged commit 053ad27 into develop Aug 4, 2022
@scott-brust scott-brust deleted the fix/sc-107188/delay_1ms_test_fix branch August 4, 2022 20:35
@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