From 4f0ec785f6609119f7e36500ad025097cd640514 Mon Sep 17 00:00:00 2001 From: Daniel Patterson Date: Thu, 14 Feb 2019 17:14:50 -0800 Subject: [PATCH 1/4] Configurable snapping behaviour (#5361) --- CHANGELOG.md | 1 + docs/http.md | 1 + docs/nodejs/api.md | 5 +++ features/car/startpoint.feature | 25 +++++++++++ include/engine/api/base_parameters.hpp | 15 ++++++- .../contiguous_internalmem_datafacade.hpp | 20 +++++---- include/engine/datafacade/datafacade_base.hpp | 12 ++++-- include/engine/geospatial_query.hpp | 42 ++++++++++++------- include/engine/plugins/plugin_base.hpp | 14 +++++-- include/nodejs/node_osrm_support.hpp | 29 +++++++++++++ .../server/api/base_parameters_grammar.hpp | 12 +++++- package.json | 2 +- test/nodejs/route.js | 21 ++++++++++ unit_tests/engine/offline_facade.cpp | 12 ++++-- unit_tests/mocks/mock_datafacade.hpp | 36 ++++++++-------- 15 files changed, 191 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 188dce29558..3a440881cce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Features: - ADDED: new waypoints parameter to the `route` plugin, enabling silent waypoints [#5345](https://github.com/Project-OSRM/osrm-backend/pull/5345) - ADDED: data timestamp information in the response (saved in new file `.osrm.timestamp`). [#5115](https://github.com/Project-OSRM/osrm-backend/issues/5115) + - ADDED: new API parameter - `snapping=any|default` to allow snapping to previously unsnappable edges [#5361](https://github.com/Project-OSRM/osrm-backend/pull/5361) - Routing: - CHANGED: allow routing past `barrier=arch` [#5352](https://github.com/Project-OSRM/osrm-backend/pull/5352) diff --git a/docs/http.md b/docs/http.md index 7caf2509c94..5cbd76673d8 100644 --- a/docs/http.md +++ b/docs/http.md @@ -32,6 +32,7 @@ To pass parameters to each location some options support an array like encoding: |hints |`{hint};{hint}[;{hint} ...]` |Hint from previous request to derive position in street network. | |approaches |`{approach};{approach}[;{approach} ...]` |Keep waypoints on curb side. | |exclude |`{class}[,{class}]` |Additive list of classes to avoid, order does not matter. | +|snapping |`default` (default), `any` |Default snapping avoids is_startpoint (see profile) edges, `any` will snap to any edge in the graph | Where the elements follow the following format: diff --git a/docs/nodejs/api.md b/docs/nodejs/api.md index 0ab53c13225..b7d3630f1ff 100644 --- a/docs/nodejs/api.md +++ b/docs/nodejs/api.md @@ -59,6 +59,7 @@ Returns the fastest route between two or more coordinates while visiting the way - `options.approaches` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)?** Keep waypoints on curb side. Can be `null` (unrestricted, default) or `curb`. - `options.waypoints` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)?** Indices to coordinates to treat as waypoints. If not supplied, all coordinates are waypoints. Must include first and last coordinate index. `null`/`true`/`false` + - `options.snapping` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)?** Which edges can be snapped to, either `default`, or `any`. `default` only snaps to edges marked by the profile as `is_startpoint`, `any` will allow snapping to any edge in the routing graph. - `callback` **[Function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function)** **Examples** @@ -91,6 +92,7 @@ Note: `coordinates` in the general options only supports a single `{longitude},{ - `options.number` **[Number](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number)** Number of nearest segments that should be returned. Must be an integer greater than or equal to `1`. (optional, default `1`) - `options.approaches` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)?** Keep waypoints on curb side. Can be `null` (unrestricted, default) or `curb`. + - `options.snapping` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)?** Which edges can be snapped to, either `default`, or `any`. `default` only snaps to edges marked by the profile as `is_startpoint`, `any` will allow snapping to any edge in the routing graph. - `callback` **[Function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function)** **Examples** @@ -133,6 +135,7 @@ tables. Optionally returns distance table. - `options.fallback_speed` **[Number](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number)?** Replace `null` responses in result with as-the-crow-flies estimates based on `fallback_speed`. Value is in metres/second. - `options.fallback_coordinate` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)?** Either `input` (default) or `snapped`. If using a `fallback_speed`, use either the user-supplied coordinate (`input`), or the snapped coordinate (`snapped`) for calculating the as-the-crow-flies diestance between two points. - `options.scale_factor` **[Number](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number)?** Multiply the table duration values in the table by this number for more controlled input into a route optimization solver. + - `options.snapping` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)?** Which edges can be snapped to, either `default`, or `any`. `default` only snaps to edges marked by the profile as `is_startpoint`, `any` will allow snapping to any edge in the routing graph. - `callback` **[Function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function)** **Examples** @@ -212,6 +215,7 @@ if they can not be matched successfully. - `options.gaps` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)?** Allows the input track splitting based on huge timestamp gaps between points. Either `split` or `ignore` (optional, default `split`). - `options.tidy` **[Boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)?** Allows the input track modification to obtain better matching quality for noisy tracks (optional, default `false`). - `options.waypoints` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)?** Indices to coordinates to treat as waypoints. If not supplied, all coordinates are waypoints. Must include first and last coordinate index. + - `options.snapping` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)?** Which edges can be snapped to, either `default`, or `any`. `default` only snaps to edges marked by the profile as `is_startpoint`, `any` will allow snapping to any edge in the routing graph. - `callback` **[Function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function)** **Examples** @@ -276,6 +280,7 @@ Right now, the following combinations are possible: - `options.source` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)** Return route starts at `any` or `first` coordinate. (optional, default `any`) - `options.destination` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)** Return route ends at `any` or `last` coordinate. (optional, default `any`) - `options.approaches` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)?** Keep waypoints on curb side. Can be `null` (unrestricted, default) or `curb`. + - `options.snapping` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)?** Which edges can be snapped to, either `default`, or `any`. `default` only snaps to edges marked by the profile as `is_startpoint`, `any` will allow snapping to any edge in the routing graph. - `callback` **[Function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function)** **Examples** diff --git a/features/car/startpoint.feature b/features/car/startpoint.feature index 965f57d4339..44cc696abc9 100644 --- a/features/car/startpoint.feature +++ b/features/car/startpoint.feature @@ -35,3 +35,28 @@ Feature: Car - Allowed start/end modes | from | to | route | modes | | 1 | 2 | ab,ab | driving,driving | | 2 | 1 | ab,ab | driving,driving | + + Scenario: Car - URL override of non-startpoints + Given the node map + """ + a 1 b c 2 d + """ + + Given the query options + | snapping | any | + + And the ways + | nodes | highway | access | + | ab | service | private | + | bc | primary | | + | cd | service | private | + + When I request a travel time matrix I should get + | | 2 | c | + | 1 | 59.1 | 35.1 | + | b | 35.1 | 11.1 | + + When I route I should get + | from | to | route | + | 1 | 2 | ab,bc,cd | + | 2 | 1 | cd,bc,ab | diff --git a/include/engine/api/base_parameters.hpp b/include/engine/api/base_parameters.hpp index 4693c67a189..91ac937b4fc 100644 --- a/include/engine/api/base_parameters.hpp +++ b/include/engine/api/base_parameters.hpp @@ -63,6 +63,13 @@ namespace api */ struct BaseParameters { + + enum class SnappingType + { + Default, + Any + }; + std::vector coordinates; std::vector> hints; std::vector> radiuses; @@ -73,15 +80,19 @@ struct BaseParameters // Adds hints to response which can be included in subsequent requests, see `hints` above. bool generate_hints = true; + SnappingType snapping = SnappingType::Default; + BaseParameters(const std::vector coordinates_ = {}, const std::vector> hints_ = {}, std::vector> radiuses_ = {}, std::vector> bearings_ = {}, std::vector> approaches_ = {}, bool generate_hints_ = true, - std::vector exclude = {}) + std::vector exclude = {}, + const SnappingType snapping_ = SnappingType::Default) : coordinates(coordinates_), hints(hints_), radiuses(radiuses_), bearings(bearings_), - approaches(approaches_), exclude(std::move(exclude)), generate_hints(generate_hints_) + approaches(approaches_), exclude(std::move(exclude)), generate_hints(generate_hints_), + snapping(snapping_) { } diff --git a/include/engine/datafacade/contiguous_internalmem_datafacade.hpp b/include/engine/datafacade/contiguous_internalmem_datafacade.hpp index 899fb86d6c4..63d4dbc3ac1 100644 --- a/include/engine/datafacade/contiguous_internalmem_datafacade.hpp +++ b/include/engine/datafacade/contiguous_internalmem_datafacade.hpp @@ -389,23 +389,25 @@ class ContiguousInternalMemoryDataFacadeBase : public BaseDataFacade std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, - const Approach approach) const override final + const Approach approach, + const bool use_all_edges) const override final { BOOST_ASSERT(m_geospatial_query.get()); return m_geospatial_query->NearestPhantomNodeWithAlternativeFromBigComponent( - input_coordinate, approach); + input_coordinate, approach, use_all_edges); } std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, const double max_distance, - const Approach approach) const override final + const Approach approach, + const bool use_all_edges) const override final { BOOST_ASSERT(m_geospatial_query.get()); return m_geospatial_query->NearestPhantomNodeWithAlternativeFromBigComponent( - input_coordinate, max_distance, approach); + input_coordinate, max_distance, approach, use_all_edges); } std::pair @@ -413,24 +415,26 @@ class ContiguousInternalMemoryDataFacadeBase : public BaseDataFacade const double max_distance, const int bearing, const int bearing_range, - const Approach approach) const override final + const Approach approach, + const bool use_all_edges) const override final { BOOST_ASSERT(m_geospatial_query.get()); return m_geospatial_query->NearestPhantomNodeWithAlternativeFromBigComponent( - input_coordinate, max_distance, bearing, bearing_range, approach); + input_coordinate, max_distance, bearing, bearing_range, approach, use_all_edges); } std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, const int bearing, const int bearing_range, - const Approach approach) const override final + const Approach approach, + const bool use_all_edges) const override final { BOOST_ASSERT(m_geospatial_query.get()); return m_geospatial_query->NearestPhantomNodeWithAlternativeFromBigComponent( - input_coordinate, bearing, bearing_range, approach); + input_coordinate, bearing, bearing_range, approach, use_all_edges); } std::uint32_t GetCheckSum() const override final { return m_check_sum; } diff --git a/include/engine/datafacade/datafacade_base.hpp b/include/engine/datafacade/datafacade_base.hpp index 157dbb1caa4..77dab643fa4 100644 --- a/include/engine/datafacade/datafacade_base.hpp +++ b/include/engine/datafacade/datafacade_base.hpp @@ -161,22 +161,26 @@ class BaseDataFacade virtual std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, - const Approach approach) const = 0; + const Approach approach, + const bool use_all_edges) const = 0; virtual std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, const double max_distance, - const Approach approach) const = 0; + const Approach approach, + const bool use_all_edges) const = 0; virtual std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, const double max_distance, const int bearing, const int bearing_range, - const Approach approach) const = 0; + const Approach approach, + const bool use_all_edges) const = 0; virtual std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, const int bearing, const int bearing_range, - const Approach approach) const = 0; + const Approach approach, + const bool use_all_edges = false) const = 0; virtual bool HasLaneData(const EdgeID id) const = 0; virtual util::guidance::LaneTupleIdPair GetLaneData(const EdgeID id) const = 0; diff --git a/include/engine/geospatial_query.hpp b/include/engine/geospatial_query.hpp index dd52d8dc72e..67e2b3b6966 100644 --- a/include/engine/geospatial_query.hpp +++ b/include/engine/geospatial_query.hpp @@ -205,18 +205,23 @@ template class GeospatialQuery std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, const double max_distance, - const Approach approach) const + const Approach approach, + const bool use_all_edges) const { bool has_small_component = false; bool has_big_component = false; auto results = rtree.Nearest( input_coordinate, - [this, approach, &input_coordinate, &has_big_component, &has_small_component]( - const CandidateSegment &segment) { + [this, + approach, + &input_coordinate, + &has_big_component, + &has_small_component, + &use_all_edges](const CandidateSegment &segment) { auto use_segment = (!has_small_component || (!has_big_component && !IsTinyComponent(segment))); auto use_directions = std::make_pair(use_segment, use_segment); - const auto valid_edges = HasValidEdge(segment); + const auto valid_edges = HasValidEdge(segment, use_all_edges); const auto admissible_segments = CheckSegmentExclude(segment); use_directions = boolPairAnd(use_directions, admissible_segments); use_directions = boolPairAnd(use_directions, valid_edges); @@ -251,19 +256,24 @@ template class GeospatialQuery // a second phantom node is return that is the nearest coordinate in a big component. std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, - const Approach approach) const + const Approach approach, + const bool use_all_edges) const { bool has_small_component = false; bool has_big_component = false; auto results = rtree.Nearest( input_coordinate, - [this, approach, &input_coordinate, &has_big_component, &has_small_component]( - const CandidateSegment &segment) { + [this, + approach, + &input_coordinate, + &has_big_component, + &has_small_component, + &use_all_edges](const CandidateSegment &segment) { auto use_segment = (!has_small_component || (!has_big_component && !IsTinyComponent(segment))); auto use_directions = std::make_pair(use_segment, use_segment); - const auto valid_edges = HasValidEdge(segment); + const auto valid_edges = HasValidEdge(segment, use_all_edges); const auto admissible_segments = CheckSegmentExclude(segment); use_directions = boolPairAnd(use_directions, admissible_segments); use_directions = boolPairAnd(use_directions, valid_edges); @@ -298,7 +308,8 @@ template class GeospatialQuery NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate input_coordinate, const int bearing, const int bearing_range, - const Approach approach) const + const Approach approach, + const bool use_all_edges) const { bool has_small_component = false; bool has_big_component = false; @@ -310,12 +321,13 @@ template class GeospatialQuery bearing, bearing_range, &has_big_component, - &has_small_component](const CandidateSegment &segment) { + &has_small_component, + &use_all_edges](const CandidateSegment &segment) { auto use_segment = (!has_small_component || (!has_big_component && !IsTinyComponent(segment))); auto use_directions = std::make_pair(use_segment, use_segment); const auto admissible_segments = CheckSegmentExclude(segment); - use_directions = boolPairAnd(use_directions, HasValidEdge(segment)); + use_directions = boolPairAnd(use_directions, HasValidEdge(segment, use_all_edges)); if (use_segment) { @@ -356,7 +368,8 @@ template class GeospatialQuery const double max_distance, const int bearing, const int bearing_range, - const Approach approach) const + const Approach approach, + const bool use_all_edges) const { bool has_small_component = false; bool has_big_component = false; @@ -368,12 +381,13 @@ template class GeospatialQuery bearing, bearing_range, &has_big_component, - &has_small_component](const CandidateSegment &segment) { + &has_small_component, + &use_all_edges](const CandidateSegment &segment) { auto use_segment = (!has_small_component || (!has_big_component && !IsTinyComponent(segment))); auto use_directions = std::make_pair(use_segment, use_segment); const auto admissible_segments = CheckSegmentExclude(segment); - use_directions = boolPairAnd(use_directions, HasValidEdge(segment)); + use_directions = boolPairAnd(use_directions, HasValidEdge(segment, use_all_edges)); if (use_segment) { diff --git a/include/engine/plugins/plugin_base.hpp b/include/engine/plugins/plugin_base.hpp index 7412627de32..78dd9e08950 100644 --- a/include/engine/plugins/plugin_base.hpp +++ b/include/engine/plugins/plugin_base.hpp @@ -270,6 +270,7 @@ class BasePlugin const bool use_bearings = !parameters.bearings.empty(); const bool use_radiuses = !parameters.radiuses.empty(); const bool use_approaches = !parameters.approaches.empty(); + const bool use_all_edges = parameters.snapping == api::BaseParameters::SnappingType::Any; BOOST_ASSERT(parameters.IsValid()); for (const auto i : util::irange(0UL, parameters.coordinates.size())) @@ -296,7 +297,8 @@ class BasePlugin *parameters.radiuses[i], parameters.bearings[i]->bearing, parameters.bearings[i]->range, - approach); + approach, + use_all_edges); } else { @@ -305,7 +307,8 @@ class BasePlugin parameters.coordinates[i], parameters.bearings[i]->bearing, parameters.bearings[i]->range, - approach); + approach, + use_all_edges); } } else @@ -314,13 +317,16 @@ class BasePlugin { phantom_node_pairs[i] = facade.NearestPhantomNodeWithAlternativeFromBigComponent( - parameters.coordinates[i], *parameters.radiuses[i], approach); + parameters.coordinates[i], + *parameters.radiuses[i], + approach, + use_all_edges); } else { phantom_node_pairs[i] = facade.NearestPhantomNodeWithAlternativeFromBigComponent( - parameters.coordinates[i], approach); + parameters.coordinates[i], approach, use_all_edges); } } diff --git a/include/nodejs/node_osrm_support.hpp b/include/nodejs/node_osrm_support.hpp index a76b5ccf5bd..2696a4c2274 100644 --- a/include/nodejs/node_osrm_support.hpp +++ b/include/nodejs/node_osrm_support.hpp @@ -1010,6 +1010,35 @@ argumentsToRouteParameter(const Nan::FunctionCallbackInfo &args, } } + if (obj->Has(Nan::New("snapping").ToLocalChecked())) + { + v8::Local snapping = obj->Get(Nan::New("snapping").ToLocalChecked()); + if (snapping.IsEmpty()) + return route_parameters_ptr(); + + if (!snapping->IsString()) + { + Nan::ThrowError("Snapping must be a string: [default, any]"); + return route_parameters_ptr(); + } + const Nan::Utf8String snapping_utf8str(snapping); + std::string snapping_str{*snapping_utf8str, *snapping_utf8str + snapping_utf8str.length()}; + + if (snapping_str == "default") + { + params->snapping = osrm::RouteParameters::SnappingType::Default; + } + else if (snapping_str == "any") + { + params->snapping = osrm::RouteParameters::SnappingType::Any; + } + else + { + Nan::ThrowError("'snapping' param must be one of [default, any]"); + return route_parameters_ptr(); + } + } + bool parsedSuccessfully = parseCommonParameters(obj, params); if (!parsedSuccessfully) { diff --git a/include/server/api/base_parameters_grammar.hpp b/include/server/api/base_parameters_grammar.hpp index bb731e8d39c..5615ee53f33 100644 --- a/include/server/api/base_parameters_grammar.hpp +++ b/include/server/api/base_parameters_grammar.hpp @@ -162,6 +162,13 @@ struct BaseParametersGrammar : boost::spirit::qi::grammar (-approach_type % ';')[ph::bind(&engine::api::BaseParameters::approaches, qi::_r1) = qi::_1]; + snapping_type.add("default", engine::api::BaseParameters::SnappingType::Default)( + "any", engine::api::BaseParameters::SnappingType::Any); + + snapping_rule = + qi::lit("snapping=") > + snapping_type[ph::bind(&engine::api::BaseParameters::snapping, qi::_r1) = qi::_1]; + exclude_rule = qi::lit("exclude=") > (qi::as_string[+qi::char_("a-zA-Z0-9")] % ',')[ph::bind(&engine::api::BaseParameters::exclude, qi::_r1) = qi::_1]; @@ -171,7 +178,8 @@ struct BaseParametersGrammar : boost::spirit::qi::grammar | bearings_rule(qi::_r1) // | generate_hints_rule(qi::_r1) // | approach_rule(qi::_r1) // - | exclude_rule(qi::_r1); + | exclude_rule(qi::_r1) // + | snapping_rule(qi::_r1); } protected: @@ -197,8 +205,10 @@ struct BaseParametersGrammar : boost::spirit::qi::grammar qi::rule base64_char; qi::rule polyline_chars; qi::rule unlimited_rule; + qi::rule snapping_rule; qi::symbols approach_type; + qi::symbols snapping_type; }; } } diff --git a/package.json b/package.json index f2c2d622d66..76e761feb20 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "osrm", - "version": "5.22.0-latest.1", + "version": "5.22.0-customsnapping.2", "private": false, "description": "The Open Source Routing Machine is a high performance routing engine written in C++14 designed to run on OpenStreetMap data.", "dependencies": { diff --git a/test/nodejs/route.js b/test/nodejs/route.js index 037a7be1028..f23eaf3eed9 100644 --- a/test/nodejs/route.js +++ b/test/nodejs/route.js @@ -689,4 +689,25 @@ test('route: throws on invalid waypoints values, waypoints must be an array of i }; assert.throws(function () { osrm.route(options, function (err, response) { console.error(`response: ${response}`); console.error(`error: ${err}`); }); }, /Waypoints must be supplied in increasing order/); +}); + +test('route: throws on invalid snapping values', function (assert) { + assert.plan(1); + var osrm = new OSRM(monaco_path); + var options = { + steps: true, + coordinates: three_test_coordinates.concat(three_test_coordinates), + snapping: "zing" + }; + assert.throws(function () { osrm.route(options, function (err, response) { console.error(`response: ${response}`); console.error(`error: ${err}`); }); }, + /'snapping' param must be one of \[default, any\]/); +}); + +test('route: snapping parameter passed through OK', function(assert) { + assert.plan(2); + var osrm = new OSRM(monaco_path); + osrm.route({snapping: "any", coordinates: [[7.448205209414596,43.754001097311544],[7.447122039202185,43.75306156811368]]}, function(err, route) { + assert.ifError(err); + assert.equal(Math.round(route.routes[0].distance * 10), 1314); // Round it to nearest 0.1m to eliminate floating point comparison error + }); }); \ No newline at end of file diff --git a/unit_tests/engine/offline_facade.cpp b/unit_tests/engine/offline_facade.cpp index dab7e232721..1307450bea8 100644 --- a/unit_tests/engine/offline_facade.cpp +++ b/unit_tests/engine/offline_facade.cpp @@ -284,7 +284,8 @@ class ContiguousInternalMemoryDataFacade std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate /*input_coordinate*/, - const Approach /*approach*/) const override + const Approach /*approach*/, + const bool /* use_all_edges */) const override { return {}; } @@ -292,7 +293,8 @@ class ContiguousInternalMemoryDataFacade std::pair NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate /*input_coordinate*/, const double /*max_distance*/, - const Approach /*approach*/) const override + const Approach /*approach*/, + const bool /* use_all_edges */) const override { return {}; } @@ -302,7 +304,8 @@ class ContiguousInternalMemoryDataFacade const double /*max_distance*/, const int /*bearing*/, const int /*bearing_range*/, - const Approach /*approach*/) const override + const Approach /*approach*/, + const bool /* use_all_edges */) const override { return {}; } @@ -311,7 +314,8 @@ class ContiguousInternalMemoryDataFacade NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate /*input_coordinate*/, const int /*bearing*/, const int /*bearing_range*/, - const Approach /*approach*/) const override + const Approach /*approach*/, + const bool /* use_all_edges */) const override { return {}; } diff --git a/unit_tests/mocks/mock_datafacade.hpp b/unit_tests/mocks/mock_datafacade.hpp index efea099d00d..7b0dc8eff0a 100644 --- a/unit_tests/mocks/mock_datafacade.hpp +++ b/unit_tests/mocks/mock_datafacade.hpp @@ -167,39 +167,39 @@ class MockBaseDataFacade : public engine::datafacade::BaseDataFacade } std::pair - NearestPhantomNodeWithAlternativeFromBigComponent( - const util::Coordinate /*input_coordinate*/, - const engine::Approach /*approach*/) const override + NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate /*input_coordinate*/, + const engine::Approach /*approach*/, + const bool /* use_all_edges */) const override { return {}; } std::pair - NearestPhantomNodeWithAlternativeFromBigComponent( - const util::Coordinate /*input_coordinate*/, - const double /*max_distance*/, - const engine::Approach /*approach*/) const override + NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate /*input_coordinate*/, + const double /*max_distance*/, + const engine::Approach /*approach*/, + const bool /* use_all_edges */) const override { return {}; } std::pair - NearestPhantomNodeWithAlternativeFromBigComponent( - const util::Coordinate /*input_coordinate*/, - const double /*max_distance*/, - const int /*bearing*/, - const int /*bearing_range*/, - const engine::Approach /*approach*/) const override + NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate /*input_coordinate*/, + const double /*max_distance*/, + const int /*bearing*/, + const int /*bearing_range*/, + const engine::Approach /*approach*/, + const bool /* use_all_edges */) const override { return {}; } std::pair - NearestPhantomNodeWithAlternativeFromBigComponent( - const util::Coordinate /*input_coordinate*/, - const int /*bearing*/, - const int /*bearing_range*/, - const engine::Approach /*approach*/) const override + NearestPhantomNodeWithAlternativeFromBigComponent(const util::Coordinate /*input_coordinate*/, + const int /*bearing*/, + const int /*bearing_range*/, + const engine::Approach /*approach*/, + const bool /* use_all_edges */) const override { return {}; } From 9ba60d0d5c823cf5abe7b0f6452339d3c3485d53 Mon Sep 17 00:00:00 2001 From: Raf Czlonka Date: Fri, 15 Feb 2019 16:59:02 +0000 Subject: [PATCH 2/4] Quote command line options which may contain special characters (#5362) I.e. the example commands fail if $PWD contains a space character. --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 155c81257ae..58dafa4fadb 100644 --- a/README.md +++ b/README.md @@ -58,16 +58,16 @@ Download OpenStreetMap extracts for example from [Geofabrik](http://download.geo Pre-process the extract with the car profile and start a routing engine HTTP server on port 5000 - docker run -t -v $(pwd):/data osrm/osrm-backend osrm-extract -p /opt/car.lua /data/berlin-latest.osm.pbf + docker run -t -v "$(pwd):/data" osrm/osrm-backend osrm-extract -p /opt/car.lua /data/berlin-latest.osm.pbf -The flag `-v $(pwd):/data` creates the directory `/data` inside the docker container and makes the current working directory `$(pwd)` available there. The file `/data/berlin-latest.osm.pbf` inside the container is referring to `$(pwd)/berlin-latest.osm.pbf` on the host. +The flag `-v "$(pwd):/data"` creates the directory `/data` inside the docker container and makes the current working directory `"$(pwd)"` available there. The file `/data/berlin-latest.osm.pbf` inside the container is referring to `"$(pwd)/berlin-latest.osm.pbf"` on the host. - docker run -t -v $(pwd):/data osrm/osrm-backend osrm-partition /data/berlin-latest.osrm - docker run -t -v $(pwd):/data osrm/osrm-backend osrm-customize /data/berlin-latest.osrm + docker run -t -v "$(pwd):/data" osrm/osrm-backend osrm-partition /data/berlin-latest.osrm + docker run -t -v "$(pwd):/data" osrm/osrm-backend osrm-customize /data/berlin-latest.osrm Note that `berlin-latest.osrm` has a different file extension. - docker run -t -i -p 5000:5000 -v $(pwd):/data osrm/osrm-backend osrm-routed --algorithm mld /data/berlin-latest.osrm + docker run -t -i -p 5000:5000 -v "$(pwd):/data" osrm/osrm-backend osrm-routed --algorithm mld /data/berlin-latest.osrm Make requests against the HTTP server From 5ddbb25237eacf452c09cfb7c1aeaf6c59b1f33d Mon Sep 17 00:00:00 2001 From: Raf Czlonka Date: Fri, 15 Feb 2019 23:47:05 +0000 Subject: [PATCH 3/4] Use an environment variable instead of command substitution (#5364) `$PWD` is already there so use it instead of running `pwd(1)` in a subshell each time. --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 58dafa4fadb..8435d558ffc 100644 --- a/README.md +++ b/README.md @@ -58,16 +58,16 @@ Download OpenStreetMap extracts for example from [Geofabrik](http://download.geo Pre-process the extract with the car profile and start a routing engine HTTP server on port 5000 - docker run -t -v "$(pwd):/data" osrm/osrm-backend osrm-extract -p /opt/car.lua /data/berlin-latest.osm.pbf + docker run -t -v "${PWD}:/data" osrm/osrm-backend osrm-extract -p /opt/car.lua /data/berlin-latest.osm.pbf -The flag `-v "$(pwd):/data"` creates the directory `/data` inside the docker container and makes the current working directory `"$(pwd)"` available there. The file `/data/berlin-latest.osm.pbf` inside the container is referring to `"$(pwd)/berlin-latest.osm.pbf"` on the host. +The flag `-v "${PWD}:/data"` creates the directory `/data` inside the docker container and makes the current working directory `"${PWD}"` available there. The file `/data/berlin-latest.osm.pbf` inside the container is referring to `"${PWD}/berlin-latest.osm.pbf"` on the host. - docker run -t -v "$(pwd):/data" osrm/osrm-backend osrm-partition /data/berlin-latest.osrm - docker run -t -v "$(pwd):/data" osrm/osrm-backend osrm-customize /data/berlin-latest.osrm + docker run -t -v "${PWD}:/data" osrm/osrm-backend osrm-partition /data/berlin-latest.osrm + docker run -t -v "${PWD}:/data" osrm/osrm-backend osrm-customize /data/berlin-latest.osrm Note that `berlin-latest.osrm` has a different file extension. - docker run -t -i -p 5000:5000 -v "$(pwd):/data" osrm/osrm-backend osrm-routed --algorithm mld /data/berlin-latest.osrm + docker run -t -i -p 5000:5000 -v "${PWD}:/data" osrm/osrm-backend osrm-routed --algorithm mld /data/berlin-latest.osrm Make requests against the HTTP server From 2c7c18fd24f9a46de04c935126fc29992f57113f Mon Sep 17 00:00:00 2001 From: Daniel Patterson Date: Fri, 15 Feb 2019 19:23:03 -0800 Subject: [PATCH 4/4] Fix bug in snapping=any when bearings or radiuses are supplied. --- features/car/startpoint.feature | 63 +++++++++++++++++++++++++++++ include/engine/geospatial_query.hpp | 6 +-- 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/features/car/startpoint.feature b/features/car/startpoint.feature index 44cc696abc9..9e753843b60 100644 --- a/features/car/startpoint.feature +++ b/features/car/startpoint.feature @@ -60,3 +60,66 @@ Feature: Car - Allowed start/end modes | from | to | route | | 1 | 2 | ab,bc,cd | | 2 | 1 | cd,bc,ab | + + Scenario: Car - URL override of non-startpoints + Given the node map + """ + a 1 b c 2 d + """ + + Given the query options + | snapping | any | + | bearings | 90,180; | + + And the ways + | nodes | highway | access | + | ab | service | private | + | bc | primary | | + | cd | service | private | + + When I route I should get + | from | to | route | + | 1 | 2 | ab,bc,cd | + | 2 | 1 | cd,bc,ab | + + Scenario: Car - URL override of non-startpoints + Given the node map + """ + a 1 b c 2 d + """ + + Given the query options + | snapping | any | + | radiuses | 100;unlimited | + + And the ways + | nodes | highway | access | + | ab | service | private | + | bc | primary | | + | cd | service | private | + + When I route I should get + | from | to | route | + | 1 | 2 | ab,bc,cd | + | 2 | 1 | cd,bc,ab | + + Scenario: Car - URL override of non-startpoints + Given the node map + """ + a 1 b c 2 d + """ + + Given the query options + | snapping | any | + | bearings | 90,180;0,180;; | + + And the ways + | nodes | highway | access | + | ab | service | private | + | bc | primary | | + | cd | service | private | + + When I request a travel time matrix I should get + | | 2 | c | + | 1 | 59.1 | 35.1 | + | b | 35.1 | 11.1 | \ No newline at end of file diff --git a/include/engine/geospatial_query.hpp b/include/engine/geospatial_query.hpp index 67e2b3b6966..b4a05b18ded 100644 --- a/include/engine/geospatial_query.hpp +++ b/include/engine/geospatial_query.hpp @@ -327,13 +327,12 @@ template class GeospatialQuery (!has_small_component || (!has_big_component && !IsTinyComponent(segment))); auto use_directions = std::make_pair(use_segment, use_segment); const auto admissible_segments = CheckSegmentExclude(segment); - use_directions = boolPairAnd(use_directions, HasValidEdge(segment, use_all_edges)); if (use_segment) { use_directions = boolPairAnd(CheckSegmentBearing(segment, bearing, bearing_range), - HasValidEdge(segment)); + HasValidEdge(segment, use_all_edges)); use_directions = boolPairAnd(use_directions, admissible_segments); use_directions = boolPairAnd( use_directions, CheckApproach(input_coordinate, segment, approach)); @@ -387,13 +386,12 @@ template class GeospatialQuery (!has_small_component || (!has_big_component && !IsTinyComponent(segment))); auto use_directions = std::make_pair(use_segment, use_segment); const auto admissible_segments = CheckSegmentExclude(segment); - use_directions = boolPairAnd(use_directions, HasValidEdge(segment, use_all_edges)); if (use_segment) { use_directions = boolPairAnd(CheckSegmentBearing(segment, bearing, bearing_range), - HasValidEdge(segment)); + HasValidEdge(segment, use_all_edges)); use_directions = boolPairAnd(use_directions, admissible_segments); use_directions = boolPairAnd( use_directions, CheckApproach(input_coordinate, segment, approach));