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 with runtime defined SCALING_FACTOR #109

Open
Sh3Rm4n opened this issue Aug 9, 2021 · 3 comments
Open

Clock with runtime defined SCALING_FACTOR #109

Sh3Rm4n opened this issue Aug 9, 2021 · 3 comments

Comments

@Sh3Rm4n
Copy link

Sh3Rm4n commented Aug 9, 2021

Hey @PTaylor-us,

first and foremost, thanks for this great crate! It makes working with times and durations much more consistent and intuitive!

I have a question about implementing clock::Clock though.

Is it at all possible to implement it for clocks and timers, which clock bases is defined at runtime as opposed to be known at compile time? If I understand it correctly, the associated constant SCALING_FACTOR is used as a basis to calculate 1 second out of the ticks provided by the clock to be compatible with Durations.

But this is not practicable. Knowing the clock at compile time (especially out of the perspective of a library author) is a rare exception. Every clock is configurable / the clock of crystals varies depending on the PCB design and even if some clocks are defined (as for example the RTC basis with its 32.768 kHz) can still be ignored and another source chosen by the user.
And of course, the clock of timer peripherals is also configurable.

So for my point of view, implementing any Clock is impossible. Am I overlooking something? So the thing I would like to see is providing a way to define the basis clock frequency at runtime when instantiating a Clock. I don't know if this is possible for example with duration::Generic which is storing the scaling factor at runtime, if I am understanding it correctly.

While writing, I'm thinking of a way to move SCALING_FACTOR to be runtime-defined, but have no solution / proposal as of now.


Some more questions about the implementation. I'm trying to read some information through the provided example, because the documentation is pretty sparse (which I'll gladly try to improve after clarifying these questions :) )

impl time::Clock for SysClock {
type T = u64;
const SCALING_FACTOR: time::fraction::Fraction = <time::fraction::Fraction>::new(1, 16_000_000);
fn try_now(&self) -> Result<time::Instant<Self>, time::clock::Error> {
self.capture_task.tasks_trigger[0].write(|write| unsafe { write.bits(1) });
let ticks =
self.low.cc[0].read().bits() as u64 | ((self.high.cc[0].read().bits() as u64) << 32);
Ok(time::Instant::new(ticks as Self::T))
}
}

Then fn try_now should return the current ticks / Instant pretending it being a monotonic timer, and that is its only purpose?

@PTaylor-us
Copy link
Member

Then fn try_now should return the current ticks / Instant pretending it being a monotonic timer, and that is its only purpose?

That is correct. In this case to get the current tick value, it's triggering a linked capture and reading the two registers of a concatenated hardware timer on the nRF52.

Knowing the clock at compile time (especially out of the perspective of a library author) is a rare exception.

Setting aside the context of library author for a sec, in my experience, clock speed is always known at compile time. At the very least, it is known what it will be after startup configuration. It's been a while since I've been in Rust, but for a library, can't the user of the library define a const value that is used in the clock implementation?

@Sh3Rm4n
Copy link
Author

Sh3Rm4n commented Aug 13, 2021

Setting aside the context of library author for a sec, in my experience, clock speed is always known at compile time. At the very least, it is known what it will be after startup configuration. It's been a while since I've been in Rust, but for a library, can't the user of the library define a const value that is used in the clock implementation?

Yeah, this is something I have though about as well. I think the problem right now is, that the SCALING_FACTOR is set via a associated const, which means that only one clock per timer is possible (which might be okay for a user, but is not for a library author). So I guess a better solution could, that the SCALING_FACTOR is parametrized with const generics.

iml<const SCALING_FACTOR: time::fraction::Fraction>  time::Clock<SCALING_FACTOR> for SysClock {
    // ...
}

But this example, as written above, is not possible, as of the current min-const-generics implantation. So it might have to be dumped down to compile. So leveraging the fact, that the scaling factor is known at compile time to reduce overhead is definitely a great thing to thrive for, but it lacks flexibility right now (because of lacking const generics features - or because it is not dynamically configurable) to be used in a library.

@PTaylor-us
Copy link
Member

I plan to migrate to const generics when they're ready. That's what I wanted from the start, but had to settle for the associated const. I need to play with some code to understand more fully where the sticking point is. I haven't had a chance to use Rust for some time, so I'm a bit .... wait for it .... rusty :).

Thank you for your interest in the crate and I look forward to working with you to find a solution.

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

2 participants