Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify parameters names for local planner (trivial change) #90

Closed
corot opened this issue Jul 18, 2013 · 9 comments
Closed

Unify parameters names for local planner (trivial change) #90

corot opened this issue Jul 18, 2013 · 9 comments
Assignees
Labels

Comments

@corot
Copy link
Contributor

corot commented Jul 18, 2013

Plugin dwa_local_planner uses path_distance_bias/goal_distance_bias/occdist_scale, but base_local_planner uses pdist_scale/gdist_scale/occdist_scale with the same semantics. I propose to unify using the latest on hydro.

The wiki page for base_local_planner was also wrong (I changed), as are the configuration for some robots, as Turtlebot: it uses base_local_planner but sets dwa_local_planner parameters.

Something similar happens with other parameters min_in_place_rotational_vel/min_in_place_vel_theta or max_rotational_vel/max_vel_theta: the dynamic parameter and the (old?) name differs.

@felix-kolbe
Copy link

+1
Having such differences there is really bad, fiddling with the navigation configuration is puzzling itself. Could it be you changed the wiki page for every distro? A note (or distro-branching) would be good to prevent pre-hydro users hit their computers or robots.

@DLu
Copy link
Member

DLu commented Oct 16, 2013

I made a change for this in my layered-groovy code. I can port to Hydro if it all looks good.

@mikeferguson
Copy link
Contributor

A little late to do this to either hydro or indigo, propose we do this in Jade.

@mikeferguson mikeferguson added this to the Jade Release (1.13.0) milestone Nov 29, 2014
@mikeferguson
Copy link
Contributor

@corot -- any chance you could put together a Pull Request to fulfill this in Jade?

@corot
Copy link
Contributor Author

corot commented May 5, 2015

mmm.... I'm horribly busy these days, so I cannot promise anything.

@corot
Copy link
Contributor Author

corot commented Apr 27, 2016

Another worse case of duplicated parameters: base local planner uses both max/min_vel_theta and max/min_vel_x. So you must set both!
👍 to keep only max/min_vel_x. Would be great to do it before Kinetic release

@mikeferguson
Copy link
Contributor

Attacking this for Melodic

@mikeferguson mikeferguson self-assigned this Mar 28, 2018
@mikeferguson
Copy link
Contributor

The defaults for path_distance_bias and pdist_scale in the two dynamic reconfigurations are vastly different (32 vs 0.6) -- are these really "the same thing"?

@DLu
Copy link
Member

DLu commented Feb 27, 2019

Closed in #725

@DLu DLu closed this as completed Feb 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants