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

Add function for getting a types fully qualified name #514

Merged
merged 3 commits into from Aug 17, 2020

Conversation

jacobperron
Copy link
Member

As opposed the type name 'foo::msg::Bar', the new function let's us get the ROS namespaced name 'foo/msg/Bar'.

This seems generally useful, and would simplify logic in ros2/rviz#591 for example.

I'm open to changing the name of the new function if there are better suggestions.

As opposed the type name 'foo::msg::Bar', the new function let's us get the ROS namespaced name 'foo/msg/Bar'.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron jacobperron added the enhancement New feature or request label Aug 13, 2020
@jacobperron jacobperron self-assigned this Aug 13, 2020
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

Makes sense to me, but should we also have just name? It just seems odd to "reserve" that term by using "fully qualified name" if we're not gonna have it. And if we intend to have it, now would be a good time to add it I guess.

@jacobperron
Copy link
Member Author

Good suggestion. What do you think about adding three new functions instead?

  • name() (e.g. returns Bar)
  • namespace() (e.g. returns foo/msg)
  • name_with_namespace() (e.g. returns foo/msg/Bar)

@wjwwood
Copy link
Member

wjwwood commented Aug 17, 2020

I like fully_qualified_name, and name seems ok, but we can't do just namespace I think due to keyword collisions. Maybe msg_namespace or namespace_ or we could prefix all with type, e.g. type_name, type_namespace, and type_fully_qualified name/fully_qualified_type_name.

The other option is just to have name return the full name and let users split it themselves.

I don't have a strong opinion one way or the other.

@jacobperron
Copy link
Member Author

The other option is just to have name return the full name and let users split it themselves.

I like this option. I think it's unlikely a user only wants the last part of the name (or just the namespace), and in this case they can do the splitting themselves.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron
Copy link
Member Author

@wjwwood I've renamed the function to name(), see 0e79466.

@jacobperron
Copy link
Member Author

Testing rosidl_generator_cpp, rosidl_runtime_cpp, and test_msgs:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@jacobperron jacobperron merged commit d098918 into master Aug 17, 2020
@jacobperron jacobperron deleted the jacob/msg_traits_fqn branch August 17, 2020 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants