Skip to content
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

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

Merged
merged 5 commits into from
Aug 30, 2016

Conversation

wkentaro
Copy link
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
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
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
Contributor Author

Choose a reason for hiding this comment

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

Copy link
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
Contributor Author

Fixed.

@dirk-thomas
Copy link
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
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
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
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
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
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
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
Contributor Author

Fixed.

@dirk-thomas
Copy link
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
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.

None yet

2 participants