Fix urdfdom dependency #2341

marcbone · 2024-07-19T14:02:32Z

The package.xml relies on liburdfdom-dev, which comes from Ubuntu instead of the one that comes from ROS 2. This usually leads to a situation where you have urdfdom installed twice on your system. Once from this package as liburdfdom-dev that is installed in /usr/lib/x86_64-linux-gnu/ and once from other ROS 2 packages on your system that use the urdfdom from ROS 2 (like the urdf package), which is installed in /opt/ros/{ROS_DISTRO}/lib/x86_64-linux-gnu. This will now lead to CMake warnings in other ROS 2 project that try to use urdfdom. It still works, but it might break in the future.

This PR changes the package.xml to rely on the urdfdom package that comes from ROS.

We now install the urdfdom that is provided by ROS instead of the package that comes from ubuntu

jcarpent

@nim65s @wxmerkt Could you provide some feedback on this PR?

nim65s · 2024-07-22T07:53:18Z

I don't understand why ROS would distribute a package which is available upstream. I also don't understand how this change could fix the situation. From what I understand, in cases you have urdfdom both in /usr and /opt/ros, this warning will always be there, right ? To me, the issue is that you have twice the same package installed from different sources.

Also, it seems easier to remove /opt/ros from your configure/build paths than removing /usr: this is not even possible on ubuntu I think.

wxmerkt · 2024-07-23T14:28:33Z

The background to this is that in some cases upstream packages were seriously out of date, had bugs, or incompatible versions were on different versions used by a ROS distro, so packages got vendored in. I believe liburdfdom had the challenge in the past that the system-/upstream-version was quite behind. Some digging shows that this may have been related to issues with locale settings.

The reason why urdfdom is vendored into ROS2 is due to an API break by removing the TinyXML compatibility and upgrading to urdfdom 4 in Rolling and newer, cf. https://discourse.ros.org/t/upcoming-api-break-in-urdfdom/34750

For this PR, we need changes however using conditional tags:

Noetic doesnt have urdfdom
Jazzy is the first one to have urdfdom released at 4.x
Iron has urdfdom at 3.1.x
Humble has urdfdom at 3.0.x

As such, let's make the switch of the tag conditional on ROS_VERSION - if ROS_VERSION = 1, we keep the existing, and if ROS_VERSION = 2, you can switch.

package.xml

Co-authored-by: Wolfgang Merkt <wxmerkt@users.noreply.github.com>

github-actions

👋 Hi,
This is a reminder message to please assign a proper label to this Pull Request.
The possible labels are:

build_collision (build pinocchio with coal support)
build_casadi (build pinoochio with casadi support)
build_autodiff (build pinocchio with cppad support)
build_codegen (build pinocchio with cppadcg support)
build_extra (build pinocchio with extra algorithms)
build_mpfr (build pinocchio with Boost.Multiprecision support)
build_sdf (build pinocchio with sdf parser)
build_accelerate
build_all (build pinocchio with all the options stated above)

Thanks.

## [3.2.0] - 2024-08-27 ### Fixed - Append pinocchio optional libraries into pkg-config file (stack-of-tasks/pinocchio#2322) - Fixed support of DAE meshes with MeshCat (stack-of-tasks/pinocchio#2331) - Fixed pointer casts in urdf parser (stack-of-tasks/pinocchio#2339) - Remove CMake CMP0167 warnings (stack-of-tasks/pinocchio#2347) - Fixed urdfdom in ROS packaging (stack-of-tasks/pinocchio#2341) - Fixed overview-urdf cpp example (stack-of-tasks/pinocchio#2384) - Fixed mjcf model without a base link parsing (stack-of-tasks/pinocchio#2386) - Fixed talos-simulation.py, simulation-contact-dynamics.py and simulation-closed-kinematic-chains.py examples (stack-of-tasks/pinocchio#2392) ### Added - Add getMotionAxis method to helical, prismatic, revolute and ubounded revolute joint (stack-of-tasks/pinocchio#2315) - Add initial compatiblity with coal (coal needs `-DCOAL_BACKWARD_COMPATIBILITY_WITH_HPP_FCL=ON`) (stack-of-tasks/pinocchio#2323) - Add compatibility with jrl-cmakemodules workspace (stack-of-tasks/pinocchio#2333) - Add ``collision_color`` parameter to `MeshcatVisualizer.loadViewerModel` (stack-of-tasks/pinocchio#2350) - Add ``BuildFromMJCF`` function to RobotWrapper (stack-of-tasks/pinocchio#2363) - Add more CasADi examples (stack-of-tasks/pinocchio#2388) ### Removed - Remove deprecated headers related to joint constraints (stack-of-tasks/pinocchio#2382) ### Changed - Use eigenpy to expose `GeometryObject::meshMaterial` variant (stack-of-tasks/pinocchio#2315) - GepettoViewer is no more the default viewer for RobotWrapper (stack-of-tasks/pinocchio#2331) - Modernize python code base with ruff (stack-of-tasks/pinocchio#2367) - Restructure CppAD and CasADi examples (stack-of-tasks/pinocchio#2388) - Enhance and fix CppAD benchmarks outputs (stack-of-tasks/pinocchio#2393) Packaging Changes: - Removed patches af, an, ap: fixed upstream - Updated patches ag, ak

