Speed is inverted when dist is negative

[mdtoom]

---

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	#1896
Primary OS tested on	Ubuntu
Robotic platform tested on	Gazebo

---

Description of contribution in a few bullet points

• Fixes that backup_action_node cannot enter a negative speed and therefore the action cannot backed up.
• Changed to invert speed when distance is negative (robot should go forward) similar to backup recovery node.

Description of documentation updates required from your changes

---

Future work that may be required in bullet points

[codecov]

Codecov Report

Merging #1897 into master will decrease coverage by 0.68%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1897      +/-   ##
==========================================
- Coverage   71.83%   71.15%   -0.69%     
==========================================
  Files         222      222              
  Lines       10673    10671       -2     
==========================================
- Hits         7667     7593      -74     
- Misses       3006     3078      +72     

Impacted Files	Coverage Δ
...v2_behavior_tree/plugins/action/back_up_action.cpp	87.50% <ø> (-1.39%)	⬇️
...v2_behavior_tree/plugins/action/back_up_action.hpp	100.00% <100.00%> (ø)
nav2_recoveries/plugins/back_up.cpp	91.66% <100.00%> (ø)
...re/include/dwb_core/illegal_trajectory_tracker.hpp	40.00% <0.00%> (-60.00%)	⬇️
...roller/dwb_core/src/illegal_trajectory_tracker.cpp	25.00% <0.00%> (-55.00%)	⬇️
...tree/include/nav2_behavior_tree/bt_action_node.hpp	79.26% <0.00%> (-9.76%)	⬇️
...map_2d/include/nav2_costmap_2d/inflation_layer.hpp	91.66% <0.00%> (-8.34%)	⬇️
nav2_costmap_2d/src/clear_costmap_service.cpp	17.89% <0.00%> (-6.32%)	⬇️
..._dwb_controller/dwb_core/src/dwb_local_planner.cpp	75.68% <0.00%> (-5.48%)	⬇️
nav2_recoveries/plugins/spin.cpp	89.47% <0.00%> (-5.27%)	⬇️
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c65e3aa...c906ec5. Read the comment docs.

[SteveMacenski]

This is not an intuitive solution. I think you'd be better off checking both variables for signs. The speed must be positive since speed is a scalar not a vector quantity. We assume this is always positive and the sign is flipped in the backup recovery. The distance is also a scalar and should also be assumed to always be positive.

[mdtoom]

I agree, and updated, but shouldn't then the backup recovery in nav2_recoveries also be changed because there a negative distance and a positive speed is required to go backward. This also not very intuitive.

[SteveMacenski]

I agree that's unintuitive for the API. Can you update the recovery behavior & the BT node to use only positive values as inputs and then set the negative signs required in the recovery behavior itself?

So at the end, it should be all positive values in the BT node and in the actual recovery behavior we add negative sign for the velocity and distance when processing.

I think it would be good to add that as a doc as well. In the Foxy migration page https://github.com/ros-planning/navigation.ros.org/blob/master/migration/Foxy.rst can you write a couple sentences explaining that change?

[mdtoom]

Interface changes documented in: ros-navigation/docs.nav2.org#60