search

cra-ros-pkg / robot_localization

robot_localization is a package of nonlinear state estimation nodes. The package was developed by Charles River Analytics, Inc. Please ask questions on answers.ros.org.
http://www.cra.com
Other
1.36k stars 879 forks source link

Integrate UTM translation into utm_transform_node? #63

Closed mikepurvis closed 9 years ago

mikepurvis commented 9 years ago

For users running nmea_navsat_driver, it's lame to have to run a separate node and depend on gps_common to get utm_odometry_node.

Would you consider adding a parameter which enables the gpsFixCallback to just compute and publish the appropriate utm-converted PoseWithCovarianceStamped? It would mean bringing in the conversions header from gps_common, and very slightly swelling the utm_odometry_node binary size, but shouldn't otherwise impact indoor users of the package.

ayrton04 commented 9 years ago

Sounds do-able. Is the dependency on gps_common an issue, or is it actually having to run a separate node that's the problem? If I were to build against gps_common as a dependency (in the catkin sense) and then use its conversion methods, would that work for you?

mikepurvis commented 9 years ago

Ehh, it's both— would prefer to avoid an extra process if unnecessary, but also not wanting to balloon the dependencies of the package.

If you only need the conversions.h header, you could <build_depend> on gps_common, and then if there's an issue with gps_common in the future, it's not a big deal to just clone the header in and drop the dependency.

ayrton04 commented 9 years ago

Sounds good. Should be pretty straightforward.

ayrton04 commented 9 years ago

Added a new gps_transform_node, as utm_transform_node is not really an appropriate name anymore. I took the liberty of cleaning it up a bit and using some proper classes instead of having a single cpp file for everything. The parameters are the same, except obviously no UTM input is required. Also, it now outputs an odometry message to /odometry/gps. There's a new template launch file for it.

My plan at the moment is to deprecate utm_transform_node (with a warning at startup) and axe it by the time we hit Jade.

Give it a shot and let me know what you think.

mikepurvis commented 9 years ago

Awesome, will give it a try!

Not sure if you have a strong preference one way or the other on this, but given that the incoming message is NavSatFix and is intended to represent fixes from GLONASS, Galileo, etc, I wonder if it might be logical to name it navsat_transform_node instead?

ayrton04 commented 9 years ago

I don't have any preference whatsoever, actually! I loathe naming. I'll change it.

ayrton04 commented 9 years ago

Renamed and pushed. Running pre-release tests now to make sure the devel branches are still building. Let me know if you have any trouble with it.