Skip to content

[rospy_tutorials] Add example of periodical publishing with rospy.Timer#34

Merged
dirk-thomas merged 5 commits intoros:kinetic-develfrom
wkentaro:talker-timer
Aug 30, 2016
Merged

[rospy_tutorials] Add example of periodical publishing with rospy.Timer#34
dirk-thomas merged 5 commits intoros:kinetic-develfrom
wkentaro:talker-timer

Conversation

@wkentaro
Copy link
Copy Markdown
Contributor

As suggested in ros/ros_comm#864 (comment)

try:
rospy.init_node('talker', anonymous=True)
pub = rospy.Publisher('chatter', String, queue_size=10)
timer = rospy.Timer(rospy.Duration(1. / 10), publish_callback) # 10Hz
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A Timer expects a Time as the first argument. While passing a Duration works it is less intuitive imo.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My comment was obviously wrong. Passing Time doesn't work and the test fails with a stacktrace. But somehow the PR job didn't detect the problem.

I will look into the job. Once it reports the failure correctly we can make it pass by removing the second commit (which I asked for).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The wrong docstring will be fixed via ros/ros_comm#878.

@wkentaro
Copy link
Copy Markdown
Contributor Author

Fixed.

@dirk-thomas
Copy link
Copy Markdown
Member

@ros-pull-request-builder retest this please

@@ -0,0 +1,4 @@
<launch>
<node name="talker" pkg="rospy_tutorials" type="talker_timer.py" />
<test test-name="talker_listener_test" pkg="rospy_tutorials" type="talker_listener_test.py" />
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The test-name is the same as for another test. Please add a suffix (_with_timer) to it in order to make it unique.

@wkentaro
Copy link
Copy Markdown
Contributor Author

Fixed.

<test test-name="talker_listener_test" pkg="rospy_tutorials" type="talker_listener_test.py" />

<test test-name="talker_listener_test_with_timer"
name="talker_listener_test_with_timer"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This shouldn't have a name attribute.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So, should the name be specified when the node name is used in testing to resolve name of private topic name or param name?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't know - you might want to try it.

But this test doesn't have anything like that.

@dirk-thomas
Copy link
Copy Markdown
Member

Great, now the PR job detected the failing test correctly. Can you please revert the second commit now and use Duration instead of Time again.

@wkentaro
Copy link
Copy Markdown
Contributor Author

Fixed.

@dirk-thomas
Copy link
Copy Markdown
Member

Thank you for iterating on this and fixing the documentation.

@wkentaro wkentaro deleted the talker-timer branch August 30, 2016 18:11
@wkentaro
Copy link
Copy Markdown
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants