rust-embedded / embedded-hal

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

Clock API #24

Open hannobraun opened 6 years ago

hannobraun commented 6 years ago

Clocks are an important part of all microcontrollers that I know of, and I believe this is a topic that should be covered in embedded-hal. I don't have a good enough overview over the topic to know everything that's required, but I have encountered some use cases that gave me a few ideas.

I've implemented a rough proposal as part of LPC82x HAL. This is not quite ready for inclusion into embedded-hal, but I've decided to open this issue to garner some feedback, and give others the opportunity to mention other use cases that I haven't covered.

Here's the API that I'd like to submit (eventually):

Use cases for the API:

As I said, this is still a bit rough. https://github.com/braun-robotics/rust-lpc82x-hal/issues/4 has some ideas for improvement. I'd love to hear what everyone thinks!

TheZoq2 commented 6 years ago

This looks like a good proposal to me. It would solve most issues I pointed out in #46.

One thing it doesn't directly solve is specifying timeouts for CountDown without knowing what Timer::Time is? Perhaps you could give them a Fn(Frequency) -> Ticks. Or perhaps that should be a separate struct since they are not completely related.

hannobraun commented 6 years ago

@TheZoq2 I think your problem is somewhat orthogonal to this proposal. Even if this were adopted, and CountDown's method would be changed to take Into<Ticks> instead, we'd still need the associated type, as Ticks as currently specified is not portable enough. The u32 needs to be replaced with a type parameter, and that type parameter would then still show up as CountDown's type Time.

I can think of only one good solution to your problem: Choose a type that has enough range for your timeouts (u16, for example) and require the user to provide a function that converts from that type to whatever CountDown's type Time happens to be.

Your driver should ideally be as general as embedded-hal is, so it shouldn't make any assumptions about the data type that the timer uses. But the end user, that ends up running your driver on a specific microcontroller, has the knowledge to convert from your type to the one used by the CountDown implementation.

If this proposal were part of embedded-hal, and a time library based on it would show up, then that time library might help with this problem. As of now, I don't know exactly what that would look like though.

TheZoq2 commented 6 years ago

Oh yea, you have a point. At some point, the amount of ticks in the timer is stored in some datatype, probably an unsigned int of some kind but it would depend on the timer. A driver could specify that the type should be a u16 for example, but what datatype is required depends on the frequency of the timer which is not known.

One solution I could think of for this would be to define some new types like Second, Millisecond and Microsecond as well as additional countdown traits like CountDownSecond etc. It would then be up to the device crate implementor to implement these for all the timers that are capable of "enough precision/range" to accurately meassure such times.

Of course, this isn't without issues either. "enough precision" is very vague and a timer might be capable of counting for 10 seconds but would then overflow. The behaviour after 10 seconds would then be undefined.

As you pointed out, these proposals are not completely related. Do you think I should reopen mine and keep the discussion goging there or should we continue here?

hannobraun commented 6 years ago

One solution I could think of for this would be to define some new types like Second, Millisecond and Microsecond as well as additional countdown traits like CountDownSecond etc. It would then be up to the device crate implementor to implement these for all the timers that are capable of "enough precision/range" to accurately meassure such times.

Of course, this isn't without issues either. "enough precision" is very vague and a timer might be capable of counting for 10 seconds but would then overflow. The behaviour after 10 seconds would then be undefined.

Your idea is interesting, but I'm not sure if this is something that should go into embedded-hal, at least at this point in time. I think this whole topic of time and its representation will require a lot of experimentation before we'll be ready to judge what a good solution would be like, and this experimentation should probably happen in dedicated crates.

Once it has become clear what a good solution is, we can decide whether to merge it into embedded-hal or keep it separate. Maybe someone with a better grasp on the topic than me might disagree.

As you pointed out, these proposals are not completely related. Do you think I should reopen mine and keep the discussion goging there or should we continue here?

I think it's fine to have this discussion here, as it relates to use cases for the proposed clock API.

TheZoq2 commented 6 years ago

Your idea is interesting, but I'm not sure if this is something that should go into embedded-hal, at least at this point in time.

Yep, that makes sense. Since I have some time available this week, I'll try implementing what I proposed and see if I run into any issues along the way.

thejpster commented 6 years ago

I just wanted to add quickly that I would hope that any time period given in seconds (or fractions thereof) would convert to clock ticks at compile time, as it would in a C program with a F_MHZ define or similar.

Jonathan

TheZoq2 commented 6 years ago

Yep, that would be ideal. However, im not sure if that's possible with the current structure of the crate. As far as I can tell (at least in stm32f103xx), the frequency of all the clocks in the system is read (and then fixed) at runtime when you create a clocks object.

TheZoq2 commented 6 years ago

This turned out to be a simpler to implement that I thought. I created a crate called embedded-hal-time containing a single trait called RealCountDown which is generic over a time type. I added 3 time types Seconds, Milliseconds and Microseconds which are simply newtypes around u32. RealCountDown also has CountDown as a supertrait for code reuse.

I also added an implementation of that trait to my fork of the stm32f103xx_hal crate. The implementation is not perfect, there is some code duplication between it and the normal Timer::start function and some probably expensivie divisions in the case of Microsecond and Millisecond.