Skip to content

Improve resolution of mp_hal_delay_ms() to subticks. #10315

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 8, 2025
Merged

Improve resolution of mp_hal_delay_ms() to subticks. #10315

merged 4 commits into from
May 8, 2025

Conversation

eightycc
Copy link
Collaborator

@eightycc eightycc commented May 6, 2025

Improves resolution of mp_hal_delay_ms() from ticks (1/1024 sec.) to subticks (1/32768). This PR also corrects port_get_raw_ticks() for these ports:

  • broadcom: Adds calculation and return of subticks. Adopts calculation used for Espressif port.
  • cxd56: Returns subticks only when pointer parameter is non-null.
  • litex: Return zero for subticks when parameter is non-null. This port does not resolve time below integral ticks.
  • raspberrypi: Correct tick/subtick calculation. Adopts calculation used for Espressif port.

eightycc added 2 commits May 6, 2025 10:57
@eightycc
Fix port_get_raw_ticks() for these ports:
7b12bc8 
o broadcom: return subticks
o cxd56: return subticks only when pointer is non-null
o litex: zero subticks, port is accurate only to ticks
o raspberrypi: correct subtick calculation
@eightycc
Improve mp_hal_delay_ms by implementing subtick resolution.
fcf7df8
@eightycc eightycc linked an issue May 6, 2025 that may be closed by this pull request
time.sleep waits less than expected #9693
Closed
@eightycc
Copy link
Collaborator Author

eightycc commented May 7, 2025 •
edited
Loading

Test on Raspberry Pi Pico 2:

Adafruit CircuitPython 10.0.0-alpha.4-dirty on 2025-05-06; Raspberry Pi Pico 2 with rp2350a
expected 10000 us, got 10007.1 us
expected 10000 us, got 10004.7 us

Test on Adafruit Metro ESP32-S3:

Adafruit CircuitPython 10.0.0-alpha.4-2-gfcf7df857e-dirty on 2025-05-07; Adafruit Metro ESP32S3 with ESP32S3
expected 10000 us, got 9986.71 us
expected 10000 us, got 9984.9 us

Accuracy now within 1 subtick (1/32768 sec = ~30.5 us).

@eightycc
Copy link
Collaborator Author

eightycc commented May 7, 2025 •
edited
Loading

ESP32-S3 failure also occurs with 10.0.0-alpha.4 without this PR and with 9.2.7. This eliminates this PR and ESP-IDF 5.4.1 as potential causes of the failure.
Failure on ESP32-S3 was due to a typo in the test (whoops!). Cleaned up the PR a bit in the process.

@eightycc
Improve update to mp_hal_delay_ms:
61071cb 
o Test for console interrupt during RUN_BACKGROUND_TASK
o Handle signedness of remaining tick/subtick values correctly for very large delays
o Improve comments
@eightycc eightycc marked this pull request as ready for review May 7, 2025 16:11
@eightycc eightycc requested a review from tannewt May 7, 2025 16:11
@eightycc
Copy link
Collaborator Author

eightycc commented May 7, 2025

Ready for review. The ESP32-S3 failure was a typo in the test.

tannewt
tannewt requested changes May 7, 2025
View reviewed changes
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

One API comment. Looks great otherwise. Thanks!

@eightycc eightycc requested a review from tannewt May 7, 2025 20:12
tannewt
tannewt approved these changes May 8, 2025
View reviewed changes
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you!

@tannewt tannewt merged commit 07ec0c6 into adafruit:main May 8, 2025
615 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tannewt tannewt tannewt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

time.sleep waits less than expected
2 participants
@eightycc @tannewt