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

General purpose delay provider #210

Merged
merged 2 commits into from
Feb 20, 2023
Merged

Conversation

dbrgn
Copy link
Contributor

@dbrgn dbrgn commented Feb 14, 2023

Fixes #199 and improves docs.

Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

I like this change, I've seen a few people stumble on this sort of thing. Thank you for improving the docs!

My nitpick would just be about the name, I would just call it Delay.

@dbrgn
Copy link
Contributor Author

dbrgn commented Feb 14, 2023

@MabezDev I initially named it Delay, but then thought that it might not be ideal because it would collide with the Delay trait.

However, I just realized that the trait(s) are called DelayMs and DelayUs, so it shouldn't be an issue 🙂 I'll rename!

@dbrgn dbrgn force-pushed the general-purpose-delay branch from 2bbafe3 to d923b32 Compare February 14, 2023 20:37
@dbrgn dbrgn force-pushed the general-purpose-delay branch from d923b32 to b485bd4 Compare February 14, 2023 20:42
@dbrgn
Copy link
Contributor Author

dbrgn commented Feb 14, 2023

Renamed, and I also fixed an embarrassing bug 😄

Additionally, I added the static methods on Delay and am calling these from the trait impls. This makes for a more convenient API and is consistent with the other delay providers.

@ivmarkov ivmarkov merged commit d477d54 into esp-rs:master Feb 20, 2023
@dbrgn dbrgn deleted the general-purpose-delay branch February 20, 2023 17:31
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.

General-Purpose Delay Provider
3 participants