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

Use of get_parameter_or not working. #1763

Closed
tgreier opened this issue May 22, 2020 · 3 comments · Fixed by #1765
Closed

Use of get_parameter_or not working. #1763

tgreier opened this issue May 22, 2020 · 3 comments · Fixed by #1765

Comments

@tgreier
Copy link
Contributor

tgreier commented May 22, 2020

Where get_parameter_or is used, those values do not seem to be read from yaml.
It seems to be a problem perhaps since introduction of declare_parameter?

Bug report

Required Info:

  • Operating System:
    Ubuntu 18.04
  • Version or commit hash:
    eloquent-devel

Steps to reproduce issue

Try to set from yaml:
odom_topic
https://github.com/ros-planning/navigation2/blob/7c0ac74514506591c0d508a6bc10e42d1d67162f/nav2_dwb_controller/nav_2d_utils/include/nav_2d_utils/odom_subscriber.hpp#L69

goal_reached_tol
https://github.com/ros-planning/navigation2/blob/9615c2ef5d3c89db901f916b136845367e5fcabb/nav2_behavior_tree/plugins/condition/goal_reached_condition.cpp#L62

required_movement_radius
https://github.com/ros-planning/navigation2/blob/9615c2ef5d3c89db901f916b136845367e5fcabb/nav2_controller/src/progress_checker.cpp#L30

movement_time_allowance_
https://github.com/ros-planning/navigation2/blob/9615c2ef5d3c89db901f916b136845367e5fcabb/nav2_controller/src/progress_checker.cpp#L32

Also, underscore at end of "movement_time_allowance_" should be removed?

Expected behavior

Values will be set

Actual behavior

Defaults are retained

Additional information

@SteveMacenski
Copy link
Member

SteveMacenski commented May 22, 2020

Please give exact examples and links to their lines in source for review.

Also, underscore at end of "movement_time_allowance_" should be removed?

Yes, most likely, though it appears to be correct in source that I can see https://github.com/ros-planning/navigation2/blob/a0279a10d72fc297e3dabca2e017b7443616b7a9/nav2_controller/src/progress_checker.cpp#L33

@tgreier
Copy link
Contributor Author

tgreier commented May 22, 2020

I'm sorry, I am using eloquent-devel branch so these changes are not in that branch. The change you referenced I believe is in master branch.

However, I originally was having a problem with "odom" topic, which is here:

https://github.com/ros-planning/navigation2/blob/7c0ac74514506591c0d508a6bc10e42d1d67162f/nav2_dwb_controller/nav_2d_utils/include/nav_2d_utils/odom_subscriber.hpp#L69

Should master branch be used for latest ros2 work? I apologize for my newbie question.

@SteveMacenski
Copy link
Member

Master should be used for latest ros2 work, yes. But you can cherry pick that PR's commit to Eloquent and then submit a PR and I'll merge it to fix eloquent.

I agree, I think in that case there's an issue because the odom_topic hasn't been declared. Without declaring, you can't get it from the file so it'll always use the default or the topic provided in the constructor.

You can submit a PR to fix that using the nav2_utils declare if not declared method before the get_parameter_or to declare it and then it should work.

Please update the original ticket with the link to each of these and I can help figure out which/what the solution is for you to patch in a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants