embedded-hal icon indicating copy to clipboard operation
embedded-hal copied to clipboard

Shared type definitions (e.g. MHz, bps...)

Open nordmoen opened this issue 6 years ago • 16 comments

I want to create this issue to discuss a shared place to store necessary hardware type definitions. These could be stored in the embedded-hal crate, but could also easily be in their own crate. I just think we should discuss a shared place to store these outside the processor specific crates.

There are many type definitions that can be useful when working with low-level sensors. At the moment there are no shared place that such type can or are stored. A couple of very handy types are currently in stm32f30x_hal::time (and also replicated in stm32f103xx_hal) for dealing with time.

An easy first step is to create a type module for embedded-hal and extract the types defined in stm32f30x_hal::time and potentially add more later if necessary.

I'm not sure if there are other definitions apart from time that should be abstracted out. I don't think this issue should be about physical types (e.g. cm or m/s), but should focus on shared types that are necessary for interacting with hardware.

nordmoen avatar Mar 08 '18 07:03 nordmoen

For most physical types there are other crates, so as the docs.rs/measurements crate that I manage.

I think frequency, bitrate and time should be in embedded-hal though as most APIs will need them.

thejpster avatar Mar 08 '18 07:03 thejpster

Not to derail the discussion after the first comment, but would it be possible to transition some of the time handling to use core::time::Duration instead of raw u32s? I understand that the additional u64 is not wanted, but for subsecond durations shouldn't it be optimized away?

The advantage would be more interoperability with the rest of the ecosystem, and also many helpful functions to ensure no overflow etc.

nordmoen avatar Mar 08 '18 08:03 nordmoen

@thejpster Great stuff! I was actually looking for that, partially also for #32. @nordmoen I fully agree that the base units required by embedded-hal should be part of it instead of being duplicated in every single implementation. I'm a bit torn about core::time::Duration because I'm not exactly sure how much baggage that would add; guess we'll have to just have to try and see.

therealprof avatar Mar 08 '18 08:03 therealprof

Partially related discussion: #24 See also (from the linked issue): https://github.com/TheZoq2/embedded-hal-time

hannobraun avatar Mar 08 '18 12:03 hannobraun

Not to derail the discussion after the first comment, but would it be possible to transition some of the time handling to use core::time::Duration instead of raw u32s? I understand that the additional u64 is not wanted, but for subsecond durations shouldn't it be optimized away?

It would be very likely that it won't be optimized away unless the CountDown implementation (or whatever is using the Duration) gets fully inlined to where the Duration is created. (This does appear to happen in my current prototype that's just blinking an LED based on a CountDown, but I don't know that we could rely on it in larger projects).

I was just thinking about whether a ShortDuration { us: u32 } or something similar should be available in embedded-hal for implementations to use. I would assume most usage of CountDown would be fine with that as the range.


EDIT: Thinking about it a little more. As long as the Duration is created and directly passed into CountDown::start, and CountDown::start is short enough, that could probably be relied upon to be inlined. The difficult cases come if you start passing the Duration around functions, or if CountDown::start is complicated enough that inlining it seems worse to the optimizer than having separate Duration functions.

Nemo157 avatar Mar 08 '18 12:03 Nemo157

True, but then again if you're counting clock cycles and just use a u32 counter, your countdowns aren't going to be very long. I think it'd be better if we just implemented the standard and if it really becomes a problem at some point check out how to address it.

therealprof avatar Mar 08 '18 13:03 therealprof

There are cases when long times don't make sense. I'd be against calling it ShortDuration, but prefer defining it as Microseconds(pub u32) instead.

Kixunil avatar Mar 13 '18 22:03 Kixunil

To name a few alternatives to the measurements crate (which forces the use of f64, which might be suboptimal for small µCs), there's dimensioned and uom, both of which support #![no_std] and allow using arbitrary types.

jonas-schievink avatar Mar 27 '18 11:03 jonas-schievink

Yeah, it's a somewhat odd choice to use f64 for a number of reasons, but especially without FPU it does hurt. I was actually going to look into what was required to change that, potentially using https://github.com/japaric/fpa .

therealprof avatar Mar 27 '18 11:03 therealprof

I am trying to implement a new device driver that should ideally only depend on embedded-hal and I found this issue while searching for a solution to the problem below:

For it to work, I need to wait up to a certain amount of microseconds for an input pin to change, check whether it has changed and wehther it is now in the desired state. This can be implemented for example using the MonoTimer of stm32f103xx-hal. Once the current 'time' / Instant is read a simple while loop could check whether the input changed, as long as the timeout has not reached yet (something like while .elapsed() < us * (.frequency() / 1_000_000)). Since a blocking behaviour might not be desired - as well as the dependency on stm32f103xx-hal - one might think the CountDown could be used. But it requires the Time type to be known - which for the stm32f103 is defined in the stm32f103xx-hal as Hertz. Both strategies require a dependency on the device-hal crate which I would like to omit. Am I missing and easy / obvious approach?

kellerkindt avatar Aug 07 '18 16:08 kellerkindt

Hey, I wanted to revive this stalled issue. I’m writing a device driver that would really like a CountDown in milliseconds, so I’m looking for a common Time type that can represent that.

Has anything changed re: core::time::Duration in the past year that would make it more or less attractive to adopt in embedded-hal?

fionawhim avatar Jul 25 '19 13:07 fionawhim

There is also https://crates.io/crates/bitrate

little-arhat avatar Jul 27 '19 14:07 little-arhat

@kellerkindt or @fionawhim did either of you ever come up with a solution?

I'm also writing a driver and found that I have to depend on the stm32f3xx_hal device crate for the same reason you both mentioned.

wbuck avatar Jan 06 '21 13:01 wbuck

Have you had a look at embedded-time? There is some discussion about it at https://github.com/rust-embedded/embedded-hal/issues/211

eldruin avatar Jan 06 '21 13:01 eldruin

@eldruin OK, I submitted a PR for integrating embedded-time for the stm32f3xx-hal device crate.

It was a pretty smooth integration for the most part except for the CountDown/Periodic timer due to the generic constraint Into<Self::Time>.

I want to see if there are some other devices crates that would be willing to accept a PR for this.

wbuck avatar Jan 12 '21 14:01 wbuck

I would love to see the same happen for https://github.com/stm32-rs/stm32l4xx-hal

MathiasKoch avatar Jan 14 '21 08:01 MathiasKoch