From 938dff011fb52fedb8b787d7403b0c8fb1d67f5a Mon Sep 17 00:00:00 2001 From: Moritz Kobitzsch Date: Wed, 7 Sep 2016 15:51:14 +0200 Subject: [PATCH] handle all our new strings correctly, introduce rotary_pronunciation --- CHANGELOG.md | 1 + docs/http.md | 2 +- features/car/names.feature | 28 +++++++++++++ include/engine/guidance/assemble_steps.hpp | 4 ++ include/engine/guidance/route_step.hpp | 2 + src/engine/api/json_factory.cpp | 6 +++ src/engine/guidance/post_processing.cpp | 48 ++++++++++++++-------- 7 files changed, 72 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95dbdf704..4feea86ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - 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 # 5.3.0 Changes from 5.3.0-rc.3 diff --git a/docs/http.md b/docs/http.md index 954165b54..04b01b653 100644 --- a/docs/http.md +++ b/docs/http.md @@ -537,7 +537,7 @@ step. | `use lane` | going straight on a specific lane | | `continue` | Turn in direction of `modifier` to stay on the same road | | `roundabout` | traverse roundabout, has additional field `exit` with NR if the roundabout is left. `the modifier specifies the direction of entering the roundabout` | - | `rotary` | a larger version of a roundabout, can offer `rotary_name` in addition to the `exit` parameter. | + | `rotary` | a larger version of a roundabout, can offer `rotary_name/rotary_pronunciation` in addition to the `exit` parameter. | | `roundabout turn`| Describes a turn at a small roundabout that should be treated as normal turn. The `modifier` indicates the turn direciton. Example instruction: `At the roundabout turn left`. | | `notification` | not an actual turn but a change in the driving conditions. For example the travel mode. If the road takes a turn itself, the `modifier` describes the direction | diff --git a/features/car/names.feature b/features/car/names.feature index 1dfe94411..e03a0308a 100644 --- a/features/car/names.feature +++ b/features/car/names.feature @@ -48,3 +48,31 @@ Feature: Car - Street names in instructions When I route I should get | from | to | route | | a | c | tertiary,residential,residential | + + Scenario: Inner city expressway with on road + Given the node map + | a | b | | | | c | g | + | | | | | f | | | + | | | | | | | | + | | | | | | | | + | | | | | | | | + | | | | | | d | | + | | | | | | | | + | | | | | | | | + | | | | | | | | + | | | | | | e | | + + And the ways + | nodes | highway | name | name:pronunciation | + | abc | primary | road | roooaad | + | cg | primary | road | roooaad | + | bfd | trunk_link | | | + | cde | trunk | trunk | truank | + + And the relations + | type | way:from | way:to | node:via | restriction | + | restriction | abc | cde | c | no_right_turn | + + When I route I should get + | waypoints | route | turns | pronunciations | + | a,e | road,trunk,trunk | depart,turn right,arrive | roooaad,truank,truank | diff --git a/include/engine/guidance/assemble_steps.hpp b/include/engine/guidance/assemble_steps.hpp index 98981cdf8..c930dab5d 100644 --- a/include/engine/guidance/assemble_steps.hpp +++ b/include/engine/guidance/assemble_steps.hpp @@ -112,6 +112,7 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa std::move(pronunciation), std::move(destinations), NO_ROTARY_NAME, + NO_ROTARY_NAME, segment_duration / 10.0, distance, path_point.travel_mode, @@ -170,6 +171,7 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa facade.GetPronunciationForID(step_name_id), facade.GetDestinationsForID(step_name_id), NO_ROTARY_NAME, + NO_ROTARY_NAME, duration / 10., distance, target_mode, @@ -195,6 +197,7 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa facade.GetPronunciationForID(source_node.name_id), facade.GetDestinationsForID(source_node.name_id), NO_ROTARY_NAME, + NO_ROTARY_NAME, duration / 10., leg_geometry.segment_distances[segment_index], source_mode, @@ -229,6 +232,7 @@ inline std::vector assembleSteps(const datafacade::BaseDataFacade &fa facade.GetPronunciationForID(target_node.name_id), facade.GetDestinationsForID(target_node.name_id), NO_ROTARY_NAME, + NO_ROTARY_NAME, ZERO_DURATION, ZERO_DISTANCE, target_mode, diff --git a/include/engine/guidance/route_step.hpp b/include/engine/guidance/route_step.hpp index 7106091c6..6435137a7 100644 --- a/include/engine/guidance/route_step.hpp +++ b/include/engine/guidance/route_step.hpp @@ -61,6 +61,7 @@ struct RouteStep std::string pronunciation; std::string destinations; std::string rotary_name; + std::string rotary_pronunciation; double duration; double distance; extractor::TravelMode mode; @@ -78,6 +79,7 @@ inline RouteStep getInvalidRouteStep() "", "", "", + "", 0, 0, TRAVEL_MODE_INACCESSIBLE, diff --git a/src/engine/api/json_factory.cpp b/src/engine/api/json_factory.cpp index 44d28b1d2..9252bd0de 100644 --- a/src/engine/api/json_factory.cpp +++ b/src/engine/api/json_factory.cpp @@ -240,7 +240,13 @@ util::json::Object makeRouteStep(guidance::RouteStep step, util::json::Value geo if (!step.destinations.empty()) route_step.values["destinations"] = std::move(step.destinations); if (!step.rotary_name.empty()) + { route_step.values["rotary_name"] = std::move(step.rotary_name); + if (!step.rotary_pronunciation.empty()) + { + route_step.values["rotary_pronunciation"] = std::move(step.rotary_pronunciation); + } + } route_step.values["mode"] = detail::modeToString(std::move(step.mode)); route_step.values["maneuver"] = makeStepManeuver(std::move(step.maneuver)); diff --git a/src/engine/guidance/post_processing.cpp b/src/engine/guidance/post_processing.cpp index ad3d7daa4..3dbe0a214 100644 --- a/src/engine/guidance/post_processing.cpp +++ b/src/engine/guidance/post_processing.cpp @@ -44,6 +44,16 @@ inline bool hasManeuver(const RouteStep &first, const RouteStep &second) second.maneuver.instruction.type != TurnType::Suppressed; } +// forward all signage/name data from one step to another. +// When we collapse a step, we might have to transfer the name, pronunciation and similar tags. +inline void forwardStepSignage(RouteStep &destination, const RouteStep &origin) +{ + destination.name_id = origin.name_id; + destination.name = origin.name; + destination.pronunciation = origin.pronunciation; + destination.destinations = origin.destinations; +} + inline bool choiceless(const RouteStep &step, const RouteStep &previous) { // if the next turn is choiceless, we consider longer turn roads collapsable than usually @@ -156,7 +166,10 @@ void fixFinalRoundabout(std::vector &steps) // remember the current name as rotary name in tha case we end in a rotary if (propagation_step.maneuver.instruction.type == TurnType::EnterRotary || propagation_step.maneuver.instruction.type == TurnType::EnterRotaryAtExit) + { propagation_step.rotary_name = propagation_step.name; + propagation_step.rotary_pronunciation = propagation_step.pronunciation; + } else if (propagation_step.maneuver.instruction.type == TurnType::EnterRoundaboutIntersection || @@ -254,7 +267,10 @@ void closeOffRoundabout(const bool on_roundabout, }; steps[1].maneuver.instruction.type = exitToEnter(step.maneuver.instruction.type); if (steps[1].maneuver.instruction.type == TurnType::EnterRotary) + { steps[1].rotary_name = steps[0].name; + steps[1].rotary_pronunciation = steps[0].pronunciation; + } } // Normal exit from the roundabout, or exit from a previously fixed roundabout. Propagate the @@ -266,8 +282,7 @@ void closeOffRoundabout(const bool on_roundabout, // intersections are locations passed along the way const auto exit_intersection = steps[step_index].intersections.front(); const auto exit_bearing = exit_intersection.bearings[exit_intersection.out]; - const auto destination_name = step.name; - const auto destinatino_name_id = step.name_id; + const auto destination_copy = step; if (step_index > 1) { // The very first route-step is head, so we cannot iterate past that one @@ -285,6 +300,7 @@ void closeOffRoundabout(const bool on_roundabout, propagation_step.maneuver.instruction.type == TurnType::EnterRotaryAtExit) { propagation_step.rotary_name = propagation_step.name; + propagation_step.rotary_pronunciation = propagation_step.pronunciation; } else if (propagation_step.maneuver.instruction.type == TurnType::EnterRoundaboutIntersection || @@ -301,8 +317,7 @@ void closeOffRoundabout(const bool on_roundabout, ::osrm::util::guidance::getTurnDirection(angle); } - propagation_step.name = destination_name; - propagation_step.name_id = destinatino_name_id; + forwardStepSignage(propagation_step, destination_copy); invalidateStep(steps[propagation_index + 1]); break; } @@ -424,8 +439,7 @@ void collapseTurnAt(std::vector &steps, util::guidance::mirrorDirectionModifier( steps[one_back_index].maneuver.instruction.direction_modifier); } - steps[one_back_index].name = current_step.name; - steps[one_back_index].name_id = current_step.name_id; + forwardStepSignage(steps[one_back_index], current_step); invalidateStep(steps[step_index]); } } @@ -441,7 +455,7 @@ void collapseTurnAt(std::vector &steps, { BOOST_ASSERT(two_back_index < steps.size()); // the simple case is a u-turn that changes directly into the in-name again - const bool direct_u_turn = steps[two_back_index].name == current_step.name; + const bool direct_u_turn = steps[two_back_index].name_id == current_step.name_id; // however, we might also deal with a dual-collapse scenario in which we have to // additionall collapse a name-change as welll @@ -451,7 +465,8 @@ void collapseTurnAt(std::vector &steps, (steps[next_step_index].maneuver.instruction.type == TurnType::UseLane || isCollapsableInstruction(steps[next_step_index].maneuver.instruction)); const bool u_turn_with_name_change = - continues_with_name_change && steps[next_step_index].name == steps[two_back_index].name; + continues_with_name_change && + steps[next_step_index].name_id == steps[two_back_index].name_id; if (direct_u_turn || u_turn_with_name_change) { @@ -466,8 +481,7 @@ void collapseTurnAt(std::vector &steps, // beginning of this function } - steps[one_back_index].name = steps[two_back_index].name; - steps[one_back_index].name_id = steps[two_back_index].name_id; + forwardStepSignage(steps[one_back_index], steps[two_back_index]); steps[one_back_index].maneuver.instruction.type = TurnType::Continue; steps[one_back_index].maneuver.instruction.direction_modifier = DirectionModifier::UTurn; @@ -780,8 +794,7 @@ std::vector collapseTurns(std::vector steps) steps[one_back_index] = elongate(std::move(steps[one_back_index]), steps[step_index]); - steps[one_back_index].name_id = steps[step_index].name_id; - steps[one_back_index].name = steps[step_index].name; + forwardStepSignage(steps[one_back_index], steps[step_index]); // the turn lanes for this turn are on the sliproad itself, so we have to // remember them steps[one_back_index].intersections.front().lanes = @@ -807,7 +820,7 @@ std::vector collapseTurns(std::vector steps) // These have to be handled in post-processing else if (isCollapsableInstruction(current_step.maneuver.instruction) && current_step.maneuver.instruction.type != TurnType::Suppressed && - steps[getPreviousNameIndex(step_index)].name == current_step.name && + steps[getPreviousNameIndex(step_index)].name_id == current_step.name_id && canCollapseAll(getPreviousNameIndex(step_index) + 1, next_step_index)) { BOOST_ASSERT(step_index > 0); @@ -831,8 +844,8 @@ std::vector collapseTurns(std::vector steps) const auto two_back_index = getPreviousIndex(one_back_index); BOOST_ASSERT(two_back_index < steps.size()); // valid, since one_back is collapsable or a turn and therefore not depart: - const auto &coming_from_name = steps[two_back_index].name; - if (current_step.name == coming_from_name) + const auto &coming_from_name_id = steps[two_back_index].name_id; + if (current_step.name_id == coming_from_name_id) { if (compatible(one_back_step, steps[two_back_index])) { @@ -861,7 +874,7 @@ std::vector collapseTurns(std::vector steps) else if (step_index + 2 < steps.size() && current_step.maneuver.instruction.type == TurnType::NewName && steps[next_step_index].maneuver.instruction.type == TurnType::NewName && - one_back_step.name == steps[next_step_index].name) + one_back_step.name_id == steps[next_step_index].name_id) { // if we are crossing an intersection and go immediately after into a name change, // we don't wan't to collapse the initial intersection. @@ -1077,8 +1090,7 @@ void trimShortSegments(std::vector &steps, LegGeometry &geometry) // as the segment before it. Thus, we have to copy the names // and travel modes from the new next_to_last step. auto &new_next_to_last = *(steps.end() - 2); - next_to_last_step.name = new_next_to_last.name; - next_to_last_step.name_id = new_next_to_last.name_id; + forwardStepSignage(next_to_last_step, new_next_to_last); next_to_last_step.mode = new_next_to_last.mode; // the geometry indices of the last step are already correct; }