diff --git a/CHANGELOG.md b/CHANGELOG.md index 598df2cc4..d51ad2837 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,7 @@ # Unreleased + - Changes from 5.21.0 + - Features: + - ADDED: new waypoints parameter to the `route` plugin, enabling silent waypoints [#5345](https://github.com/Project-OSRM/osrm-backend/pull/5345) # 5.21.0 - Changes from 5.20.0 diff --git a/docs/http.md b/docs/http.md index fb7811c79..bd47fcc7e 100644 --- a/docs/http.md +++ b/docs/http.md @@ -195,6 +195,7 @@ In addition to the [general options](#general-options) the following options are |geometries |`polyline` (default), `polyline6`, `geojson` |Returned route geometry format (influences overview and per step) | |overview |`simplified` (default), `full`, `false` |Add overview geometry either full, simplified according to highest zoom level it could be display on, or not at all.| |continue\_straight |`default` (default), `true`, `false` |Forces the route to keep going straight at waypoints constraining uturns there even if it would be faster. Default value depends on the profile. | +|waypoints | `{index};{index};{index}...` |Treats input coordinates indicated by given indices as waypoints in returned Match object. Default is to treat all input coordinates as waypoints. | \* Please note that even if alternative routes are requested, a result cannot be guaranteed. diff --git a/docs/nodejs/api.md b/docs/nodejs/api.md index ba3f18bd8..0ab53c132 100644 --- a/docs/nodejs/api.md +++ b/docs/nodejs/api.md @@ -57,6 +57,7 @@ Returns the fastest route between two or more coordinates while visiting the way - `options.overview` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)** Add overview geometry either `full`, `simplified` according to highest zoom level it could be display on, or not at all (`false`). (optional, default `simplified`) - `options.continue_straight` **[Boolean](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Boolean)?** Forces the route to keep going straight at waypoints and don't do a uturn even if it would be faster. Default value depends on the profile. - `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` - `callback` **[Function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function)** @@ -210,6 +211,7 @@ if they can not be matched successfully. - `options.radiuses` **[Array](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array)?** Standard deviation of GPS precision used for map matching. If applicable use GPS accuracy. Can be `null` for default value `5` meters or `double >= 0`. - `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. - `callback` **[Function](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function)** **Examples** diff --git a/features/testbot/via.feature b/features/testbot/via.feature index c851fb87c..017cc54e6 100644 --- a/features/testbot/via.feature +++ b/features/testbot/via.feature @@ -18,6 +18,53 @@ Feature: Via points | waypoints | route | | a,b,c | abc,abc,abc,abc | + Scenario: Simple via point with waypoints collapsing + Given the node map + """ + a + + b 1c d + 2 + + e + """ + + And the ways + | nodes | + | ace | + | bcd | + + Given the query options + | waypoints | 0;2 | + + When I route I should get + | waypoints | route | turns | + | b,1,e | bcd,ace,ace | depart,turn right,arrive | + | b,2,e | bcd,ace,ace | depart,turn right,arrive | + + Scenario: Simple via point with waypoints collapsing + Given the node map + """ + a 2 b + + c d + 1 3 + """ + + And the ways + | nodes | + | ab | + | bd | + | cd | + | ac | + + Given the query options + | waypoints | 0;2 | + + When I route I should get + | waypoints | route | turns | + | 1,2,3 | cd,ac,ab,bd,cd | depart,new name right,new name right,new name right,arrive | + Scenario: Simple via point with core factor Given the contract extra arguments "--core 0.8" Given the node map diff --git a/include/engine/api/match_parameters.hpp b/include/engine/api/match_parameters.hpp index 79ce84c55..b54076582 100644 --- a/include/engine/api/match_parameters.hpp +++ b/include/engine/api/match_parameters.hpp @@ -63,7 +63,7 @@ struct MatchParameters : public RouteParameters RouteParameters::GeometriesType::Polyline, RouteParameters::OverviewType::Simplified, {}), - gaps(GapsType::Split), tidy(false), waypoints() + gaps(GapsType::Split), tidy(false) { } @@ -79,24 +79,19 @@ struct MatchParameters : public RouteParameters bool tidy_, std::vector waypoints_, Args... args_) - : RouteParameters{std::forward(args_)...}, timestamps{std::move(timestamps_)}, - gaps(gaps_), tidy(tidy_), waypoints{std::move(waypoints_)} + : RouteParameters{std::forward(args_)..., waypoints_}, + timestamps{std::move(timestamps_)}, gaps(gaps_), tidy(tidy_) { } std::vector timestamps; GapsType gaps; bool tidy; - std::vector waypoints; bool IsValid() const { - const auto valid_waypoints = - std::all_of(waypoints.begin(), waypoints.end(), [this](const auto &w) { - return w < coordinates.size(); - }); return RouteParameters::IsValid() && - (timestamps.empty() || timestamps.size() == coordinates.size()) && valid_waypoints; + (timestamps.empty() || timestamps.size() == coordinates.size()); } }; } diff --git a/include/engine/api/route_api.hpp b/include/engine/api/route_api.hpp index a24277901..d5736cbfe 100644 --- a/include/engine/api/route_api.hpp +++ b/include/engine/api/route_api.hpp @@ -44,8 +44,11 @@ class RouteAPI : public BaseAPI { } - void MakeResponse(const InternalManyRoutesResult &raw_routes, - util::json::Object &response) const + void + MakeResponse(const InternalManyRoutesResult &raw_routes, + const std::vector + &all_start_end_points, // all used coordinates, ignoring waypoints= parameter + util::json::Object &response) const { BOOST_ASSERT(!raw_routes.routes.empty()); @@ -62,8 +65,7 @@ class RouteAPI : public BaseAPI route.target_traversed_in_reverse)); } - response.values["waypoints"] = - BaseAPI::MakeWaypoints(raw_routes.routes[0].segment_end_coordinates); + response.values["waypoints"] = BaseAPI::MakeWaypoints(all_start_end_points); response.values["routes"] = std::move(jsRoutes); response.values["code"] = "Ok"; } diff --git a/include/engine/api/route_parameters.hpp b/include/engine/api/route_parameters.hpp index c78ec70ea..ba200050d 100644 --- a/include/engine/api/route_parameters.hpp +++ b/include/engine/api/route_parameters.hpp @@ -98,7 +98,8 @@ struct RouteParameters : public BaseParameters annotations_type{AnnotationsType::None}, geometries{geometries_}, overview{overview_}, - continue_straight{continue_straight_} + continue_straight{continue_straight_}, + waypoints() { } @@ -114,7 +115,9 @@ struct RouteParameters : public BaseParameters : BaseParameters{std::forward(args_)...}, steps{steps_}, alternatives{alternatives_}, number_of_alternatives{alternatives_ ? 1u : 0u}, annotations{annotations_}, annotations_type{annotations_ ? AnnotationsType::All : AnnotationsType::None}, - geometries{geometries_}, overview{overview_}, continue_straight{continue_straight_} + geometries{geometries_}, overview{overview_}, continue_straight{continue_straight_}, + waypoints() + { } @@ -131,7 +134,43 @@ struct RouteParameters : public BaseParameters number_of_alternatives{alternatives_ ? 1u : 0u}, annotations{annotations_ == AnnotationsType::None ? false : true}, annotations_type{annotations_}, geometries{geometries_}, overview{overview_}, - continue_straight{continue_straight_} + continue_straight{continue_straight_}, waypoints() + { + } + + // RouteParameters constructor adding the `waypoints` parameter + template + RouteParameters(const bool steps_, + const bool alternatives_, + const bool annotations_, + const GeometriesType geometries_, + const OverviewType overview_, + const boost::optional continue_straight_, + std::vector waypoints_, + const Args... args_) + : BaseParameters{std::forward(args_)...}, steps{steps_}, alternatives{alternatives_}, + number_of_alternatives{alternatives_ ? 1u : 0u}, annotations{annotations_}, + annotations_type{annotations_ ? AnnotationsType::All : AnnotationsType::None}, + geometries{geometries_}, overview{overview_}, continue_straight{continue_straight_}, + waypoints{waypoints_} + { + } + + // RouteParameters constructor adding the `waypoints` parameter + template + RouteParameters(const bool steps_, + const bool alternatives_, + const AnnotationsType annotations_, + const GeometriesType geometries_, + const OverviewType overview_, + const boost::optional continue_straight_, + std::vector waypoints_, + Args... args_) + : BaseParameters{std::forward(args_)...}, steps{steps_}, alternatives{alternatives_}, + number_of_alternatives{alternatives_ ? 1u : 0u}, + annotations{annotations_ == AnnotationsType::None ? false : true}, + annotations_type{annotations_}, geometries{geometries_}, overview{overview_}, + continue_straight{continue_straight_}, waypoints{waypoints_} { } @@ -144,12 +183,17 @@ struct RouteParameters : public BaseParameters GeometriesType geometries = GeometriesType::Polyline; OverviewType overview = OverviewType::Simplified; boost::optional continue_straight; + std::vector waypoints; bool IsValid() const { const auto coordinates_ok = coordinates.size() >= 2; const auto base_params_ok = BaseParameters::IsValid(); - return coordinates_ok && base_params_ok; + const auto valid_waypoints = + std::all_of(waypoints.begin(), waypoints.end(), [this](const auto &w) { + return w < coordinates.size(); + }); + return coordinates_ok && base_params_ok && valid_waypoints; } }; @@ -173,8 +217,8 @@ inline RouteParameters::AnnotationsType operator|=(RouteParameters::AnnotationsT { return lhs = lhs | rhs; } -} -} -} +} // ns api +} // ns engine +} // ns osrm #endif diff --git a/include/nodejs/node_osrm_support.hpp b/include/nodejs/node_osrm_support.hpp index 4ff697576..a76b5ccf5 100644 --- a/include/nodejs/node_osrm_support.hpp +++ b/include/nodejs/node_osrm_support.hpp @@ -944,6 +944,72 @@ argumentsToRouteParameter(const Nan::FunctionCallbackInfo &args, } } + if (obj->Has(Nan::New("waypoints").ToLocalChecked())) + { + v8::Local waypoints = obj->Get(Nan::New("waypoints").ToLocalChecked()); + if (waypoints.IsEmpty()) + return route_parameters_ptr(); + + // must be array + if (!waypoints->IsArray()) + { + Nan::ThrowError( + "Waypoints must be an array of integers corresponding to the input coordinates."); + return route_parameters_ptr(); + } + + auto waypoints_array = v8::Local::Cast(waypoints); + // must have at least two elements + if (waypoints_array->Length() < 2) + { + Nan::ThrowError("At least two waypoints must be provided"); + return route_parameters_ptr(); + } + auto coords_size = params->coordinates.size(); + auto waypoints_array_size = waypoints_array->Length(); + + const auto first_index = Nan::To(waypoints_array->Get(0)).FromJust(); + const auto last_index = + Nan::To(waypoints_array->Get(waypoints_array_size - 1)).FromJust(); + if (first_index != 0 || last_index != coords_size - 1) + { + Nan::ThrowError("First and last waypoints values must correspond to first and last " + "coordinate indices"); + return route_parameters_ptr(); + } + + for (uint32_t i = 0; i < waypoints_array_size; ++i) + { + v8::Local waypoint_value = waypoints_array->Get(i); + // all elements must be numbers + if (!waypoint_value->IsNumber()) + { + Nan::ThrowError("Waypoint values must be an array of integers"); + return route_parameters_ptr(); + } + // check that the waypoint index corresponds with an inpute coordinate + const auto index = Nan::To(waypoint_value).FromJust(); + if (index >= coords_size) + { + Nan::ThrowError("Waypoints must correspond with the index of an input coordinate"); + return route_parameters_ptr(); + } + params->waypoints.emplace_back(static_cast(waypoint_value->NumberValue())); + } + + if (!params->waypoints.empty()) + { + for (std::size_t i = 0; i < params->waypoints.size() - 1; i++) + { + if (params->waypoints[i] >= params->waypoints[i + 1]) + { + Nan::ThrowError("Waypoints must be supplied in increasing order"); + return route_parameters_ptr(); + } + } + } + } + bool parsedSuccessfully = parseCommonParameters(obj, params); if (!parsedSuccessfully) { diff --git a/include/server/api/match_parameter_grammar.hpp b/include/server/api/match_parameter_grammar.hpp index bb9b6f943..2d78cc0e0 100644 --- a/include/server/api/match_parameter_grammar.hpp +++ b/include/server/api/match_parameter_grammar.hpp @@ -42,17 +42,12 @@ struct MatchParametersGrammar final : public RouteParametersGrammar -qi::lit(".json") > -('?' > (timestamps_rule(qi::_r1) | BaseGrammar::base_rule(qi::_r1) | - waypoints_rule(qi::_r1) | (qi::lit("gaps=") > gaps_type[ph::bind(&engine::api::MatchParameters::gaps, qi::_r1) = qi::_1]) | (qi::lit("tidy=") > @@ -63,7 +58,6 @@ struct MatchParametersGrammar final : public RouteParametersGrammar root_rule; qi::rule timestamps_rule; - qi::rule waypoints_rule; qi::rule size_t_; qi::symbols gaps_type; diff --git a/include/server/api/route_parameters_grammar.hpp b/include/server/api/route_parameters_grammar.hpp index 2572532c9..21463f6ee 100644 --- a/include/server/api/route_parameters_grammar.hpp +++ b/include/server/api/route_parameters_grammar.hpp @@ -48,6 +48,14 @@ struct RouteParametersGrammar : public BaseParametersGrammar &root_rule_) : BaseGrammar(root_rule_) { +#ifdef BOOST_HAS_LONG_LONG + if (std::is_same::value) + size_t_ = qi::ulong_long; + else + size_t_ = qi::ulong_; +#else + size_t_ = qi::ulong_; +#endif using AnnotationsType = engine::api::RouteParameters::AnnotationsType; const auto add_annotation = [](engine::api::RouteParameters &route_parameters, @@ -70,8 +78,12 @@ struct RouteParametersGrammar : public BaseParametersGrammar waypoint_legs(route_parameters.coordinates.size(), false); + std::for_each(route_parameters.waypoints.begin(), + route_parameters.waypoints.end(), + [&](const std::size_t waypoint_index) { + BOOST_ASSERT(waypoint_index < waypoint_legs.size()); + waypoint_legs[waypoint_index] = true; + }); + // First and last coordinates should always be waypoints + // This gets validated earlier, but double-checking here, jic + BOOST_ASSERT(waypoint_legs.front()); + BOOST_ASSERT(waypoint_legs.back()); + for (std::size_t i = 0; i < routes.routes.size(); i++) + { + routes.routes[i] = CollapseInternalRouteResult(routes.routes[i], waypoint_legs); + } + } + + route_api.MakeResponse(routes, start_end_nodes, json_result); } else { diff --git a/test/nodejs/match.js b/test/nodejs/match.js index da4d7853b..3161060e9 100644 --- a/test/nodejs/match.js +++ b/test/nodejs/match.js @@ -319,6 +319,30 @@ test('match: throws on invalid waypoints values, waypoints must correspond with 'Waypoints must correspond with the index of an input coordinate'); }); +test('match: throws on invalid waypoints values, waypoints must be an array', function (assert) { + assert.plan(1); + var osrm = new OSRM(data_path); + var options = { + steps: true, + coordinates: three_test_coordinates, + waypoints: "string" + }; + assert.throws(function () { osrm.match(options, function (err, response) { console.log(err); }); }, + 'Waypoints must be an array of integers corresponding to the input coordinates.'); +}); + +test('match: throws on invalid waypoints values, waypoints must be an array of integers', function (assert) { + assert.plan(1); + var osrm = new OSRM(data_path); + var options = { + steps: true, + coordinates: three_test_coordinates, + waypoints: [0,1,"string"] + }; + assert.throws(function () { osrm.match(options, function (err, response) { console.log(err); }); }, + 'Waypoint values must be an array of integers'); +}); + test('match: error on split trace', function(assert) { assert.plan(1); var osrm = new OSRM(data_path); diff --git a/test/nodejs/route.js b/test/nodejs/route.js index 011a098a3..037a7be10 100644 --- a/test/nodejs/route.js +++ b/test/nodejs/route.js @@ -606,3 +606,87 @@ test('route: route in Monaco without motorways', function(assert) { }); }); + +test('route: throws on invalid waypoints values needs at least two', function (assert) { + assert.plan(1); + var osrm = new OSRM(monaco_path); + var options = { + steps: true, + coordinates: three_test_coordinates, + waypoints: [0] + }; + assert.throws(function () { osrm.route(options, function (err, response) { }); }, + 'At least two waypoints must be provided'); +}); + +test('route: throws on invalid waypoints values, needs first and last coordinate indices', function (assert) { + assert.plan(1); + var osrm = new OSRM(monaco_path); + var options = { + steps: true, + coordinates: three_test_coordinates, + waypoints: [1, 2] + }; + assert.throws(function () { osrm.route(options, function (err, response) { console.log(err); }); }, + 'First and last waypoints values must correspond to first and last coordinate indices'); +}); + +test('route: throws on invalid waypoints values, order matters', function (assert) { + assert.plan(1); + var osrm = new OSRM(monaco_path); + var options = { + steps: true, + coordinates: three_test_coordinates, + waypoints: [2, 0] + }; + assert.throws(function () { osrm.route(options, function (err, response) { console.log(err); }); }, + 'First and last waypoints values must correspond to first and last coordinate indices'); +}); + +test('route: throws on invalid waypoints values, waypoints must correspond with a coordinate index', function (assert) { + assert.plan(1); + var osrm = new OSRM(monaco_path); + var options = { + steps: true, + coordinates: three_test_coordinates, + waypoints: [0, 3, 2] + }; + assert.throws(function () { osrm.route(options, function (err, response) { console.log(err); }); }, + 'Waypoints must correspond with the index of an input coordinate'); +}); + +test('route: throws on invalid waypoints values, waypoints must be an array', function (assert) { + assert.plan(1); + var osrm = new OSRM(monaco_path); + var options = { + steps: true, + coordinates: three_test_coordinates, + waypoints: "string" + }; + assert.throws(function () { osrm.route(options, function (err, response) { console.log(err); }); }, + 'Waypoints must be an array of integers corresponding to the input coordinates.'); +}); + +test('route: throws on invalid waypoints values, waypoints must be an array of integers', function (assert) { + assert.plan(1); + var osrm = new OSRM(monaco_path); + var options = { + steps: true, + coordinates: three_test_coordinates, + waypoints: [0,1,"string"] + }; + assert.throws(function () { osrm.route(options, function (err, response) { console.log(err); }); }, + 'Waypoint values must be an array of integers'); +}); + +test('route: throws on invalid waypoints values, waypoints must be an array of integers in increasing order', function (assert) { + assert.plan(1); + var osrm = new OSRM(monaco_path); + var options = { + steps: true, + coordinates: three_test_coordinates.concat(three_test_coordinates), + waypoints: [0,2,1,5] + }; + assert.throws(function () { osrm.route(options, function (err, response) { console.error(`response: ${response}`); console.error(`error: ${err}`); }); }, + /Waypoints must be supplied in increasing order/); +}); \ No newline at end of file