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

How do I declare a Timer field in a struct? #75

Open
Maldus512 opened this issue Sep 8, 2020 · 11 comments
Open

How do I declare a Timer field in a struct? #75

Maldus512 opened this issue Sep 8, 2020 · 11 comments
Milestone

Comments

@Maldus512
Copy link

Hello,
I'm moving to this library to manage time durations in my system. I was able to substitute my previous u32 timestamp checks and wanted to use the Timer type to articulate period tasks.

To this end I'm trying to add a Timer field to a struct that then executes a task every second. Unfortunately the Timer struct requires 4 type parameters, out of which two (State and Type) are privately owned by the timer module.
How am I supposed to declare a struct field of type Timer<Periodic, Running, Clock, Duration>?

Considering that the state of the timer (Running, Armed,...) is part of the type, I fear that mine is a use case that was not considered, as I wouldn't be able to stop or start the timer from my statically declared field. Is there no way to achieve what I want to?

@PTaylor-us
Copy link
Member

The Timer type acts as a builder. The Timer::new() gives you a OneShot, Armed Timer object. From there, you can call Timer.into_periodic() to get a Periodic, Armed Timer object. From there you call Timer.start() to make it Running. I guess this scheme does require you to start the timer upon initialization of your struct. I can see that as being non-optimal.

my_struct.timer = Timer::new(my_clock, 1_u32.millisecond()).into_periodic().start();

If this doesn't work for your usage, let me know and I'll change it.

@Maldus512
Copy link
Author

This is what I was trying to accomplish:

use embedded_timer::{Timer, Clock, Duration::Milliseconds};

struct MyDevice<C : Clock> {
    timer : Timer<Something, Something, C, Milliseconds<C::T>>
    /* Other stuff */
}

impl<C : Clock> MyDevice<C> {
    pub fn new(period: u32, clock : &C) -> MyDevice<C> {
        MyDevice {
            timer: Timer::new(clock, Milliseconds::new(C::T::from(period))).into_periodic(),
        }
    }

    pub fn periodic_task(&self) {
        if (self.timer.is_expired()) {
            // Do something...
            // Re-arm timer
        }
    }
}

I can get the same result (as I am currently doing) by separately storing an Instant timestamp and a Duration period to be checked, but I felt the timer route would have been more elegant. The most obvious problem is that I cannot declare the field timer of the struct, as I do not have access to the required types (where I wrote Something in my example).

Even if were able to skip that I'm afraid something would break when passing the Clock reference to the timer, unless I stored that same Clock inside the struct and made the lifetime explicit...

@PTaylor-us
Copy link
Member

I will definitely take a close look at this and figure out what I need to change.

Even if were able to skip that I'm afraid something would break when passing the Clock reference to the timer, unless I stored that same Clock inside the struct and made the lifetime explicit...

Would you mind elaborating on this? It's entirely possible I overlooked something, but I'm not sure what issues you are concerned about. I can imagine a situation where mutable access to the Clock is needed by the implementation. Is this what you're worried about.

@Maldus512
Copy link
Author

Would you mind elaborating on this? It's entirely possible I overlooked something, but I'm not sure what issues you are concerned about. I can imagine a situation where mutable access to the Clock is needed by the implementation. Is this what you're worried about.

Not really; truth to be told I'm still a beginner when it comes to Rust and I forgot I can indeed have multiple immutable references to an object (in this case Clock). Lifetimes and references were not clear to me in this particular context; after some more study I understood that if I want to declare a Timer field inside a struct I will just have to add an explicit lifetime parameter to account for the Clock reference inside the Timer.

@PTaylor-us
Copy link
Member

That is good for me to know too. I too am relatively new and there are many patterns that I am not very familiar with. At the very least, I will document an example of containing a Timer in a struct. I'm guessing that perhaps you can leave one or more Timer type parameters as anonymous in the struct declaration?

@Maldus512
Copy link
Author

I'm afraid I'm not exactly sure of what you mean by "anonymous type parameters"

@PTaylor-us
Copy link
Member

@Maldus512 I'm updating this code. I believe making the Timer type parameters public would simplify this issue greatly. Can you confirm?

@Maldus512
Copy link
Author

@PTaylor-us I think so. My main goal right now would be to declare a Periodic and Running timer as field/variable, and periodically check it, so exposing both type parameters as public would make it possible.

@jhbruhn
Copy link

jhbruhn commented Apr 20, 2021

I am struggling with the same issue. Adding the state of the timers to the type via generics is a nice idea in principle, but in prevents us from storing the timer in a struct in different states. Or am I misusing the Timer struct?
As Maldus512 said, an example on how to use the timers stored in a struct would be very nice!

Edit: On further inspection (at least for my use case), having the Timers state (Running/Armed) in the type-system is a very nice implementation in theory, but hinders me from declaring an armed Timer in a struct, which gets started in a different point in time (other than storing said timer in two Options, obviously not very elegant). Perhaps at least the Armed/Running state could be changed into an internal bool, and additionally the Markers are made public so the Timer can be defined in a struct. That would solve the problem.

@PTaylor-us
Copy link
Member

PTaylor-us commented Apr 28, 2021

@jhbruhn I think you're right. Given that this type may be stored in a struct, it needs to be rewritten without the state-types.

To start with, I am making the Timer param types public.

@PTaylor-us
Copy link
Member

Related: #102

@PTaylor-us PTaylor-us added this to the v0.13.0 milestone Aug 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants