Navigation /amcl_pose topic should be better named

[rotu]

Currently, the Nav2 stack subscribes to the rover's pose as on the /amcl_pose topic. This is somewhat silly since it doesn't care how the pose is derived - (it need not be from an AMC localization filter). Perhaps a better name would be "world_pose" or "global_pose".

[mkhansenbot]

I think this also overlaps with #595. Besides the robot model, I'm not sure what else subscribes to the /amcl_pose topic and I don't think we should be using the pose at all in that case because it's incompatible with SLAM / Cartographer.

[SteveMacenski]

I staunchly disagree. the amcl_pose topic is published by AMCL and AMCL alone. There is no expectation for other implementations to use that topic. Making something like those suggested contains less embedded information about what that topic is and how it should be utilized.

That pose should not be used by anything as means to get a pose of the robot, that's what TF is for. That should really only be used for logging, debugging, and a really high level sense of where about you might be.

I would close this ticket, thoughts?

[rotu]

@SteveMacenski, I stand by this issue. It's a global pose with covariance which represents the robot's location in space - nothing more or less. amcl_pose is simultaneously too specific and too ambiguous: AMCL is merely today's implementation detail (that could be subbed out for any other positioning algorithm, like SLAM or even EKF with a GPS or IPS signal). And amcl_pose sounds could be either a pose coming out of AMCL or going into AMCL.

I agree that my suggestions are not necessarily too much better. Maybe localized_pose?

[SteveMacenski]

@rotu your ticket mentions the reason you want it changed is because the nav2 stack subscribes to it, which I can only find 1 instance of that, in robot.cpp for the get position function. We've had discussions about changing that over to using TF to get smoother and filtered pose estimates. I think that part of your issue should be resolved soon.

This is the pose produced by AMCL, I'm not sure the idea of a pose going into AMCL is a meaningful concept. AMCL is just the flavor of the day, but again, that topic is not meant to be used as a robots position, you should always be using TF for that so that you're incorporating the odometric filtered state of motion between AMCL updates which are few and far between. If you have additional implementations of localizers / SLAM there's no requirement for them to broadcast or even be aware of that topic. We could even just remove that topic all together and I think that would be reasonable. The API for ROS localization and mapping solutions is only to publish to TF, the topics are purely implementation specific and/or debug. If the original AMCL authors wanted that debug topic to look at, that's great and its good to have that information, but my feeling is that this is a perceived issue out of (what I think is) misuse.

I'm not opposed to having topics change, but I would like to make sure we're doing it for the right reasons and choosing something that either adds clarity or embeds more information. I think in this case amcl_pose is clear and embeds the information about what that pose represents.

[rotu]

We've had discussions about changing that over to using TF to get smoother and filtered pose estimates. I think that part of your issue should be resolved soon.

If nav2 no longer subscribes to the amcl_pose topic, then this is no longer an issue to me.

which I can only find 1 instance of that, in robot.cpp for the get position function

Exactly. I'm trying to get Cartographer SLAM hooked up to the nav stack so I can do click-to-navigate while using SLAM. Remapping the output of my SLAM implementation to the /amcl_pose topic is misleading to anyone reading the code.

I'm not sure the idea of a pose going into AMCL is a meaningful concept

Yes it is. AMCL as currently implemented needs a pose estimate to seed the particle filter. It's not a good name, for that reason alone.

[SteveMacenski]

I've added TF positioning to my Priority 1 queue

[SteveMacenski]

#993

Can I close this ticket once this PR is merged implementing TF positioning?

[rotu]

@SteveMacenski I support that