From 2e38d3c170cea32c5d0be2874c75fc80be70296b Mon Sep 17 00:00:00 2001 From: Duane Gearhart Date: Fri, 20 Apr 2018 05:41:53 -0400 Subject: [PATCH] Do not combine a segregated edge with a roundabout (#5040) * Do not combine a segregated edge with a roundabout, add test --- CHANGELOG.md | 5 ++ features/testbot/fixed.feature | 63 +++++++++++++++++++++++++- scripts/osm2cucumber.js | 43 ++++++++++++++++++ src/engine/guidance/collapse_turns.cpp | 7 +-- 4 files changed, 113 insertions(+), 5 deletions(-) create mode 100644 scripts/osm2cucumber.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 603f540c69b..424a6312cca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# 5.17.1 + - Changes from 5.17.0: + - Bugfixes: + - FIXED: Do not combine a segregated edge with a roundabout [#5039](https://github.com/Project-OSRM/osrm-backend/issues/5039) + # 5.17.0 - Changes from 5.16.0: - Bugfixes: diff --git a/features/testbot/fixed.feature b/features/testbot/fixed.feature index 9d83649ffc4..1eb66b89f72 100644 --- a/features/testbot/fixed.feature +++ b/features/testbot/fixed.feature @@ -39,5 +39,64 @@ Feature: Fixed bugs, kept to check for regressions | de | yes | When I route I should get - | from | to | route | - | 1 | 2 | bcd,bcd | + | from | to | route | + | 1 | 2 | bcd,bcd | + + ############################# + # This test models the OSM map at the location for + # https://github.com/Project-OSRM/osrm-backend/issues/5039 + ############################# + Scenario: Mixed Entry and Exit and segregated + Given the profile file "car" initialized with + """ + profile.properties.left_hand_driving = true + """ + Given the node locations + | node | lon | lat | + | a | 171.12889297029 | -42.58425289548 | + | b | 171.1299357 | -42.5849295 | + | c | 171.1295427 | -42.5849385 | + | d | 171.1297356 | -42.5852029 | + | e | 171.1296909 | -42.5851986 | + | f | 171.1295097 | -42.585007 | + | g | 171.1298225 | -42.5851928 | + | h | 171.1300262 | -42.5859122 | + | i | 171.1292651 | -42.584698 | + | j | 171.1297209 | -42.5848569 | + | k | 171.1297188 | -42.5854056 | + | l | 171.1298326 | -42.5857266 | + | m | 171.1298871 | -42.5848922 | + | n | 171.1296505 | -42.585189 | + | o | 171.1295206 | -42.5850862 | + | p | 171.1296106 | -42.5848862 | + | q | 171.1299784 | -42.5850191 | + | r | 171.1298867 | -42.5851671 | + | s | 171.1306955 | -42.5845812 | + | t | 171.129525 | -42.584807 | + | u | 171.1299705 | -42.584984 | + | v | 171.1299067 | -42.5849073 | + | w | 171.1302061 | -42.5848173 | + | x | 171.12975 | -42.5855753 | + | y | 171.129969 | -42.585079 | + | 1 | 171.131511926651| -42.584306746421966 | + | 2 | 171.128743886947| -42.58414875714669 | + And the ways + | nodes | highway | maxspeed | name | ref | surface | junction | oneway | + | ws | primary | 100 | Taramakau Highway | SH 6 | asphalt | | | + | kxlh | trunk | | Otira Highway | SH 73 | | | | + | ai | primary | 100 | Kumara Junction Highway | SH 6 | asphalt | | | + | qyrgdenof | primary | 100 | Kumara Junction | | | roundabout | yes | + | ke | trunk | | Otira Highway | SH 73 | | | yes | + | itj | primary | 100 | Kumara Junction Highway | SH 6 | | | yes | + | gk | trunk | | Otira Highway | SH 73 | | | yes | + | fi | primary | 100 | Kumara Junction Highway | SH 6 | | | yes | + | wq | primary | 100 | Taramakau Highway | SH 6 | | | yes | + | vw | primary | 100 | Taramakau Highway | SH 6 | | | yes | + | vbuq | primary | 100 | Kumara Junction | | | roundabout | yes | + | jmv | primary | 100 | Kumara Junction | | | roundabout | yes | + | fcpj | primary | 100 | Kumara Junction | | | roundabout | yes | + + When I route I should get + | waypoints | route | turns | + | 1,2 | Taramakau Highway,Kumara Junction Highway,Kumara Junction Highway,Kumara Junction Highway | depart,Kumara Junction-exit-2,exit rotary slight left,arrive | + diff --git a/scripts/osm2cucumber.js b/scripts/osm2cucumber.js new file mode 100644 index 00000000000..d123c6fe67d --- /dev/null +++ b/scripts/osm2cucumber.js @@ -0,0 +1,43 @@ +/********************************* + * Takes an XML `.osm` file and converts it into a cucumber scenario definition like + * Given the node locations + * | node | lon | lat | + * ..... + * Given the ways + * | nodes | tag1 | tag2 | tag3 | + * ..... + * + * Note that cucumber tests are limited to 26 nodes (labelled a-z), so + * you'll need to use pretty small OSM extracts to get this to work. + *****************************************/ +var fs = require('fs'); +var parseString = require('xml2js').parseString; + +var data = fs.readFileSync('filename.osm', 'utf8'); + +const items = parseString(data, (err, result) => { +var idmap = {}; + +console.log('Given the node locations'); +console.log(' | node | lon | lat |'); +result.osm.node.filter(n => !n.$.action || n.$.action !== 'delete').forEach(i => { + var code = String.fromCharCode(97 + Object.keys(idmap).length) + idmap[i.$.id] = code; + console.log(` | ${code} | ${i.$.lon} | ${i.$.lat} |`); +}); + +var allkeys = {}; +var waytags = {}; + +result.osm.way.filter(n => !n.$.action || n.$.action !== 'delete').forEach(w => { + if (!waytags[w.$.id]) waytags[w.$.id] = {}; + w.tag.forEach(t => { allkeys[t.$.k] = t.$.v; waytags[w.$.id][t.$.k] = t.$.v; }); +}); + +console.log('And the ways'); +console.log(` | nodes | ${Object.keys(allkeys).join(' | ')} |`); + +result.osm.way.filter(n => !n.$.action || n.$.action !== 'delete').forEach(w => { + console.log(` | ${w.nd.map(n => idmap[n.$.ref]).join('')} | ${Object.keys(allkeys).map(k => waytags[w.$.id][k] || '').join(' | ')} |`); +}); +}); diff --git a/src/engine/guidance/collapse_turns.cpp b/src/engine/guidance/collapse_turns.cpp index be3680827d8..8bf6570d710 100644 --- a/src/engine/guidance/collapse_turns.cpp +++ b/src/engine/guidance/collapse_turns.cpp @@ -618,9 +618,10 @@ RouteSteps collapseSegregatedTurnInstructions(RouteSteps steps) TransferLanesStrategy()); ++next_step; } - // else if the current step is segregated and the next step is not then combine with turn - // adjustment - else if (curr_step->is_segregated && !next_step->is_segregated) + // else if the current step is segregated and the next step is not segregated + // and the next step is not a roundabout then combine with turn adjustment + else if (curr_step->is_segregated && !next_step->is_segregated && + !hasRoundaboutType(next_step->maneuver.instruction)) { // Determine if u-turn if (bearingsAreReversed(