search

ros / urdfdom

URDF parser
http://ros.org/wiki/urdf
Other
97 stars 131 forks source link

urdfdom version 2 updates #157

Closed clalancette closed 2 years ago

clalancette commented 3 years ago

Ever since we merged the ROS 2 version of urdfdom into this repository, there has been something of a split-brain here. This repository is really meant to be ROS-agnostic, but the stuff on the ros2 branch here clearly was not. This is a problem for downstream packagers because the latest tag on the master branch is 1.0.4, but the latest tag overall on this repository is on the ros2 branch (2.3.3). One bug that mentions this is https://github.com/ros/urdfdom/issues/156 , but there have been others.

This PR is an attempt to solve these problems. What this PR does is to rebase the ros2 branch onto master, making a few changes along the way. By getting the changes from the ros2 branch onto master, it resolves the above ambiguity and also brings some of the latest development onto the master branch. The changes are necessary so that the code from the ros2 branch is not ROS 2 specific.

The changes on this branch can be summarized in the following:

  1. The only change to the API is the addition of the parseCamera, parseRay, parseSensor, and parseModelState to the public API. Since those functions are already implemented internally, here is no ABI change here at all. In some sense this change gets us further away from our goal of removing TinyXML, but I didn't want to do too many things in this PR, so I left it as-is.
  2. Use modern CMake wherever possible. This includes getting rid of global include directives (include_directories), global link directives (link_directories), custom setting of the CXX_STANDARD, etc. It also includes creating modern exported interfaces for the libraries in this package. One place where we could not use modern CMake was finding the include directories for urdfdom_headers. That's because the modern interfaces for urdfdom_headers is not available in the version released in Ubuntu 20.04, so we can't rely on it.
  3. Make finding the "vendor" packages QUIET, which essentially makes them optional. That means that when building standalone (for Debian packaging) or in ROS 1, it will just use the system versions of the packages. On ROS 2 (and more specifically ROS 2 Windows), it will find the vendor packages.
  4. Add a package.xml into the source repository. For ROS 1 we typically relied on overlaying the package.xml at bloom time, but there is no real reason to do that. Systems that do understand package.xml can take easier advantage of it in the source, and systems that don't understand it will just ignore it.

I've tested this PR by building it standalone (mkdir build ; cd build ; cmake .. ; make -j10), building it with catkin_make_isolated in a ROS 1 overlay, and by building it with colcon in a ROS 2 workspace (on Linux and Windows), and it seems to work in all of those circumstances.

If we merge this, this will obsolete #146 .

rhaschke commented 3 years ago

Thanks @clalancette for this attempt to cleanup history and sync ROS1 and ROS2 branches. But, I don't understand yet, why you prefer rebasing over merging? Do you also plan to rebase all release tags on the ROS2 branch then? If not, this means that we keep both branches, the old/current ROS2 branch and the rebased one. I suggest to merge and apply required modifications afterwards. This would cleanly resemble the history of both branches.

What about the release strategy? As far as I remember, ROS1 relies on the system package released via the Debian release process (which was cumbersome). However, ROS2 releases the package via the ROS release process, right? Do you plan to change the release process for ROS1 as well?

clalancette commented 2 years ago

Superseded by #158, where we use the merge strategy as suggested by @rhaschke .