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

Timestamp is not available for interrupts #89

Open
kryptan opened this issue May 1, 2021 · 2 comments
Open

Timestamp is not available for interrupts #89

kryptan opened this issue May 1, 2021 · 2 comments

Comments

@kryptan
Copy link

kryptan commented May 1, 2021

Linux kernel provides timestamps for each GPIO event but it is not available in RPPAL API. It is stored in the gpio::ioctl::Event struct but is discarded and not returned in the InputPin API.

Measuring these timestamps manually using std::time introduces imprecision because of context switches.

@golemparts
Copy link
Owner

Thanks for pointing this out! While the timestamp provided by the GPIO driver also isn't accurate, it will still be more precise than measuring timestamps yourself, so I agree it's a good idea to return both the logic level and timestamp. I'll add it to the to-do list.

@achary
Copy link

achary commented Jan 5, 2024

I've looked a bit into the situation, and things do not look straightforward yet.

I've noticed that the ioctrl::v1::Event (as well as unused yet ioctl::v2::Event) structures does store events' timestamps, but wrap them in std::time::Duration type. This looks almost ready.

However, passing a Duration to the callbacks would create a weird, IMO incorrect API. At least as far as Rust'y time handling style goes:

  • using Instant type to represent moments in time,

  • using Duration for representing time intervals.

  • Ideally, the callback signature should be FnMut(Level, std::time:Instant).

But using std::time::Instant would be only a valid here, if:

a) there is a hard guarantee that the nanosecond count reported by the Linux Kernel for each interrupt event always comes and always will come from the very same clock source that the Unix variant of std::time::Instant happens to use.

If not (now or at any point in the future), a potential comparisons of Instant from interrupt callback with ones obtained using other means, would create incorrect intervals ans super hard to find bugs. I think the assumption a) cannot be safely taken for granted.

Also, the std:time::Instant struct is super opaque and I've found no way to simply construct it from a custom time value of offset from the UNIX epoch etc.

A possible workaround for the two issues above could be to provide own (that is implemented here) rppal::time::Instant type. It would internally store the low-level interrupt event timestamps as u64-typed number of nanoseconds, and could supports similar convenience methods from std::time::Instant, like duration_since as well as arithmetic traits (Add, Sub etc) involving Instant and Duration types.

Side note: a custom Instant type is something Tokio library did. For different reasons, but just pointing this out here. In my view, the idea of a new type where timestamping info is prevented from being confused of incorrectly mixed with others, may require few lines of code more, but is not as crazy IMO as it may first sound.

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

No branches or pull requests

3 participants