-
Notifications
You must be signed in to change notification settings - Fork 250
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
Newtype fugit Rate, Instant and Duration #3083
Conversation
7878da0
to
a1f9a4a
Compare
e204b06
to
728dd66
Compare
a5747a0
to
ab9491f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
We could consider a From<core::time::Duration>
impl but not necessarily in this PR (it's already big enough)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, thanks for taking care of the impl @bugadani!
The only thing left to do now is bikeshed megahertz, I've offered my opinion, and a few alternatives.
fugit forces the exclusive choice of u32 or u64. this creates frustrations when wanting to interface with other crates that use usize, or some other int-len. Awhile ago, I ran into this issue, and it's a pain. I would recommend something that bases itself more on using the I've been doing some work on this recently, but it's unfinished and unrefined, but it certainly serves well as a PoC. |
This PR implements this plan: #2923 (comment) where we've chosen fixed backing types for our implementation, so no need to worry about that here :).
I'd like to avoid using typenum where possible, slow compile times and too many generics. |
What's your use case? |
Use case was ages ago, so I forget the details, but the key issues is that there were incompatable types embedded into const generics, and just made the whole thing messy.
Noted. What do you plan to do for the cases where the types chosen for the const generics cause headaches? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
In general, we're trying to avoid generics (even the const kind) in the public API, and instead to the type checking in the constructor then convert the driver to a "concrete" type. |
Closes #2923
skip-changelog because of embassy/wifi updates.