Skip to content

Conversation

@sloretz
Copy link
Contributor

@sloretz sloretz commented Oct 13, 2017

Replace sleep calls with calls to wait_for_service(). Labeled in-progress because it requires ros2/rclpy#127 to be merged first.

@sloretz sloretz added the in progress Actively being worked on (Kanban column) label Oct 13, 2017
@sloretz sloretz self-assigned this Oct 13, 2017
# wait for connection to be established
# (no wait for service in Python yet)
time.sleep(1)
cli.wait_for_service()
Copy link
Member

Choose a reason for hiding this comment

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

Now that I've approved it 😄, I noticed that this will block forever. If you look at the C++ version it will do wait_for_service for say a second, then loop with a message like "still waiting for service..."

Not required, just an idea to exercise the timeout and return value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sloretz sloretz force-pushed the rclpy_wait_for_service branch from 021f949 to 8ee5ca3 Compare November 29, 2017 23:16
@sloretz sloretz added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Nov 29, 2017
@sloretz
Copy link
Contributor Author

sloretz commented Nov 29, 2017

Rebased onto master. I think this can be merged now that ros2/rclpy#161 has been merged. I'll merge it before the end of the day to give time for objections.

Copy link
Member

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

lgtm
I'll leave @dhood the opportunity to comment as I don't know if there is pending work to convert these to use the loggers rather than printing to console like it's been done for the demos

Copy link
Member

@dhood dhood left a comment

Choose a reason for hiding this comment

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

logging can be updated in another pass, it will touch other lines/files

@mikaelarguedas mikaelarguedas merged commit ba02775 into master Nov 30, 2017
@mikaelarguedas mikaelarguedas deleted the rclpy_wait_for_service branch November 30, 2017 18:41
@mikaelarguedas mikaelarguedas removed the in review Waiting for review (Kanban column) label Nov 30, 2017
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.

5 participants