## [3.2.0] - 2024-08-27 ### Fixed - Append pinocchio optional libraries into pkg-config file (stack-of-tasks/pinocchio#2322) - Fixed support of DAE meshes with MeshCat (stack-of-tasks/pinocchio#2331) - Fixed pointer casts in urdf parser (stack-of-tasks/pinocchio#2339) - Remove CMake CMP0167 warnings (stack-of-tasks/pinocchio#2347) - Fixed urdfdom in ROS packaging (stack-of-tasks/pinocchio#2341) - Fixed overview-urdf cpp example (stack-of-tasks/pinocchio#2384) - Fixed mjcf model without a base link parsing (stack-of-tasks/pinocchio#2386) - Fixed talos-simulation.py, simulation-contact-dynamics.py and simulation-closed-kinematic-chains.py examples (stack-of-tasks/pinocchio#2392) ### Added - Add getMotionAxis method to helical, prismatic, revolute and ubounded revolute joint (stack-of-tasks/pinocchio#2315) - Add initial compatiblity with coal (coal needs `-DCOAL_BACKWARD_COMPATIBILITY_WITH_HPP_FCL=ON`) (stack-of-tasks/pinocchio#2323) - Add compatibility with jrl-cmakemodules workspace (stack-of-tasks/pinocchio#2333) - Add ``collision_color`` parameter to `MeshcatVisualizer.loadViewerModel` (stack-of-tasks/pinocchio#2350) - Add ``BuildFromMJCF`` function to RobotWrapper (stack-of-tasks/pinocchio#2363) - Add more CasADi examples (stack-of-tasks/pinocchio#2388) ### Removed - Remove deprecated headers related to joint constraints (stack-of-tasks/pinocchio#2382) ### Changed - Use eigenpy to expose `GeometryObject::meshMaterial` variant (stack-of-tasks/pinocchio#2315) - GepettoViewer is no more the default viewer for RobotWrapper (stack-of-tasks/pinocchio#2331) - Modernize python code base with ruff (stack-of-tasks/pinocchio#2367) - Restructure CppAD and CasADi examples (stack-of-tasks/pinocchio#2388) - Enhance and fix CppAD benchmarks outputs (stack-of-tasks/pinocchio#2393) ## [3.1.0] - 2024-07-04 ### Fixed - Fix `appendModel` when joints after the base are in parallel (stack-of-tasks/pinocchio#2295) - Fix `appendModel` build when called with template arguments different than the ones from `context` (stack-of-tasks/pinocchio#2284) - Fix `TransformRevoleTpl::rotation` and `TransformHelicalTpl::rotation` build (stack-of-tasks/pinocchio#2284) - Fix compilation issue for Boost 1.85 (stack-of-tasks/pinocchio#2255) - Fix python bindings of `contactInverseDynamics` (stack-of-tasks/pinocchio#2263) - Deactivate `BUILD_WITH_LIBPYTHON` when building with PyPy (stack-of-tasks/pinocchio#2274) - Fix Python bindings cross building with `hpp-fcl` (stack-of-tasks/pinocchio#2288) - Fix build issue on Windows when a deprecated header is included (stack-of-tasks/pinocchio#2292) - Fix build issue on Windows when building in Debug mode (stack-of-tasks/pinocchio#2292) - Fix visualization of meshes in meshcat (stack-of-tasks/pinocchio#2294) - Fix Anymal simulation test (stack-of-tasks/pinocchio#2299) - Fix contact derivatives and impulse dynamics tests (stack-of-tasks/pinocchio#2300) - Fix CMake compatibility with old console_bridge version (stack-of-tasks/pinocchio#2312) ### Added - Python unittest for `contactInverseDynamics` function (stack-of-tasks/pinocchio#2263) - Added helper functions to return operation count of CasADi functions. (stack-of-tasks/pinocchio#2275) - C++ and Python unittest for `dIntegrateTransport` to check vector transport and its inverse (stack-of-tasks/pinocchio#2273) - Add kinetic and potential energy regressors (stack-of-tasks/pinocchio#2282) ### Removed - Remove header `list.hpp` include for bindings of model and rnea (stack-of-tasks/pinocchio#2263) Packaging Changes: - Removed patches af, an, ap: fixed upstream - Updated patches ag, ak

marcbone and others added 2 commits July 19, 2024 15:04

fix urdfdom dependency in package.xml

1b534af

We now install the urdfdom that is provided by ROS instead of the package that comes from ubuntu

changelog: sync

8563f9e

jcarpent previously approved these changes Jul 20, 2024

View reviewed changes

jcarpent reviewed Jul 21, 2024

View reviewed changes

wxmerkt reviewed Jul 23, 2024

View reviewed changes

package.xml Outdated Show resolved Hide resolved

jcarpent dismissed their stale review via 5881268 July 24, 2024 06:14

jcarpent and others added 2 commits July 24, 2024 08:14

ros: handle ROS1 and ROS2 context switch for urdfdom

5881268

Co-authored-by: Wolfgang Merkt <wxmerkt@users.noreply.github.com>

Merge branch 'devel' into patch-1

9d6c697

github-actions bot reviewed Jul 24, 2024

View reviewed changes

jcarpent merged commit 205a7bc into stack-of-tasks:devel Jul 24, 2024
20 of 21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix urdfdom dependency #2341

Fix urdfdom dependency #2341

marcbone commented Jul 19, 2024

jcarpent left a comment

nim65s commented Jul 22, 2024

wxmerkt commented Jul 23, 2024

github-actions bot left a comment

Fix urdfdom dependency #2341

Fix urdfdom dependency #2341

Conversation

marcbone commented Jul 19, 2024

jcarpent left a comment

Choose a reason for hiding this comment

nim65s commented Jul 22, 2024

wxmerkt commented Jul 23, 2024

github-actions bot left a comment

Choose a reason for hiding this comment