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

feat(vda5050_msgs): mapping rules for Action & InstantActions #36

Open
wants to merge 1 commit into
base: ros2
Choose a base branch
from

Conversation

moritz-boeker-node-robotics

Adding mapping rules so that Action msgs and InstantActions msgs are bridged properly from ROS2 to ROS1.

@ipa-pfs
Copy link
Contributor

ipa-pfs commented Sep 20, 2023

Do we need it for both ways meaning we need an equivalent for the ros1 branch?

@moritz-boeker-node-robotics
Copy link
Author

Do we need it for both ways meaning we need an equivalent for the ros1 branch?

I think not, because in https://github.com/ros2/ros1_bridge/blob/master/doc/index.rst#how-can-i-install-mapping-rule-files it says the mapping rules must only be added to the ROS2 package.

@@ -0,0 +1,27 @@
-
ros1_package_name: 'vda5050_msgs'
Copy link
Contributor

Choose a reason for hiding this comment

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

these are incomplete though aren't they?
if we want to add those we probably would want it for all the messages types in here.
The advantage of https://github.com/ipa320/vda5050_msgs/tree/ros2/vda5050_serializer/vda5050_serializer is that it's not hardcoded and therefore actually more resistent to change in the message definitions here. I'm not sure if something like that is supported in ros1 ros2 bridge, otherwise I probably would keep these mapping files in your specific repo or at least autogenerate them in ci in here.

Choose a reason for hiding this comment

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

I just added those message types that I wanted the ros1_bridge to be able to bridge for a specific customer. This is the way that Kemal suggested me to do. I have no clue how the ros1_bridge could use our serializer to automatically translate dromedarCase messages to snake_case messages. IMO it is necessary to either add the mapping rules to

  • our ros1_bridge repo (which might not be the nicest thing)
  • or here
    AFAIK it's rather complicated to add the mapping rules directly to our customer repo given the fact that we cannot easily add the mapping rules simply as an argument but need to rebuild the ros1_bridge docker image image with the mapping rules.

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