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

Apply correct traversal permissions to barrier vertex #5369

Merged
merged 10 commits into from
Sep 26, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.opentripplanner.street.model.StreetTraversalPermission;
import org.opentripplanner.street.model.edge.StreetEdge;
import org.opentripplanner.street.model.edge.StreetEdgeBuilder;
import org.opentripplanner.street.model.vertex.BarrierVertex;
import org.opentripplanner.street.model.vertex.IntersectionVertex;
import org.opentripplanner.street.model.vertex.Vertex;
import org.slf4j.Logger;
Expand Down Expand Up @@ -143,6 +144,7 @@ private void build() {

buildBasicGraph();
buildWalkableAreas(!params.areaVisibility());
validateBarriers();

if (params.staticParkAndRide()) {
List<AreaGroup> areaGroups = groupAreas(osmdb.getParkAndRideAreas());
Expand Down Expand Up @@ -400,6 +402,11 @@ private void buildBasicGraph() {
LOG.info(progress.completeMessage());
}

private void validateBarriers() {
List<BarrierVertex> vertices = graph.getVerticesOfType(BarrierVertex.class);
vertices.forEach(bv -> bv.makeBarrierAtEndReachable());
}

private void setWayName(OSMWithTags way) {
if (!way.hasTag("name")) {
I18NString creativeName = way.getOsmProvider().getWayPropertySet().getCreativeNameForWay(way);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package org.opentripplanner.openstreetmap.model;

import java.util.Set;
import org.locationtech.jts.geom.Coordinate;
import org.opentripplanner.street.model.StreetTraversalPermission;

public class OSMNode extends OSMWithTags {

static final Set<String> MOTOR_VEHICLE_BARRIERS = Set.of("bollard", "bar", "chain");

public double lat;
public double lon;

Expand Down Expand Up @@ -34,13 +38,13 @@ public boolean hasCrossingTrafficLight() {
return hasTag("crossing") && "traffic_signals".equals(getTag("crossing"));
}

/**
* Checks if this node is bollard
/* Checks if this node is a barrier which prevents motor vehicle traffic
*
* @return true if it is
*/
public boolean isBollard() {
return isTag("barrier", "bollard");
public boolean isMotorVehicleBarrier() {
var barrier = this.getTag("barrier");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's too late now because I don't want to hold up the merge of this PR but whenever we touch this code again, there is the following new helper method for checking if it's one of a set of tag values:

/**
* Takes a tag key and checks if the value is any of those in {@code oneOfTags}.
*/
public boolean isOneOfTags(String key, Set<String> oneOfTags) {
return oneOfTags.stream().anyMatch(value -> isTag(key, value));
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, I forgot that new method. Let's fix this in some upcoming OSM related PR.

return barrier != null && MOTOR_VEHICLE_BARRIERS.contains(barrier);
}

/**
Expand All @@ -50,7 +54,7 @@ public boolean isBollard() {
*/
public boolean isBarrier() {
return (
isBollard() ||
isMotorVehicleBarrier() ||
isPedestrianExplicitlyDenied() ||
isBicycleExplicitlyDenied() ||
isMotorcarExplicitlyDenied() ||
Expand All @@ -59,6 +63,18 @@ public boolean isBarrier() {
);
}

/**
* Consider barrier tag in permissions. Leave the rest for the super class.
*/
@Override
public StreetTraversalPermission overridePermissions(StreetTraversalPermission def) {
StreetTraversalPermission permission = def;
if (isMotorVehicleBarrier()) {
permission = permission.remove(StreetTraversalPermission.CAR);
}
return super.overridePermissions(permission);
}

@Override
public String url() {
return String.format("https://www.openstreetmap.org/node/%d", getId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ public class OSMWithTags {
"escape"
);

static final Set<String> LEVEL_TAGS = Set.of("level", "layer");

/* To save memory this is only created when an entity actually has tags. */
private Map<String, String> tags;

Expand All @@ -55,8 +57,6 @@ public class OSMWithTags {

private OsmProvider osmProvider;

static final Set<String> levelTags = Set.of("level", "layer");

public static boolean isFalse(String tagValue) {
return ("no".equals(tagValue) || "0".equals(tagValue) || "false".equals(tagValue));
}
Expand Down Expand Up @@ -589,7 +589,7 @@ private boolean isTagDeniedAccess(String tagName) {
* Some entities can have a semicolon separated list of levels (e.g. elevators)
*/
public Set<String> getLevels() {
var levels = getMultiTagValues(levelTags);
var levels = getMultiTagValues(LEVEL_TAGS);
if (levels.isEmpty()) {
// default
return Set.of("0");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@
*/
public class BarrierVertex extends OsmVertex {

//According to OSM default permissions are access=no, foot=yes, bicycle=yes
public static final StreetTraversalPermission defaultBarrierPermissions =
StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE;
StreetTraversalPermission.ALL;
private StreetTraversalPermission barrierPermissions;

public BarrierVertex(double x, double y, long nodeId) {
Expand All @@ -33,4 +32,36 @@ public StreetTraversalPermission getBarrierPermissions() {
public void setBarrierPermissions(StreetTraversalPermission barrierPermissions) {
this.barrierPermissions = barrierPermissions;
}

/*
* Barrier vertex at the end of a way does not make sense, because
* it creates discontinuity of routing in a single point.
* This method examines if traversal limitations can be removed.
* The logic examines edges referring to the vertex, so it should be
* applied only after the vertex has been linked to the graph.
*/
public void makeBarrierAtEndReachable() {
var edgeCount = this.getDegreeOut() + this.getDegreeIn();
var needsFix = false;
if (edgeCount == 1) {
// only one edge connects the vertex, must be end point
needsFix = true;
} else if (edgeCount == 2) {
var out = this.getOutgoing();
var in = this.getIncoming();
if (
// if only outgoing edges or incoming edges -> vertex does not act as a pass-through point and barrier makes no sense
out.isEmpty() ||
in.isEmpty() ||
// in+out edge pair connects the vertex to a single adjacent vertex -> must be street end point
out.iterator().next().getToVertex() ==
in.iterator().next().getFromVertex()
) {
needsFix = true;
}
}
if (needsFix) {
this.barrierPermissions = StreetTraversalPermission.ALL;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,41 @@ void createArtificalEntrancesToUnlikedParkingLots() {
});
}

/**
* Test that a barrier vertex created when street ends to an access restriction
* will not prevent routing to that vertex
*/
@Test
private void testBarrierAtEnd() {
var deduplicator = new Deduplicator();
var graph = new Graph(deduplicator);

File file = new File(
URLDecoder.decode(
getClass().getResource("noaccess-at-end.pbf").getFile(),
StandardCharsets.UTF_8
)
);
OsmProvider provider = new OsmProvider(file, false);
OsmModule loader = OsmModule.of(provider, graph).build();
loader.buildGraph();

RouteRequest request = new RouteRequest();

// Route along a simple 3 vertex highway which ends to a 'access=none' node
Vertex start = graph.getVertex(VertexLabel.osm(1));
Vertex end = graph.getVertex(VertexLabel.osm(3));

GraphPathFinder graphPathFinder = new GraphPathFinder(null);
List<GraphPath<State, Edge, Vertex>> pathList = graphPathFinder.graphPathFinderEntryPoint(
request,
Set.of(start),
Set.of(end)
);
assertNotNull(pathList);
assertFalse(pathList.isEmpty());
}

@Nonnull
private Graph buildParkingLots() {
var graph = new Graph();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,42 +24,42 @@ public class BarrierVertexTest {

@Test
public void testBarrierPermissions() {
OSMNode simpleBarier = new OSMNode();
assertFalse(simpleBarier.isBollard());
simpleBarier.addTag("barrier", "bollard");
assertTrue(simpleBarier.isBollard());
OSMNode simpleBarrier = new OSMNode();
assertFalse(simpleBarrier.isMotorVehicleBarrier());
simpleBarrier.addTag("barrier", "bollard");
assertTrue(simpleBarrier.isMotorVehicleBarrier());
String label = "simpleBarrier";
BarrierVertex bv = new BarrierVertex(simpleBarier.lon, simpleBarier.lat, 0);
BarrierVertex bv = new BarrierVertex(simpleBarrier.lon, simpleBarrier.lat, 0);
bv.setBarrierPermissions(
simpleBarier.overridePermissions(BarrierVertex.defaultBarrierPermissions)
simpleBarrier.overridePermissions(BarrierVertex.defaultBarrierPermissions)
);
assertEquals(StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE, bv.getBarrierPermissions());

simpleBarier.addTag("foot", "yes");
simpleBarrier.addTag("foot", "yes");
bv.setBarrierPermissions(
simpleBarier.overridePermissions(BarrierVertex.defaultBarrierPermissions)
simpleBarrier.overridePermissions(BarrierVertex.defaultBarrierPermissions)
);
assertEquals(StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE, bv.getBarrierPermissions());
simpleBarier.addTag("bicycle", "yes");
simpleBarrier.addTag("bicycle", "yes");
bv.setBarrierPermissions(
simpleBarier.overridePermissions(BarrierVertex.defaultBarrierPermissions)
simpleBarrier.overridePermissions(BarrierVertex.defaultBarrierPermissions)
);
assertEquals(StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE, bv.getBarrierPermissions());
simpleBarier.addTag("access", "no");
simpleBarrier.addTag("access", "no");
bv.setBarrierPermissions(
simpleBarier.overridePermissions(BarrierVertex.defaultBarrierPermissions)
simpleBarrier.overridePermissions(BarrierVertex.defaultBarrierPermissions)
);
assertEquals(StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE, bv.getBarrierPermissions());

simpleBarier.addTag("motor_vehicle", "no");
simpleBarrier.addTag("motor_vehicle", "no");
bv.setBarrierPermissions(
simpleBarier.overridePermissions(BarrierVertex.defaultBarrierPermissions)
simpleBarrier.overridePermissions(BarrierVertex.defaultBarrierPermissions)
);
assertEquals(StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE, bv.getBarrierPermissions());

simpleBarier.addTag("bicycle", "no");
simpleBarrier.addTag("bicycle", "no");
bv.setBarrierPermissions(
simpleBarier.overridePermissions(BarrierVertex.defaultBarrierPermissions)
simpleBarrier.overridePermissions(BarrierVertex.defaultBarrierPermissions)
);
assertEquals(StreetTraversalPermission.PEDESTRIAN, bv.getBarrierPermissions());

Expand Down Expand Up @@ -94,8 +94,8 @@ public void testBarrierPermissions() {
@Test
public void testStreetsWithBollard() {
Graph graph = new Graph();
//default permissions are PEDESTRIAND and BICYCLE
BarrierVertex bv = new BarrierVertex(2.0, 2.0, 0);
bv.setBarrierPermissions(StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE);

StreetVertex endVertex = StreetModelForTest.intersectionVertex("end_vertex", 1.0, 2.0);

Expand Down Expand Up @@ -139,18 +139,18 @@ public void testStreetsWithBollard() {
assertTrue(endVertex_to_bv_forward.canTraverse(TraverseMode.BICYCLE));
assertTrue(endVertex_to_bv_forward.canTraverse(TraverseMode.WALK));

//tests bollard which doesn't allow cycling
BarrierVertex noBicycleBollard = new BarrierVertex(1.5, 1, 0);
noBicycleBollard.setBarrierPermissions(StreetTraversalPermission.PEDESTRIAN);
StreetEdge no_bike_to_endVertex = edge(noBicycleBollard, endVertex, 100, false);
//tests bollard which allows only walking
BarrierVertex onlyWalkBollard = new BarrierVertex(1.5, 1, 0);
onlyWalkBollard.setBarrierPermissions(StreetTraversalPermission.PEDESTRIAN);
StreetEdge edge = edge(onlyWalkBollard, endVertex, 100, false);

assertTrue(no_bike_to_endVertex.canTraverse(new TraverseModeSet(TraverseMode.CAR)));
assertTrue(no_bike_to_endVertex.canTraverse(new TraverseModeSet(TraverseMode.BICYCLE)));
assertTrue(no_bike_to_endVertex.canTraverse(new TraverseModeSet(TraverseMode.WALK)));
assertTrue(edge.canTraverse(new TraverseModeSet(TraverseMode.CAR)));
assertTrue(edge.canTraverse(new TraverseModeSet(TraverseMode.BICYCLE)));
assertTrue(edge.canTraverse(new TraverseModeSet(TraverseMode.WALK)));

assertFalse(no_bike_to_endVertex.canTraverse(TraverseMode.CAR));
assertFalse(no_bike_to_endVertex.canTraverse(TraverseMode.BICYCLE));
assertTrue(no_bike_to_endVertex.canTraverse(TraverseMode.WALK));
assertFalse(edge.canTraverse(TraverseMode.CAR));
assertFalse(edge.canTraverse(TraverseMode.BICYCLE));
assertTrue(edge.canTraverse(TraverseMode.WALK));
}

/**
Expand Down
Binary file not shown.