-
Notifications
You must be signed in to change notification settings - Fork 60
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
Feature: generate ROS2 compatible type naming #22
Feature: generate ROS2 compatible type naming #22
Conversation
…DL parser ROS2 scoped name to set the typename
@TSC21 Please, update the IDL-Parser submodule and I'll approve the PR. |
Ah yes of course. Will do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, please update the submodule IDL parser to the new generated commit after the squash (generated when merging the PR)
src/main/java/com/eprosima/fastrtps/idl/templates/RTPSPubSubTypeSource.stg
Show resolved
Hide resolved
Please, update IDL parser submodule to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@LuisGP @richiware thank you both. This will definitely improve the experience for people using standalone Fast-RTPS apps linked in the same DDS domain as ROS2 nodes. Before you do a release, I would like to add as a feature an option to install the script and binary system wide (in |
@MiguelCompany this message is for you :P. |
@MiguelCompany @LuisGP @richiware are you planning to document this? |
Fixes #19. @LuisGP thank you for the tips. Please check if this solution makes sense to you:
-typeros2
option setsm_type_ros2
totrue
, which is defined in the Context;isGenerateTypesROS2
is true, meaningm_type_ros2
, then it's used a scoped name generated from the IDL parser, which has the required valid structure to be used in the ROS2 context.Requires eProsima/IDL-Parser#39 to be merged first.