Add UTM zone to frame_id; added reverse_utm_odometry_node

[dheera]

Adding the UTM zone to the frame_id (e.g. frame_id/utm_10S) ensures that no data is lost (i.e. odometry message is self-contained) when conversion is made from NavSatFix to Odometry. This also helps differentiate Odometry data that can be theoretically specified in two different ways in overlapping UTM zones.

Also added a reverse_utm_odometry_node that reverses the UTM odometry data back into a NavSatFix which is extremely useful for NavSatFix-compatible map visualization tools to be able to directly visualize Odometry data in UTM format.

[kriskozak]

I don't know who all might be using the utm_odometry_node, so I would recommend not changing the default behavior. Adding a parameter, that defaults to false, to append the zone in the frame_id would probably be sufficient.

Also, the purpose of the reverse_utm_odometry_node was not obvious to me from the name. I would recommend a name like utm_odometry_to_navsatfix_node, which is clearer though a little wordy, instead.

[dheera]

@kriskozak Thanks for the feedback! I use utm_odometry_node, and store data in UTM coordinates. But an example of why reversing the UTM back to NavSatFix is useful:

• when I use a Kalman filter to fuse multiple sensor data, and want to convert the result back to a NavSatFix and compare against the raw GPS
• when I use localization algorithms that operate in UTM coordinates, but I want the robot to report its location in lat/lon coordinates back to a server for other purposes (e.g. tracking)

I called it 'reverse_' because it does exactly the opposite of utm_odometry_node. I'm open to calling it utm_odometry_to_navsatfix_node or anything else, but then shouldn't we then call NavSatFix->Odometry utm_navsatfix_to_odometry_node for consistency?

I like the idea of appending the zone as optional parameter to maintain backward compatibility. But in that case we should allow the reverse node to specify a fixed UTM zone as a ROS parameter (in case the user chose not to append it to the frame_id). Let me know your thoughts!

[kriskozak]

@dheera I understand and agree with the usefulness of the node, I just object to the name. I agree with your comment about the symmetry of the naming, but at this point utm_odometry_node has existed with that name for 7 years, so it doesn't make sense to change it now.

The reason that I object to the name is that reverse made me think of reversing direction rather than reversing a process (so the purpose wasn't clear until I read your explanation). If I understand it correctly, the node's function is to convert Odometry messages in a UTM frame to NavSatFix messages, so calling it utm_odometry_to_navsatfix_node (or utm_odometry_to_navsatfix the _node suffix is not really necessary except that it parallels the suffix for utm_odometry_node) is clearer as to its purpose.

Finally, it occurs to me that you may should probably also consider two other issues:

1. Appending only zone leaves an ambiguity about whether the coordinate is northern or southern hemisphere (perhaps also appending N/S or appending Band letter would be appropriate).
2. You may want to add a "zone" (and N/S or band) parameter to the reverse_utm_odometry_node to make it possible to determine latitude in the event that that info is not in the field of the utm_odom message.

[dheera]

@kriskozak Good points! I'll fix these over the weekend.

[dheera]

@kriskozak @pjreed I have made the following requested changes:

• renamed node to utm_odometry_to_navsatfix_node
• the appending of the zone in the frame_id is disabled by default, and enabled by a boolean ROS parameter "append_zone"
• the utm_odometry_to_navsatfix_node will accept a ROS parameter called "zone", and if not given, it will look for an appended zone as above

Please review. Thanks!

[pjreed]

Pinging @kriskozak , did you ever have a chance to look at this?

[kriskozak]

Thanks for the reminder. I have not had a chance to test it, but the changes to utm_odometry_node.cpp appear to give the desired behavior without changing the default behavior. Even though it's a simple and straightforward change, I would prefer to see it tested before merging, but @pjreed, if you want to make a call, feel free. I will give it a go later, if I get the chance, but you may be pinging me again in a month ;-).

The utm_odometry_to_navsatfix_node.cpp looks fine, but also not tested -- I'm fine with merging this one without testing, though.

[dheera]

Any update on this? Thanks!

[kriskozak]

Sorry about the massive delay on this. See the one comment about the default value of the append_zone parameter.