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

Update our time driver for upcoming embassy changes #2701

Draft
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

bugadani
Copy link
Contributor

@bugadani bugadani commented Dec 6, 2024

These changes build on embassy-rs/embassy#3593 #2684 and embassy-rs/embassy#3656

We now manage our own timer queues, which means that

  • if there are multiple queues (with integrated-timers), we can use a priority-limited mutex. This means that higher priority executors don't get blocked by lower priority ones (except for Priority1 which may be blocked by the thread mode executor, but that's it).
  • if there is only a single queue (integrated or generic), we use a max priority lock (same thing as above, but instead of a critical section we end up with a spin lock).
  • the embassy-side changes mean processing the timer queue has a much lower overhead when using integrated timers.

Configuration-wise, users have the options to select between:

  • N integrated queues (requires N alarms)
  • 1 integrated queue
  • generic timer queue (for RTIC, for example) - this is the default if integrated-timers is not set, but probably should be inverted at this point, or at least default to single-queue.
  • The size of the generic queue is configurable using ESP_HAL_EMBASSY_GENERIC_QUEUE_SIZE

@bugadani bugadani force-pushed the time-driver-redo branch 7 times, most recently from 1f73ebd to 3690f3b Compare December 8, 2024 18:28
@bugadani bugadani force-pushed the time-driver-redo branch 2 times, most recently from 64f0c56 to bf8e06f Compare December 17, 2024 09:14
@bugadani bugadani added the skip-changelog No changelog modification needed label Dec 17, 2024
@bugadani bugadani force-pushed the time-driver-redo branch 2 times, most recently from 32ccce2 to fa124d8 Compare December 17, 2024 20:16
println!("cargo:rustc-check-cfg=cfg(single_queue)");
println!("cargo:rustc-check-cfg=cfg(generic_timers)");

match &crate_config["ESP_HAL_EMBASSY_TIMER_QUEUE"] {
Copy link
Contributor Author

@bugadani bugadani Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to figure out passing config options to tests for this. We should also figure out testing multiple feature sets, or configs in this case - otherwise we won't be exhaustive in our async tests. Although only the integrated timer is our implementation, it would be interesting to test the time driver's ability to provide multiple alarms, too.

@bugadani bugadani changed the title Experimental embassy changes Update our time driver for upcoming embassy changes Dec 17, 2024
@bugadani bugadani force-pushed the time-driver-redo branch 2 times, most recently from 6b2a8ef to 25ddfba Compare December 18, 2024 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog No changelog modification needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant