Basic Timer implementation #440
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Added cancel behavior
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
* Added timer_period_ns * Adds Timer::is_ready(). Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com> * Added comments to avoid warnings * Added the getter for rcl_clock_t --------- Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com> Co-authored-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
* Added timer_period_ns * Adds Timer::is_ready(). Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com> * Added comments to avoid warnings * Added the getter for rcl_clock_t * WIP add create_timer to node * Cleaning some stuff --------- Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com> Co-authored-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
|
In reference to your discussion points, I think that having timers use |
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
- Adds documentation. - Removes the no-callback constructor. - Removes vertical space. Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
Signed-off-by: Agustin Alba Chicar <ag.albachicar@gmail.com>
|
For visibility, I've opened agalbachicar#5 which targets this PR and addresses most of the feedback that I expect the working group will have for this PR. I recommend that others hold off on reviewing this until agalbachicar#5 gets settled because any reviews in the meantime will likely become outdated soon. @agalbachicar separately from the PR that I opened, you'll need to merge the latest |
|
And also here, thanks a lot for such a feedback, @mxgrey ! I'll review it with my peers and get back as soon as possible. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is a basic Timer implementation. It is heavily based on what other client libraries do, i.e. use RCL bindings and provide a safe API against the bindings.
Details
It is by no means a final version but it is a good first approach to something close to the final version. We will happily change everything you tell us to change :)
It is important to look at the
TimerCallbacktype trait definition.We tried to keep mutable and non-mutable functions within
Timerbut then when integrating it againstWaitSet,NodeandExecutorwe had to make it all "non-mutable".This has been done in collaboration (peer programming) with @tulku , @JesusSilvaUtrera, @jballoffet 🎊
How to test it?
We wrote an end to end example, we will soon add it so it can be show how to do it.
We introduced some unit tests, we can certainly extend them as soon as we define the final API.
Notes
We observed some non-trivial delays in the callback execution. Certainly the overhead between the bindings and what we are doing is non-negligible. We would appreciate some input here.
Identified discussion points
Durationtypes?Clock::with_source()makes the test inTimercrash with SIGSEGV. See the comment and test in timer.rs.