fix roundabout-handling when name changes

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:

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 |

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) ||
+    const auto obvious_change =
-                                (names_are_equal && ref_is_contained) ||
+        (names_are_empty && refs_are_empty) || (names_are_equal && ref_is_contained) ||
-                                (names_are_equal && refs_are_empty) || name_is_removed ||
+        (names_are_equal && refs_are_empty) || (ref_is_contained && name_is_removed) ||
-                                ref_is_removed || is_suffix_change;
+        (names_are_equal && ref_is_removed) || is_suffix_change;
 
     return !obvious_change;
 }

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<RouteStep> collapseTurns(std::vector<RouteStep> steps)
     for (std::size_t step_index = 1; step_index + 1 < steps.size(); ++step_index)
     {
         const auto &current_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());

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<unsigned> roundabout_name_ids;
     std::unordered_set<unsigned> 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;