Skip to content

smac_planner: fix goal checking in getAnalyticPath()#2159

Closed
mkolodziejczyk-piap wants to merge 1 commit intoros-navigation:foxy-develfrom
mkolodziejczyk-piap:foxy_smac_astar_fix
Closed

smac_planner: fix goal checking in getAnalyticPath()#2159
mkolodziejczyk-piap wants to merge 1 commit intoros-navigation:foxy-develfrom
mkolodziejczyk-piap:foxy_smac_astar_fix

Conversation

@mkolodziejczyk-piap
Copy link
Copy Markdown


Basic Info

Info Please fill out this column
Ticket(s) this addresses #2040
Primary OS tested on Ubuntu
Robotic platform tested on my own simulation stack

Description of contribution in a few bullet points

  • Added condition to exclude goal from poses added through analytic path expansion, because the goal is added always at the end and adding it twice causes infinite loops in the path

Copy link
Copy Markdown
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a styling change but LGTM. Can you explain in a few sentences what the specific issue you hit? I'm curious how we didn't also run into this in our testing.

for (const auto & node_pose : possible_nodes) {
const auto & n = node_pose.first;
if (!n->wasVisited()) {
if (!n->wasVisited() and n->getIndex()!=_goal->getIndex()) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!n->wasVisited() and n->getIndex()!=_goal->getIndex()) {
if (!n->wasVisited() && n->getIndex() != _goal->getIndex()) {

Just to be consistent in styling

@mkolodziejczyk-piap
Copy link
Copy Markdown
Author

IMO the issue arises, when the vehicle is crossing the goal but the orientation is not the goal orientation and after crossing this goal ompl plans to correct the yaw with some back and forth manouvers. In my case, this loop happend almost all the time near the goal, probably because (1) my controller was not tuned enought to follow the path exactly (so the initial "smooth" global plan turned into some wild manouvers to correct the yaw) (2) my vehicle has large turning radius

@SteveMacenski
Copy link
Copy Markdown
Member

SteveMacenski commented Jan 29, 2021

vehicle is crossing the goal but the orientation is not the goal orientation

Ooooooh, yeah I did not test for that. Thanks!

Please make the update / main PR and we can merge the set. Thanks for debugging this!

@SteveMacenski
Copy link
Copy Markdown
Member

@mkolodziejczyk-piap just following up here. Just need the small styling update & main PR to merge both

@SteveMacenski
Copy link
Copy Markdown
Member

SteveMacenski commented Feb 9, 2021

@mkolodziejczyk-piap just following up

This is essentially done, just needs superficial update. If I don't hear from you, I'll merge in my own patch sometime next week.

@SteveMacenski
Copy link
Copy Markdown
Member

SteveMacenski commented Feb 24, 2021

Closing - superseded by linked PRs that meet the Nav2 coding styling

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 this pull request may close these issues.

2 participants