-
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
[not ready] Left hand driving #2670
Conversation
@@ -162,6 +162,8 @@ class BaseDataFacade | |||
|
|||
virtual bool GetContinueStraightDefault() const = 0; | |||
|
|||
virtual bool UseLeftSideDriving() const = 0; |
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.
MockDataFacade
needs a dummy impl. for this, too. Otherwise unit tests won't compile.
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.
(try with make tests
)
Signed-off-by: Lauren Budorick <lauren@mapbox.com>
|
||
When I route I should get | ||
| waypoints | route | turns | lanes | | ||
| a,h | ab,ch,ch | depart,roundabout-exit-5,arrive | ,slight right:false slight right:true, | |
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.
I think you forgot to change the expected lanes
output here from slight right
to slight left.
If you take a look at the way ab
and dc
above, they only have slight left
lanes.
Counterclockwise vs clockwise does not change the turn lane's direction.
It only changes lane anticipation's behavior in keeping the user to the left/right.
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.
yes, here must be slight left
, but the test fails because query returns an empty result, so i set todo
state and kept the original expecation, because had no idea what should be be the expected result.
I will try to find out the reason during the next week. @MoKob showed me the starting place https://github.com/Project-OSRM/osrm-backend/blob/master/src/extractor/guidance/turn_lane_matcher.cpp#L54-L103
Given #2707 is merged and marked as a duplicate of this, any reason to keep this one around? |
@MoKob nope forgot to close this one. 👍 |
Starting toward #2269. Needs more tests 🚑
cc @oxidase