-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Adds cucumber scenario for u turn restrictions at intersection #4484
Conversation
Currently this is blogged by #4465 |
features/car/restrictions.feature
Outdated
|
||
When I route I should get | ||
| waypoints | route | | ||
| a,x,a | ax,ax,xb,xb,ax,ax | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use bearing values here to work around the limitation of snapping to x
and trigger a real u-turn.
| restriction | cx | cx | x | no_u_turn | | ||
| restriction | dx | dx | x | no_u_turn | | ||
|
||
When I route I should get |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mjjbell as you worked on PR closing #4465 and this PR was blocked by it could you take a look? This test currently failing on master:
2022-10-06T14:33:17.0903046Z Message:
2022-10-06T14:33:17.0903377Z �[31mTables were not identical:
2022-10-06T14:33:17.0903835Z | waypoints | route |
2022-10-06T14:33:17.0904218Z | a,x,a | (-) ax,ax,xb,xb,ax,ax |
2022-10-06T14:33:17.0904606Z | a,x,a | (+) ax,xb,xb,xb,ax,ax |�[39m
But it seems in this context ax,ax,xb,xb,ax,ax
is the same as ax,xb,xb,xb,ax,ax
, or not? Asking because tbh I don't have enough knowledge yet to now for sure if we can merge it or not(but probably would be great to merge it if it is ready) :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Short answer: yes
Longer answer:
Adding the turns
to see what's going on.
| a,x,a | (-) ax,ax,xb,xb,ax,ax | (-) |
| a,x,a | (+) ax,xb,xb,xb,ax,ax | (+) depart,new name straight,continue uturn,arrive,depart,arrive |
So the response is splitting the route into two as a->x->b(uturn)->x , x->a
whereas the test wants a->x , x->b(uturn)->x->a
They will have equivalent weight, so it's just whichever OSRM happens to select out of the two.
This wouldn't have worked in the past because the mid journey stop might have been on cx
or somewhere else that wasn't optimal.
@srividyacb here's a cucumber scenario for what we talked about. See how the route does not make a u-turn at
x
but instead goes to theb
node and makes a u-turn there.