search

AutoRally / autorally

Software for the AutoRally platform
http://autorally.github.io
731 stars 228 forks source link

child_frame_id of odometry message is not set in autorally_plant.cpp #90

Open sayali-purdue opened 4 years ago

sayali-purdue commented 4 years ago

As per the http://docs.ros.org/melodic/api/nav_msgs/html/msg/Odometry.html, The twist in the odometry message should be specified in the coordinate frame given by the child_frame_id.

In https://github.com/AutoRally/autorally/blob/melodic-devel/autorally_control/src/path_integral/autorally_plant.cpp:

the twist has been set for the variable 'subscribed_state' of type nav_msgs::Odometry (declared at line #310), however no child_frame_id is set.

I would like to know the following:

  1. Which frame the twist in this message belongs to?
  2. Why is the child_frame_id not set?

Thank you!

JasonGibson274 commented 4 years ago

This repo is rather horrible about properly using an TF features. The frame of the odometry message would be the center of the vehicle at ground level (z = 0).

sayali-purdue commented 4 years ago

Thanks a lot for providing the frame details. I am doing a static review of the code. I would really appreciate if you could provide me a couple of details:

  1. Can I know why there was no need for the child_frame_id info in your application?
  2. Also, as a developer of this code, do you think this odometry frame info (i.e., setting child_frame_id field) is useful to others, who want to use your node?

Thanks again!

JasonGibson274 commented 4 years ago

There was no need since it always means the same thing in our code. Usually subscribed pose is just used for plotting and debugging. If you would like to submit a PR I'll happily review it, it's not a feature we personally have a use for so it has been left blank.

We also do not interface with any standard ROS code when running our platform (all our code is custom). If people are using any of the code externally to this repository I will support that as my own personal time allows, but in general that's outside of the scope of the research project as it stands right now.