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

Reformat the visibility file adding and dependency exporting #7

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

t0ny-peng
Copy link
Contributor

@t0ny-peng t0ny-peng commented Oct 13, 2021

  1. The way how visibility file is included was changed from using -include (affecting all compiling units) to adding to each PB headers.
  2. Some dependencies exporting was reformatted so that the correct one fromrosidl_generator_c and rosidl_runtime_c will be exported, depending on the ROS2 version.

Tested over Foxy.

@brakmic-aleksandar Would you please review this PR? One thing I'm not so sure is this line. Seems that this is the only way to use msg from the same package while not installing them(e.g., for test purpose). It's a weird use case but actually exists somewhere.

https://github.com/t0ny-peng/rosidl_typesupport_protobuf/blob/41c0b20c7087e642a35b2f0d6ec6902e9c89c75f/rosidl_typesupport_protobuf_cpp/cmake/rosidl_typesupport_protobuf_cpp_generate_interfaces.cmake#L142 #

@brakmic-aleksandar
Copy link
Contributor

@t0ny-peng Regarding your question, we should leave that line in as is so we can cover that use case, we can always change it in the future if we find a better way to do it.

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.

2 participants