-
Notifications
You must be signed in to change notification settings - Fork 996
runtime (scheduler): generalize scheduler beyond timers #1006
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some stylistic comments, although I'm still not understanding what is going on here (see #1010 (comment)).
| } | ||
| const pollInterval = 256 | ||
|
|
||
| // Run the scheduler until all tasks have finished. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not believe this sentence is true anymore with this PR: the scheduler runs until the main task has finished.
|
|
||
| // poll checks for any events that are ready and pushes them onto the runqueue. | ||
| // It returns true if any tasks were ready. | ||
| func poll() bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name of this function is rather broad. Perhaps something like pollForEvents or checkForNewTasks? Or fillRunqueue?
| } | ||
|
|
||
| // wait sleeps until any tasks are awoken by external events. | ||
| func wait() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for this function. I think something like waitForEvents would be clearer.
(I'm assuming this will wait for the next interrupt soon, with wfe?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, #1040 adds an implementation for atsamd21 which directly uses wfe.
|
Closing in favor of #1142 |
No description provided.