Closed gkouros closed 6 years ago
Thanks for this PR, it looks like a useful enhancement.
At first glance, the code looks fine (with very minor comments).
But, someone needs to spend enough time reviewing this change to make sure all the parameters have retained their original default values.
Sorry about that, I didn't notice the pattern.
It's no big deal, but it's nice to keep lists of dependencies ordered. Makes it easier to see what's there and not there, especially if the lists get long, which this one is not.
I rechecked the default values and bounds of the parameters and noticed that I accidentally used 0.0 lower bound for the gains k_int and k_diff. However, both parameters as well as the tolerance_timeout parameter are not actually used anywhere in the code. Should I remove all three, or simply fix the lower bounds of the two gain parameters?
If you are certain they are not used, they should probably be removed.
I removed the unused parameters. Now, everything looks ok to me, but as you suggested, a second pair of eyes would be advisable.
@jack-oquin: I can take a look and test out this PR.
I've just tested this PR, and it looks great to me. Thanks @gkouros for the contribution!
I'll merge it in, and updated documentation on the ROS wiki page with parameter descriptions.
Add dynamic reconfigure for the parameters of the eband planner, trajectory controller and visualizer.