rst-tu-dortmund / teb_local_planner

An optimal trajectory planner considering distinctive topologies for mobile robots based on Timed-Elastic-Bands (ROS Package)
http://wiki.ros.org/teb_local_planner
BSD 3-Clause "New" or "Revised" License
984 stars 545 forks source link

LookupTransform error when using nav2 behavior tree #378

Open stu00608 opened 1 year ago

stu00608 commented 1 year ago

Problem

Conclusion

Looking forwarfd to your reply, thanks!

jcmonteiro commented 1 year ago

I would say that tf2::TimePointZero is not the "correct" either. TEB will plan the path with respect to its knowledge of the world at a given time. In my opinion, it would make more sense to use the timestamp from the scan message. I say this because, at the end of the day, you are planning to avoid obstacles and your best guess of their poses comes from your previous measurement.

stu00608 commented 1 year ago

I would say that tf2::TimePointZero is not the "correct" either. TEB will plan the path with respect to its knowledge of the world at a given time. In my opinion, it would make more sense to use the timestamp from the scan message. I say this because, at the end of the day, you are planning to avoid obstacles and your best guess of their poses comes from your previous measurement.

I agree with you, timestamp from scan message should not affect by behavior tree. Also, I wanna know if the version that I modified whether use timeFromSec(0) or timestamp in latest scan message, is it gonna break the other parts inside TEB?

I think this should be fixed eventually with humble update. Since from nav2 humble release, they do have more example behavior trees that will cause this issue.

Thank you!

jcmonteiro commented 1 year ago

Also, I wanna know if the version that I modified whether use timeFromSec(0) or timestamp in latest scan message, is it gonna break the other parts inside TEB?

Not that I can think of right now since the overall behavior depends a lot on all the timestamps in your tf-tree.

By using the zero stamp you are allowing that particular lookup to consider the latest tfs in the tree, regardless of how old they are. The possible outcomes of doing so will be platform-specific.

automech-rb commented 1 year ago

It seems like ROS1 TEB uses ros::Time() instead of plan_pose.header.stamp in ROS2. https://github.com/rst-tu-dortmund/teb_local_planner/blob/0e839074c3407ff9ee2206a49567a20bd49fd8cc/src/teb_local_planner_ros.cpp#L718

How about changing it similar to ROS1 method?

stu00608 commented 1 year ago

There is a comment block seems doing the same thing in ros2_master branch. Is there any reason to use plan_pose.header.stamp in newer version?

https://github.com/rst-tu-dortmund/teb_local_planner/blob/4501233e21871ed86a5569642f8e5770dadce27b/teb_local_planner/src/teb_local_planner_ros.cpp#L728

automech-rb commented 1 year ago

This seems to be ported to crystal long time ago from this PR by @vinnamkim

Hey @vinnamkim , if possible can you explain your decision for changing the lookup time stamp? Thanks