-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
ROS2 rolling/galactic conversion #1622
base: ros2-dashing
Are you sure you want to change the base?
ROS2 rolling/galactic conversion #1622
Conversation
Friendly ping @MichaelGrupp, can you have a look? We are ready to take care of the rviz part next. |
Apparently the format bot uses a bleeding edge clang-format that uses the new Google style and reformats a bunch of files in every PR. This is an empty commit to trigger this in a separate commit. See llvm-mirror/clang@62e3198 Signed-off-by: Guillaume Doisy <[email protected]>
…ject#1262) Some service handlers need to check if a trajectory is in a valid state for the requested operation. If it's not, they return an error response. `Node::CheckTrajectoryState` allows to avoid code duplication in such situations. Signed-off-by: Guillaume Doisy <[email protected]>
I have added a few lines to the doc to alert users to remove ros abseil-cpp and also to change the rosinstall file if they wish to checkout a different version of cartographer. As discussed in cartographer-project#1208 Signed-off-by: Guillaume Doisy <[email protected]>
…pher-project#1204) This allows to serialize the state also when no bagfile is given, e.g. when the offline node is used to run an optimization and/or trimming configuration on a pbstream. Or simply when one wants to use a custom name directly. This doesn't break compatibility with the previous CLI. Signed-off-by: Guillaume Doisy <[email protected]>
…er-project#1222) This `/trajectory_query` service allows to look up trajectory segments from the pose graph. This can be useful if one wants to get optimized trajectory data from the current SLAM run and listening to live TF data would be too noisy. For example, to stitch buffered point cloud data from a depth sensor based on a recent localization trajectory segment of a robot. Before, the pose graph trajectory nodes were not available except for the trajectory marker topic (only positions, no orientation; inefficient) or after serialization (which is not practical in live operation). Signed-off-by: Guillaume Doisy <[email protected]>
* Simplify start_trajectory. * Ran clang-format. * Make corrections based on the review * Make corrections based on review cartographer-project#2, remove obsolete functions * Remove unnecessary local variables Signed-off-by: Guillaume Doisy <[email protected]>
…cartographer-project#1282) Signed-off-by: Guillaume Doisy <[email protected]>
Indigo and Lunar are both EOL Signed-off-by: Guillaume Doisy <[email protected]>
…t#1278) Seems to have been used in the past, but isn't needed anymore. Signed-off-by: Guillaume Doisy <[email protected]>
…apher-project#1484) Essentially revert cartographer-project#1021 as image seems to be fixed Signed-off-by: Bo Chen <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
Signed-off-by: Guillaume Doisy <[email protected]>
* Fix build status on front page. Changes "googlecartographer" to "cartographer-project" for references to CI and GitHub. Following cartographer-project/cartographer#1693. Signed-off-by: Wolfgang Hess <[email protected]> * Remove Indigo and Lunar Dockerfiles. Signed-off-by: Guillaume Doisy <[email protected]>
cartographer-project/cartographer#1711 removed building Abseil from the Cartographer library. We follow here the same approach as in cartographer CI. An alternative approach which is now possible is adding an Abseil package for catkin and depending on that. Signed-off-by: Wolfgang Hess <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
It has reached end-of-life with the end of LTS on June 30, 2020. Signed-off-by: Wolfgang Hess <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
This follows Cartographer installing libgmock-dev. Signed-off-by: Wolfgang Hess <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
Signed-off-by: Michael Grupp <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
- it has been used only at Google for a quality pipeline - the worker.py is Python 2 and closely tied to Google Cloud, so I don't believe there is much to be reused by someone else nowadays and not worth the effort to port to Python 3. Signed-off-by: Michael Grupp <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
Signed-off-by: Geonhee-LEE <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
…ographer-project#1511) This should solve cartographer-project#1506. - Use osrf/ros-<distro>-desktop as base image for CI The basic ROS base image requires rosdep to install packages with many dependencies, e.g. rviz. Using the desktop base image should speed up CI because it has a good overlap with what we need and only little extra stuff. - Remove apt list only after installing everything. - Don't build protobuf 3 from source on Ubuntu 18 & 20. It should come with rosdep. Only on Ubuntu 16 we need to build manually because it ships with Protobuf 2. - No need to build Ceres from source on Ubuntu 18 & 20. The versions are greater or equal to 1.13, which is the one specified in the source build script. The apt packages should work just fine. The cartographer library is built from source in all cases, which is good to ensure that we always test compatibility with the latest version of it. Signed-off-by: Guillaume Doisy <[email protected]>
…1516) Signed-off-by: Wolfgang Hess <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
…1518) Signed-off-by: Wolfgang Hess <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
Signed-off-by: Wolfgang Hess <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
…rtographer-project#1521) This uses the more modern issue template mechanism of GitHub. Users will be able to choose one of these after clicking "add issue". --- The template for tuning is reworded to reflect that tuning issues are not guaranteed to be handled by maintainers. Signed-off-by: Michael Grupp <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
…oject#1524) Signed-off-by: Michael Grupp <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
Fixes broken links, code blocks and other things that `sphinx-build docs/source docs/out -E` complained about. Signed-off-by: Michael Grupp <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
…1525) Signed-off-by: Wolfgang Hess <[email protected]> Signed-off-by: Guillaume Doisy <[email protected]>
Sure, I'v uploaded all relevant file to google drive. We'v also tested other bags and they all crash at same place with save error message.
Yes, the pipeline works fine in ros1, we use it extensively. |
Signed-off-by: Guillaume Doisy <[email protected]>
Signed-off-by: Guillaume Doisy <[email protected]>
Signed-off-by: Guillaume Doisy <[email protected]>
@hengjiUSTC , thanks for the feedback and the bag, I actually only basically tested the asset writer node before with the
Assuming you have all your files in |
Thanks @doisyg for replying so quickly, it works now. We didn't see any degradation. |
Great! Did you notice significant differences in CPU or completion time ? |
@doisyg We do facing new crash and issue in ROS2 galgactic and still investigating. Sharing our status as reference. The bag is recorded when we are doing on vehicle test
When playing back Analyze: vehicle_model: https://drive.google.com/file/d/1o-_M3eGknuyZQTUhtHy45vU-aQXcb0ne/view?usp=sharing
Not sure if it's appropriate to discuss here, hope our use case could help make cartographer in galactic more mature. |
FYI, issue is fixed. The problem is caused by rslidar. After we change lidar to velodyne, cartographer in galactic works perfectly. |
@doisyg I think we've waited long enough here, so I'm going to suggest that we move on with a fork. I can spend some time getting things together, though as this is a side project for me I can't commit to any particular timeline. The very first question I have is: which version of Once that is done (and building in Rolling), I'll come back here so that we can discuss exactly where this code should live, the tagged version it should be, etc. |
Hi Chris,
The very last commit of the master branch (which is basically the tag 2.0.0 + one commit for CI changes): https://github.com/cartographer-project/cartographer/tree/b8228ee6564f5a7ad0d6d0b9a30516521cff2ee9
Great, once you have a rolling bin, I will be happy to make the changes for it to compile against it (currently it is galactic). |
It's actually easier for us to release tags, so if there are no important changes beyond the 2.0.0 tag, then I'll just release that. |
Great, releasing to tag 2.0.0 will work just fine |
I took a brief look at this today, and we need a couple of fixes to release cartographer. First, we need to get this rosdep key in: ros/rosdistro#32226 The second one we can patch in at release time if need be, but we definitely can't release without the first one. |
Signed-off-by: Guillaume Doisy <[email protected]>
b23ef10
to
c91cac5
Compare
I did not know that abseil would exist in Jammy, that would simplify things! Though, cartographer asks for a specific commit, see https://github.com/cartographer-project/cartographer/blob/105c034577220268cd28a304a185adbec46b729f/scripts/install_abseil.sh
Yes, I can make a PR once the rosdep key is added and that it is confirmed that it works with that version of abseil. Tough it would require, same as for the ros wrapper, a responsive maintainer. |
Also, I just pushed a change in the folder structure of the wrapper to conform more to the classic ros folder organization (and with *.cpp extension instead of *.cc) + cleaner CMakeLists |
I took a bit of time to spin a Jammy VM + Ros2 rolling (testing bin).
|
Thanks for checking in on this. Because of the issues with getting things into upstream, what I'm going to do for now is to continue to use the fork of cartographer at https://github.com/ros2/cartographer. I'll update that to the latest 2.0.0 tag of cartographer, then apply the patch to the package.xml on top. Once I get that done, I'll see if I can do a successful release into Rolling. |
All right, I successfully released cartographer 2.0.0 into Rolling: https://build.ros2.org/job/Rbin_uJ64__cartographer__ubuntu_jammy_amd64__binary/18/ . I've run out of time to work on this today, but the next step would be to take the patches in here, apply them to https://github.com/ros2/cartographer_ros, and do a release there. @doisyg if I'm going to do that, what tagged version of |
Great news ! |
There is already the Before doing that, it would be a good idea to do a build-n-test of your branch of cartographer_ros against the version of cartographer 2.0.0 that I built into Rolling. You should be able to accomplish that by installing an Ubuntu 22.04 container, installing ROS 2 Rolling on it, and then enabling the testing repository (by changing /etc/apt/sources.list.d/ros2.list to contain Once we have confirmation that the cartographer package is good, and you've opened the PR against the |
I actually did it yesterday after you published it in testing, I get the following error at compilation:
|
I think there is some kind of packaging bug with Ceres on Jammy. Regardless, cartographer was adding the include directories for Ceres in a non-supported way, so I've now removed that. |
Yes, compiling and working fine with the last update |
I naively made a PR : ros2#59, but the ros2/cartographer_ros |
Signed-off-by: Guillaume Doisy <[email protected]>
@clalancette any update on your side ? |
I needed an updated ros2 wrapper compiling against rolling/galactic, so here it is. I started from the master branch but took inspiration from https://github.com/ros2/cartographer_ros when needed.
Still a wip but the main nodes are working. Would be nice to target a galactic release.
Converted to this point (28/01/2022):
use_sim_time
to true. Tested with converted legacy museam bagscartographer_ros_msgs
packagecartographer_rviz
packageStill to do but less critical:
Converted launch files and demo bags: