search

rust-embedded / embedded-hal

A Hardware Abstraction Layer (HAL) for embedded systems
Apache License 2.0
2.01k stars 202 forks source link

Remove Into<Time> generics #352

Closed burrbull closed 2 years ago

burrbull commented 2 years ago

Reason: fugit uses it's own convertion technic and works bad with Into. Same as embedded-time don't use Into for potentially fallible convertions

rust-highfive commented 2 years ago

r? @therealprof

(rust-highfive has picked a reviewer for you, use r? to override)

burrbull commented 2 years ago

Could you elaborate why something like Into<T> is problematic? If you already have T, as you would need to have if we merged this, then Into<T> should always be available, right? What am I missing?

If you look at fugit::ExtU32 it let you pass 10.millis() type inference for automatic detecting of NOM and DENOM regardless of the resulting type. It works very well if resulting type is known (without any Into<T>). If you don't know resulting type than type inference don't work and you need to specify it by hand.

Saying about embedded-time Into<T> works only for part of conversions. Other require TryInto<T>

eldruin commented 2 years ago

Thank you for bringing this up @burrbull. As we agreed yesterday to merge #324 and thus remove the traits for now, I will close this. However, we should definitely remember about this when we discuss the associated type constraints when adding the traits back. Please bring this back up for proper discussion then if we forget about it.

burrbull commented 2 years ago

If you look at fugit::ExtU32 it let you pass 10.millis() type inference for automatic detecting of NOM and DENOM regardless of the resulting type. It works very well if resulting type is known (without any Into<T>). If you don't know resulting type than type inference don't work and you need to specify it by hand.

Looks like this was fixed. But can't understand how.