robotology / cer

Contains SW specific to the R1 robots
GNU General Public License v2.0
10 stars 13 forks source link

[WIP] Handle ROS config files - Proposal #75

Closed barbalberto closed 4 years ago

barbalberto commented 7 years ago

This PR is a proposed way to handle the installation of ROS config files contained in a yarp repo. NOTE: right now this PR handle launch and config files used ONLY BY ROS, and not by yarp applications.

Those files are in config and launch folder placed in the root od the repo. This installer works in 4 different ways, depending on the PC's set-up

1) The 'package' (repo) is compiled and installed using plain CMake (the YARP way) and no special env var is set. In this case the ROS config files will be installed using YARP standard paths, i.e. by mean of ${{PROJECT_NAME}_DATA_INSTALL_DIR} In order to have ROS find this repo, the environment variable ROS_PACKAGE_PATH must be extended to include this folder.

2) The 'package' (repo) is compiled and installed using plain CMake (the YARP way) and the ROS_WS env var is set. (the name ROS_WS can be changed to whatever we like) This approach works like the iCub-Common package, so all packages which need to install ROS config file, will place them in the folder pointed to by ROS_WS In order to have ROS find this repo at runtime, the environment variable ROS_PACKAGE_PATH must be extended to include this folder. This approach mimic the behaviour of iCubContribCommon, but uses an env var to achieve the goal. Obviously the behaviour can be changed to use the same approach used by contrib if we like it more. I used an env var because it was faster to implement.

3) The package is compiled using catkin ROS build system: this opens up 3 senarios:

There is some logic in the macro, so that the cmake is aware of what's going on and choose what to do depending on the environment it is compiled into.

For a package to use this installer it has to do:

## Install ROS config file in a ROS-like way
include(ROSconfigHelpers)
set_ros_config_install_prefix()
ROS_config_install()

It could be expanded to install other folders other then config and launch. File package.xml is copied as well.

traversaro commented 7 years ago

Great! This is also related to https://github.com/robotology-playground/icub-model-generator/issues/28 .

Some comments:

  1. I would leave to the users the freedom to specify the location of where to install the ROS configuration files. ${{PROJECT_NAME}_DATA_INSTALL_DIR} is defined only if yarp_configure_external_installation(${PROJECT_NAME}) has been called. However, we have a lot of projects for which yarp_configure_external_installation is called with an argument different from ${PROJECT_NAME}, and the use of ${PROJECT_NAME} is neither enforced in the code or recommended in the documentation. In a lot of recent repository, for example, I started to just install data in ${YARP_DATA_INSTALL_DIR} (see the workflow explained in https://github.com/robotology/yarp/pull/1111/files) to avoid having an infinite list of YARP_DATA_DIRS in the superbuild configurations.

  2. I do not understand the use case. iCubContrib works by modifying the CMAKE_INSTALL_PREFIX, and if the CMAKE_INSTALL_PREFIX is modified all the variables used in 1. are modified accordingly. Furthermore, ROS_WS is a bit misleading because all the enviromental variables that start with ROS_ are tipically defined/used by ROS itself (see http://wiki.ros.org/ROS/EnvironmentVariables).

  3. & 4. I do not understand the use case. First, this is a repository that is using a plain CMake build system, so it will always use cmake as the "build system", even if it uses catkin or ament as "build tools", right? For the difference between "build system" and "build tools", see https://github.com/ros2/design/pull/115 , in particular https://github.com/ros2/design/blob/e99e8a966b1b12202717c1d4c15d72ea5ec8b685/articles/101_build_tool.md#build-tool-vs-build-system .

traversaro commented 7 years ago

See https://github.com/gerkey/ros1_external_use#installing-for-use-by-tools-like-roslaunch for a "official" way of deal with ROS packages without using Catkin .

barbalberto commented 7 years ago

Thanks. I can add the dummy .catkin file if it is useful. In my test it worked also without it, but should not harm having it. I can remove the case 2, as discussed.

traversaro commented 7 years ago

Yes, apparently in ROS documentation [1] they claim that the ROS_PACKAGE_PATH is not necessary anymore after the introduction of Catkin (basically the tools should search in the directory indicated by CMAKE_PREFIX_PATH), but I was not able to get to work rospack without the ROS_PACKAGE_PATH, so I suspect that ROS_PACKAGE_PATH is always necessary.

[1] : http://wiki.ros.org/ROS/EnvironmentVariables#ROS_PACKAGE_PATH