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

Fix parameter bridge for topic if ros1 and ros2 type have a different name #177

Merged
merged 2 commits into from
Jun 10, 2019

Conversation

cyrilleberger
Copy link
Contributor

I have set ros1 type to "" since in create_bidirectional_bridge it can find out which type to use for ros1 when only specifying ros2 type.

An alternative fix would be to allow to specify different type for ros1 and ros2 in the parameters.

@tfoote tfoote added the in review Waiting for review (Kanban column) label Mar 21, 2019
…fferent type name)

Signed-off-by: Cyrille Berger <cyrille.berger@liu.se>
@cyrilleberger cyrilleberger force-pushed the fix-different-msg-name branch from 4132759 to 67898df Compare March 21, 2019 09:35
@dirk-thomas
Copy link
Member

The message printed above provides misleading information now. Please update that accordingly.

@dirk-thomas dirk-thomas added in progress Actively being worked on (Kanban column) requires-changes and removed in review Waiting for review (Kanban column) labels Mar 22, 2019
@cyrilleberger
Copy link
Contributor Author

Do you mean the commit message?

@dirk-thomas
Copy link
Member

Sorry, I though I commented on a specific line. Without that context its not obvious. I was referring to the message printed a few lines above the change: line 69-72.

…type

Signed-off-by: Cyrille Berger <cyrille.berger@liu.se>
@cyrilleberger
Copy link
Contributor Author

Is there anything else I need to change?

@dirk-thomas dirk-thomas added in review Waiting for review (Kanban column) and removed requires-changes in progress Actively being worked on (Kanban column) labels Jun 6, 2019
@dirk-thomas
Copy link
Member

dirk-thomas commented Jun 10, 2019

Is there anything else I need to change?

Please comment after pushing additional commits to your PRs. Otherwise nobody gets a notification and it is unlikely someone will review the changes.

The change looks good to me. Thanks for the improvement.

Build Status

@dirk-thomas
Copy link
Member

Build Status

It seems like your fork is outdated which failed the CI build.

Testing a rebased version of your fork - the result looks good: Build Status

@dirk-thomas dirk-thomas merged commit fcdc6e5 into ros2:master Jun 10, 2019
dhananjaysathe pushed a commit to rapyuta-robotics/ros1_bridge that referenced this pull request Aug 22, 2019
… name (ros2#177)

* don't specify ros1 type (it fixes the bridge if ros1 and ros2 have different type name)

Signed-off-by: Cyrille Berger <cyrille.berger@liu.se>

* remove ROS1 type from messages, since that type is guessed from ROS2 type

Signed-off-by: Cyrille Berger <cyrille.berger@liu.se>
Signed-off-by: Dhananjay Sathe <dhananjay.sathe@rapyuta-robotics.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in review Waiting for review (Kanban column)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants