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

Improve and use timer abstractions #1753

Merged
merged 14 commits into from
Jul 9, 2024

Conversation

bjoernQ
Copy link
Contributor

@bjoernQ bjoernQ commented Jul 4, 2024

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • I have updated existing examples or added new ones (if applicable).
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • My changes are in accordance to the esp-rs API guidelines

Extra:

Pull Request Details 📖

Description

  • make esp-wifi::initialize take a PeriodicTimer<ErasedTimer>
  • make the embassy time-driver take OneShotTimer<ErasedTimer> references

This enables the user to use any hw-timer for esp-wifi (no matter if from TIMG0, TIMG1 or SYSTIMER) and use any combination of hw-timers for the embassy-time-driver. It also gets rid of the features to select the embassy time-driver implementation.

Testing

Run the (changed) examples ideally at least on ESP32, ESP32-S2 and any other chip

@jessebraham jessebraham changed the title Improve and use timer absractions Improve and use timer abstractions Jul 4, 2024
@bjoernQ bjoernQ force-pushed the improve-and-use-timer-absractions branch from 6c50198 to 88522a0 Compare July 4, 2024 14:21
Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

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

There's a lot to take in here, but overall I think it's looking quite good. Thanks for taking care of this, nice to finally have this implemented!

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Jul 5, 2024

Side note: The underlying timers probably should wrap modifications on shared registers in a critical-section (#1739 already addresses that for SYSTIMER). That is out of scope for this PR

Sorry for the huge amount of changed code here but I think it makes sense to have this in one PR - fortunately much of the changes are just adapted examples. BTW the examples use different timers and patterns for initialization - that's to show the flexibility of the API and on purpose

Copy link
Contributor

@JurajSadel JurajSadel left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jessebraham jessebraham added this pull request to the merge queue Jul 9, 2024
Merged via the queue into esp-rs:main with commit a5be31f Jul 9, 2024
29 of 30 checks passed
@Dominaezzz Dominaezzz mentioned this pull request Aug 15, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make OneShotTimer / PeriodicTimer useable in esp-wifi and esp-hal-embassy
3 participants