korken89 / fugit

`fugit` provides a comprehensive library of `Duration` and `Instant` for the handling of time in embedded systems, doing all it can at compile time.
Apache License 2.0
56 stars 16 forks source link

impl TryFrom<core::time::Duration> for Duration<...> #25

Open chrysn opened 2 years ago

chrysn commented 2 years ago

It appears to me that it should be well possible to implement this conversion -- it wouldn't be as compile-time constant as the rest of fugit, but it would help gluing it to more generic code that works with standard, aeh, core durations.

(I'm considering to move my bespoke timers for RIOT-OS to just be a newtype around, and after the next breaking API change, just a type alias for fugit's duration, in particular TimerDurationU32, but don't want to lose functionality for simple demos whose values are expressed in what the user may be familiar with).

[edit: When I do switch over and there is soft approval of this, I can probably provide an implementation PR.]

adamgreig commented 2 years ago

I guess we could have TryFrom<fugit::Duration> for core::time::duration while we're about it, too. Probably can't make either infallible since I guess you could have a fugit Duration that's u64 backed and counts minutes or whatever, unfortunately. What would the error condition be on converting core's duration to fugit's duration? core represents nanoseconds, so most fugit durations won't be an exact/lossless conversion... do we only error on overrange, but not on loss of accuracy?

chrysn commented 2 years ago

Yes, good point; that direction might even be infallible given Duration's fixed 64bit-seconds semantics that go back to before time even made sense.

As for acceptability of rounding, core durations already have a one-sided statement on this: "If the underlying system does not support nanosecond-level precision, APIs binding a system timeout will typically round up the number of nanoseconds." -- while the "typically" gives us leeway, the conservative conversion would be to round up from core::time::Duration to fugit::Duration as well, ensuring that when it's passed on to, say, a hardware timer, that too rather fires later than sooner. For the inverse direction (from fugit::Duration to core::time::Duration), they probably didn't think about that because which system gives accurate sub-nanosecond times ... but I'd take the "typically" in the docs to indicate that such conversions are not absolutely expected to be correct, so any rounding is probably acceptable.

korken89 commented 2 years ago

Seems like a good idea!

Does anyone of you two feel like making a PR? I have too much other things on my plate to take this on.

chrysn commented 2 years ago

I can, but only when I get back to my timers, which I can't predict when.

adamgreig commented 2 years ago

What do we think about making fugit -> core infallible? In principle an fugit's HoursDurationU64 can express a longer duration than core's Duration, which would have to fail or saturate, but in practice core's Duration can express up to 584 billion years, so it seems a little annoying to require try_into() for a failure that seems rather unlikely.

Perhaps we could document that the conversion saturates after 584 billion years and then use Into instead. Or stick to TryInto.

And we could do the same the other way, converting a core Duration that's longer than a fugit Duration into the maximum value for the Duration, but I think in that case it's much more likely to happen (a TmerDurationU32 might have a period of like 30 seconds) and so should probably be an Error.

korken89 commented 2 years ago

Infallible is fine for me :)

chrysn commented 2 years ago

For the other direction I'd prefer to stick with fallible, at least for the u32 ones (but probably all because it's so generic).