-
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
Add function to set the initial pose from RVIZ for pure localization mode #1284
base: master
Are you sure you want to change the base?
Add function to set the initial pose from RVIZ for pure localization mode #1284
Conversation
I got the following error while I built your code
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I can see that this code is useful for your type of experiments, I don't think this code serves the general use case as it makes a lot of assumptions... For example, a pose sent on the /initialpose
topic is not necessarily pure 2D and 3D users probably don't always want the z-component to be automatically set.
But I fear that initial pose handling is something that almost always requires some customization, so I don't think we will find an implementation that is generally useful so it can be integrated into this repository... especially one that supports both 2D and 3D.
P.S.: But maybe it makes sense to help people with implementing their custom initial pose tool by providing a minimal example, e.g. in the documentation?
Because a minimal initial pose handler can be implemented with a few lines of Python or C++ code using the ROS services /get_trajectory_states
, /finish_trajectory
and /start_trajectory
.
"Filename of a pbstream to draw a map from."); | ||
|
||
namespace { | ||
int current_trajectory_id_ = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a hack, and there are numerous possible situations where this would be wrong.
A proper implementation would be to check the trajectory states for an active trajectory. The ROS service for that is /get_trajectory_states
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your advice. I have revised based on your comment (f0b35a4)
FLAGS_configuration_directory, FLAGS_configuration_basename); | ||
map_builder_ = absl::make_unique<cartographer::mapping::MapBuilder>( | ||
node_options.map_builder_options); | ||
map_builder_->LoadState(&reader, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you have a map builder? It's not used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
map build is used to get the frozen trajectory information:
https://github.com/googlecartographer/cartographer_ros/blob/5b09b08daf6a6f1f51296f44e11f8c1f5ba48d40/cartographer_ros/cartographer_ros/set_initpose_main.cc#L109
The trajectory information is required here, since the initial pose of the new trajectory should be provided w.r.t the reference trajectory (i.e. the frozen trajectory). Is it right?
|
||
// find the best submap, which is closest to the initial pose | ||
// and assign the height (i.e. z value) of this submap to the initial pose | ||
if (delta_pos.length() < min_dist) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oof... this looks like it could have funny side effects in buildings with multiple floors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed this part (5b09b08)
…he rosserical "/get_trajectory_states"
@MichaelGrupp Thank you for your review. I agree that my original implementation is specialized for the 2.5D case. So I improve the code according to your comments. But, about following comment, I want to discuss further:
Besides these three services, I think the information about the reference trajectory (i.e. the frozen trajectory) is also necessary, since the initial pose should be provided w.r.t. the reference trajectory; otherwise, users should covert the transformation by themselves (but common robotics users are tend to use simple gui like rviz): BTW, calling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tongtybj |
Maybe it's more elegant to do the rviz initial pose feature 'outside' Cartographer via a simple Python script? At least that's how I did it :-) #1652 |
As mentioned in the closed PR #637, manually "tuning" of the robot initial pose from GUI (e.g. Rviz) is a very important function for home service robot, which is supported by the ROS default localization method AMCL.
I hope this PR can facilitate the development of this RFC.
To test my commit, please try following commands
Use the icon

2D Pose Estimation
in Rviz to set the initial pose as shown in following image.Play the rosbag:
The whole sequence can be found in this video, and you can also try the 3D mode in this same way.
We have confirmed the 3D performance with our original data, please check this video.
Limitations: the multi-floor map is not supported, since we can not assign the height (i.e. z value) from Rviz.