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

Add mock for timers #40

Merged
merged 4 commits into from
Jan 6, 2023
Merged

Add mock for timers #40

merged 4 commits into from
Jan 6, 2023

Conversation

niclashoyer
Copy link
Contributor

@niclashoyer niclashoyer commented Jan 18, 2022

Added a mock for timers using embedded-time with nanosecond precision. A single clock can be used to create multiple timers. The clock can be freely skipped by any time value supported by embedded_time (e.g. microseconds, seconds, nanoseconds, …).

To support sharing a clock and timer between threads I relied on Arc<AtomicU64> for the time base. That also allows each timer to actually own the clock, so that they can be freely shared with any driver implementation.

I added this on top of #39. If wanted, I could also try to backport this.

@niclashoyer
Copy link
Contributor Author

niclashoyer commented Jan 18, 2022

I think embedded_time needs a newer rust version. They don't have a MSRV given, so we need to figure that out somehow.

Edit: 1.51 seems to be right. It is now possible to also set the MSRV as package attribute in Cargo.toml since rust 1.56. This will generate a warning in versions < 1.56, but will not fail, see here.

@dbrgn
Copy link
Owner

dbrgn commented Aug 8, 2022

Sorry for the long delay. I'm quite busy at the moment (as always 😂), so if someone else with timer experience would do a first review, that would already be helpful. (Anyone can do a review, not just maintainers 🙂)

The branch also needs a rebase.

@eldruin
Copy link
Contributor

eldruin commented Aug 8, 2022

In the mean time we have decided to remove timers in embedded-hal 1.0 so I think it would not make much sense to merge this as-is.
Something that would make sense, though, would be to adapt the mock proposed here to the version available in embedded-hal 0.2.7. Pretty much cherry picking commit b8e03b8 on top of master would work as the interface was the same.
From a quick look the mock proposed here seemed fine but I can do a proper review of this afterwards.
One additional thing to think about would be whether to use embedded-time or fugit (or both).

@dbrgn
Copy link
Owner

dbrgn commented Oct 9, 2022

@niclashoyer would you be interested in doing the backport suggested by @eldruin?

@niclashoyer
Copy link
Contributor Author

@dbrgn @eldruin done! I'm currently moving to embedded-hal-async, but I think it still makes sense to backport this.

@dbrgn
Copy link
Owner

dbrgn commented Jan 6, 2023

Sorry for the long delay again.

I rebased your branch against current master and will now review the contribution 🙂

Regarding the added dependencies, I'm thinking about putting the timer feature behind a cargo flag.

@dbrgn dbrgn mentioned this pull request Jan 6, 2023
@dbrgn
Copy link
Owner

dbrgn commented Jan 6, 2023

I pushed two changes:

  • For consistency with other modules, I've renamed SimClock to MockClock and SimTimer to MockTimer.
  • I put the module behind a Cargo feature (enabled by default)

Thanks! 👍🏻

This is required to be able to use dep: syntax in Cargo.toml
@dbrgn dbrgn merged commit bfa658d into dbrgn:master Jan 6, 2023
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

Successfully merging this pull request may close these issues.

3 participants