Update version in CMakeLists.txt

This fixes situations where XCode is installed with a newer SDK than the current OS. (#3198)

CHANGELOG.md

@@ -1,3 +1,54 @@
+# 5.4.3
+  - Changes from 5.4.2
+    - Bugfixes
+      - #3254 Fixed a bug that could end up hiding roundabout instructions
+      - #3260 fixed a bug that provided the wrong location in the arrival instruction
+
+# 5.4.2
+  - Changes from 5.4.1
+    - Bugfixes
+      - #3032 Fixed a bug that could result in emitting `invalid` as an instruction type on sliproads with mode changes
+      - #3085 Fixed an outdated assertion that could throw without a cause for concern
+      - #3037 Fixed omitting the last coordinate for overview=simplified
+      - #3176 Fixed exposing wrong OSM ids in matching
+      - Fixes splitting logic in map matching
+
+# 5.4.1
+  - Changes from 5.4.0
+    - Bugfixes
+      - #3016: Fixes shared memory updates while queries are running
+
+# 5.4.0
+  - Changes from 5.3.0
+    - Profiles
+      - includes library guidance.lua that offers preliminary configuration on guidance.
+      - added left_hand_driving flag in global profile properties
+      - modified turn penalty function for car profile - better fit to real data
+      - return `ref` and `name` as separate fields. Do no use ref or destination as fallback for name value
+    - Guidance
+      - Handle Access tags for lanes, only considering valid lanes in lane-guidance (think car | car | bike | car)
+    - API:
+      - `annotations=true` now returns the data source id for each segment as `datasources`
+      - Reduced semantic of merge to refer only to merges from a lane onto a motorway-like road
+      - new `ref` field in the `RouteStep` object. It contains the reference code or name of a way. Previously merged into the `name` property like `name (ref)` and are now separate fields.
+    - Bugfixes
+      - Fixed an issue that would result in segfaults for viaroutes with an invalid intermediate segment when u-turns were allowed at the via-location
+      - Invalid only_* restrictions could result in loss of connectivity. As a fallback, we assume all turns allowed when the restriction is not valid
+      - Fixed a bug that could result in an infinite loop when finding information about an upcoming intersection
+      - Fixed a bug that led to not discovering if a road simply looses a considered prefix
+      - BREAKING: Fixed a bug that could crash postprocessing of instructions on invalid roundabout taggings. This change requires reprocessing datasets with osrm-extract and osrm-contract
+      - Fixed an issue that could emit `invalid` as instruction when ending on a sliproad after a traffic-light
+      - Fixed an issue that would detect turning circles as sliproads
+      - Fixed a bug where post-processing instructions (e.g. left + left -> uturn) could result in false pronunciations
+      - Fixes a bug where a bearing range of zero would cause exhaustive graph traversals
+      - Fixes a bug where certain looped geometries could cause an infinite loop during extraction
+    - Infrastructure:
+      - Adds a feature to limit results in nearest service with a default of 100 in `osrm-routed`
+
+# 5.4.0-rc.7
+  - Chages from 5.4.0-rc.6
+    - Bugfixes re-introduce space between two entries in summaries
+
 # 5.4.0-rc.6
   - Changes from 5.4.0-rc.5
     - Bugfixes
@@ -29,34 +80,6 @@
     - Trip Plugin
       - changed internal behaviour to prefer the smallest lexicographic result over the largest one
 
-# 5.4.0
-  - Changes from 5.3.0
-    - Profiles
-      - includes library guidance.lua that offers preliminary configuration on guidance.
-      - added left_hand_driving flag in global profile properties
-      - modified turn penalty function for car profile - better fit to real data
-      - return `ref` and `name` as separate fields. Do no use ref or destination as fallback for name value
-    - Guidance
-      - Handle Access tags for lanes, only considering valid lanes in lane-guidance (think car | car | bike | car)
-    - API:
-      - `annotations=true` now returns the data source id for each segment as `datasources`
-      - Reduced semantic of merge to refer only to merges from a lane onto a motorway-like road
-      - new `ref` field in the `RouteStep` object. It contains the reference code or name of a way. Previously merged into the `name` property like `name (ref)` and are now separate fields.
-    - Bugfixes
-      - Fixed an issue that would result in segfaults for viaroutes with an invalid intermediate segment when u-turns were allowed at the via-location
-      - Invalid only_* restrictions could result in loss of connectivity. As a fallback, we assume all turns allowed when the restriction is not valid
-      - Fixed a bug that could result in an infinite loop when finding information about an upcoming intersection
-      - Fixed a bug that led to not discovering if a road simply looses a considered prefix
-      - BREAKING: Fixed a bug that could crash postprocessing of instructions on invalid roundabout taggings. This change requires reprocessing datasets with osrm-extract and osrm-contract
-      - Fixed an issue that could emit `invalid` as instruction when ending on a sliproad after a traffic-light
-      - Fixed an issue that would detect turning circles as sliproads
-      - Fixed a bug where post-processing instructions (e.g. left + left -> uturn) could result in false pronunciations
-      - Fixes a bug where a bearing range of zero would cause exhaustive graph traversals
-      - Fixes a bug where certain looped geometries could cause an infinite loop during extraction
-
-    - Infrastructure:
-      - Adds a feature to limit results in nearest service with a default of 100 in `osrm-routed`
-
 # 5.3.0
   - Changes from 5.3.0-rc.3
     - Guidance

CMakeLists.txt

@@ -10,7 +10,7 @@ endif()
 project(OSRM C CXX)
 set(OSRM_VERSION_MAJOR 5)
 set(OSRM_VERSION_MINOR 4)
-set(OSRM_VERSION_PATCH 0)
+set(OSRM_VERSION_PATCH 3)
 
 # these two functions build up custom variables:
 #   OSRM_INCLUDE_PATHS and OSRM_DEFINES
@@ -127,8 +127,8 @@ if(ENABLE_GOLD_LINKER)
 
         # Issue 2785: check gold binutils version and don't use gc-sections for versions prior 2.25
         string(REGEX REPLACE ".*\\(GNU Binutils[^\\)0-9]+([0-9]+\\.[0-9]+)[^\\)]*\\).*" "\\1" GOLD_BINUTILS_VERSION "${LD_VERSION}")
-        if ("${GOLD_BINUTILS_VERSION}" VERSION_LESS "2.25")
+        if ("${GOLD_BINUTILS_VERSION}" VERSION_LESS "2.26")
-          message(STATUS "Disabling gc-sections on gold binutils < 2.25, see: https://sourceware.org/bugzilla/show_bug.cgi?id=17639")
+          message(STATUS "Disabling gc-sections on gold binutils < 2.26, see: https://sourceware.org/bugzilla/show_bug.cgi?id=17639")
           set(LD_AVOID_GC_SECTIONS TRUE)
         endif()
     else()
@@ -249,6 +249,8 @@ endif()
 
 # Configuring other platform dependencies
 if(APPLE)
+  set(CMAKE_OSX_DEPLOYMENT_TARGET "10.10")
+  execute_process(COMMAND xcrun --sdk macosx --show-sdk-path OUTPUT_VARIABLE CMAKE_OSX_SYSROOT OUTPUT_STRIP_TRAILING_WHITESPACE)
   set(CMAKE_OSX_ARCHITECTURES "x86_64")
   message(STATUS "Set Architecture to x64 on OS X")
   exec_program(uname ARGS -v  OUTPUT_VARIABLE DARWIN_VERSION)

example/example.cpp

@@ -34,7 +34,7 @@ int main(int argc, const char *argv[])
     config.use_shared_memory = false;
 
     // Routing machine with several services (such as Route, Table, Nearest, Trip, Match)
-    OSRM osrm{config};
+    const OSRM osrm{config};
 
     // The following shows how to use the Route service; configure this service
     RouteParameters params;

features/car/summaries.feature

@@ -23,10 +23,10 @@ Feature: Basic Routing
             | df    | street |
 
         When I route I should get
-            | waypoints | route                               | summary               |
+            | waypoints | route                               | summary                 |
-            | a,e       | road,,1 st,1 st                     | road,1 st             |
+            | a,e       | road,,1 st,1 st                     | road, 1 st              |
-            | a,d,f     | road,,,street,street                | road;street           |
+            | a,d,f     | road,,,street,street                | road;street             |
-            | a,e,f     | road,,1 st,1 st,1 st,street,street  | road,1 st;1 st,street |
+            | a,e,f     | road,,1 st,1 st,1 st,street,street  | road, 1 st;1 st, street |
 
      Scenario: Name Empty
         Given the node map
@@ -51,8 +51,8 @@ Feature: Basic Routing
             | bc    |      | 101 |
 
         When I route I should get
-            | waypoints | route | summary  |
+            | waypoints | route | summary   |
-            | a,c       | road, | road,101 |
+            | a,c       | road, | road, 101 |
 
      Scenario: Only Refs
         Given the node map
@@ -64,8 +64,8 @@ Feature: Basic Routing
             | bc    |      | 101 |
 
         When I route I should get
-            | waypoints | route  | summary |
+            | waypoints | route  | summary  |
-            | a,c       | ,      | 100,101 |
+            | a,c       | ,      | 100, 101 |
 
      Scenario: Single Ref
         Given the node map

features/guidance/roundabout.feature

@@ -403,3 +403,48 @@ Feature: Basic Roundabout
             #| w,x       | ll,egg,egg,tr,tr | depart,roundabout-exit-1,roundabout-exit-2,arrive       |
             | w,x       | ll,egg,egg,tr,tr | depart,turn right,continue left,turn slight left,arrive |
 
+    @3254
+    Scenario: Driving up to and through a roundabout
+        Given the node map
+            |   | g |  |  |  |  |   |  | a |  |   |  |   |
+            |   |   |  |  |  |  |   |  |   |  |   |  |   |
+            | e | f |  |  |  |  | b |  |   |  | d |  | h |
+            |   |   |  |  |  |  |   |  |   |  |   |  |   |
+            |   | i |  |  |  |  |   |  | c |  |   |  |   |
+            |   |   |  |  |  |  |   |  |   |  |   |  |   |
+            |   |   |  |  |  |  |   |  | k |  |   |  |   |
+
+        And the ways
+            | nodes | junction   | name       | highway     |
+            | abcda | roundabout | roundabout | residential |
+            | gfi   |            | side       | residential |
+            | efb   |            | left       | residential |
+            | dh    |            | right      | residential |
+            | ck    |            | bottom     | residential |
+
+        When I route I should get
+            | waypoints | route            | turns                           |
+            | e,h       | left,right,right | depart,roundabout-exit-2,arrive |
+
+    @3254
+    Scenario: Driving up to and through a roundabout
+        Given the node map
+            |   | g |  |   |  | a |  |   |  |   |
+            |   |   |  |   |  |   |  |   |  |   |
+            | e | f |  | b |  |   |  | d |  | h |
+            |   |   |  |   |  |   |  |   |  |   |
+            |   | i |  |   |  | c |  |   |  |   |
+            |   |   |  |   |  |   |  |   |  |   |
+            |   |   |  |   |  | k |  |   |  |   |
+
+        And the ways
+            | nodes | junction   | name       | highway     |
+            | abcda | roundabout | roundabout | residential |
+            | gfi   |            | side       | residential |
+            | efb   |            | left       | residential |
+            | dh    |            | right      | residential |
+            | ck    |            | bottom     | residential |
+
+        When I route I should get
+            | waypoints | route            | turns                           |
+            | e,h       | left,right,right | depart,roundabout-exit-2,arrive |

features/guidance/turn-location.feature

@@ -0,0 +1,21 @@
+@routing  @guidance
+Feature: Turn Location Feature
+
+    Background:
+        Given the profile "car"
+        Given a grid size of 10 meters
+
+    Scenario: Simple feature to test turn locations
+        Given the node map
+            |   | c |   |
+            | a | b | d |
+
+        And the ways
+            | nodes  | highway |
+            | ab     | primary |
+            | cb     | primary |
+            | db     | primary |
+
+       When I route I should get
+            | waypoints | route    | turns                   | locations |
+            | a,c       | ab,cb,cb | depart,turn left,arrive | a,b,c     |

features/step_definitions/matching.js

@@ -1,5 +1,6 @@
+'use strict';
+
 var util = require('util');
-var d3 = require('d3-queue');
 var polyline = require('polyline');
 
 module.exports = function () {
@@ -43,7 +44,28 @@ module.exports = function () {
 
                     if (res.statusCode === 200) {
                         if (headers.has('matchings')) {
-                            subMatchings = json.matchings.filter(m => !!m).map(sub => sub.matched_points);
+                            subMatchings = [];
+
+                            // find the first matched
+                            let start_index = 0;
+                            while (start_index < json.tracepoints.length && json.tracepoints[start_index] === null) start_index++;
+
+                            var sub = [];
+                            let prev_index = null;
+                            for(var i = start_index; i < json.tracepoints.length; i++){
+                                if (json.tracepoints[i] === null) continue;
+
+                                let current_index = json.tracepoints[i].matchings_index;
+
+                                if(prev_index !== current_index) {
+                                    if (sub.length > 0) subMatchings.push(sub);
+                                    sub = [];
+                                    prev_index = current_index;
+                                }
+
+                                sub.push(json.tracepoints[i].location);
+                            }
+                            subMatchings.push(sub);
                         }
 
                         if (headers.has('turns')) {
@@ -68,11 +90,11 @@ module.exports = function () {
 
                         if (headers.has('geometry')) {
                             if (json.matchings.length != 1) throw new Error('*** Checking geometry only supported for matchings with one subtrace');
-                            geometry = json.matchings[0].geometry;
+                            geometry = json.matchings[0].geometry.coordinates;
                         }
 
                         if (headers.has('OSM IDs')) {
-                            if (json.matchings.length != 1) throw new Error('*** CHecking annotation only supported for matchings with one subtrace');
+                            if (json.matchings.length != 1) throw new Error('*** Checking annotation only supported for matchings with one subtrace');
                             OSMIDs = this.OSMIDList(json.matchings[0]);
                         }
                     }
@@ -108,59 +130,55 @@ module.exports = function () {
                     var encodedResult = '',
                         extendedTarget = '';
 
-                    var q = d3.queue();
+                    var testSubMatching = (sub, si) => {
+                        var testSubNode = (ni) => {
+                            var node = this.findNodeByName(sub[ni]),
+                                outNode = subMatchings[si][ni];
 
-                    var testSubMatching = (sub, si, scb) => {
+                            if (this.FuzzyMatch.matchLocation(outNode, node)) {
-                        if (si >= subMatchings.length) {
+                                encodedResult += sub[ni];
-                            ok = false;
+                                extendedTarget += sub[ni];
-                            q.abort();
+                            } else {
-                            scb();
+                                if (outNode != null) {
-                        } else {
-                            var sq = d3.queue();
-
-                            var testSubNode = (ni, ncb) => {
-                                var node = this.findNodeByName(sub[ni]),
-                                    outNode = subMatchings[si][ni];
-
-                                if (this.FuzzyMatch.matchLocation(outNode, node)) {
-                                    encodedResult += sub[ni];
-                                    extendedTarget += sub[ni];
-                                } else {
                                     encodedResult += util.format('? [%s,%s]', outNode[0], outNode[1]);
-                                    extendedTarget += util.format('%s [%d,%d]', node.lat, node.lon);
+                                } else {
-                                    ok = false;
+                                    encodedResult += '?';
                                 }
-                                ncb();
+                                extendedTarget += util.format('%s [%d,%d]', node.lat, node.lon);
-                            };
+                                ok = false;
-
-                            for (var i=0; i<sub.length; i++) {
-                                sq.defer(testSubNode, i);
                             }
+                        };
 
-                            sq.awaitAll(scb);
+                        for (var i=0; i<sub.length; i++) {
+                            testSubNode(i);
                         }
                     };
 
-                    row.matchings.split(',').forEach((sub, si) => {
+                    if (headers.has('matchings')) {
-                        q.defer(testSubMatching, sub, si);
+                        if (subMatchings.length != row.matchings.split(',').length) {
-                    });
+                            ok = false;
-
+                            cb(new Error('*** table matchings and api response are not the same'));
-                    q.awaitAll(() => {
-                        if (ok) {
-                            if (headers.has('matchings')) {
-                                got.matchings = row.matchings;
-                            }
-
-                            if (headers.has('timestamps')) {
-                                got.timestamps = row.timestamps;
-                            }
-                        } else {
-                            got.matchings = encodedResult;
-                            row.matchings = extendedTarget;
                         }
 
-                        cb(null, got);
+                        row.matchings.split(',').forEach((sub, si) => {
-                    });
+                            testSubMatching(sub, si);
+                        });
+                    }
+
+                    if (ok) {
+                        if (headers.has('matchings')) {
+                            got.matchings = row.matchings;
+                        }
+
+                        if (headers.has('timestamps')) {
+                            got.timestamps = row.timestamps;
+                        }
+                    } else {
+                        got.matchings = encodedResult;
+                        row.matchings = extendedTarget;
+                    }
+
+                    cb(null, got);
                 };
 
                 if (row.request) {

features/support/data.js

@@ -126,6 +126,20 @@ module.exports = function () {
         return fromNode;
     };
 
+    // find a node based on an array containing lon/lat
+    this.findNodeByLocation = (node_location) => {
+        var searched_coordinate = new classes.Location(node_location[0],node_location[1]);
+        for (var node in this.nameNodeHash)
+        {
+            var node_coordinate = new classes.Location(this.nameNodeHash[node].lon,this.nameNodeHash[node].lat);
+            if (this.FuzzyMatch.matchCoordinate(searched_coordinate, node_coordinate, this.zoom))
+            {
+                return node;
+            }
+        }
+        return '_';
+    };
+
     this.findWayByName = (s) => {
         return this.nameWayHash[s.toString()] || this.nameWayHash[s.toString().split('').reverse().join('')];
     };

features/support/data_classes.js

@@ -107,8 +107,16 @@ module.exports = {
         }
 
         matchLocation (got, want) {
+            if (got == null || want == null) return false;
             return this.match(got[0], util.format('%d ~0.0025%', want.lon)) &&
                 this.match(got[1], util.format('%d ~0.0025%', want.lat));
         }
+
+        matchCoordinate (got, want, zoom) {
+            if (got == null || want == null) return false;
+            return this.match(got.lon, util.format('%d +- %d', want.lon, 0.25*zoom)) &&
+                this.match(got.lat, util.format('%d +- %d', want.lat, 0.25*zoom));
+        }
+
     }
 };

features/support/env.js

@@ -51,8 +51,26 @@ module.exports = function () {
         } else {
             this.TERMSIGNAL = 'SIGTERM';
             this.EXE = '';
-            // TODO autodetect if this was build with shared or static libraries
+
-            this.LIB = process.env.BUILD_SHARED_LIBS && '.so' || '.a';
+            // heuristically detect .so/.a suffix
+            this.LIB = null;
+
+            try {
+                const dot_a = util.format('%s/libosrm%s', this.BIN_PATH, '.a');
+                fs.accessSync(dot_a, fs.F_OK);
+                this.LIB = '.a';
+            } catch(e) { /*nop*/ }
+
+            try {
+                const dot_so = util.format('%s/libosrm%s', this.BIN_PATH, '.so');
+                fs.accessSync(dot_so, fs.F_OK);
+                this.LIB = '.so';
+            } catch(e) { /*nop*/ }
+
+            if (!this.LIB) {
+                throw new Error('*** Unable to detect dynamic or static libosrm libraries');
+            }
+
             this.QQ = '';
         }
 
@@ -65,7 +83,7 @@ module.exports = function () {
 
         // eslint-disable-next-line no-console
         console.info(util.format('Node Version', process.version));
-        if (parseInt(process.version.match(/v(\d)/)[1]) < 4) throw new Error('*** PLease upgrade to Node 4.+ to run OSRM cucumber tests');
+        if (parseInt(process.version.match(/v(\d)/)[1]) < 4) throw new Error('*** Please upgrade to Node 4.+ to run OSRM cucumber tests');
 
         fs.exists(this.TEST_PATH, (exists) => {
             if (exists)

features/support/route.js

@@ -211,6 +211,14 @@ module.exports = function () {
             .join(',');
     };
 
+    this.locations = (instructions) => {
+        return instructions.legs.reduce((m, v) => m.concat(v.steps), [])
+            .map(v => {
+                return this.findNodeByLocation(v.maneuver.location);
+            })
+            .join(',');
+    };
+
     this.intersectionList = (instructions) => {
         return instructions.legs.reduce((m, v) => m.concat(v.steps), [])
             .map( v => {

features/support/shared_steps.js

@@ -35,7 +35,7 @@ module.exports = function () {
                     if (err) return cb(err);
                     if (body && body.length) {
                         let destinations, pronunciations, instructions, refs, bearings, turns, modes, times,
-                            distances, summary, intersections, lanes;
+                            distances, summary, intersections, lanes, locations;
 
                         let json = JSON.parse(body);
 
@@ -54,6 +54,7 @@ module.exports = function () {
                             distances = this.distanceList(json.routes[0]);
                             lanes = this.lanesList(json.routes[0]);
                             summary = this.summary(json.routes[0]);
+                            locations = this.locations(json.routes[0]);
                         }
 
                         if (headers.has('status')) {
@@ -125,6 +126,10 @@ module.exports = function () {
                                 got.intersections = (intersections || '').trim();
                             }
 
+                            if (headers.has('locations')){
+                                got.locations = (locations || '').trim();
+                            }
+
                             var putValue = (key, value) => {
                                 if (headers.has(key)) got[key] = instructions ? value : '';
                             };

features/testbot/matching.feature

@@ -5,6 +5,8 @@ Feature: Basic Map Matching
         Given the profile "testbot"
         Given a grid size of 10 meters
         Given the extract extra arguments "--generate-edge-lookup"
+        Given the query options
+            | geometries | geojson |
 
     Scenario: Testbot - Map matching with outlier that has no candidate
         Given a grid size of 100 meters
@@ -21,7 +23,7 @@ Feature: Basic Map Matching
 
         When I match I should get
             | trace | timestamps | matchings |
-            | ab1d  | 0 1 2 3    | abcd      |
+            | ab1d  | 0 1 2 3    | ad        |
 
     Scenario: Testbot - Map matching with trace splitting
         Given the node map
@@ -102,13 +104,13 @@ Feature: Basic Map Matching
             | fe    | yes    |
 
         When I match I should get
-            | trace | matchings |
+            | trace | matchings   |
-            | dcba  | hg,gf,fe  |
+            | dcba  | hgfe        |
-            | efgh  | ab,bc,cd  |
+            | efgh  | abcd        |
 
     Scenario: Testbot - Duration details
         Given the query options
-            | annotations | true |
+            | annotations | true    |
 
         Given the node map
             | a | b | c | d | e |   | g | h |
@@ -128,23 +130,47 @@ Feature: Basic Map Matching
 
         When I match I should get
             | trace | matchings | annotation                                                                                     |
-            | abeh  | abcedgh   | 1:9.897633:1,0:0:0,1:10.008842:0,1:10.008842:0,1:10.008842:0,0:0:0,2:20.017685:0,1:10.008842:0 |
+            | abeh  | abeh      | 1:9.897633:1,0:0:0,1:10.008842:0,1:10.008842:0,1:10.008842:0,0:0:0,2:20.017685:0,1:10.008842:0 |
-            | abci  | abc,ci    | 1:9.897633:1,0:0:0,1:10.008842:0,0:0.111209:0,1:10.010367:0                                    |
+            | abci  | abci      | 1:9.897633:1,0:0:0,1:10.008842:0,0:0.111209:0,1:10.010367:0                                    |
 
         # The following is the same as the above, but separated for readability (line length)
         When I match I should get
             | trace | matchings | OSM IDs               |
-            | abeh  | abcedgh   | 1,2,3,2,3,4,5,4,5,6,7 |
+            | abeh  | abeh      | 1,2,3,2,3,4,5,4,5,6,7 |
-            | abci  | abc,ci    | 1,2,3,2,3,8,3,8       |
+            | abci  | abci      | 1,2,3,2,3,8,3,8       |
+
+    Scenario: Testbot - Regression test for #3037
+        Given the query options
+            | overview   | simplified |
+            | geometries | geojson  |
+
+        Given the node map
+            | a |   | b |   | c |
+            |   |   |   |   |   |
+            |   |   |   |   |   |
+            |   |   |   |   |   |
+            | e |   | f |   | g |
+
+        And the ways
+            | nodes | oneway |
+            | abc   | yes    |
+            | efg   | yes    |
+            | ae    | yes    |
+            | cg    | yes    |
+            | fb    | yes    |
+
+        When I match I should get
+            | trace | matchings | geometry                                         |
+            | efbc  | efbc      | 1,0.99964,1.000178,0.99964,1.000178,1,1.000359,1 |
 
     Scenario: Testbot - Geometry details
         Given the query options
             | overview   | full     |
-            | geometries | polyline |
+            | geometries | geojson  |
 
         Given the node map
-            | a | b | c |
+            | a |   | b |   | c |
-            |   | d |   |
+            |   |   | d |   |   |
 
         And the ways
             | nodes | oneway |
@@ -152,5 +178,64 @@ Feature: Basic Map Matching
             | bd    | no     |
 
         When I match I should get
-            | trace | matchings | geometry                                |
+            | trace | matchings | geometry                                   |
-            | abd   | abd       | 1,1,1,1.00009,1,1.00009,0.99991,1.00009 |
+            | abd   | abd       | 1,1,1.000179,1,1.000178,1,1.000178,0.99991 |
+
+    # Regression test 1 for issue 3176
+    Scenario: Testbot - multiuple segments: properly expose OSM IDs
+        Given the query options
+            | annotations | true    |
+
+        Given the node map
+            | a | 1 | b |  | c |  | d |  | e |   | f | 2 | g |
+
+        And the nodes
+            | node | id |
+            | a    | 1  |
+            | b    | 2  |
+            | c    | 3  |
+            | d    | 4  |
+            | e    | 5  |
+            | f    | 6  |
+            | g    | 7  |
+
+        And the ways
+            | nodes | oneway |
+            | ab    | no     |
+            | bc    | no     |
+            | cd    | no     |
+            | de    | no     |
+            | ef    | no     |
+            | fg    | no     |
+
+        When I match I should get
+            | trace | OSM IDs       |
+            | 12    | 1,2,3,4,5,6,7 |
+            | 21    | 7,6,5,4,3,2,1 |
+
+    # Regression test 2 for issue 3176
+    Scenario: Testbot - same edge: properly expose OSM IDs
+        Given the query options
+            | annotations | true    |
+
+        Given the node map
+            | a | 1 | b |  | c |  | d |  | e | 2 | f |
+
+        And the nodes
+            | node | id |
+            | a    | 1  |
+            | b    | 2  |
+            | c    | 3  |
+            | d    | 4  |
+            | e    | 5  |
+            | f    | 6  |
+
+        And the ways
+            | nodes   | oneway |
+            | abcdef  | no     |
+
+        When I match I should get
+            | trace | OSM IDs     |
+            | 12    | 1,2,3,4,5,6 |
+            | 21    | 6,5,4,3,2,1 |
+

features/testbot/summary.feature

@@ -18,12 +18,12 @@ Feature: Basic Routing
             | de    |
 
         When I route I should get
-            | from | to | route          | summary |
+            | from | to | route          | summary  |
-            | a    | e  | ab,bc,cd,de,de | ab,bc   |
+            | a    | e  | ab,bc,cd,de,de | ab, bc   |
-            | e    | a  | de,cd,bc,ab,ab | de,bc   |
+            | e    | a  | de,cd,bc,ab,ab | de, bc   |
-            | a    | b  | ab,ab          | ab      |
+            | a    | b  | ab,ab          | ab       |
-            | b    | d  | bc,cd,cd       | bc,cd   |
+            | b    | d  | bc,cd,cd       | bc, cd   |
-            | 1    | c  | bc,bc          | bc      |
+            | 1    | c  | bc,bc          | bc       |
 
     @smallest
     Scenario: Check handling empty values
@@ -40,8 +40,8 @@ Feature: Basic Routing
             | df    | df   |
 
         When I route I should get
-            | from | to | route          | summary |
+            | from | to | route          | summary  |
-            | e    | a  | de,,bc,ab,ab   | de,bc   |
+            | e    | a  | de,,bc,ab,ab   | de, bc   |
 
     @smallest @todo
     Scenario: Summaries when routing on a simple network
@@ -74,6 +74,6 @@ Feature: Basic Routing
             | xey   | cross  |we need this because phantom node segments are not considered for the summary |
 
         When I route I should get
-            | from | to | route                     | summary      |
+            | from | to | route                     | summary       |
-            | a    | 1  | first,first,second,second | first,second |
+            | a    | 1  | first,first,second,second | first, second |
 

include/engine/api/route_api.hpp

@@ -94,8 +94,12 @@ class RouteAPI : public BaseAPI
             const bool reversed_source = source_traversed_in_reverse[idx];
             const bool reversed_target = target_traversed_in_reverse[idx];
 
-            auto leg_geometry = guidance::assembleGeometry(
+            auto leg_geometry = guidance::assembleGeometry(BaseAPI::facade,
-                BaseAPI::facade, path_data, phantoms.source_phantom, phantoms.target_phantom);
+                                                           path_data,
+                                                           phantoms.source_phantom,
+                                                           phantoms.target_phantom,
+                                                           reversed_source,
+                                                           reversed_target);
             auto leg = guidance::assembleLeg(facade,
                                              path_data,
                                              leg_geometry,

include/engine/api/route_parameters.hpp

@@ -69,6 +69,21 @@ struct RouteParameters : public BaseParameters
 
     RouteParameters() = default;
 
+    template <typename... Args>
+    RouteParameters(const bool steps_,
+                    const bool alternatives_,
+                    const GeometriesType geometries_,
+                    const OverviewType overview_,
+                    const boost::optional<bool> continue_straight_,
+                    Args... args_)
+        : BaseParameters{std::forward<Args>(args_)...}, steps{steps_}, alternatives{alternatives_},
+          annotations{false}, geometries{geometries_}, overview{overview_},
+          continue_straight{continue_straight_}
+    // Once we perfectly-forward `args` (see #2990) this constructor can delegate to the one below.
+    {
+    }
+
+    // RouteParameters constructor adding the `annotations` setting in a API-compatible way.
     template <typename... Args>
     RouteParameters(const bool steps_,
                     const bool alternatives_,

include/engine/engine.hpp

@@ -1,7 +1,6 @@
 #ifndef ENGINE_HPP
 #define ENGINE_HPP
 
-#include "storage/shared_barriers.hpp"
 #include "engine/status.hpp"
 #include "util/json_container.hpp"
 
@@ -20,6 +19,11 @@ struct Object;
 }
 }
 
+namespace storage
+{
+struct SharedBarriers;
+}
+
 // Fwd decls
 namespace engine
 {
@@ -52,10 +56,7 @@ class BaseDataFacade;
 class Engine final
 {
   public:
-    // Needs to be public
+    explicit Engine(const EngineConfig &config);
-    struct EngineLock;
-
-    explicit Engine(EngineConfig &config);
 
     Engine(Engine &&) noexcept;
     Engine &operator=(Engine &&) noexcept;
@@ -63,15 +64,15 @@ class Engine final
     // Impl. in cpp since for unique_ptr of incomplete types
     ~Engine();
 
-    Status Route(const api::RouteParameters &parameters, util::json::Object &result);
+    Status Route(const api::RouteParameters &parameters, util::json::Object &result) const;
-    Status Table(const api::TableParameters &parameters, util::json::Object &result);
+    Status Table(const api::TableParameters &parameters, util::json::Object &result) const;
-    Status Nearest(const api::NearestParameters &parameters, util::json::Object &result);
+    Status Nearest(const api::NearestParameters &parameters, util::json::Object &result) const;
-    Status Trip(const api::TripParameters &parameters, util::json::Object &result);
+    Status Trip(const api::TripParameters &parameters, util::json::Object &result) const;
-    Status Match(const api::MatchParameters &parameters, util::json::Object &result);
+    Status Match(const api::MatchParameters &parameters, util::json::Object &result) const;
-    Status Tile(const api::TileParameters &parameters, std::string &result);
+    Status Tile(const api::TileParameters &parameters, std::string &result) const;
 
   private:
-    std::unique_ptr<EngineLock> lock;
+    std::unique_ptr<storage::SharedBarriers> lock;
 
     std::unique_ptr<plugins::ViaRoutePlugin> route_plugin;
     std::unique_ptr<plugins::TablePlugin> table_plugin;

include/engine/guidance/assemble_geometry.hpp

@@ -35,7 +35,9 @@ namespace guidance
 inline LegGeometry assembleGeometry(const datafacade::BaseDataFacade &facade,
                                     const std::vector<PathData> &leg_data,
                                     const PhantomNode &source_node,
-                                    const PhantomNode &target_node)
+                                    const PhantomNode &target_node,
+                                    const bool reversed_source,
+                                    const bool reversed_target)
 {
     LegGeometry geometry;
 
@@ -43,16 +45,30 @@ inline LegGeometry assembleGeometry(const datafacade::BaseDataFacade &facade,
     geometry.segment_offsets.push_back(0);
     geometry.locations.push_back(source_node.location);
 
-    // Need to get the node ID preceding the source phantom node
+    //                          u       *      v
-    // TODO: check if this was traversed in reverse?
+    //                          0 -- 1 -- 2 -- 3
-    std::vector<NodeID> reverse_geometry;
+    // fwd_segment_position:  1
-    facade.GetUncompressedGeometry(source_node.reverse_packed_geometry_id, reverse_geometry);
+    // source node fwd:       1      1 -> 2 -> 3
-    geometry.osm_node_ids.push_back(facade.GetOSMNodeIDOfNode(
+    // source node rev:       2 0 <- 1 <- 2
-        reverse_geometry[reverse_geometry.size() - source_node.fwd_segment_position - 1]));
+    const auto source_segment_start_coordinate =
+        source_node.fwd_segment_position + (reversed_source ? 1 : 0);
 
-    std::vector<uint8_t> forward_datasource_vector;
+    // we don't save the first node id in the forward geometry, we need to get it as last coordinate from the reverse
-    facade.GetUncompressedDatasources(source_node.forward_packed_geometry_id,
+    // geometry
-                                      forward_datasource_vector);
+    if (source_segment_start_coordinate == 0)
+    {
+        std::vector<NodeID> source_geometry;
+        facade.GetUncompressedGeometry(source_node.reverse_packed_geometry_id, source_geometry);
+        geometry.osm_node_ids.push_back(
+            facade.GetOSMNodeIDOfNode(source_geometry.back()));
+    }
+    else
+    {
+        std::vector<NodeID> source_geometry;
+        facade.GetUncompressedGeometry(source_node.forward_packed_geometry_id, source_geometry);
+        geometry.osm_node_ids.push_back(
+            facade.GetOSMNodeIDOfNode(source_geometry[source_segment_start_coordinate - 1]));
+    }
 
     auto cumulative_distance = 0.;
     auto current_distance = 0.;
@@ -94,12 +110,29 @@ inline LegGeometry assembleGeometry(const datafacade::BaseDataFacade &facade,
     geometry.segment_offsets.push_back(geometry.locations.size());
     geometry.locations.push_back(target_node.location);
 
-    // Need to get the node ID following the destination phantom node
+    //                           u       *      v
-    // TODO: check if this was traversed in reverse??
+    //                           0 -- 1 -- 2 -- 3
-    std::vector<NodeID> forward_geometry;
+    // fwd_segment_position:  1
-    facade.GetUncompressedGeometry(target_node.forward_packed_geometry_id, forward_geometry);
+    // target node fwd:       2  0 -> 1 -> 2
-    geometry.osm_node_ids.push_back(
+    // target node rev:       1       1 <- 2 <- 3
-        facade.GetOSMNodeIDOfNode(forward_geometry[target_node.fwd_segment_position]));
+    const auto target_segment_end_coordinate =
+        target_node.fwd_segment_position + (reversed_target ? 0 : 1);
+    // we don't save the first node id in the forward geometry, we need to get it as last coordinate from the reverse
+    // geometry
+    if (target_segment_end_coordinate == 0)
+    {
+        std::vector<NodeID> target_geometry;
+        facade.GetUncompressedGeometry(target_node.reverse_packed_geometry_id, target_geometry);
+        geometry.osm_node_ids.push_back(
+            facade.GetOSMNodeIDOfNode(target_geometry.back()));
+    }
+    else
+    {
+        std::vector<NodeID> target_geometry;
+        facade.GetUncompressedGeometry(target_node.forward_packed_geometry_id, target_geometry);
+        geometry.osm_node_ids.push_back(
+            facade.GetOSMNodeIDOfNode(target_geometry[target_segment_end_coordinate - 1]));
+    }
 
     BOOST_ASSERT(geometry.segment_distances.size() == geometry.segment_offsets.size() - 1);
     BOOST_ASSERT(geometry.locations.size() > geometry.segment_distances.size());

include/engine/guidance/assemble_leg.hpp

@@ -198,7 +198,7 @@ inline RouteLeg assembleLeg(const datafacade::BaseDataFacade &facade,
 
         const auto summary_names = summary_array | boost::adaptors::transformed(name_id_to_string) |
                                    boost::adaptors::filtered(not_empty);
-        summary = boost::algorithm::join(summary_names, ",");
+        summary = boost::algorithm::join(summary_names, ", ");
     }
 
     return RouteLeg{duration, distance, summary, {}};

include/engine/guidance/assemble_steps.hpp

@@ -213,13 +213,6 @@ inline std::vector<RouteStep> assembleSteps(const datafacade::BaseDataFacade &fa
 
     BOOST_ASSERT(segment_index == number_of_segments - 1);
     bearings = detail::getArriveBearings(leg_geometry);
-    // This step has length zero, the only reason we need it is the target location
-    maneuver = {intersection.location,
-                bearings.first,
-                bearings.second,
-                extractor::guidance::TurnInstruction::NO_TURN(),
-                WaypointType::Arrive,
-                0};
 
     intersection = {
         target_node.location,
@@ -230,6 +223,15 @@ inline std::vector<RouteStep> assembleSteps(const datafacade::BaseDataFacade &fa
         util::guidance::LaneTupel(),
         {}};
 
+    // This step has length zero, the only reason we need it is the target location
+    maneuver = {intersection.location,
+                bearings.first,
+                bearings.second,
+                extractor::guidance::TurnInstruction::NO_TURN(),
+                WaypointType::Arrive,
+                0};
+
+
     BOOST_ASSERT(!leg_geometry.locations.empty());
     steps.push_back(RouteStep{target_node.name_id,
                               facade.GetNameForID(target_node.name_id),

include/engine/routing_algorithms/map_matching.hpp

@@ -176,24 +176,133 @@ class MapMatching final : public BasicRoutingInterface<DataFacadeT, MapMatching<
         prev_unbroken_timestamps.push_back(initial_timestamp);
         for (auto t = initial_timestamp + 1; t < candidates_list.size(); ++t)
         {
+
+            const bool gap_in_trace = [&, use_timestamps]() {
+                // use temporal information if available to determine a split
+                if (use_timestamps)
+                {
+                    return trace_timestamps[t] - trace_timestamps[prev_unbroken_timestamps.back()] >
+                           max_broken_time;
+                }
+                else
+                {
+                    return t - prev_unbroken_timestamps.back() > MAX_BROKEN_STATES;
+                }
+            }();
+
+            if (!gap_in_trace)
+            {
+                BOOST_ASSERT(!prev_unbroken_timestamps.empty());
+                const std::size_t prev_unbroken_timestamp = prev_unbroken_timestamps.back();
+
+                const auto &prev_viterbi = model.viterbi[prev_unbroken_timestamp];
+                const auto &prev_pruned = model.pruned[prev_unbroken_timestamp];
+                const auto &prev_unbroken_timestamps_list =
+                    candidates_list[prev_unbroken_timestamp];
+                const auto &prev_coordinate = trace_coordinates[prev_unbroken_timestamp];
+
+                auto &current_viterbi = model.viterbi[t];
+                auto &current_pruned = model.pruned[t];
+                auto &current_parents = model.parents[t];
+                auto &current_lengths = model.path_distances[t];
+                const auto &current_timestamps_list = candidates_list[t];
+                const auto &current_coordinate = trace_coordinates[t];
+
+                const auto haversine_distance = util::coordinate_calculation::haversineDistance(
+                    prev_coordinate, current_coordinate);
+                // assumes minumum of 0.1 m/s
+                const int duration_upper_bound =
+                    ((haversine_distance + max_distance_delta) * 0.25) * 10;
+
+                // compute d_t for this timestamp and the next one
+                for (const auto s : util::irange<std::size_t>(0UL, prev_viterbi.size()))
+                {
+                    if (prev_pruned[s])
+                    {
+                        continue;
+                    }
+
+                    for (const auto s_prime :
+                         util::irange<std::size_t>(0UL, current_viterbi.size()))
+                    {
+                        const double emission_pr = emission_log_probabilities[t][s_prime];
+                        double new_value = prev_viterbi[s] + emission_pr;
+                        if (current_viterbi[s_prime] > new_value)
+                        {
+                            continue;
+                        }
+
+                        forward_heap.Clear();
+                        reverse_heap.Clear();
+
+                        double network_distance;
+                        if (super::facade->GetCoreSize() > 0)
+                        {
+                            forward_core_heap.Clear();
+                            reverse_core_heap.Clear();
+                            network_distance = super::GetNetworkDistanceWithCore(
+                                forward_heap,
+                                reverse_heap,
+                                forward_core_heap,
+                                reverse_core_heap,
+                                prev_unbroken_timestamps_list[s].phantom_node,
+                                current_timestamps_list[s_prime].phantom_node,
+                                duration_upper_bound);
+                        }
+                        else
+                        {
+                            network_distance = super::GetNetworkDistance(
+                                forward_heap,
+                                reverse_heap,
+                                prev_unbroken_timestamps_list[s].phantom_node,
+                                current_timestamps_list[s_prime].phantom_node);
+                        }
+
+                        // get distance diff between loc1/2 and locs/s_prime
+                        const auto d_t = std::abs(network_distance - haversine_distance);
+
+                        // very low probability transition -> prune
+                        if (d_t >= max_distance_delta)
+                        {
+                            continue;
+                        }
+
+                        const double transition_pr = transition_log_probability(d_t);
+                        new_value += transition_pr;
+
+                        if (new_value > current_viterbi[s_prime])
+                        {
+                            current_viterbi[s_prime] = new_value;
+                            current_parents[s_prime] = std::make_pair(prev_unbroken_timestamp, s);
+                            current_lengths[s_prime] = network_distance;
+                            current_pruned[s_prime] = false;
+                            model.breakage[t] = false;
+                        }
+                    }
+                }
+
+                if (model.breakage[t])
+                {
+                    // save start of breakage -> we need this as split point
+                    if (t < breakage_begin)
+                    {
+                        breakage_begin = t;
+                    }
+
+                    BOOST_ASSERT(prev_unbroken_timestamps.size() > 0);
+                    // remove both ends of the breakage
+                    prev_unbroken_timestamps.pop_back();
+                }
+                else
+                {
+                    prev_unbroken_timestamps.push_back(t);
+                }
+            }
+
             // breakage recover has removed all previous good points
-            bool trace_split = prev_unbroken_timestamps.empty();
+            const bool trace_split = prev_unbroken_timestamps.empty();
 
-            // use temporal information if available to determine a split
+            if (trace_split || gap_in_trace)
-            if (use_timestamps)
-            {
-                trace_split =
-                    trace_split ||
-                    (trace_timestamps[t] - trace_timestamps[prev_unbroken_timestamps.back()] >
-                     max_broken_time);
-            }
-            else
-            {
-                trace_split =
-                    trace_split || (t - prev_unbroken_timestamps.back() > MAX_BROKEN_STATES);
-            }
-
-            if (trace_split)
             {
                 std::size_t split_index = t;
                 if (breakage_begin != map_matching::INVALID_STATE)
@@ -217,111 +326,9 @@ class MapMatching final : public BasicRoutingInterface<DataFacadeT, MapMatching<
                 // Important: We potentially go back here!
                 // However since t > new_start >= breakge_begin
                 // we can only reset trace_coordindates.size() times.
-                t = new_start + 1;
+                t = new_start;
-            }
+                // note: the head of the loop will call ++t, hence the next
-
+                // iteration will actually be on new_start+1
-            BOOST_ASSERT(!prev_unbroken_timestamps.empty());
-            const std::size_t prev_unbroken_timestamp = prev_unbroken_timestamps.back();
-
-            const auto &prev_viterbi = model.viterbi[prev_unbroken_timestamp];
-            const auto &prev_pruned = model.pruned[prev_unbroken_timestamp];
-            const auto &prev_unbroken_timestamps_list = candidates_list[prev_unbroken_timestamp];
-            const auto &prev_coordinate = trace_coordinates[prev_unbroken_timestamp];
-
-            auto &current_viterbi = model.viterbi[t];
-            auto &current_pruned = model.pruned[t];
-            auto &current_parents = model.parents[t];
-            auto &current_lengths = model.path_distances[t];
-            const auto &current_timestamps_list = candidates_list[t];
-            const auto &current_coordinate = trace_coordinates[t];
-
-            const auto haversine_distance = util::coordinate_calculation::haversineDistance(
-                prev_coordinate, current_coordinate);
-            // assumes minumum of 0.1 m/s
-            const int duration_uppder_bound =
-                ((haversine_distance + max_distance_delta) * 0.25) * 10;
-
-            // compute d_t for this timestamp and the next one
-            for (const auto s : util::irange<std::size_t>(0UL, prev_viterbi.size()))
-            {
-                if (prev_pruned[s])
-                {
-                    continue;
-                }
-
-                for (const auto s_prime : util::irange<std::size_t>(0UL, current_viterbi.size()))
-                {
-                    const double emission_pr = emission_log_probabilities[t][s_prime];
-                    double new_value = prev_viterbi[s] + emission_pr;
-                    if (current_viterbi[s_prime] > new_value)
-                    {
-                        continue;
-                    }
-
-                    forward_heap.Clear();
-                    reverse_heap.Clear();
-
-                    double network_distance;
-                    if (super::facade->GetCoreSize() > 0)
-                    {
-                        forward_core_heap.Clear();
-                        reverse_core_heap.Clear();
-                        network_distance = super::GetNetworkDistanceWithCore(
-                            forward_heap,
-                            reverse_heap,
-                            forward_core_heap,
-                            reverse_core_heap,
-                            prev_unbroken_timestamps_list[s].phantom_node,
-                            current_timestamps_list[s_prime].phantom_node,
-                            duration_uppder_bound);
-                    }
-                    else
-                    {
-                        network_distance = super::GetNetworkDistance(
-                            forward_heap,
-                            reverse_heap,
-                            prev_unbroken_timestamps_list[s].phantom_node,
-                            current_timestamps_list[s_prime].phantom_node);
-                    }
-
-                    // get distance diff between loc1/2 and locs/s_prime
-                    const auto d_t = std::abs(network_distance - haversine_distance);
-
-                    // very low probability transition -> prune
-                    if (d_t >= max_distance_delta)
-                    {
-                        continue;
-                    }
-
-                    const double transition_pr = transition_log_probability(d_t);
-                    new_value += transition_pr;
-
-                    if (new_value > current_viterbi[s_prime])
-                    {
-                        current_viterbi[s_prime] = new_value;
-                        current_parents[s_prime] = std::make_pair(prev_unbroken_timestamp, s);
-                        current_lengths[s_prime] = network_distance;
-                        current_pruned[s_prime] = false;
-                        model.breakage[t] = false;
-                    }
-                }
-            }
-
-            if (model.breakage[t])
-            {
-                // save start of breakage -> we need this as split point
-                if (t < breakage_begin)
-                {
-                    breakage_begin = t;
-                }
-
-                BOOST_ASSERT(prev_unbroken_timestamps.size() > 0);
-                // remove both ends of the breakage
-                prev_unbroken_timestamps.pop_back();
-            }
-            else
-            {
-                prev_unbroken_timestamps.push_back(t);
             }
         }
 

include/engine/routing_algorithms/routing_base.hpp

@@ -215,12 +215,13 @@ template <class DataFacadeT, class Derived> class BasicRoutingInterface
                     const PhantomNodes &phantom_node_pair,
                     std::vector<PathData> &unpacked_path) const
     {
+        BOOST_ASSERT(std::distance(packed_path_begin, packed_path_end) > 0);
+
         const bool start_traversed_in_reverse =
             (*packed_path_begin != phantom_node_pair.source_phantom.forward_segment_id.id);
         const bool target_traversed_in_reverse =
             (*std::prev(packed_path_end) != phantom_node_pair.target_phantom.forward_segment_id.id);
 
-        BOOST_ASSERT(std::distance(packed_path_begin, packed_path_end) > 0);
         std::stack<std::pair<NodeID, NodeID>> recursion_stack;
 
         // We have to push the path in reverse order onto the stack because it's LIFO.

include/extractor/profile_properties.hpp

@@ -12,7 +12,7 @@ struct ProfileProperties
 {
     ProfileProperties()
         : traffic_signal_penalty(0), u_turn_penalty(0), continue_straight_at_waypoint(true),
-          use_turn_restrictions(false), left_hand_driving(false)
+        use_turn_restrictions(false), left_hand_driving(false)
     {
     }
 

include/osrm/osrm.hpp

@@ -83,7 +83,7 @@ class OSRM final
      * \return Status indicating success for the query or failure
      * \see Status, RouteParameters and json::Object
      */
-    Status Route(const RouteParameters &parameters, json::Object &result);
+    Status Route(const RouteParameters &parameters, json::Object &result) const;
 
     /**
      * Distance tables for coordinates.
@@ -92,7 +92,7 @@ class OSRM final
      * \return Status indicating success for the query or failure
      * \see Status, TableParameters and json::Object
      */
-    Status Table(const TableParameters &parameters, json::Object &result);
+    Status Table(const TableParameters &parameters, json::Object &result) const;
 
     /**
      * Nearest street segment for coordinate.
@@ -101,7 +101,7 @@ class OSRM final
      * \return Status indicating success for the query or failure
      * \see Status, NearestParameters and json::Object
      */
-    Status Nearest(const NearestParameters &parameters, json::Object &result);
+    Status Nearest(const NearestParameters &parameters, json::Object &result) const;
 
     /**
      * Trip: shortest round trip between coordinates.
@@ -110,7 +110,7 @@ class OSRM final
      * \return Status indicating success for the query or failure
      * \see Status, TripParameters and json::Object
      */
-    Status Trip(const TripParameters &parameters, json::Object &result);
+    Status Trip(const TripParameters &parameters, json::Object &result) const;
 
     /**
      * Match: snaps noisy coordinate traces to the road network
@@ -119,7 +119,7 @@ class OSRM final
      * \return Status indicating success for the query or failure
      * \see Status, MatchParameters and json::Object
      */
-    Status Match(const MatchParameters &parameters, json::Object &result);
+    Status Match(const MatchParameters &parameters, json::Object &result) const;
 
     /**
      * Tile: vector tiles with internal graph representation
@@ -128,7 +128,7 @@ class OSRM final
      * \return Status indicating success for the query or failure
      * \see Status, TileParameters and json::Object
      */
-    Status Tile(const TileParameters &parameters, std::string &result);
+    Status Tile(const TileParameters &parameters, std::string &result) const;
 
   private:
     std::unique_ptr<engine::Engine> engine_;

include/storage/shared_barriers.hpp

@@ -3,6 +3,7 @@
 
 #include <boost/interprocess/sync/named_condition.hpp>
 #include <boost/interprocess/sync/named_mutex.hpp>
+#include <boost/interprocess/sync/named_sharable_mutex.hpp>
 
 namespace osrm
 {
@@ -13,25 +14,13 @@ struct SharedBarriers
 
     SharedBarriers()
         : pending_update_mutex(boost::interprocess::open_or_create, "pending_update"),
-          update_mutex(boost::interprocess::open_or_create, "update"),
+          query_mutex(boost::interprocess::open_or_create, "query")
-          query_mutex(boost::interprocess::open_or_create, "query"),
-          no_running_queries_condition(boost::interprocess::open_or_create, "no_running_queries"),
-          update_ongoing(false), number_of_queries(0)
     {
     }
 
     // Mutex to protect access to the boolean variable
     boost::interprocess::named_mutex pending_update_mutex;
-    boost::interprocess::named_mutex update_mutex;
+    boost::interprocess::named_sharable_mutex query_mutex;
-    boost::interprocess::named_mutex query_mutex;
-
-    // Condition that no update is running
-    boost::interprocess::named_condition no_running_queries_condition;
-
-    // Is there an ongoing update?
-    bool update_ongoing;
-    // Is there any query?
-    int number_of_queries;
 };
 }
 }

include/util/debug.hpp

@@ -23,6 +23,7 @@ inline void print(const engine::guidance::RouteStep &step)
     std::cout << static_cast<int>(step.maneuver.instruction.type) << " "
               << static_cast<int>(step.maneuver.instruction.direction_modifier) << "  "
               << static_cast<int>(step.maneuver.waypoint_type) << " "
+              << step.maneuver.location << " "
               << " Duration: " << step.duration << " Distance: " << step.distance
               << " Geometry: " << step.geometry_begin << " " << step.geometry_end
               << "\n\tIntersections: " << step.intersections.size() << " [";

src/engine/engine.cpp

@@ -1,5 +1,5 @@
-#include "engine/engine.hpp"
 #include "engine/api/route_parameters.hpp"
+#include "engine/engine.hpp"
 #include "engine/engine_config.hpp"
 #include "engine/status.hpp"
 
@@ -21,6 +21,7 @@
 #include <boost/assert.hpp>
 #include <boost/interprocess/sync/named_condition.hpp>
 #include <boost/interprocess/sync/scoped_lock.hpp>
+#include <boost/interprocess/sync/sharable_lock.hpp>
 #include <boost/thread/lock_types.hpp>
 
 #include <algorithm>
@@ -28,68 +29,17 @@
 #include <utility>
 #include <vector>
 
-namespace osrm
-{
-namespace engine
-{
-struct Engine::EngineLock
-{
-    // will only be initialized if shared memory is used
-    storage::SharedBarriers barrier;
-    // decrease number of concurrent queries
-    void DecreaseQueryCount();
-    // increase number of concurrent queries
-    void IncreaseQueryCount();
-};
-
-// decrease number of concurrent queries
-void Engine::EngineLock::DecreaseQueryCount()
-{
-    // lock query
-    boost::interprocess::scoped_lock<boost::interprocess::named_mutex> query_lock(
-        barrier.query_mutex);
-
-    // decrement query count
-    --(barrier.number_of_queries);
-    BOOST_ASSERT_MSG(0 <= barrier.number_of_queries, "invalid number of queries");
-
-    // notify all processes that were waiting for this condition
-    if (0 == barrier.number_of_queries)
-    {
-        barrier.no_running_queries_condition.notify_all();
-    }
-}
-
-// increase number of concurrent queries
-void Engine::EngineLock::IncreaseQueryCount()
-{
-    // lock update pending
-    boost::interprocess::scoped_lock<boost::interprocess::named_mutex> pending_lock(
-        barrier.pending_update_mutex);
-
-    // lock query
-    boost::interprocess::scoped_lock<boost::interprocess::named_mutex> query_lock(
-        barrier.query_mutex);
-
-    // unlock update pending
-    pending_lock.unlock();
-
-    // increment query count
-    ++(barrier.number_of_queries);
-}
-} // ns engine
-} // ns osrm
-
 namespace
 {
 // Abstracted away the query locking into a template function
 // Works the same for every plugin.
 template <typename ParameterT, typename PluginT, typename ResultT>
-osrm::engine::Status RunQuery(const std::unique_ptr<osrm::engine::Engine::EngineLock> &lock,
+osrm::engine::Status
-                              osrm::engine::datafacade::BaseDataFacade &facade,
+RunQuery(const std::unique_ptr<osrm::storage::SharedBarriers> &lock,
-                              const ParameterT &parameters,
+         osrm::engine::datafacade::BaseDataFacade &facade,
-                              PluginT &plugin,
+         const ParameterT &parameters,
-                              ResultT &result)
+         PluginT &plugin,
+         ResultT &result)
 {
     if (!lock)
     {
@@ -97,7 +47,10 @@ osrm::engine::Status RunQuery(const std::unique_ptr<osrm::engine::Engine::Engine
     }
 
     BOOST_ASSERT(lock);
-    lock->IncreaseQueryCount();
+    // this locks aquires shared ownership of the query mutex: other requets are allowed
+    // to run, but data updates need to wait for all queries to finish until they can aquire an exclusive lock
+    boost::interprocess::sharable_lock<boost::interprocess::named_sharable_mutex> query_lock(
+        lock->query_mutex);
 
     auto &shared_facade = static_cast<osrm::engine::datafacade::SharedDataFacade &>(facade);
     shared_facade.CheckAndReloadFacade();
@@ -107,7 +60,6 @@ osrm::engine::Status RunQuery(const std::unique_ptr<osrm::engine::Engine::Engine
 
     osrm::engine::Status status = plugin.HandleRequest(parameters, result);
 
-    lock->DecreaseQueryCount();
     return status;
 }
 
@@ -124,11 +76,12 @@ namespace osrm
 namespace engine
 {
 
-Engine::Engine(EngineConfig &config)
+Engine::Engine(const EngineConfig &config)
+    : lock(config.use_shared_memory ? std::make_unique<storage::SharedBarriers>()
+                                    : std::unique_ptr<storage::SharedBarriers>())
 {
     if (config.use_shared_memory)
     {
-        lock = util::make_unique<EngineLock>();
         query_data_facade = util::make_unique<datafacade::SharedDataFacade>();
     }
     else
@@ -157,32 +110,32 @@ Engine::~Engine() = default;
 Engine::Engine(Engine &&) noexcept = default;
 Engine &Engine::operator=(Engine &&) noexcept = default;
 
-Status Engine::Route(const api::RouteParameters &params, util::json::Object &result)
+Status Engine::Route(const api::RouteParameters &params, util::json::Object &result) const
 {
     return RunQuery(lock, *query_data_facade, params, *route_plugin, result);
 }
 
-Status Engine::Table(const api::TableParameters &params, util::json::Object &result)
+Status Engine::Table(const api::TableParameters &params, util::json::Object &result) const
 {
     return RunQuery(lock, *query_data_facade, params, *table_plugin, result);
 }
 
-Status Engine::Nearest(const api::NearestParameters &params, util::json::Object &result)
+Status Engine::Nearest(const api::NearestParameters &params, util::json::Object &result) const
 {
     return RunQuery(lock, *query_data_facade, params, *nearest_plugin, result);
 }
 
-Status Engine::Trip(const api::TripParameters &params, util::json::Object &result)
+Status Engine::Trip(const api::TripParameters &params, util::json::Object &result) const
 {
     return RunQuery(lock, *query_data_facade, params, *trip_plugin, result);
 }
 
-Status Engine::Match(const api::MatchParameters &params, util::json::Object &result)
+Status Engine::Match(const api::MatchParameters &params, util::json::Object &result) const
 {
     return RunQuery(lock, *query_data_facade, params, *match_plugin, result);
 }
 
-Status Engine::Tile(const api::TileParameters &params, std::string &result)
+Status Engine::Tile(const api::TileParameters &params, std::string &result) const
 {
     return RunQuery(lock, *query_data_facade, params, *tile_plugin, result);
 }

src/engine/guidance/assemble_overview.cpp

@@ -43,37 +43,11 @@ unsigned calculateOverviewZoomLevel(const std::vector<LegGeometry> &leg_geometri
     return util::viewport::getFittedZoom(south_west, north_east);
 }
 
-std::vector<util::Coordinate> simplifyGeometry(const std::vector<LegGeometry> &leg_geometries,
-                                               const unsigned zoom_level)
-{
-    std::vector<util::Coordinate> overview_geometry;
-    auto leg_index = 0UL;
-    for (const auto &geometry : leg_geometries)
-    {
-        auto simplified_geometry =
-            douglasPeucker(geometry.locations.begin(), geometry.locations.end(), zoom_level);
-        // not the last leg
-        if (leg_index < leg_geometries.size() - 1)
-        {
-            simplified_geometry.pop_back();
-        }
-        overview_geometry.insert(
-            overview_geometry.end(), simplified_geometry.begin(), simplified_geometry.end());
-    }
-    return overview_geometry;
-}
 }
 
 std::vector<util::Coordinate> assembleOverview(const std::vector<LegGeometry> &leg_geometries,
                                                const bool use_simplification)
 {
-    if (use_simplification)
-    {
-        const auto zoom_level = std::min(18u, calculateOverviewZoomLevel(leg_geometries));
-        return simplifyGeometry(leg_geometries, zoom_level);
-    }
-    BOOST_ASSERT(!use_simplification);
-
     auto overview_size =
         std::accumulate(leg_geometries.begin(),
                         leg_geometries.end(),
@@ -85,16 +59,34 @@ std::vector<util::Coordinate> assembleOverview(const std::vector<LegGeometry> &l
     std::vector<util::Coordinate> overview_geometry;
     overview_geometry.reserve(overview_size);
 
+    using GeometryIter = decltype(overview_geometry)::const_iterator;
+
     auto leg_reverse_index = leg_geometries.size();
-    for (const auto &geometry : leg_geometries)
+    const auto insert_without_overlap = [&leg_reverse_index, &overview_geometry](GeometryIter begin, GeometryIter end) {
-    {
+        // not the last leg
-        auto begin = geometry.locations.begin();
+        if (leg_reverse_index > 1)
-        auto end = geometry.locations.end();
-        if (--leg_reverse_index > 0)
         {
+            --leg_reverse_index;
             end = std::prev(end);
         }
         overview_geometry.insert(overview_geometry.end(), begin, end);
+    };
+
+    if (use_simplification)
+    {
+        const auto zoom_level = std::min(18u, calculateOverviewZoomLevel(leg_geometries));
+        for (const auto &geometry : leg_geometries)
+        {
+            const auto simplified = douglasPeucker(geometry.locations.begin(), geometry.locations.end(), zoom_level);
+            insert_without_overlap(simplified.begin(), simplified.end());
+        }
+    }
+    else
+    {
+        for (const auto &geometry : leg_geometries)
+        {
+            insert_without_overlap(geometry.locations.begin(), geometry.locations.end());
+        }
     }
 
     return overview_geometry;

src/engine/guidance/post_processing.cpp

@@ -1,5 +1,5 @@
-#include "engine/guidance/post_processing.hpp"
 #include "extractor/guidance/turn_instruction.hpp"
+#include "engine/guidance/post_processing.hpp"
 
 #include "engine/guidance/assemble_steps.hpp"
 #include "engine/guidance/lane_processing.hpp"
@@ -260,7 +260,8 @@ void closeOffRoundabout(const bool on_roundabout,
         BOOST_ASSERT(leavesRoundabout(steps[1].maneuver.instruction) ||
                      steps[1].maneuver.instruction.type == TurnType::StayOnRoundabout ||
                      steps[1].maneuver.instruction.type == TurnType::Suppressed ||
-                     steps[1].maneuver.instruction.type == TurnType::NoTurn);
+                     steps[1].maneuver.instruction.type == TurnType::NoTurn ||
+                     steps[1].maneuver.instruction.type == TurnType::UseLane);
         steps[0].geometry_end = 1;
         steps[1].geometry_begin = 0;
         steps[1] = forwardInto(steps[1], steps[0]);
@@ -354,6 +355,10 @@ void collapseTurnAt(std::vector<RouteStep> &steps,
     BOOST_ASSERT(one_back_index < steps.size());
     const auto &current_step = steps[step_index];
     const auto &one_back_step = steps[one_back_index];
+    // Don't collapse roundabouts
+    if (entersRoundabout(current_step.maneuver.instruction) ||
+        entersRoundabout(one_back_step.maneuver.instruction))
+        return;
 
     // FIXME: this function assumes driving on the right hand side of the streat
     const auto bearingsAreReversed = [](const double bearing_in, const double bearing_out) {
@@ -520,6 +525,11 @@ void collapseTurnAt(std::vector<RouteStep> &steps,
 //
 bool isStaggeredIntersection(const RouteStep &previous, const RouteStep &current)
 {
+    //don't touch roundabouts
+    if (entersRoundabout(previous.maneuver.instruction) ||
+        entersRoundabout(current.maneuver.instruction))
+        return false;
+
     // Base decision on distance since the zig-zag is a visual clue.
     // If adjusted, make sure to check validity of the is_right/is_left classification below
     const constexpr auto MAX_STAGGERED_DISTANCE = 3; // debatable, but keep short to be on safe side
@@ -838,6 +848,11 @@ std::vector<RouteStep> collapseTurns(std::vector<RouteStep> steps)
                         ::osrm::util::guidance::getTurnDirection(angle);
                     invalidateStep(steps[step_index]);
                 }
+                else
+                {
+                    // the sliproad turn is incompatible. So we handle it as a turn
+                    steps[one_back_index].maneuver.instruction.type = TurnType::Turn;
+                }
             }
         }
         // Due to empty segments, we can get name-changes from A->A

src/extractor/guidance/intersection_handler.cpp

@@ -510,17 +510,45 @@ std::size_t IntersectionHandler::findObviousTurn(const EdgeID via_edge,
          !best_data.road_classification.IsRampClass()))
     {
         // Find left/right deviation
-        const double left_deviation = angularDeviation(
+        // skipping over service roads
-            intersection[(best + 1) % intersection.size()].turn.angle, STRAIGHT_ANGLE);
+        const std::size_t left_index = [&]() {
+            const auto index_candidate = (best + 1) % intersection.size();
+            if (index_candidate == 0)
+                return index_candidate;
+            const auto &candidate_data =
+                node_based_graph.GetEdgeData(intersection[index_candidate].turn.eid);
+            if (obvious_by_road_class(in_data.road_classification,
+                                      best_data.road_classification,
+                                      candidate_data.road_classification))
+                return (index_candidate + 1) % intersection.size();
+            else
+                return index_candidate;
+
+        }();
+        const auto right_index = [&]() {
+            BOOST_ASSERT(best > 0);
+            const auto index_candidate = best - 1;
+            if (index_candidate == 0)
+                return index_candidate;
+            const auto candidate_data =
+                node_based_graph.GetEdgeData(intersection[index_candidate].turn.eid);
+            if (obvious_by_road_class(in_data.road_classification,
+                                      best_data.road_classification,
+                                      candidate_data.road_classification))
+                return index_candidate - 1;
+            else
+                return index_candidate;
+        }();
+
+        const double left_deviation =
+            angularDeviation(intersection[left_index].turn.angle, STRAIGHT_ANGLE);
         const double right_deviation =
-            angularDeviation(intersection[best - 1].turn.angle, STRAIGHT_ANGLE);
+            angularDeviation(intersection[right_index].turn.angle, STRAIGHT_ANGLE);
 
         if (best_deviation < MAXIMAL_ALLOWED_NO_TURN_DEVIATION &&
             std::min(left_deviation, right_deviation) > FUZZY_ANGLE_DIFFERENCE)
             return best;
 
-        const auto left_index = (best + 1) % intersection.size();
-        const auto right_index = best - 1;
         const auto &left_data = node_based_graph.GetEdgeData(intersection[left_index].turn.eid);
         const auto &right_data = node_based_graph.GetEdgeData(intersection[right_index].turn.eid);
 

src/osrm/osrm.cpp

@@ -21,32 +21,32 @@ OSRM &OSRM::operator=(OSRM &&) noexcept = default;
 
 // Forward to implementation
 
-engine::Status OSRM::Route(const engine::api::RouteParameters &params, util::json::Object &result)
+engine::Status OSRM::Route(const engine::api::RouteParameters &params, util::json::Object &result) const
 {
     return engine_->Route(params, result);
 }
 
-engine::Status OSRM::Table(const engine::api::TableParameters &params, json::Object &result)
+engine::Status OSRM::Table(const engine::api::TableParameters &params, json::Object &result) const
 {
     return engine_->Table(params, result);
 }
 
-engine::Status OSRM::Nearest(const engine::api::NearestParameters &params, json::Object &result)
+engine::Status OSRM::Nearest(const engine::api::NearestParameters &params, json::Object &result) const
 {
     return engine_->Nearest(params, result);
 }
 
-engine::Status OSRM::Trip(const engine::api::TripParameters &params, json::Object &result)
+engine::Status OSRM::Trip(const engine::api::TripParameters &params, json::Object &result) const
 {
     return engine_->Trip(params, result);
 }
 
-engine::Status OSRM::Match(const engine::api::MatchParameters &params, json::Object &result)
+engine::Status OSRM::Match(const engine::api::MatchParameters &params, json::Object &result) const
 {
     return engine_->Match(params, result);
 }
 
-engine::Status OSRM::Tile(const engine::api::TileParameters &params, std::string &result)
+engine::Status OSRM::Tile(const engine::api::TileParameters &params, std::string &result) const
 {
     return engine_->Tile(params, result);
 }

src/storage/storage.cpp

@@ -42,11 +42,6 @@ namespace osrm
 namespace storage
 {
 
-using RTreeLeaf = engine::datafacade::BaseDataFacade::RTreeLeaf;
-using RTreeNode =
-    util::StaticRTree<RTreeLeaf, util::ShM<util::Coordinate, true>::vector, true>::TreeNode;
-using QueryGraph = util::StaticGraph<contractor::QueryEdge::EdgeData>;
-
 // delete a shared memory region. report warning if it could not be deleted
 void deleteRegion(const SharedDataType region)
 {
@@ -76,6 +71,11 @@ void deleteRegion(const SharedDataType region)
     }
 }
 
+using RTreeLeaf = engine::datafacade::BaseDataFacade::RTreeLeaf;
+using RTreeNode =
+    util::StaticRTree<RTreeLeaf, util::ShM<util::Coordinate, true>::vector, true>::TreeNode;
+using QueryGraph = util::StaticGraph<contractor::QueryEdge::EdgeData>;
+
 Storage::Storage(StorageConfig config_) : config(std::move(config_)) {}
 
 int Storage::Run()
@@ -738,20 +738,15 @@ int Storage::Run()
     SharedDataTimestamp *data_timestamp_ptr =
         static_cast<SharedDataTimestamp *>(data_type_memory->Ptr());
 
-    boost::interprocess::scoped_lock<boost::interprocess::named_mutex> query_lock(
-        barrier.query_mutex);
-
-    // notify all processes that were waiting for this condition
-    if (0 < barrier.number_of_queries)
     {
-        barrier.no_running_queries_condition.wait(query_lock);
+        boost::interprocess::scoped_lock<boost::interprocess::named_sharable_mutex> query_lock(barrier.query_mutex);
-    }
 
-    data_timestamp_ptr->layout = layout_region;
+        data_timestamp_ptr->layout = layout_region;
-    data_timestamp_ptr->data = data_region;
+        data_timestamp_ptr->data = data_region;
-    data_timestamp_ptr->timestamp += 1;
+        data_timestamp_ptr->timestamp += 1;
-    deleteRegion(previous_data_region);
+        deleteRegion(previous_data_region);
-    deleteRegion(previous_layout_region);
+        deleteRegion(previous_layout_region);
+    }
     util::SimpleLogger().Write() << "all data loaded";
 
     return EXIT_SUCCESS;

src/tools/unlock_all_mutexes.cpp

@@ -10,6 +10,5 @@ int main()
     osrm::storage::SharedBarriers barrier;
     barrier.pending_update_mutex.unlock();
     barrier.query_mutex.unlock();
-    barrier.update_mutex.unlock();
     return 0;
 }