From 306744e5cb705b501d6d78d7b34f5934636d24ff Mon Sep 17 00:00:00 2001 From: Moritz Kobitzsch Date: Fri, 17 Jun 2016 11:00:17 +0200 Subject: [PATCH] fix roundabout-handling when name changes --- CHANGELOG.md | 4 ++ features/guidance/roundabout.feature | 37 +++++++++++++++++++ include/extractor/guidance/toolkit.hpp | 8 ++-- src/engine/guidance/post_processing.cpp | 6 ++- src/extractor/guidance/roundabout_handler.cpp | 35 +++++++++++------- 5 files changed, 71 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdde15610..677ca7f21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 5.2.3 + - Bugfixes: + - Fixed an issue with name changes in roundabouts that could result in crashes + # 5.2.2 Changes from 5.2.1 - Bugfixes: diff --git a/features/guidance/roundabout.feature b/features/guidance/roundabout.feature index 37b9b08f6..6d262bae8 100644 --- a/features/guidance/roundabout.feature +++ b/features/guidance/roundabout.feature @@ -361,3 +361,40 @@ Feature: Basic Roundabout | a,f | ab,ef,ef | depart,roundabout-exit-2,arrive | 0->180,180->270,180->0 | | a,h | ab,gh,gh | depart,roundabout-exit-1,arrive | 0->180,180->270,270->0 | + Scenario: Motorway Roundabout + #See 39.933742 -75.082345 + Given the node map + | | | | | l | | | | a | | i | + | | | | | | | | | | | | + | | | | | | | | | | | | + | | | | | | | b | | | | | + | | | | c | | | | | | | | + | | | | | | | | | | | | + | | | | | | | | | h | | | + | n | | | | | | | | | | | + | | | | | | | | | | | | + | | | d | | | | | | | | j | + | | | | | | | | | | | | + | | | | | m | | | g | | | | + | | | | | | | | | | | | + | | | | | | | | | | | | + | | | e | | f | | | | | | | + + And the ways + | nodes | junction | name | highway | oneway | ref | + | ab | | crescent | trunk | yes | US 130 | + | bcd | roundabout | crescent | trunk | yes | US 130 | + | de | | crescent | trunk | yes | US 130 | + | fg | | crescent | trunk | yes | US 130 | + | gh | roundabout | crescent | trunk | yes | US 130 | + | hi | | crescent | trunk | yes | US 130 | + | jh | | | trunk_link | yes | NJ 38 | + | hb | roundabout | | trunk_link | yes | NJ 38 | + | bl | | | trunk_link | yes | NJ 38 | + | cnd | | kaighns | trunk_link | yes | | + | dmg | roundabout | | trunk_link | yes | | + + When I route I should get + | waypoints | route | turns | + | a,e | crescent (US 130),crescent (US 130),crescent (US 130) | depart,roundabout-exit-3,arrive | + | j,l | NJ 38,NJ 38,NJ 38 | depart,roundabout-exit-2,arrive | diff --git a/include/extractor/guidance/toolkit.hpp b/include/extractor/guidance/toolkit.hpp index 0c7d26738..bd21137cd 100644 --- a/include/extractor/guidance/toolkit.hpp +++ b/include/extractor/guidance/toolkit.hpp @@ -405,10 +405,10 @@ inline bool requiresNameAnnounced(const std::string &from, (from_ref.find(to_ref) != std::string::npos || to_ref.find(from_ref) != std::string::npos); const auto ref_is_removed = !from_ref.empty() && to_ref.empty(); - const auto obvious_change = (names_are_empty && refs_are_empty) || - (names_are_equal && ref_is_contained) || - (names_are_equal && refs_are_empty) || name_is_removed || - ref_is_removed || is_suffix_change; + const auto obvious_change = + (names_are_empty && refs_are_empty) || (names_are_equal && ref_is_contained) || + (names_are_equal && refs_are_empty) || (ref_is_contained && name_is_removed) || + (names_are_equal && ref_is_removed) || is_suffix_change; return !obvious_change; } diff --git a/src/engine/guidance/post_processing.cpp b/src/engine/guidance/post_processing.cpp index afedcd0d6..dfad1e028 100644 --- a/src/engine/guidance/post_processing.cpp +++ b/src/engine/guidance/post_processing.cpp @@ -231,7 +231,9 @@ void closeOffRoundabout(const bool on_roundabout, // instruction and move it right to the beginning to make sure to immediately announce the // exit. BOOST_ASSERT(leavesRoundabout(steps[1].maneuver.instruction) || - steps[1].maneuver.instruction.type == TurnType::StayOnRoundabout); + steps[1].maneuver.instruction.type == TurnType::StayOnRoundabout || + steps[1].maneuver.instruction.type == TurnType::Suppressed || + steps[1].maneuver.instruction.type == TurnType::NoTurn); steps[0].geometry_end = 1; steps[1] = forwardInto(steps[1], steps[0]); steps[0].duration = 0; @@ -627,7 +629,7 @@ std::vector collapseTurns(std::vector steps) for (std::size_t step_index = 1; step_index + 1 < steps.size(); ++step_index) { const auto ¤t_step = steps[step_index]; - if( current_step.maneuver.instruction.type == TurnType::NoTurn ) + if (current_step.maneuver.instruction.type == TurnType::NoTurn) continue; const auto one_back_index = getPreviousIndex(step_index); BOOST_ASSERT(one_back_index < steps.size()); diff --git a/src/extractor/guidance/roundabout_handler.cpp b/src/extractor/guidance/roundabout_handler.cpp index a79075d54..2d0e54d95 100644 --- a/src/extractor/guidance/roundabout_handler.cpp +++ b/src/extractor/guidance/roundabout_handler.cpp @@ -1,5 +1,5 @@ -#include "extractor/guidance/roundabout_handler.hpp" #include "extractor/guidance/constants.hpp" +#include "extractor/guidance/roundabout_handler.hpp" #include "extractor/guidance/toolkit.hpp" #include "util/coordinate_calculation.hpp" @@ -210,11 +210,11 @@ RoundaboutType RoundaboutHandler::getRoundaboutType(const NodeID nid) const return util::Coordinate(node_info_list[node].lon, node_info_list[node].lat); }; - unsigned roundabout_name_id = 0; + std::unordered_set roundabout_name_ids; std::unordered_set connected_names; const auto getNextOnRoundabout = - [this, &roundabout_name_id, &connected_names](const NodeID node) { + [this, &roundabout_name_ids, &connected_names](const NodeID node) { EdgeID continue_edge = SPECIAL_EDGEID; for (const auto edge : node_based_graph.GetAdjacentEdgeRange(node)) { @@ -226,16 +226,24 @@ RoundaboutType RoundaboutHandler::getRoundaboutType(const NodeID nid) const // fork in roundabout return SPECIAL_EDGEID; } - // roundabout does not keep its name - if (roundabout_name_id != 0 && roundabout_name_id != edge_data.name_id && - requiresNameAnnounced(name_table.GetNameForID(roundabout_name_id), - name_table.GetNameForID(edge_data.name_id), - street_name_suffix_table)) - { - return SPECIAL_EDGEID; - } - roundabout_name_id = edge_data.name_id; + if (EMPTY_NAMEID != edge_data.name_id) + { + bool add = true; + for (auto name_id : roundabout_name_ids) + { + + if (!requiresNameAnnounced(name_table.GetNameForID(name_id), + name_table.GetNameForID(edge_data.name_id), + street_name_suffix_table)) + { + add = false; + break; + } + } + if (add) + roundabout_name_ids.insert(edge_data.name_id); + } continue_edge = edge; } @@ -320,7 +328,8 @@ RoundaboutType RoundaboutHandler::getRoundaboutType(const NodeID nid) const // used with a reference and without. This will be fixed automatically // when we handle references separately or if the useage is more consistent - if (0 != roundabout_name_id && 0 == connected_names.count(roundabout_name_id)) + if (1 == roundabout_name_ids.size() && + 0 == connected_names.count(*roundabout_name_ids.begin())) return RoundaboutType::Rotary; else return RoundaboutType::Roundabout;