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

Turn restriction (via node) ignored #1346

Closed
hlaw opened this issue Jan 15, 2015 · 7 comments
Closed

Turn restriction (via node) ignored #1346

hlaw opened this issue Jan 15, 2015 · 7 comments

Comments

@hlaw
Copy link

hlaw commented Jan 15, 2015

http://osrm.at/aEd
screenshot from 2015-01-16 01 23 19

The sharp turn at the northwest of the route should not occur, as this should be prohibited by an "only" turn restriction:
http://www.openstreetmap.org/relation/2436379#map=18/22.32655/114.20857

Incidentally the "from" way of the turn restriction is also the "to" way in another turn restriction, similar to the case mentioned in #1041 (closed). Not sure if this is relevant.

Thank you.

@DennisOSRM
Copy link
Collaborator

Thanks, will look into it

@hlaw
Copy link
Author

hlaw commented Jan 15, 2015

I am trying to trace this and found two duplicate tests, this and this, in the same chain of if-else statements in the turn restriction parsing code.

Seems to stem from the refactoring commit 0d9b705 for lines 140 / 142 of ExtractionContainers.cpp. Not sure if this is related to this issue or perhaps the outstanding issues at #1286.

@emiltin
Copy link
Contributor

emiltin commented Jan 15, 2015

good catch, looks wrong to me

@DennisOSRM
Copy link
Collaborator

Thanks for the pointer into the source code. I was able to remove redundant code and also found another issue where it miscalculated the total number of restrictions (and thus ignored some). Locally this looks much better now:

screenshot 2015-01-16 15 34 29

I am now working on getting the patch into shape and will push it during the day to develop branch. Not sure how to put this edge case into a cucumber test though.

DennisOSRM added a commit that referenced this issue Jan 16, 2015
- use const_iterator where it makes sense
- fix renumbering of turn restriction members
- remove redundant code
- fix counting of usable turn restrictions
@DennisOSRM
Copy link
Collaborator

The fix should be live on the demo site in roughly 10-12 hours from now. Leaving the issue open until we verify it there.

@hlaw
Copy link
Author

hlaw commented Jan 16, 2015

Looks good, thanks a lot!

@DennisOSRM
Copy link
Collaborator

Looking good now.

screenshot 2015-01-21 09 27 15

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

No branches or pull requests

3 participants