Skip to content
This repository has been archived by the owner on Oct 7, 2021. It is now read-only.

support wait_for_service #137

Merged
merged 6 commits into from
May 28, 2016
Merged

support wait_for_service #137

merged 6 commits into from
May 28, 2016

Conversation

wjwwood
Copy link
Member

@wjwwood wjwwood commented May 20, 2016

Connects to ros2/ros2#215

@wjwwood wjwwood added the in review Waiting for review (Kanban column) label May 20, 2016
@wjwwood
Copy link
Member Author

wjwwood commented May 27, 2016

This is also ready for review. The latest commit 6f65a4e fixed the bug I was struggling with where the publisher and subscription count was always going up and never down.

) noexcept
{
if (!is_available) {
return "argument is_available is null";
Copy link
Contributor

Choose a reason for hiding this comment

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

should be return nullptr with setting error message?

Copy link
Member Author

Choose a reason for hiding this comment

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

@tfoote
Copy link
Contributor

tfoote commented May 28, 2016

lgtm, there are several error catches with fprintfs such that a user cannot deal with them. Most are clean up errors. Presumeably if we cannot deallocate things there's something bad happening. I don't think we have a way to expose that sort of failure at the moment though.

@wjwwood
Copy link
Member Author

wjwwood commented May 28, 2016

Yeah, I try to only use fprintf when returning an error is not possible, like in the "fail" goto label of a create function.

@wjwwood wjwwood merged commit 64015bc into master May 28, 2016
@wjwwood wjwwood removed the in review Waiting for review (Kanban column) label May 28, 2016
@wjwwood wjwwood deleted the wait_for_service branch May 28, 2016 22:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants