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

Fetch cartographer_ros to release-1.0 tag and add ROS2 API #25

Closed
wants to merge 59 commits into from

Conversation

routiful
Copy link

@routiful routiful commented Feb 1, 2019

I fetched cartographer_ros code to release-1.0 tag in googlecartographer/cartographer_ros repo.
I have been tested this turtlebot3 and gazebo.

If you want to test this, please following below link
http://emanual.robotis.com/docs/en/platform/turtlebot3/ros2/#ros2

clalancette and others added 30 commits May 17, 2018 20:49
Remove boost from cartographer_ros.

It was only being used for boost::array, which we can easily
replace with std::array.

Signed-off-by: Chris Lalancette <[email protected]>

Port cartographer_ros_msgs over to ROS2.

The messages and services now build.

Signed-off-by: Chris Lalancette <[email protected]>

Port cartographer_ros over to ROS2.

Signed-off-by: Chris Lalancette <[email protected]>

Always get the tf transform right "now".

We are currently having problems with rounding in tf, which
causes it to always miss the transform.  To work around this
for now, just always ask for the latest transform.  This
isn't technically correct, but will probably be good enough
for Turtlebot where our transforms are static.

Signed-off-by: Chris Lalancette <[email protected]>

Disable testing in cartographer_ros.

This is to avoid an error when building without google-mock
installed.

Signed-off-by: Chris Lalancette <[email protected]>

Add in configuration files for turtlebot 2d and 3d.

Signed-off-by: Chris Lalancette <[email protected]>

Make sure to ignore building the cartographer_ros_docs.

We don't currently have a configuration that will allow these
to build.

Signed-off-by: Chris Lalancette <[email protected]>

Make sure to include trajectory_builder.lua for 3d.

Signed-off-by: Chris Lalancette <[email protected]>

Update to use tf2::durationFromSec (ros2#3)

Internal storage has been converted from <ns, double> to <ns, int>

use CMAKE_X_STANDARD and check compiler rather than platform

Use standard urdfdom-headers rosdep key. (ros2#6)

We're shadowing this key for r2b2 since upstream is out of date but we should
use the rosdep key so we can eventually target upstream.

ros2/robot_model#1

Revert "Use standard urdfdom-headers rosdep key. (ros2#6)" (ros2#7)

This reverts commit 0dbd82a.

Move executables to libexec directory (ros2#9)

* Move into lib directory.

Signed-off-by: Chris Lalancette <[email protected]>

* move turtlebot config files to turtlebot_cartographer package

Make cartographer_ros compile again with recent changes to rclcpp::Time (ros2#12)

* Make cartographer_ros compile again with recent changes to rclcpp::Time

Signed-off-by: Chris Lalancette <[email protected]>

* Fix nits from review.

Signed-off-by: Chris Lalancette <[email protected]>

* Fix up alphabetical order.

Signed-off-by: Chris Lalancette <[email protected]>

* Store the timesource and the clock in the node.

This way we don't re-initialize it every time.

Signed-off-by: Chris Lalancette <[email protected]>

migrate to format 3 and declare membership to rosidl_interface_packages (ros2#13)

Update for rclcpp namespace removals (ros2#14)

* Remove ::publisher:: namespace

* Remove subscription:: namespace

* Remove service:: namespace

* Remove timer:: namespace

* Remove node:: namespace

Update the maintainer to me. (ros2#15)

* Update the maintainer to me.

* Updates from review.

* fixup

2.0.0
@mkhansenbot
Copy link

I have tested this and it is working on my TB3, can we get a code review and merge?

@mjcarroll
Copy link
Member

@mkhansen-intel So @clalancette and I discussed this a few weeks ago, and we thought that the best course of action would be to try to get ROS2 cartographer improvements merged back upstream in the original repositories. Unfortunately, I had the wrong date for the Cartographer open house where I was going to propose this, so I haven't made much progress with the upstream devs.

@mkhansenbot
Copy link

I agree that merging upstream would be ideal. I've filed an issue upstream in that regard also here: cartographer-project#1162

In the meantime, can we merge it here so at least other ROS2 users have a central place to get the code and we don't have to keep forking?

@mkhansenbot
Copy link

@mjcarroll - So in order to get this merged upstream, an RFC needs to be filed for the cartographer project. (see this issue: cartographer-project#1162) I don't have the time to do this, would you be able to do it so we can get these changes upstreamed in time for Dashing? Otherwise, can you merge this PR so at least we can release the code from this repo for Dashing?

@mjcarroll
Copy link
Member

I don't think that I will have time to get this through the RFC process before Dashing, so I'm okay with merging this in that case. Full transparency, I haven't really reviewed or evaluated this code, as it is quite a substantial diff, so I'm going to have to take your word that it's working.

Based on the linked discussion, it seems that introducing a cartographer_ros2 package would be the best approach long term, so that may be a good thing to start pursuing in the future.

@mkhansenbot
Copy link

I think the same issues exist with creating a cartographer_ros2 package as with this repo. It's a fork that has to be maintained by someone, and changes in the ROS version may not be ported to the ROS2 version. At least if they're branches in the same repo, they might have a chance of staying synched. But that's just my 2 cents, as I can't volunteer to maintain it in either case.

@routiful
Copy link
Author

routiful commented Apr 12, 2019

@mjcarroll @mkhansen-intel
Interesting conversation :)
This code have already tested with TurtleBot3 platform even Gazebo.
But I am agree that someone maintain this code to reliable performance.
I'm ready to improve migrated cartographer_ros for ROS2.

@robotpilot robotpilot mentioned this pull request Apr 29, 2019
20 tasks
@mkhansenbot
Copy link

@mjcarroll and @clalancette, can we get this merged?

@dirk-thomas
Copy link
Member

@mjcarroll @clalancette Friendly ping.

@clalancette
Copy link

Closing out as it was superceded by #26

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.

9 participants