Add support for timers on reset callback #1979
Merged
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
|
CI including rcl PR
|
|
Any updates here? |
|
mauropasse
force-pushed
the
mauro/add-timer-on-reset-callback
branch
from
052aa82
to
0673586
Compare
|
The CI fails because ros2/rcl#995 is needed. |
|
The main issue here is the Windows CI that is failing. |
|
When I looked at the Win CI output it seemed that something related to the parameters interface in tf2_ros was failing. This seems to be kind of unrelated. Can we maybe try to tun the CI against some documentation PR to see if this is an issue with the current master of tf2? |
mauropasse
force-pushed
the
mauro/add-timer-on-reset-callback
branch
from
0673586
to
d5f1949
Compare
mauropasse
requested review from
ivanpauno,
hidmic and
wjwwood
as code owners
fujitatomoya
approved these changes
Nov 28, 2022
|
CI:
|
|
There seems to be another Windows build issue: |
|
Also seems unrelated imo... |
|
Seems like a random CI network issue, running it again:
|
The Windows CI still seems to have Network issues: Relevant Jenkins Traceback |
Yes, I will run it again in some hours, after checking with the infra team if there are any windows CI issues. |
|
|
it seems we are still having problems with the windows agents ... |
|
Thanks for everyone's patience on these infrastructure issues. We use low-bid spot instances in order to keep costs down but these cause us trouble every year around the shopping season. The ROS infra team is currently working to add some on-demand capacity to allow jobs to continue. In order to run that transition I am unfortunately going to abort |
|
Alright. Because |
|
The run still fails :/, but it at least failed relatively early with an infrastructure related error. |
|
|
|
CI
|
|
CI after fixing rcl PR
|
|
CI
|
|
Finally 🎉 |
|
@fujitatomoya @ivanpauno @hidmic it looks like all comments have been addressed; can you have a final look before we merge this PR and its related rcl PR? |
|
I am good to go with this, but requesting another review from maintainers. CC: @ivanpauno @hidmic @wjwwood be advised that this must be merged with ros2/rcl#995 |
added 2 commits
December 16, 2022 11:16
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
mauropasse
force-pushed
the
mauro/add-timer-on-reset-callback
branch
from
d5f1949
to
43dee48
Compare
|
Rebased after #2059 got merged |
|
Ping; it looks like the rcl PR (ros2/rcl#995) has been approved, can we get an additional review here? |
this protects against the callback being modified while we are resetting the timer Signed-off-by: Alberto Soragna <alberto.soragna@gmail.com>
|
Hi, any update on this? |
tonynajjar
pushed a commit
to pixel-robotics/rclcpp
that referenced
this pull request
Apr 3, 2023
Signed-off-by: Mauro Passerino <mpasserino@irobot.com> Co-authored-by: Alberto Soragna <alberto.soragna@gmail.com>
tonynajjar
pushed a commit
to pixel-robotics/rclcpp
that referenced
this pull request
Apr 3, 2023
Signed-off-by: Mauro Passerino <mpasserino@irobot.com> Co-authored-by: Alberto Soragna <alberto.soragna@gmail.com>
alsora
added a commit
to irobot-ros/rclcpp
that referenced
this pull request
Apr 29, 2023
Signed-off-by: Mauro Passerino <mpasserino@irobot.com> Co-authored-by: Alberto Soragna <alberto.soragna@gmail.com>
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.
Based on ros2/rcl#995, this PR adds support for the "on reset" callback for timers.
The issue which originated this need, is to wake up an events based executor when a timer is reset (which could also be caused by a time jump).
@alsora