locusrobotics / robot_navigation

Spiritual successor to ros-planning/navigation.
450 stars 148 forks source link

Incorrect parameter overrides for Twirling #57

Closed SteveMacenski closed 4 years ago

SteveMacenski commented 4 years ago

Hi,

As reported in https://github.com/ros-planning/navigation2/issues/1766 and cross referenced here, the twirling param will try to get scale in the TrajectoryCritic initialization function (https://github.com/locusrobotics/robot_navigation/blob/master/dwb_local_planner/include/dwb_local_planner/trajectory_critic.h#L98) but then calling its implementations onInit(), trying to get again and set to zero if it doesn't exist. In this situation, there's no way for it not to exist (https://github.com/locusrobotics/robot_navigation/blob/master/dwb_critics/src/twirling.cpp#L43).

Suggested fix it just to remove it from onInit().

DLu commented 4 years ago

I believe this is only a ROS2 problem, since in ROS1, checking the parameter in initialize will NOT set the ROS parameter. Am I missing something?

SteveMacenski commented 4 years ago

I see what you mean, I think you are correct