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

When generating service mappings cast pair to set to avoid duplicate pairs #268

Merged
merged 2 commits into from
May 29, 2020
Merged

When generating service mappings cast pair to set to avoid duplicate pairs #268

merged 2 commits into from
May 29, 2020

Conversation

suddrey-qut
Copy link
Contributor

Fixes issue #257

Recreation of PR #259 to do master->master

…rvice name cause redefinition errors

Signed-off-by: Gavin Suddrey <g.suddrey@qut.edu.au>
ros1_bridge/__init__.py Outdated Show resolved Hide resolved
Signed-off-by: Gavin Suddrey <g.suddrey@qut.edu.au>
@dirk-thomas
Copy link
Member

Thanks for the patch.

@dirk-thomas dirk-thomas merged commit 7aaf32f into ros2:master May 29, 2020
@peterpena
Copy link

Should this be backported?

@dirk-thomas
Copy link
Member

dirk-thomas commented Jun 22, 2020

Should this be backported?

@peterpena Into which ROS distro?

@peterpena
Copy link

@dirk-thomas I am not sure what ROS distro it will be. The bug seems to be introduced all the way back to Ardent. What is the protocol to decide the ROS distro?

@dirk-thomas
Copy link
Member

What is the protocol to decide the ROS distro?

Only active distro can be patched (so the earliest would be Dashing). Another criteria is if it is necessary / severe enough. Based on the original issue it seems trivial to avoid this case by not defining a custom mapping which replicates the default behavior. So I would think the need to backport is fairly low.

@peterpena
Copy link

I see. Thanks for the explanation. I felt the need to ask if it needed to be backported, but based on what you said, it seems that it might not be necessary.

hsd-dev pushed a commit to hsd-dev/ros1_bridge that referenced this pull request Nov 17, 2020
…pairs (ros2#268)

* Fixed issue where explicitly mapped packages with the same package/service name cause redefinition errors

Signed-off-by: Gavin Suddrey <g.suddrey@qut.edu.au>

* Removing unneeded whitespace

Signed-off-by: Gavin Suddrey <g.suddrey@qut.edu.au>
Signed-off-by: Harsh Deshpande <harshavardhan.deshpande@ipa.fraunhofer.de>
ErickKramer added a commit to ErickKramer/ros1_bridge that referenced this pull request Jun 22, 2023
I was landing into a redefinition error when using `mapping_rules.yaml`
to bridge ROS actions.

I applied the patch done in ros2#268
for the ROS services to the ROS actions and it solved the redefinition
error.
ErickKramer added a commit to ErickKramer/ros1_bridge that referenced this pull request Jun 28, 2023
This fixes a redefinition error when using `mapping_rules.yaml`
to bridge ROS actions.

I applied the patch done in ros2#268
for the ROS services to the ROS actions and it solved the redefinition
error.

Additional improvements:
- Iterate over both the ros1 and ros2 component directly in the for loop
  by using `zip`.

Signed-off-by: Erick Kramer <e.kramer@rethinkrobotics.com>
ErickKramer added a commit to ErickKramer/ros1_bridge that referenced this pull request Oct 26, 2023
This fixes a redefinition error when using `mapping_rules.yaml`
to bridge ROS actions.

I applied the patch done in ros2#268
for the ROS services to the ROS actions and it solved the redefinition
error.

Additional improvements:
- Iterate over both the ros1 and ros2 component directly in the for loop
  by using `zip`.

Signed-off-by: Erick Kramer <e.kramer@rethinkrobotics.com>
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