Added rosidl_runtime c and cpp dependencies

[ahcorde]

This PR is related to the changes introduced in this PR ros2/rosidl#442. The full process can be followed here ros2/rosidl#443

Signed-off-by: ahcorde [email protected]

[ahcorde]

• rmw_cyclonedds_cpp is using rosidl_generator_c headers which are located here rosidl_runtime_c
  • headers are used here:
    • https://github.com/ahcorde/rmw_cyclonedds/blob/ahcorde/rosidl_runtime/rmw_cyclonedds_cpp/include/rmw_cyclonedds_cpp/TypeSupport_impl.hpp#L33
    • https://github.com/ahcorde/rmw_cyclonedds/blob/ahcorde/rosidl_runtime/rmw_cyclonedds_cpp/include/rmw_cyclonedds_cpp/TypeSupport.hpp#L19
• there is no need to export rosidl_generator_cpp, headers of this package are not used
• rosidl_runtime_c is a <depend> , it needs to build, build_export and exec

[eboasson]

It sounds and looks entirely reasonable to me, but not having any detailed knowledge of what each package is supposed to do, I am unsure what that statement is worth in terms of an approval …

[wjwwood]

lgtm

@eboasson basically we're breaking the rosidl_generator_c into two parts, one is the actual code generation which uses Python and has template files for messages and stuff, and the other part is the runtime which is like headers and libraries in support of the generated messages which are needed at runtime.

[eboasson]

@wjwwood would this change in dependencies mean it will only build on Foxy after merging this PR? In that case dashing should get its own branch, just like eloquent got in November.

It's easy enough for me to make a branch for this repo, but I don't know what things I need to update to make sure Dashing builds stay away from the master branch. Perhaps you (or someone else who knows) can take care of that before this gets merged?

[rotu]

It seems like this is a great use case for a conditional dependency so as not to break the build.

[dirk-thomas]

would this change in dependencies mean it will only build on Foxy after merging this PR?

Yes, this patch will only work for Foxy since the runtime package doesn't exist in Eloqent or Dashing.

In that case dashing should get its own branch, just like eloquent got in November.

We leave this decision up to you. If you do want to continue targeting multiple distros from the master branch we would ask you though to make update all contributed pull requests to be compatible with older distros.

It's easy enough for me to make a branch for this repo, but I don't know what things I need to update to make sure Dashing builds stay away from the master branch. Perhaps you (or someone else who knows) can take care of that before this gets merged?

Instead of letting others do this I will summarize the places where the branch is stored so you can update them if you decide to do so:

• The ros2.repos file:
  • The master branch targets the upcoming ROS distro (atm Foxy): https://github.com/ros2/ros2/blob/master/ros2.repos
  • There are branches for each already released ROS distro, e.g. eloquent: https://github.com/ros2/ros2/blob/eloquent/ros2.repos
• The rosdistro repository:
  • The doc and source entry are referencing the branch name, e.g. for eloquent: https://github.com/ros/rosdistro/blob/5b9013042d665bd0a24578b702f3dd5fc3379fd5/eloquent/distribution.yaml#L1898-L1916
  • There are sibling directories for other ROS distros, e.g. dashing. For Foxy the entries haven't been added yet but that will likely happen in the next weeks.
• The gbp repository:
  • The tracks.yaml file contains the devel_branch for each track: https://github.com/ros2-gbp/rmw_cyclonedds-release/blob/master/tracks.yaml

[rotu]

I'm so sorry I did not mean to close this PR

[rotu]

The changes you made are on branch pr-110. Please pull.

[ahcorde]

I can't see the branch pr-110

[rotu]

https://github.com/ros2/rmw_cyclonedds/tree/pr-110