search

ros2 / ros1_bridge

ROS 2 package that provides bidirectional communication between ROS 1 and ROS 2
Apache License 2.0
425 stars 275 forks source link

Mapping Rules Limited to ROS2 Package #364

Closed jaelrod closed 2 years ago

jaelrod commented 2 years ago

Feature request

Allow mapping rules for ROS2 messages defined in other packages.

Feature description

Open-source ROS2 package in use has message B defined. Internal ROS1 package has message A defined. ROS1 package and ROS2 package have different names, establishing mapping of A<->B requires custom mapping rules, ROS2 package is checked out as a git submodule, and branching of this package is strongly discouraged. Having a way to apply mapping rules to ROS2 packages not owned by the current workspace, unlocking that functionality for message types in git submodules, etc., would be helpful in complex bridging scenarios.

Implementation considerations

I see that there is an explicit check for this in the python code used to generate the C++ bridge code template specializations, so possibly there is a reason I'm not seeing for imposing this limitation on the use of mapping rules. However, applying custom mapping rules to a third-party open-source package locally and building the bridge to establish the custom pairings bridge works as desired. I realize that removing this check entirely results in the possibility for duplicate and/or conflicting mapping rules, so perhaps rather than removing it entirely, a more verbose qualification in the configuration might better provide the additional flexibility. https://github.com/ros2/ros1_bridge/blob/b5e4931cefce6cc3351b31b04b0399a6d3bacac4/ros1_bridge/__init__.py#L386

methylDragon commented 2 years ago

Could you spin up a minimal example for the custom mappings you're looking for (in a repo that contains the src directory of a workspace) so this feature could be iterated on?

I think exposing an additional (optional) configuration member in the .yaml file would be good, but the code generator uses a parsed version of that in the ament resource index that I'm not too clear on at this juncture.

Edit: Ah, got it, I'll likely release a PR editing the docs and add a new configuration rule. A minimal example would still be good to help check if the feature works though.

jaelrod commented 2 years ago

Thanks for the reply! Glad my description ended up making sense.

I can look into setting up a minimal example, but it could also be a good idea to have that set up in ros1_bridge so that tests can validate the behavior. You can just include a git submodule that defines a message, such as this apriltag detection message, define its ROS1 counterpart in a package somewhere, and throw in a mapping rules yaml trying to map it to the ROS1 message: https://github.com/NVIDIA-AI-IOT/isaac_ros_apriltag/blob/main/isaac_ros_apriltag_interfaces/msg/AprilTagDetectionArray.msg

methylDragon commented 2 years ago

Thanks for the reply! Glad my description ended up making sense.

I can look into setting up a minimal example, but it could also be a good idea to have that set up in ros1_bridge so that tests can validate the behavior. You can just include a git submodule that defines a message, such as this apriltag detection message, define its ROS1 counterpart in a package somewhere, and throw in a mapping rules yaml trying to map it to the ROS1 message: https://github.com/NVIDIA-AI-IOT/isaac_ros_apriltag/blob/main/isaac_ros_apriltag_interfaces/msg/AprilTagDetectionArray.msg

Alternatively, you could test it out on your own setup with the PR :eyes: https://github.com/ros2/ros1_bridge/pull/367 I'll independently look into setting up tests.

Edit: Probably not, it'll need more in-depth integration test setup, and we already don't test for static bridges...