Add sensor_msgs_library target and install headers to include/${PROJECT_NAME}

[sloretz]

This adds a modern CMake target sensor_msgs_library usable downstream as sensor_msgs::sensor_msgs_library and installs headers to a unique include directory as part of ros2/ros2#1150

It might depend on ros2/rosidl#662, I'm not sure.

[clalancette]

Looks good to me.

[sloretz]

CI (build: --packages-above-and-dependencies sensor_msgs test: --packages-select sensor_msgs)

• Linux
• Linux-aarch64
• Windows

[pablogs9]

This makes micro-ROS type support fails because we are not using rosidl_generator_cpp at all, and we rely on rosidl_generator_c. Details here

Is there any possibility of making this modification optional? Or maybe selecting rosidl_generator_c or rosidl_generator_cpp depending on which one is found?

I found that just replacing

rosidl_get_typesupport_target(cpp_target "${PROJECT_NAME}" "rosidl_generator_cpp")

with

rosidl_get_typesupport_target(cpp_target "${PROJECT_NAME}" "rosidl_generator_c")

it works as expected.

Should we assume that cpp generator and/or cpp typesupport are mandatory?

Any idea?

[pablogs9]

Another question @sloretz @clalancette , why this has been done only in this sensor_msgs package and not in others?

[sloretz]

Another question @sloretz @clalancette , why this has been done only in this sensor_msgs package and not in others?

sensor_msgs is the only package that also provides non-generated C++ headers to do things with the generated c++ code.

Is there any possibility of making this modification optional?

Yes, I think so. The headers in the sensor_msgs::sensor_msgs_library target need C++ code to be generated for them, but the target and header installation could be conditioned on that generator being present. I recommend opening a PR that checks if(TARGET ${cpp_target}) and only installs headers and creates the sensor_msgs::sensor_msgs_library target if that check passes.

I found that just replacing [...] with [...]

That probably worked because downstream packages using those are depending on ${sensor_msgs_TARGETS} which also includes the generated c++ target. I wouldn't expect that to work if someone depended on sensor_msgs::sensor_msgs_library only.

[pablogs9]

sensor_msgs is the only package that also provides non-generated C++ headers to do things with the generated c++ code.

As far as I understood .msg and other type definition formats provided by ROS 2 are intended to declare a certain type independently of the programming language. Just like IDL. In that sense, you are assuming that this .msg and .srv are going to have a C++ type support. IMHO this should be divided into a sensor_msgs package with type definition and other sensor_msgs_utils_cpp with the non-generated C++ code.

Yes, I think so. The headers in the sensor_msgs::sensor_msgs_library target need C++ code to be generated for them, but the target and header installation could be conditioned on that generator being present. I recommend opening a PR that checks if(TARGET ${cpp_target}) and only installs headers and creates the sensor_msgs::sensor_msgs_library target if that check passes.

I do not understand how I can do if(TARGET ${cpp_target}) if cpp_target is set in rosidl_get_typesupport_target(cpp_target "${PROJECT_NAME}" "rosidl_generator_cpp") and rosidl_get_typesupport_target result in a fatal error if rosidl_generator_cpp is not available.

In any case, I have proposed an approach here: #183

[RFRIEDM-Trimble]

Is there a reason to not also add an alias library? It's supported in the version of cmake used here.
https://cmake.org/cmake/help/v3.5/command/add_library.html#alias-libraries

You said in the PR you added the alias, but I cannot see where it was created in the diff.