search

ros2 / rosidl_typesupport

Packages which provide the typesupport for ROS messages and services
Apache License 2.0
13 stars 34 forks source link

Decouple idl generation pipeline from rmw implementations #50

Closed ivanpauno closed 4 years ago

ivanpauno commented 4 years ago

Feature request

Feature description

Currently, rosidl_generate_interfaces from rosidl_cmake can't be called before the rmw implementations are built. Decoupling idl generation from the rmw implementations will allow to use the same message abstractions at rmw level for implementing https://github.com/ros2/design/pull/250.

Implementation considerations

Currently, rosidl_typesupport_c and rosidl_typesupport_cpp have a build dependency on rmw_implementation, which depends on the available implementations (see 1 2). I first believed that they depend on rmw_implementation, because the typesupports are being registered there (e.g.: rmw_fastrtps_cpp), but I later realize that that resource isn't needed in rmw_typesupport_* packages nor in rosidl_cmake. If that the case, we could simply delete that dependency.

For using the idl pipeline generation (calling rosidl_generate_interfaces macro), you may want to use the default generators by adding a build tool dependency on rosidl_default_generators, which depends on the rmw_typesupport_* and also in rosidl_generator_py. The latest, also depends on the rmw_implementation package, because it is using the previously commented resource registered by the rmw implementations. I think that we could avoid using that resource, and directly query this other. As I don't need the python generated file, I will only add a dependency on the packages I need instead of using rosidl_default_generators.

ivanpauno commented 4 years ago

@dirk-thomas Can you check if my description is correct?

dirk-thomas commented 4 years ago

I first believed that they depend on rmw_implementation, because the typesupports are being registered there (e.g.: rmw_fastrtps_cpp), but I later realize that that resource isn't needed in rmw_typesupport_* packages nor in rosidl_cmake.

The RMW implementations register their typesupports with this: https://github.com/ros2/rmw/blob/9f2e181306c66ac0f09674086a108d4657611766/rmw/cmake/register_rmw_implementation.cmake#L65

And the rosidl_typesupport_c/cpp packages query that information (https://github.com/ros2/rosidl_typesupport/blob/38eb801f1f856a503676bb79875786b3a3b6d92d/rosidl_typesupport_cpp/cmake/rosidl_typesupport_cpp_generate_interfaces.cmake#L65) to determine how many typesupports for a specific language exist since it generates different code / logic depending on if there is a single typesupport or multiple available.

Therefore I doubt that dependency can simply be eliminated.

ivanpauno commented 4 years ago

I first believed that they depend on rmw_implementation, because the typesupports are being registered there (e.g.: rmw_fastrtps_cpp), but I later realize that that resource isn't needed in rmw_typesupport_* packages nor in rosidl_cmake.

The RMW implementations register their typesupports with this: https://github.com/ros2/rmw/blob/9f2e181306c66ac0f09674086a108d4657611766/rmw/cmake/register_rmw_implementation.cmake#L65

And the rosidl_typesupport_c/cpp packages query that information (

https://github.com/ros2/rosidl_typesupport/blob/38eb801f1f856a503676bb79875786b3a3b6d92d/rosidl_typesupport_cpp/cmake/rosidl_typesupport_cpp_generate_interfaces.cmake#L65

) to determine how many typesupports for a specific language exist since it generates different code / logic depending on if there is a single typesupport or multiple available. Therefore I doubt that dependency can simply be eliminated.

Yes, I ran into that problem after trying to build the interfaces before the rmw implementations. What is the rationale behing checking that the available typesupports are used by an rmw implementation? Why not delete all this checking and generate interfaces for all the available typesupports?

dirk-thomas commented 4 years ago

What is the rationale behing checking that the available typesupports are used by an rmw implementation?

If you only have a single typesupport you avoid the overhead of the indirection by directly linking against it.

ivanpauno commented 4 years ago

Closing, as I've already found a way of solving the problem. See:

dirk-thomas commented 4 years ago

I will reopen this since the previous work around has reached its limits with recent changes. The goal described by this ticket is what is imo the correct approach to solve ros2/rmw_dds_common#8.

dirk-thomas commented 4 years ago

Packaging: Build Status

ivanpauno commented 4 years ago

I've cancelled both linux jobs, as there are a lot of failures in the macos and Windows ones. It seems to be related with the change in rosidl_python.

dirk-thomas commented 4 years ago

The cpplint / uncrustify warnings were resolved by https://github.com/ros2/rmw_fastrtps/pull/364/commits/ff60121400229b8dae4d2d1536f66afcff63eb76.

The flake8 warnings are unrelated and targeted by ros2/rclpy#539.