Skip to content
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

Clock API #24

Open
hannobraun opened this issue Nov 28, 2017 · 8 comments
Open

Clock API #24

hannobraun opened this issue Nov 28, 2017 · 8 comments
Labels

Comments

@hannobraun
Copy link
Member

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:

  • Implementations of the traits: LowPowerClock, IrcDerivedClock
  • Sleep code that depends on various bits: Sleep
  • I also have an unrelease time library that depends on this infrastructure. I haven't had time to polish and release it yet, but I'll happily send the code to anyone who wants to take a look.

As I said, this is still a bit rough. lpc-rs/lpc8xx-hal#4 has some
ideas for improvement. I'd love to hear what everyone thinks!

@TheZoq2
Copy link

TheZoq2 commented Feb 28, 2018

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
Copy link
Member Author

@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
Copy link

TheZoq2 commented Feb 28, 2018

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
Copy link
Member Author

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
Copy link

TheZoq2 commented Feb 28, 2018

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
Copy link
Contributor

thejpster commented Feb 28, 2018 via email

@TheZoq2
Copy link

TheZoq2 commented Feb 28, 2018

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
Copy link

TheZoq2 commented Feb 28, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants