common_interfaces
common_interfaces copied to clipboard
ros2
Modern CMake Targets
I would appreciate modern cmake targets, so things like ament_target_dependencies don't have to be used anymore (see https://github.com/ament/ament_cmake/issues/292). It seems modern cmake targets are produced already by many interfaces such as std_msgs. For instance: std_msgs::std_msgs__rosidl_typesupport_cpp
But I saw some interfaces define a nicer wrapper library for this: For instance https://github.com/ros2/common_interfaces/blob/rolling/sensor_msgs/CMakeLists.txt#L67 (Although an alias target should be added as well for this).
So I wonder what the general strategy is. It seems a little bit inconsistent. I personally don't like these generated names such as std_msgs::std_msgs__rosidl_typesupport_cpp. So I would be in favor of having wrapper targets. On the other hand, it seems like this could be implemented directly in rosidl_generate_interfaces and similar functions?
Yes, the goal is definitely to have std_msgs::std_msgs as a target. We attempted to do this at one point, but ran into some issues. @sloretz do you happen to remember what problems we had?
Anyway, if you have some ideas on how to make this nicer, we'd be happy to consider them.
There is a variable with all the CMake targets: ${<package name>_TARGETS}, ex: ${std_msgs_TARGETS}. There isn't a wrapper target at the moment. I don't remember what problems were had with it.
Anyway, if you have some ideas on how to make this nicer, we'd be happy to consider them.
Well I mean the approach that was taken seems like a first improvement. The PR for this was https://github.com/ros2/common_interfaces/pull/178#issuecomment-1076516076. Which also mentions a few issues, but seems like they all were resolved. For my own library I use similar code currently.
But yeah in general the ideal solution would be to have a method in ros which does all the heavy work. Or if the existing methods like rosidl_generate_interfaces would just define the target itself.
The new method rosidl_get_typesupport_target already helps a little bit, but I don't see a reason why some ros method shouldn't do the target definition as well.
