Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check required tags of maneuver relations #4905

Merged
merged 1 commit into from
Feb 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- 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)
- CHANGED #4893: Handle motorway forks with links as normal motorway intersections[#4893](https://github.com/Project-OSRM/osrm-backend/issues/4893)
- 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)
Expand Down
29 changes: 22 additions & 7 deletions features/guidance/maneuver-tag.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 |
38 changes: 21 additions & 17 deletions src/extractor/maneuver_override_relation_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::uint64_t> from = boost::none, via = boost::none, to = boost::none;
std::vector<std::uint64_t> via_ways;
bool valid_relation = true;
OSMNodeID via_node = SPECIAL_OSM_NODEID;
OSMWayID from = SPECIAL_OSM_WAYID, to = SPECIAL_OSM_WAYID;
std::vector<OSMWayID> via_ways;

for (const auto &member : relation.members())
{
Expand All @@ -74,24 +76,27 @@ ManeuverOverrideRelationParser::TryParse(const osmium::Relation &relation) const
continue;
}
BOOST_ASSERT(0 == strcmp("via", role));
via = static_cast<std::uint64_t>(member.ref());
// set via node id
valid_relation &= via_node == SPECIAL_OSM_NODEID;
via_node = OSMNodeID{static_cast<std::uint64_t>(member.ref())};
break;
}
case osmium::item_type::way:
BOOST_ASSERT(0 == strcmp("from", role) || 0 == strcmp("to", role) ||
0 == strcmp("via", role));
if (0 == strcmp("from", role))
{
from = static_cast<std::uint64_t>(member.ref());
valid_relation &= from == SPECIAL_OSM_WAYID;
from = OSMWayID{static_cast<std::uint64_t>(member.ref())};
}
else if (0 == strcmp("to", role))
{
to = static_cast<std::uint64_t>(member.ref());
valid_relation &= to == SPECIAL_OSM_WAYID;
to = OSMWayID{static_cast<std::uint64_t>(member.ref())};
}
else if (0 == strcmp("via", role))
{
via_ways.push_back(static_cast<std::uint64_t>(member.ref()));
via_ways.push_back(OSMWayID{static_cast<std::uint64_t>(member.ref())});
}
break;
case osmium::item_type::relation:
Expand All @@ -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
{
Expand Down