-
Notifications
You must be signed in to change notification settings - Fork 3.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
Fix turn.roads_on_the_left and turn.roads_on_the right for two-way roads #5131
Conversation
The problem with original approach is that |
👋 Hi @themylogin ! Thanks so much for catching this. I looked at the code and code wise it looks great. Travis is failing, so I'll look into that a bit. Just letting you know because this has been already sitting for over 20 days 🙈 |
I am seeing
@danpat do you have an idea why the changes in this PR would affect the turn layers in tiles? |
It's strange, all tests were passing in my environment |
I'm also confused how these are connected. But travis is seeing the same errors so I don't think it is my local environment that is the problem. |
The test fails for me locally too. I'm not sure what the problem is here. @themylogin Can you do I modified the test to write out the generated vector tile data, and converted it to GeoJSON with --- master.geojson 2018-07-24 11:35:41.000000000 -0700
+++ branch.geojson 2018-07-24 11:31:55.000000000 -0700
@@ -5554,9 +5554,9 @@
,
{ "type": "Feature", "id": 105, "properties": { "bearing_in": 295, "turn_angle": 90, "cost": 1.899999976158142, "weight": 1.899999976158142, "type": "enter roundabout turn", "modifier": "right" }, "geometry": { "type": "Point", "coordinates": [ 7.419111, 43.737153 ] } }
,
-{ "type": "Feature", "id": 106, "properties": { "bearing_in": 41, "turn_angle": -168, "cost": 27.399999618530275, "weight": 27.399999618530275, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.418172, 43.737579 ] } }
+{ "type": "Feature", "id": 106, "properties": { "bearing_in": 41, "turn_angle": -168, "cost": 7.400000095367432, "weight": 7.400000095367432, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.418172, 43.737579 ] } }
,
-{ "type": "Feature", "id": 107, "properties": { "bearing_in": 41, "turn_angle": 36, "cost": 0, "weight": 0, "type": "(noturn)", "modifier": "slight right" }, "geometry": { "type": "Point", "coordinates": [ 7.418172, 43.737579 ] } }
+{ "type": "Feature", "id": 107, "properties": { "bearing_in": 41, "turn_angle": 36, "cost": 0.10000000149011612, "weight": 0.10000000149011612, "type": "(noturn)", "modifier": "slight right" }, "geometry": { "type": "Point", "coordinates": [ 7.418172, 43.737579 ] } }
,
{ "type": "Feature", "id": 108, "properties": { "bearing_in": 43, "turn_angle": 0, "cost": 0, "weight": 0, "type": "(noturn)", "modifier": "straight" }, "geometry": { "type": "Point", "coordinates": [ 7.415801, 43.735302 ] } }
,
@@ -5566,13 +5566,13 @@
,
{ "type": "Feature", "id": 111, "properties": { "bearing_in": 208, "turn_angle": 90, "cost": 1.899999976158142, "weight": 1.899999976158142, "type": "turn", "modifier": "right" }, "geometry": { "type": "Point", "coordinates": [ 7.415484, 43.734976 ] } }
,
-{ "type": "Feature", "id": 112, "properties": { "bearing_in": 71, "turn_angle": -2, "cost": 0, "weight": 0, "type": "new name", "modifier": "left" }, "geometry": { "type": "Point", "coordinates": [ 7.417673, 43.737172 ] } }
+{ "type": "Feature", "id": 112, "properties": { "bearing_in": 71, "turn_angle": -2, "cost": 0.5, "weight": 0.5, "type": "new name", "modifier": "left" }, "geometry": { "type": "Point", "coordinates": [ 7.417673, 43.737172 ] } }
,
-{ "type": "Feature", "id": 113, "properties": { "bearing_in": 71, "turn_angle": -30, "cost": 27.399999618530275, "weight": 27.399999618530275, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.417673, 43.737172 ] } }
+{ "type": "Feature", "id": 113, "properties": { "bearing_in": 71, "turn_angle": -30, "cost": 6.5, "weight": 6.5, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.417673, 43.737172 ] } }
,
{ "type": "Feature", "id": 114, "properties": { "bearing_in": 209, "turn_angle": 15, "cost": 0, "weight": 0, "type": "(noturn)", "modifier": "straight" }, "geometry": { "type": "Point", "coordinates": [ 7.416866, 43.735936 ] } }
,
-{ "type": "Feature", "id": 115, "properties": { "bearing_in": 342, "turn_angle": 36, "cost": 0.10000000149011612, "weight": 0.10000000149011612, "type": "exit roundabout", "modifier": "right" }, "geometry": { "type": "Point", "coordinates": [ 7.416812, 43.735817 ] } }
+{ "type": "Feature", "id": 115, "properties": { "bearing_in": 342, "turn_angle": 36, "cost": 0, "weight": 0, "type": "exit roundabout", "modifier": "right" }, "geometry": { "type": "Point", "coordinates": [ 7.416812, 43.735817 ] } }
,
{ "type": "Feature", "id": 116, "properties": { "bearing_in": 342, "turn_angle": -17, "cost": 0, "weight": 0, "type": "(stay on roundabout)", "modifier": "slight left" }, "geometry": { "type": "Point", "coordinates": [ 7.416812, 43.735817 ] } }
,
@@ -5774,7 +5774,7 @@
,
{ "type": "Feature", "id": 215, "properties": { "bearing_in": 91, "turn_angle": -94, "cost": 2.299999952316284, "weight": 2.299999952316284, "type": "turn", "modifier": "left" }, "geometry": { "type": "Point", "coordinates": [ 7.417346, 43.737174 ] } }
,
-{ "type": "Feature", "id": 216, "properties": { "bearing_in": 306, "turn_angle": -53, "cost": 27.399999618530275, "weight": 27.399999618530275, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.417668, 43.737310 ] } }
+{ "type": "Feature", "id": 216, "properties": { "bearing_in": 306, "turn_angle": -53, "cost": 5.599999904632568, "weight": 5.599999904632568, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.417668, 43.737310 ] } }
,
{ "type": "Feature", "id": 217, "properties": { "bearing_in": 38, "turn_angle": -135, "cost": 6.800000190734863, "weight": 6.800000190734863, "type": "turn", "modifier": "left" }, "geometry": { "type": "Point", "coordinates": [ 7.417365, 43.737240 ] } }
,
@@ -6082,7 +6082,7 @@
,
{ "type": "Feature", "id": 369, "properties": { "bearing_in": 145, "turn_angle": 97, "cost": 1.399999976158142, "weight": 1.399999976158142, "type": "turn", "modifier": "right" }, "geometry": { "type": "Point", "coordinates": [ 7.421517, 43.736955 ] } }
,
-{ "type": "Feature", "id": 370, "properties": { "bearing_in": 145, "turn_angle": 73, "cost": 0.10000000149011612, "weight": 0.10000000149011612, "type": "new name", "modifier": "slight left" }, "geometry": { "type": "Point", "coordinates": [ 7.421517, 43.736955 ] } }
+{ "type": "Feature", "id": 370, "properties": { "bearing_in": 145, "turn_angle": 73, "cost": 0, "weight": 0, "type": "new name", "modifier": "slight left" }, "geometry": { "type": "Point", "coordinates": [ 7.421517, 43.736955 ] } }
,
{ "type": "Feature", "id": 371, "properties": { "bearing_in": 5, "turn_angle": 153, "cost": 2.799999952316284, "weight": 2.799999952316284, "type": "turn", "modifier": "right" }, "geometry": { "type": "Point", "coordinates": [ 7.422418, 43.732478 ] } }
,
@@ -6090,7 +6090,7 @@
,
{ "type": "Feature", "id": 373, "properties": { "bearing_in": 51, "turn_angle": 5, "cost": 0, "weight": 0, "type": "new name", "modifier": "slight left" }, "geometry": { "type": "Point", "coordinates": [ 7.421745, 43.736903 ] } }
,
-{ "type": "Feature", "id": 374, "properties": { "bearing_in": 51, "turn_angle": -14, "cost": 27.399999618530275, "weight": 27.399999618530275, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.421745, 43.736903 ] } }
+{ "type": "Feature", "id": 374, "properties": { "bearing_in": 51, "turn_angle": -14, "cost": 5.099999904632568, "weight": 5.099999904632568, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.421745, 43.736903 ] } }
,
{ "type": "Feature", "id": 375, "properties": { "bearing_in": 27, "turn_angle": 36, "cost": 0, "weight": 0, "type": "fork", "modifier": "slight right" }, "geometry": { "type": "Point", "coordinates": [ 7.422383, 43.732327 ] } }
,
@@ -6182,17 +6182,17 @@
,
{ "type": "Feature", "id": 419, "properties": { "bearing_in": 239, "turn_angle": -18, "cost": 0, "weight": 0, "type": "turn", "modifier": "slight left" }, "geometry": { "type": "Point", "coordinates": [ 7.421157, 43.736744 ] } }
,
-{ "type": "Feature", "id": 420, "properties": { "bearing_in": 84, "turn_angle": -43, "cost": 27.399999618530275, "weight": 27.399999618530275, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.421595, 43.736802 ] } }
+{ "type": "Feature", "id": 420, "properties": { "bearing_in": 84, "turn_angle": -43, "cost": 7.400000095367432, "weight": 7.400000095367432, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.421595, 43.736802 ] } }
,
-{ "type": "Feature", "id": 421, "properties": { "bearing_in": 84, "turn_angle": -20, "cost": 7.400000095367432, "weight": 7.400000095367432, "type": "turn", "modifier": "sharp left" }, "geometry": { "type": "Point", "coordinates": [ 7.421595, 43.736802 ] } }
+{ "type": "Feature", "id": 421, "properties": { "bearing_in": 84, "turn_angle": -20, "cost": 7.300000190734863, "weight": 7.300000190734863, "type": "turn", "modifier": "sharp left" }, "geometry": { "type": "Point", "coordinates": [ 7.421595, 43.736802 ] } }
,
-{ "type": "Feature", "id": 422, "properties": { "bearing_in": 245, "turn_angle": -3, "cost": 2.299999952316284, "weight": 2.299999952316284, "type": "new name", "modifier": "left" }, "geometry": { "type": "Point", "coordinates": [ 7.421517, 43.736955 ] } }
+{ "type": "Feature", "id": 422, "properties": { "bearing_in": 245, "turn_angle": -3, "cost": 0.10000000149011612, "weight": 0.10000000149011612, "type": "new name", "modifier": "left" }, "geometry": { "type": "Point", "coordinates": [ 7.421517, 43.736955 ] } }
,
-{ "type": "Feature", "id": 423, "properties": { "bearing_in": 245, "turn_angle": -27, "cost": 27.399999618530275, "weight": 27.399999618530275, "type": "turn", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.421517, 43.736955 ] } }
+{ "type": "Feature", "id": 423, "properties": { "bearing_in": 245, "turn_angle": -27, "cost": 5.599999904632568, "weight": 5.599999904632568, "type": "turn", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.421517, 43.736955 ] } }
,
-{ "type": "Feature", "id": 424, "properties": { "bearing_in": 245, "turn_angle": 80, "cost": 0, "weight": 0, "type": "turn", "modifier": "slight right" }, "geometry": { "type": "Point", "coordinates": [ 7.421517, 43.736955 ] } }
+{ "type": "Feature", "id": 424, "properties": { "bearing_in": 245, "turn_angle": 80, "cost": 0.800000011920929, "weight": 0.800000011920929, "type": "turn", "modifier": "slight right" }, "geometry": { "type": "Point", "coordinates": [ 7.421517, 43.736955 ] } }
,
-{ "type": "Feature", "id": 425, "properties": { "bearing_in": 284, "turn_angle": -22, "cost": 27.399999618530275, "weight": 27.399999618530275, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.421715, 43.736999 ] } }
+{ "type": "Feature", "id": 425, "properties": { "bearing_in": 284, "turn_angle": -22, "cost": 7.099999904632568, "weight": 7.099999904632568, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.421715, 43.736999 ] } }
,
{ "type": "Feature", "id": 426, "properties": { "bearing_in": 262, "turn_angle": 0, "cost": 0, "weight": 0, "type": "new name", "modifier": "left" }, "geometry": { "type": "Point", "coordinates": [ 7.421715, 43.736999 ] } }
,
@@ -6288,7 +6288,7 @@
,
{ "type": "Feature", "id": 472, "properties": { "bearing_in": 32, "turn_angle": 48, "cost": 0.10000000149011612, "weight": 0.10000000149011612, "type": "enter roundabout", "modifier": "right" }, "geometry": { "type": "Point", "coordinates": [ 7.416716, 43.735715 ] } }
,
-{ "type": "Feature", "id": 473, "properties": { "bearing_in": 223, "turn_angle": 66, "cost": 0.8999999761581421, "weight": 0.8999999761581421, "type": "enter roundabout", "modifier": "right" }, "geometry": { "type": "Point", "coordinates": [ 7.416761, 43.735856 ] } }
+{ "type": "Feature", "id": 473, "properties": { "bearing_in": 223, "turn_angle": 66, "cost": 0.4000000059604645, "weight": 0.4000000059604645, "type": "enter roundabout", "modifier": "right" }, "geometry": { "type": "Point", "coordinates": [ 7.416761, 43.735856 ] } }
,
{ "type": "Feature", "id": 474, "properties": { "bearing_in": 149, "turn_angle": -130, "cost": 7.400000095367432, "weight": 7.400000095367432, "type": "turn", "modifier": "sharp left" }, "geometry": { "type": "Point", "coordinates": [ 7.417890, 43.734327 ] } }
,
@@ -6324,7 +6324,7 @@
,
{ "type": "Feature", "id": 490, "properties": { "bearing_in": 18, "turn_angle": 11, "cost": 0, "weight": 0, "type": "(noturn)", "modifier": "straight" }, "geometry": { "type": "Point", "coordinates": [ 7.416866, 43.735936 ] } }
,
-{ "type": "Feature", "id": 491, "properties": { "bearing_in": 18, "turn_angle": -154, "cost": 27.399999618530275, "weight": 27.399999618530275, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.416866, 43.735936 ] } }
+{ "type": "Feature", "id": 491, "properties": { "bearing_in": 18, "turn_angle": -154, "cost": 7.400000095367432, "weight": 7.400000095367432, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.416866, 43.735936 ] } }
,
{ "type": "Feature", "id": 492, "properties": { "bearing_in": 25, "turn_angle": -15, "cost": 0.10000000149011612, "weight": 0.10000000149011612, "type": "(stay on roundabout)", "modifier": "slight left" }, "geometry": { "type": "Point", "coordinates": [ 7.419025, 43.732463 ] } }
,
@@ -6396,7 +6396,7 @@
,
{ "type": "Feature", "id": 526, "properties": { "bearing_in": 173, "turn_angle": 71, "cost": 0.800000011920929, "weight": 0.800000011920929, "type": "exit roundabout", "modifier": "right" }, "geometry": { "type": "Point", "coordinates": [ 7.422997, 43.738513 ] } }
,
-{ "type": "Feature", "id": 527, "properties": { "bearing_in": 257, "turn_angle": -24, "cost": 0, "weight": 0, "type": "new name", "modifier": "slight left" }, "geometry": { "type": "Point", "coordinates": [ 7.418172, 43.737579 ] } }
+{ "type": "Feature", "id": 527, "properties": { "bearing_in": 257, "turn_angle": -24, "cost": 0.10000000149011612, "weight": 0.10000000149011612, "type": "new name", "modifier": "slight left" }, "geometry": { "type": "Point", "coordinates": [ 7.418172, 43.737579 ] } }
,
{ "type": "Feature", "id": 528, "properties": { "bearing_in": 48, "turn_angle": 67, "cost": 0.5, "weight": 0.5, "type": "exit roundabout turn", "modifier": "right" }, "geometry": { "type": "Point", "coordinates": [ 7.419111, 43.737153 ] } }
,
@@ -6614,7 +6614,7 @@
,
{ "type": "Feature", "id": 635, "properties": { "bearing_in": 133, "turn_angle": -65, "cost": 6.400000095367432, "weight": 6.400000095367432, "type": "end of road", "modifier": "left" }, "geometry": { "type": "Point", "coordinates": [ 7.424971, 43.731397 ] } }
,
-{ "type": "Feature", "id": 636, "properties": { "bearing_in": 40, "turn_angle": -115, "cost": 27.399999618530275, "weight": 27.399999618530275, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.425489, 43.731569 ] } }
+{ "type": "Feature", "id": 636, "properties": { "bearing_in": 40, "turn_angle": -115, "cost": 7.400000095367432, "weight": 7.400000095367432, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.425489, 43.731569 ] } }
,
{ "type": "Feature", "id": 637, "properties": { "bearing_in": 40, "turn_angle": 28, "cost": 0, "weight": 0, "type": "new name", "modifier": "straight" }, "geometry": { "type": "Point", "coordinates": [ 7.425489, 43.731569 ] } }
,
@@ -6638,7 +6638,7 @@
,
{ "type": "Feature", "id": 647, "properties": { "bearing_in": 78, "turn_angle": 2, "cost": 0, "weight": 0, "type": "(noturn)", "modifier": "slight left" }, "geometry": { "type": "Point", "coordinates": [ 7.422362, 43.731924 ] } }
,
-{ "type": "Feature", "id": 648, "properties": { "bearing_in": 78, "turn_angle": -28, "cost": 27.399999618530275, "weight": 27.399999618530275, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.422362, 43.731924 ] } }
+{ "type": "Feature", "id": 648, "properties": { "bearing_in": 78, "turn_angle": -28, "cost": 7.300000190734863, "weight": 7.300000190734863, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.422362, 43.731924 ] } }
,
{ "type": "Feature", "id": 649, "properties": { "bearing_in": 236, "turn_angle": 4, "cost": 0, "weight": 0, "type": "(suppressed)", "modifier": "straight" }, "geometry": { "type": "Point", "coordinates": [ 7.416324, 43.731247 ] } }
,
@@ -6858,7 +6858,7 @@
,
{ "type": "Feature", "id": 757, "properties": { "bearing_in": 278, "turn_angle": 11, "cost": 0, "weight": 0, "type": "(suppressed)", "modifier": "straight" }, "geometry": { "type": "Point", "coordinates": [ 7.420007, 43.738771 ] } }
,
-{ "type": "Feature", "id": 758, "properties": { "bearing_in": 66, "turn_angle": -154, "cost": 27.399999618530275, "weight": 27.399999618530275, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.419736, 43.738815 ] } }
+{ "type": "Feature", "id": 758, "properties": { "bearing_in": 66, "turn_angle": -154, "cost": 7.400000095367432, "weight": 7.400000095367432, "type": "continue", "modifier": "uturn" }, "geometry": { "type": "Point", "coordinates": [ 7.419736, 43.738815 ] } }
,
{ "type": "Feature", "id": 759, "properties": { "bearing_in": 66, "turn_angle": 29, "cost": 0, "weight": 0, "type": "new name", "modifier": "slight right" }, "geometry": { "type": "Point", "coordinates": [ 7.419736, 43.738815 ] } }
,
|
I'd like to merge this. On the OSM lists the other day there was some discussion of crossing penalties (esp. https://lists.openstreetmap.org/pipermail/tagging/2021-January/058962.html by @1ec5) which reminded me - it would be great for OSRM to have a reliable solution for this. @themylogin There's a fairly trivial conflict (as a result of #5907) to resolve - if you want to fix that then great, otherwise I can. @danpat Just to check - the extra call to |
CHANGELOG.md
Outdated
@@ -16,6 +16,8 @@ | |||
- Profile: | |||
- ADDED: Parse `source:maxspeed` and `maxspeed:type` tags to apply maxspeeds and add belgian flanders rural speed limit. [#5217](https://github.com/Project-OSRM/osrm-backend/pull/5217) | |||
- CHANGED: Refactor maxspeed parsing to use common library. [#5144](https://github.com/Project-OSRM/osrm-backend/pull/5144) | |||
- Bugfixes: |
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.
This changelog entry needs to be moved to the top.
@systemed The effect is probably minimal - and worth the hit for the improved feature. No point in it being fast if it's wrong. |
Great, thanks. When travis has done its stuff I'll merge this. |
@systemed On the Travis topic - I migrated this repo to travis-ci.com yesterday, builds were never running on the old .org setup. This means we have 10,000 free minutes per month of CI time. Just FYI. |
@danpat That's great - thanks! Travis seems to have hung on "Expected — Waiting for status to be reported" - any thoughts? |
@systemed I think you can ignore that - I think it's old configuration looking for status from travis-ci.org. The "Travis CI - Pull Request" is the status from travis-ci.com. I'll disable the |
Issue
#5128
Tasklist