ubi-agni / tactile_toolbox

software to handle tactile sensors in ROS
9 stars 5 forks source link

Export includes to allow other plugins to instantiate tactile displays #27

Closed guihomework closed 2 years ago

guihomework commented 2 years ago

This PR permits external plugins to re-use classes or plugin displays of rviz_tactile_plugins

Similar exports are done in the main rviz https://github.com/ros-visualization/rviz/blob/noetic-devel/CMakeLists.txt#L184

guihomework commented 2 years ago

the catkin_lint issue is relative to header files not being installed.


I do not manage to find a solution to install the header files correctly so that the compilation works for both devel and install space.

I tried to mimic what rviz does [here](https://github.com/ros-visualization/rviz/blob/6a234b4404c5ee71051f0b338ab8d5d2d931c27b/src/rviz/CMakeLists.txt) and [here](https://github.com/ros-visualization/rviz/blob/6a234b4404c5ee71051f0b338ab8d5d2d931c27b/src/rviz/default_plugin/CMakeLists.txt)
including the GereneratedExportHeader, as found [in this other branch](https://github.com/robomantic/tactile_toolbox/tree/export_includes_install) but I seem to not understand the concept fully yet

some mechanism must be set up so that in devel space one can still use the `#include <pkg_name/*.h>` even if files are under the folder src/ directly but I cannot figure out how.

@rhaschke Do you have any pointer or suggestion how to solve this install problem ?
rhaschke commented 2 years ago

The standard way to solve this issue is to move installed include files into folder include/<package>/ and declare INCLUDE_DIRS include. rviz has all its source and include files in src/rviz. Hence INCLUDE_DIRS src works there because includes are found both, from devel and install space, via rviz/*.h. You are also missing the actual install of files.

guihomework commented 2 years ago

Thanks for answering quickly. I indeed saw RViz uses source and header in src) but rviz_tactile_plugins uses the same way, and I am talking about the latter.

You are also missing the actual install of files.

I think in a second attempt in the new branch as pointed in my comment, I had the installation of the actual files, but they will necessarily go in include/rviz_tactile_plugins/*.h (we don't want them loose in include directly) and so I would still face the problem, because in devel they are found without package name. I will retry to solve this issue, but to be sure I understood, where do you expect the header files of rviz_tactile_plugins to be installed ?

rhaschke commented 2 years ago

The key difference between rviz and rviz_tactile_plugins is that the former has all files in src/<package>, while the latter has them in src only.

Where do you expect the header files of rviz_tactile_plugins to be installed ?

They should go - as you said - to include/rviz_tactile_plugins/*.h.

rhaschke commented 2 years ago

I just noticed that this repo's GHA fail for unrelated reasons. These issues are fixed now again. Note that I force-pushed melodic-devel. Please rebase your branch.

guihomework commented 2 years ago

@rhaschke now I understood that you were fine with changing the place of header files in the standard location in the source not only in the installed location. I will provide a new PR for that.