From 4f0418684c3d1e7537f16a1758f22de9ca198cea Mon Sep 17 00:00:00 2001 From: Daniel Patterson Date: Wed, 23 Jan 2019 00:20:30 -0800 Subject: [PATCH] Add silent waypoints to viaroute API. --- features/testbot/via.feature | 17 ++++++ include/engine/api/base_api.hpp | 10 ++-- include/engine/api/route_api.hpp | 6 +-- include/engine/api/route_parameters.hpp | 18 +++---- include/nodejs/node_osrm_support.hpp | 54 +++++++++++++++++++ .../server/api/match_parameter_grammar.hpp | 12 ++--- .../server/api/route_parameters_grammar.hpp | 24 +++++++-- src/engine/plugins/match.cpp | 7 ++- src/engine/plugins/viaroute.cpp | 51 ++++++++++++++++-- 9 files changed, 160 insertions(+), 39 deletions(-) diff --git a/features/testbot/via.feature b/features/testbot/via.feature index c851fb87c..9626c17d8 100644 --- a/features/testbot/via.feature +++ b/features/testbot/via.feature @@ -18,6 +18,23 @@ 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 c + """ + + And the ways + | nodes | + | abc | + + Given the query options + | waypoints | 0;2 | + + When I route I should get + | waypoints | route | + | a,b,c | abc,abc | + 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/base_api.hpp b/include/engine/api/base_api.hpp index e0c924348..38c209cdb 100644 --- a/include/engine/api/base_api.hpp +++ b/include/engine/api/base_api.hpp @@ -31,10 +31,10 @@ class BaseAPI util::json::Array MakeWaypoints(const std::vector &segment_end_coordinates) const { BOOST_ASSERT(parameters.coordinates.size() > 0); - BOOST_ASSERT(parameters.coordinates.size() == segment_end_coordinates.size() + 1); + // BOOST_ASSERT(parameters.coordinates.size() == segment_end_coordinates.size() + 1); util::json::Array waypoints; - waypoints.values.resize(parameters.coordinates.size()); + waypoints.values.resize(segment_end_coordinates.size() + 1); waypoints.values[0] = MakeWaypoint(segment_end_coordinates.front().source_phantom); auto out_iter = std::next(waypoints.values.begin()); @@ -75,8 +75,8 @@ class BaseAPI const BaseParameters ¶meters; }; -} // ns api -} // ns engine -} // ns osrm +} // namespace api +} // namespace engine +} // namespace osrm #endif diff --git a/include/engine/api/route_api.hpp b/include/engine/api/route_api.hpp index a24277901..2251c5a87 100644 --- a/include/engine/api/route_api.hpp +++ b/include/engine/api/route_api.hpp @@ -359,8 +359,8 @@ class RouteAPI : public BaseAPI const RouteParameters ¶meters; }; -} // ns api -} // ns engine -} // ns osrm +} // namespace api +} // namespace engine +} // namespace osrm #endif diff --git a/include/engine/api/route_parameters.hpp b/include/engine/api/route_parameters.hpp index c78ec70ea..0a3fb4880 100644 --- a/include/engine/api/route_parameters.hpp +++ b/include/engine/api/route_parameters.hpp @@ -90,14 +90,9 @@ struct RouteParameters : public BaseParameters Args... args_) // Once we perfectly-forward `args` (see #2990) this constructor can delegate to the one // below. - : BaseParameters{std::forward(args_)...}, - steps{steps_}, - alternatives{alternatives_}, - number_of_alternatives{alternatives_ ? 1u : 0u}, - annotations{false}, - annotations_type{AnnotationsType::None}, - geometries{geometries_}, - overview{overview_}, + : BaseParameters{std::forward(args_)...}, steps{steps_}, alternatives{alternatives_}, + number_of_alternatives{alternatives_ ? 1u : 0u}, annotations{false}, + annotations_type{AnnotationsType::None}, geometries{geometries_}, overview{overview_}, continue_straight{continue_straight_} { } @@ -144,6 +139,7 @@ struct RouteParameters : public BaseParameters GeometriesType geometries = GeometriesType::Polyline; OverviewType overview = OverviewType::Simplified; boost::optional continue_straight; + std::vector waypoints; bool IsValid() const { @@ -173,8 +169,8 @@ inline RouteParameters::AnnotationsType operator|=(RouteParameters::AnnotationsT { return lhs = lhs | rhs; } -} -} -} +} // namespace api +} // namespace engine +} // namespace osrm #endif diff --git a/include/nodejs/node_osrm_support.hpp b/include/nodejs/node_osrm_support.hpp index 4ff697576..ba79a3b3d 100644 --- a/include/nodejs/node_osrm_support.hpp +++ b/include/nodejs/node_osrm_support.hpp @@ -944,6 +944,60 @@ 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())); + } + } + 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..c87c1b44d 100644 --- a/include/server/api/match_parameter_grammar.hpp +++ b/include/server/api/match_parameter_grammar.hpp @@ -18,7 +18,7 @@ namespace { namespace ph = boost::phoenix; namespace qi = boost::spirit::qi; -} +} // namespace template @@ -42,10 +42,6 @@ struct MatchParametersGrammar final : public RouteParametersGrammar gaps_type; }; -} -} -} +} // namespace api +} // namespace server +} // namespace osrm #endif diff --git a/include/server/api/route_parameters_grammar.hpp b/include/server/api/route_parameters_grammar.hpp index 2572532c9..52a3ed607 100644 --- a/include/server/api/route_parameters_grammar.hpp +++ b/include/server/api/route_parameters_grammar.hpp @@ -18,7 +18,7 @@ namespace { namespace ph = boost::phoenix; namespace qi = boost::spirit::qi; -} +} // namespace template @@ -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= route_parameters.waypoints[i + 1]) + { + return Error( + "InvalidValue", "Waypoints must be supplied in increasing order", json_result); + } + } + } + if (!CheckAlgorithms(route_parameters, algorithms, json_result)) return Status::Error; @@ -132,6 +154,29 @@ Status ViaRoutePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithm if (routes.routes[0].is_valid()) { + auto collapse_legs = !route_parameters.waypoints.empty(); + if (collapse_legs) + { + std::vector waypoint_legs; + auto waypoints_itr = route_parameters.waypoints.begin(); + for (std::size_t i = 0; i < route_parameters.coordinates.size(); i++) + { + if (i == *waypoints_itr) + { + waypoint_legs.push_back(true); + waypoints_itr++; + } + else + { + waypoint_legs.push_back(false); + } + } + for (std::size_t i = 0; i < routes.routes.size(); i++) + { + routes.routes[i] = CollapseInternalRouteResult(routes.routes[i], waypoint_legs); + } + } + route_api.MakeResponse(routes, json_result); } else @@ -155,6 +200,6 @@ Status ViaRoutePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithm return Status::Ok; } -} -} -} +} // namespace plugins +} // namespace engine +} // namespace osrm