-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Initial simple turn lane support #2954
base: master
Are you sure you want to change the base?
Conversation
…t should still work
@karussell nice! I don't see any changes to https://github.com/graphhopper/graphhopper/tree/c64febe4522984a61b2d80dcf0d78135c19bfea5/navigation - am I correct to assume this will not improve the navigation module? If not, any plans to update that too? |
The |
Thanks for the info - I had assumed that the navigate endpoint would need some "conversion" so that it matches that format, glad to hear its in. I wish I could help review but alas Java has been a while - looking forward to seeing this in Graphhopper, and happy to test this against the Maplibre Navigation SDKs once its in and we can deploy it. Thanks for the PR! |
Oh, yes, you are right it will need some modification in the converter! Maybe you can investigate the required format and contribute this change? Shouldn't be much I guess even without much Java knowledge :) |
I'd need to set up the whole toolchain, best someone else tackles this 😅. I'll reach out to our backend team and see if they can help. For now at least linking some documentation. While this is documentation for the newest API version, I quickly checked and it appears to still match the older format used by MapLibre Navigation SDK: https://docs.mapbox.com/api/navigation/directions/#lane-object Some sample code of a route step containing lane info: {
"intersections": [
{
"out": 1,
"location": [13.424671, 52.508812],
"bearings": [120, 210, 300],
"entry": [false, true, true],
"in": 0,
"stop_sign": true,
"lanes": [
{
"valid": true,
"active": true,
"valid_indication": "straight",
"indications": ["left", "straight"]
},
{
"valid": true,
"active": false,
"valid_indication": "straight",
"indications": ["straight", "right"]
},
{
"valid": false,
"active": false,
"indications": ["right"]
}
]
}
],
"geometry": "asn_Ie_}pAdKxG",
"maneuver": {
"bearing_after": 202,
"type": "turn",
"modifier": "left",
"bearing_before": 299,
"location": [13.424671, 52.508812],
"instruction": "Turn left onto Adalbertstraße"
},
"duration": 59.1,
"distance": 236.9,
"driving_side": "right",
"weight": 59.1,
"name": "Adalbertstraße",
"mode": "driving"
} |
I'm unsure if these are the necessary objects. Because there is another place for the sub component if JSON "sub": {
"text": "",
"components": [
{
"text": "",
"type": "lane",
"directions": ["left"],
"active": true
},
{
"text": "",
"type": "lane",
"directions": ["left", "straight"],
"active": true
},
{
"text": "",
"type": "lane",
"directions": ["right"],
"active": false
}
]
} But it is unclear to me, where the distance of the lane info (before the turn) is specified. Because for the client side this is important to always show the current lane and this could change multiple times per turn. So I'd prefer if someone else steps in for the /navigate endpoint, who can also check if the format changes are picked up properly on the client side. |
You're right, there's more to it. I went through the iOS navigation code, and the mentioned lane info for banners comes from where you say it does (https://github.com/flitsmeister/mapbox-directions-swift/blob/6c19ecc4e1324887ae3250802b8d13d8d8b3ff2d/MapboxDirections/MBVisualInstruction.swift#L73) Here's a test file from the MapLibre Navigation iOS SDK, it contains lane info on both the level that I described but also at the level you described - should show how it should look like. |
hi @karussell, I want to add lane support to the /navigate response. I'll start with adding it to the bannerInstructions but I have some questions:
|
Great, that's much appreciated!
I don't know. But there should be a way to specify multiple InstructionDetails instances for every turn. Imagine there is a left turn and initially you have two lanes (left|through and right) but a few meters further it are three lanes (left|through|right) and so there must be a way to specify multiple InstructionDetails per turn instruction. Could it be that there are multiple banner instruction objects per turn? Because the "distanceAlongGeometry" is per banner instruction object and this seems to be related to our InstructionDetails.beforeTurn.
Yes, sounds good. At the moment we only have "valid". |
* added lanes support to navigate endpoint * added tests for turn lane support in navigate endpoint
Fixes #1131 and replaces #1269. These turn lane details help with the instructions to e.g. announce turns earlier.
Every instruction can contain multiple lane lists. For example one lane list 500m before the turn, then the lane list changes because the right turn was done (or the "right" and "through" lane separated) and then 200m before the turn there is another lane list.
Screenshot from a prototype client that can consume this data (
turn_lanes
branch).Work to do:
valid
although it should pick the right lane (continue
). Lane info isleft|continue
. GraphHopper detects a "slight right" turn although it should use a "continue", but turn angle is too big for a "continue" due to the lane merge. So also related to ignore instructions if directions of a road are switching between explicitly <-> implicitly tagged #2953. Example link: https://graphhopper.com/maps/?point=52.528168%2C13.402536&point=52.52994%2C13.401086&profile=car