Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix libfranka linking #37

Conversation

christian-rauch
Copy link
Contributor

Compiling the targets franka_example_controllers_parameters and franka_robot_state_broadcaster_parameters fails due to the missing linking to libfranka. This PR fixes the issue by linking the libfranka target Franka::Franka. This also makes sure that the include paths or correctly imported.

@christian-rauch
Copy link
Contributor Author

ping @BarisYazici

@BarisYazici
Copy link
Collaborator

BarisYazici commented Sep 26, 2023

not sure why you are having the linking issue in the franka_example_controllers. Everything seems to be fine in the CI build stage. What's exactly the error?

peterdavidfagan pushed a commit to peterdavidfagan/franka_ros2 that referenced this pull request Oct 1, 2023
…dapt-ros-2-humble-implementation to humble

* commit '98f75fa0ba7b6721a65cced1b85db3677e5f2f22':
  modified Dockerfile to checkout certain libfranka version
  refactor: removed unused variables, updated rt-kernel-warning and minor changes
  changed franka_harware::Robot to use franka::Robot instead of franka::RobotInterface
  updated and reactivated franka_hardware unittests
  changed franka_hardware Robot class to use external control loop functionality in libfranka
  added missing dependency to franka_semantic_components
@christian-rauch
Copy link
Contributor Author

not sure why you are having the linking issue in the franka_example_controllers. Everything seems to be fine in the CI build stage.

I am building libfranka as a CMake package of a colcon workspace. Does the CI pipeline use the manually built Debian package?

What's exactly the error?

On v0.1.0 I get the error:

[workspace]/install/franka_semantic_components/include/franka_semantic_components/franka_robot_state.hpp:21:10: fatal error: franka/robot_state.h: No such file or directory
   21 | #include "franka/robot_state.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~

However, with the current libfranka (0.12.1) and the ROS node (0.1.4) I don't see the error any more.

@christian-rauch
Copy link
Contributor Author

I also see that v0.1.0 is the last humble version compatible with libfranka 0.9.2. @BarisYazici Could you create a branch for the 0.1.0 version so that I can rebase the PR there?

@christian-rauch
Copy link
Contributor Author

@BarisYazici What do you think about having a branch for the "legacy" Franka robots that use libfranka 0.9?

@PhDittmann
Copy link

This happens, because libfranka does not contain a package.xml, which determines the right compile order in colcon. In this case editing CMakeLists.txt won't help. It stays a race condition. You can solve this by running:

colcon build --packages-up-to libfranka
colcon build

@christian-rauch
Copy link
Contributor Author

This happens, because libfranka does not contain a package.xml, which determines the right compile order in colcon. In this case editing CMakeLists.txt won't help. It stays a race condition.

I don't see how this is a race condition. If header files cannot be found, it means that they are not installed or their include dir has not been added, which can in newer CMake projects be done via target_link_libraries. If this were a race condition, the find_package would fail before.

Note, while this PR is against the humble branch, it is meant to be based on a "legacy" branch from the v0.1.0 tag for the old 0.9.2 compatible firmware. This issue might have been resolved on the humble branch already, but we still need to use older hardware.

@PhDittmann
Copy link

You're absolutely right. WITHIN a CMakeLists.txt, it ensures finding and compiling dependencies in the right order, but colcon decides the order and parallel execution of multiple CMakeLists.txt. I.e. on my machine it's 4 jobs (CMakeLists.txt) at the same time.

frankalib has no package.xml, which could fix the compile order in colcon. Because of that and compiling frankalib needs quite some time is the reason, why other packages are compiled in parallel and fail, if frankalib is not being build, yet. Thats's the race condition.

I tested the latest humble and your PR yesterday, that's why I promise you calling colcon build --packages-up-to libfranka && colcon build would avoid this race condition or a package.xml must be added to frankalib and all packages using it must be edited in franka_ros2.

@christian-rauch
Copy link
Contributor Author

frankalib has no package.xml, which could fix the compile order in colcon. Because of that and compiling frankalib needs quite some time is the reason, why other packages are compiled in parallel and fail, if frankalib is not being build, yet. Thats's the race condition.

