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

add flag to ignore local publications #17

Merged
merged 1 commit into from
Jun 23, 2015

Conversation

dirk-thomas
Copy link
Member

Required for ros2/ros1_bridge#1

Please review.

@dirk-thomas dirk-thomas added the in review Waiting for review (Kanban column) label Jun 23, 2015
@dirk-thomas dirk-thomas self-assigned this Jun 23, 2015
@tfoote
Copy link

tfoote commented Jun 23, 2015

+1, though this feels very application specific and I wonder if this is something we should consider going around the standard API for the bridge instead of extending the standard API

@wjwwood
Copy link
Member

wjwwood commented Jun 23, 2015

+1. I think this will be a necessary feature to expose to the user. However, I agree that it maybe better exposed to the users through a more generic "filtering" scheme or even provide the necessary tools to the user so the subscriber can do the filtering (caller id).

This particular option name is also somewhat vague, for instance does it mean block communications to the same node/participant? or does that include other nodes in the same process?

This pokes at another issue we have to nail down, is there a direct one to one mapping between ROS nodes and DDS participants. It may be necessary to break with that design to accomplish our intra-process goals.

Do you plan to open a pull request for rmw_opensplice, rcl, and rclc?

At some point I think we'll need to setup a settings struct for each of the primitives. Using a similar patter to DDS, where you get the "default", mutate it, and then pass it to the create_* function. Fancier things could be done in rclcpp like using a named parameter idiom.

@dirk-thomas
Copy link
Member Author

I am currently working on the rmw_opensplice PR. I don't plan to touch rcl / rclc since the bridge does not use them and there is no test coverage for them atm.

Update: the PR on rmw_opensplice is ros2/rmw_opensplice#44

@wjwwood
Copy link
Member

wjwwood commented Jun 23, 2015

Tested or not we shouldn't let them diverge. I'll do the pull requests for them.

@wjwwood
Copy link
Member

wjwwood commented Jun 23, 2015

dirk-thomas added a commit that referenced this pull request Jun 23, 2015
add flag to ignore local publications
@dirk-thomas dirk-thomas merged commit 0f0d7a2 into master Jun 23, 2015
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Jun 23, 2015
@dirk-thomas dirk-thomas deleted the ignore_local_publications branch June 23, 2015 23:57
mauropasse pushed a commit to mauropasse/rmw that referenced this pull request Mar 2, 2021
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.

3 participants