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

fix bug in conflict resolution #4375

Merged
merged 1 commit into from
Aug 11, 2017
Merged

fix bug in conflict resolution #4375

merged 1 commit into from
Aug 11, 2017

Conversation

MoKob
Copy link

@MoKob MoKob commented Aug 7, 2017

Issue

resolves #4374

@@ -773,17 +774,18 @@ void TurnHandler::handleDistinctConflict(const EdgeID via_edge,
return;
}

if (getTurnDirection(left.angle) == DirectionModifier::Right)
// turn to the right
if (getTurnDirection(left.angle) <= 180)
Copy link
Author

Choose a reason for hiding this comment

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

this is the main bug here. By checking for the modifier, we do not use right turns when the angles involved are slight/sharp

Copy link
Member

Choose a reason for hiding this comment

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

Urgh this compiled because of the enum not being a strong typedef, right?

{
if (angularDeviation(left.angle, 85) >= angularDeviation(right.angle, 85))
{
left.instruction = {left_type, DirectionModifier::Right};
right.instruction = {right_type, DirectionModifier::SharpRight};
left.instruction = {left_type, DirectionModifier::SlightRight};
Copy link
Author

Choose a reason for hiding this comment

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

I notices this bug in the process of fixing the above.
Left is farther away from a 90 degree turn than right. So left cannot be a stronger turn than right.
These two were mixed up.

@MoKob MoKob requested a review from daniel-j-h August 9, 2017 08:49
@MoKob MoKob added this to the 5.10.1 milestone Aug 9, 2017
Copy link
Member

@daniel-j-h daniel-j-h left a comment

Choose a reason for hiding this comment

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

This needs to go in asap - reporting the opposite direction makes a terrible user experience. Fortunately this is only the conflict resolution code path.

Please rebase, merge, and cherry-pick into the 5.10 branch.

@MoKob MoKob merged commit a17b07b into master Aug 11, 2017
@MoKob MoKob deleted the fix/4374 branch August 11, 2017 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conflict handling results in invalid turn directions
2 participants