You are correct about the order of package compilation with colcon etc. But the issue I am facing is different from this. It would also appear with libfranka already installed in the workspace.

Since I created this PR, the humble branch advanced and the issue I reported might not be valid any longer. However, I am still experiencing linking issues with libfranka which I have fixed on a different branch, which is only compatible with the "legacy" code based on the v0.1.0 tag. I created #49 for asking for a "legacy" branch that I can send a PR to. I simply kept this PR open so I can rebase it onto the "legacy" branch once this has been sorted out.

@PhDittmann
Copy link

PhDittmann commented Aug 19, 2024

I reproduced the error using libfranka 0.9.2 and franka_ros2 v0.1.0, but found another source of the error. Building via colcon drops the Franka dependency in ~/ros2_ws/install/franka_semantic_components/share/franka_semantic_components/cmake/ament_cmake_export_dependencies-extras.cmake. That's why the includes are not transient anymore. It was fixed in here (v0.1.3) on Aug 17, 2023.

 # generated from ament_cmake_export_dependencies/cmake/ament_cmake_export_dependencies-extras.cmake.in

set(_exported_dependencies "franka_hardware;franka_msgs;hardware_interface;rclcpp_lifecycle")

A minimal change is adding Franka to ament_export_dependencies in franka_semantic_components/CMakeLists.txt. Of course, adding Franka to the THIS_PACKAGE_INCLUDE_DEPENDS variable and cleaning up hard-coded references would be much nicer.

diff --git a/franka_semantic_components/CMakeLists.txt b/franka_semantic_components/CMakeLists.txt
index 9bc3bef..7e1ce8c 100644
--- a/franka_semantic_components/CMakeLists.txt
+++ b/franka_semantic_components/CMakeLists.txt
@@ -85,5 +85,5 @@ endif()
 
 ament_export_include_directories(include)
 ament_export_libraries(franka_semantic_components)
-ament_export_dependencies(${THIS_PACKAGE_INCLUDE_DEPENDS})
+ament_export_dependencies(Franka ${THIS_PACKAGE_INCLUDE_DEPENDS})
 ament_package()

These are the commands I used to compile:

mkdir -p ~/ros2_ws/src
cd ~/ros2_ws
git clone --recursive https://github.com/frankaemika/libfranka.git -b 0.9.2 src/libfranka
git clone https://github.com/frankaemika/franka_ros2.git -b v0.1.0 src/franka_ros2
colcon build --packages-up-to libfranka
source install/setup.bash
# modify ~/ros2_ws/src/franka_ros2/franka_semantic_components/CMakeLists.txt as mentioned
colcon build

Using libfranka as stand-alone build works without code modification, because the library is set up properly:

mkdir ~/workspace
cd ~/workspace
git clone --recursive https://github.com/frankaemika/libfranka.git -b 0.9.2
cd libfranka
cmake -B build -S .
cd build
make -j$(nproc)
make install
mkdir -p ~/ros2_ws/src
cd ~/ros2_ws
git clone https://github.com/frankaemika/franka_ros2.git -b v0.1.0 src/franka_ros2
colcon build

@christian-rauch
Copy link
Contributor Author

That's why the includes are not transient anymore. It was fixed in here (v0.1.3) on Aug 17, 2023.

A minimal change is adding Franka to ament_export_dependencies in franka_semantic_components/CMakeLists.txt. Of course, adding Franka to the THIS_PACKAGE_INCLUDE_DEPENDS variable and cleaning up hard-coded references would be much nicer.

This is exactly what we did here: https://github.com/boschresearch/franka_ros2/tree/fer1/fix_libfranka_linking with a365e3e.

There are linking issues with the v0.1.0 tag, which is the last one supporting the legacy EOL "Franka Emika Robot". Therefore, I would like to rebase the linked branch onto a new upstream branch from v0.1.0 in order to get these fixes merged for users of the legacy hardware.

@christian-rauch christian-rauch deleted the fix_libfranka_linking branch September 2, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants