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

getObstacleHeuristic will return negative values #3104

Closed
mangoschorle opened this issue Aug 4, 2022 · 3 comments · Fixed by #3122
Closed

getObstacleHeuristic will return negative values #3104

mangoschorle opened this issue Aug 4, 2022 · 3 comments · Fixed by #3122

Comments

@mangoschorle
Copy link

requested_node_cost is negative. requested_node_cost is never overwritten and we reach the return only if requested_node_cost is negative.

I have the feeling it should be

return 2.0 * shared_data_->obstacle_heuristic_lookup_table[start_index];

https://github.com/ros-planning/navigation2/blob/104bdc51fb72e05af4ccb4fb96a2fd421239f016/nav2_smac_planner/src/node_hybrid.cpp#L542

@SteveMacenski
Copy link
Member

SteveMacenski commented Aug 18, 2022

requested_node_cost is a reference

const float & requested_node_cost = obstacle_heuristic_lookup_table[start_index];

So when the lookup table values are changed in the body of the function, this is updated too. The only way it would continue to be negative is if the cell was unreachable (e.g. disconnected part of the map with no allowable way to get there).

If https://github.com/ros-planning/navigation2/blob/104bdc51fb72e05af4ccb4fb96a2fd421239f016/nav2_smac_planner/src/node_hybrid.cpp#L598 is changed from == 0 to <= 0 that would guarantee that the heuristic is never negative either way.

@SteveMacenski
Copy link
Member

#3122 changes that, so no matter what, the H is never negative since the distance H would be computed if the obstacle H was not possible due to non-reachability.

@SteveMacenski SteveMacenski linked a pull request Aug 18, 2022 that will close this issue
@mangoschorle
Copy link
Author

@SteveMacenski My bad! In my code base i search and replaced all const prim_type & with the actual prim_type because I got annoyed with functions that have const bool & as param type. Shouldn't have done that.

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

Successfully merging a pull request may close this issue.

2 participants