From d7ad051ef8eeccbf03d0ea0448ff630d29dd394a Mon Sep 17 00:00:00 2001 From: Michael Krasnyk Date: Wed, 21 Feb 2018 12:56:02 +0100 Subject: [PATCH] Check required tags of `maneuver` relations --- CHANGELOG.md | 1 + features/guidance/maneuver-tag.feature | 29 ++++++++++---- .../maneuver_override_relation_parser.cpp | 38 ++++++++++--------- 3 files changed, 44 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5be7976adf1..fe942116854 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - CHANGED #4835: MAXIMAL_ALLOWED_SEPARATION_WIDTH increased to 12 meters - CHANGED #4842: Lower priority links from a motorway now are used as motorway links [#4842](https://github.com/Project-OSRM/osrm-backend/pull/4842) - CHANGED #4895: Use ramp bifurcations as fork intersections [#4895](https://github.com/Project-OSRM/osrm-backend/issues/4895) + - FIXED #4905: Check required tags of `maneuver` relations [#4905](https://github.com/Project-OSRM/osrm-backend/pull/4905) - Profile: - FIXED: `highway=service` will now be used for restricted access, `access=private` is still disabled for snapping. - ADDED #4775: Exposes more information to the turn function, now being able to set turn weights with highway and access information of the turn as well as other roads at the intersection [#4775](https://github.com/Project-OSRM/osrm-backend/issues/4775) diff --git a/features/guidance/maneuver-tag.feature b/features/guidance/maneuver-tag.feature index c2326aa85dd..951da1452e8 100644 --- a/features/guidance/maneuver-tag.feature +++ b/features/guidance/maneuver-tag.feature @@ -30,7 +30,7 @@ Feature: Maneuver tag support When I route I should get | waypoints | route | turns | - # Testing directly connected from/to + # Testing directly connected from/to | a,j | A Street,C Street,J Street,J Street | depart,turn sharp right,turn left,arrive | | b,g | A Street,C Street,C Street | depart,turn sharp right,arrive | # Testing re-awakening suppressed turns @@ -198,11 +198,26 @@ Feature: Maneuver tag support | pt | 395 | no | primary | And the relations - | type | way:from | node:via | way:via | way:to | maneuver | - | maneuver | zy | p | yp | pt | suppress | + | type | way:from | node:via | way:via | way:to | maneuver | # | + | maneuver | zy | p | yp | pt | suppress | original: depart,on ramp left,fork slight left,arrive | - When I route I should get - | waypoints | route | turns | - | z,t | NY Ave,395,395 | depart,on ramp left,arrive | - #original | z,t | NY Ave,,395,395 | depart,on ramp left,fork slight left,arrive | + And the relations + | type | way:from | way:via | way:to | maneuver | # | + | maneuver | zy | yp | pb | suppress | invalid relation: missing node:via | + And the relations + | type | node:via | way:via | way:to | maneuver | # | + | maneuver | p | yp | pb | suppress | invalid relation: missing way:from | + + And the relations + | type | way:from | node:via | way:via | maneuver | # | + | maneuver | zy | p | yp | suppress | invalid relation: missing way:to | + + And the relations + | type | way:from | node:via | way:via | way:to | maneuver | # | + | maneuver | zy | y, p | yp | pb | suppress | invalid relation: multiple node:via | + + When I route I should get + | waypoints | route | turns | + | z,t | NY Ave,395,395 | depart,on ramp left,arrive | + | z,b | NY Ave,,4th St,4th St | depart,on ramp left,fork slight right,arrive | diff --git a/src/extractor/maneuver_override_relation_parser.cpp b/src/extractor/maneuver_override_relation_parser.cpp index 3dbc1896f95..812b8136a93 100644 --- a/src/extractor/maneuver_override_relation_parser.cpp +++ b/src/extractor/maneuver_override_relation_parser.cpp @@ -52,8 +52,10 @@ ManeuverOverrideRelationParser::TryParse(const osmium::Relation &relation) const maneuver_override.maneuver = relation.tags().get_value_by_key("maneuver", ""); maneuver_override.direction = relation.tags().get_value_by_key("direction", ""); - boost::optional from = boost::none, via = boost::none, to = boost::none; - std::vector via_ways; + bool valid_relation = true; + OSMNodeID via_node = SPECIAL_OSM_NODEID; + OSMWayID from = SPECIAL_OSM_WAYID, to = SPECIAL_OSM_WAYID; + std::vector via_ways; for (const auto &member : relation.members()) { @@ -74,8 +76,9 @@ ManeuverOverrideRelationParser::TryParse(const osmium::Relation &relation) const continue; } BOOST_ASSERT(0 == strcmp("via", role)); - via = static_cast(member.ref()); // set via node id + valid_relation &= via_node == SPECIAL_OSM_NODEID; + via_node = OSMNodeID{static_cast(member.ref())}; break; } case osmium::item_type::way: @@ -83,15 +86,17 @@ ManeuverOverrideRelationParser::TryParse(const osmium::Relation &relation) const 0 == strcmp("via", role)); if (0 == strcmp("from", role)) { - from = static_cast(member.ref()); + valid_relation &= from == SPECIAL_OSM_WAYID; + from = OSMWayID{static_cast(member.ref())}; } else if (0 == strcmp("to", role)) { - to = static_cast(member.ref()); + valid_relation &= to == SPECIAL_OSM_WAYID; + to = OSMWayID{static_cast(member.ref())}; } else if (0 == strcmp("via", role)) { - via_ways.push_back(static_cast(member.ref())); + via_ways.push_back(OSMWayID{static_cast(member.ref())}); } break; case osmium::item_type::relation: @@ -103,18 +108,17 @@ ManeuverOverrideRelationParser::TryParse(const osmium::Relation &relation) const } } - if (from && (via || via_ways.size() > 0) && to) + // Check required roles + valid_relation &= from != SPECIAL_OSM_WAYID; + valid_relation &= to != SPECIAL_OSM_WAYID; + valid_relation &= via_node != SPECIAL_OSM_NODEID; + + if (valid_relation) { - via_ways.insert(via_ways.begin(), *from); - via_ways.push_back(*to); - if (via) - { - maneuver_override.via_node = {*via}; - } - for (const auto &n : via_ways) - { - maneuver_override.via_ways.push_back(OSMWayID{n}); - } + maneuver_override.via_ways.push_back(from); + std::copy(via_ways.begin(), via_ways.end(), std::back_inserter(maneuver_override.via_ways)); + maneuver_override.via_ways.push_back(to); + maneuver_override.via_node = via_node; } else {