-
Notifications
You must be signed in to change notification settings - Fork 164
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
ros2 topic pub
starts publishing right away.
#626
ros2 topic pub
starts publishing right away.
#626
Conversation
@clalancette @ivanpauno what do you think? i'd like to hear your opinion. |
I think we should punt on this for now; the issue isn't that important and I don't think we should change it for Galactic. I'll add this to H-Turtle board instead. |
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 change looks good and I tested that it worked on my machine.
I think maybe the true solution is to change the timer behavior in rcl
, but this might be an easier way to make ros2 topic pub
perform how the user expects.
I don't think that's a good idea. |
yeah i am with @ivanpauno on this. application can have more control for use cases. |
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
c0bba6d
to
5e807e6
Compare
I believe that rebase required to include #616, retry CI: |
@ros-pull-request-builder retest this please |
@fujitatomoya could you confirm if the Rpr failure is unrelated or not? Besides that failure, this seems to be ready to be merged. |
@ivanpauno thanks! https://build.ros2.org/job/Rpr__ros2cli__ubuntu_focal_amd64/223/testReport/ros2topic.ros2topic.test/test_cli/test_cli/ is not related to the fix. it is failure of (Just FYI, https://build.ros2.org/job/Rpr__ros2cli__ubuntu_focal_amd64/223/testReport/ros2topic.ros2topic.test/test_cli/test_cli/ does have the expected output which is |
Ah, I know the problem
ros2/rclpy#722, that change wasn't released into rolling when the Rpr builder last run (and the tests here were already updated). It should pass now: @ros-pull-request-builder retest this please. |
@ivanpauno thanks for the support! |
address #624
Signed-off-by: Tomoya Fujita Tomoya.Fujita@sony.com