Bike share stations may be linked to un-rideable roads

[abyrd]

Expected behavior

A bike share station can be reached by both walking (when going to rent a bike or walking after dropping one off) and by the vehicle being rented (when leaving or arriving at the station on the vehicle).

Observed behavior

The bike share station is linked using the method that defaults to WALK mode, so we only check that the way it's linked to is walkable, not whether it has bicycle permissions. It would be possible to link to a road that allows walking but not biking. This could happen around walking paths in areas with separate bikeways.

There is a stopgap configuration option that allows expressing carshare systems with GBFS, which essentially sets up a bikeshare system where the mode after renting a vehicle is CAR. The problem is most pronounced with that option enabled, because it is even more likely to find walking roads that do not allow cars.

Version of OTP used (exact commit hash or JAR name)

Current head of master: 6096412cccce24bb0f47dad8a19d3e33d824261a

Data sets in use (links to GTFS and OSM PBF files)

Netherlands latest Geofabrik extract.

Router config and graph build config JSON

Critical config is this router-config.json for an OVFlex system in the Netherlands:

{
  routingDefaults: {
    bikeRentalPickupTime: 60,
    bikeRentalPickupCost: 120,
    bikeRentalDropoffTime: 30,
    bikeRentalDropoffCost: 30
  },
  "updaters": [
         {
              "type": "bike-rental",
              "frequencySec": -1,
              "sourceType": "gbfs",
              "url": "http://gbfs.openov.nl/brengflex-arnhem/",
              "routeAsCar": true
         }
   ]
}

Steps to reproduce the problem

Planning a trip with these parameters:

"requestParameters": {
            "date": "07-19-2018",
            "mode": "WALK,BICYCLE_RENT",
            "arriveBy": "false",
            "wheelchair": "false",
            "fromPlace": "53.004001, 4.780883",
            "toPlace": "53.053751, 4.797230",
            "time": "18:00",
            "maxWalkDistance": "50.0",
            "locale": "nl"
        }

or URL http://planner.plannerstack.com/otp/routers/default/plan?fromPlace=53.004001, 4.780883&toPlace=53.053751, 4.797230&time=18:00&date=07-19-2018&locale=nl&maxWalkDistance=50.0&wheelchair=false&arriveBy=false&mode=BICYCLE_RENT,WALK

does not yield a trip using nearby rental stations because they are not properly linked. This can be verified using the ?debug_layers=true URL parameter in the default OTP UI, looking at the permissions of the roads where "bike" rental stations are linked.

[abyrd]

Adapting a suggestion from @skinkie I propose the following fix:

--- a/src/main/java/org/opentripplanner/updater/bike_rental/BikeRentalUpdater.java
+++ b/src/main/java/org/opentripplanner/updater/bike_rental/BikeRentalUpdater.java
@@ -176,8 +176,11 @@ public class BikeRentalUpdater extends PollingGraphUpdater {
                 if (vertex == null) {
                     vertex = new BikeRentalStationVertex(graph, station);
                     if (!linker.link(vertex)) {
-                        // the toString includes the text "Bike rental station"
-                        LOG.warn("{} not near any streets; it will not be usable.", station);
+                        // station's toString() includes the text "Bike rental station"
+                        LOG.warn("{} not near any walkable streets; it will not be usable.", station);
+                    }
+                    if (!linker.link(vertex, vertex.getVehicleMode(), null)) {
+                        LOG.warn("{} not near any streets accessible to rental vehicles; it will not be usable.", station);
                     }
                     verticesByStation.put(station, vertex);
                     new RentABikeOnEdge(vertex, vertex, station.networks);

so we create two link edges, one to a walking road and one to any road allowing the vehicle type being rented. The first conditional can also be modified to explicitly mention WALK for readability: if (!linker.link(vertex, TraverseMode.WALK, null)).

However this approach can (and often will) lead to two link edges to the same road though. So we will want to add a check in the linker to avoid double-linkage.

[abyrd]

It turns out that the bicycle case is already handled, but in a non-ideal way. In SimpleStreetSplitter#link() there is a special case with no explanatory comments that sets the mode to BICYCLE,WALK when it is set to BICYCLE. This causes it to link to only roads that support both bicycles and walking. We could do the same thing for cars by changing this to final TraverseModeSet traverseModeSet = new TraverseModeSet(TraverseMode.WALK, traverseMode);. Then every linking target would have to support both walking and the additional mode, if one is specified. The question is, do we want to link to a single road that supports both modes or multiple roads each supporting one mode? It will depend on typical tagging and topology in OSM, we should check that in the OTP debug layers for the regions where problems were seen in the past.

[abyrd]

One issue with double-linking, linking the stops to a walking road and a (possibly separate) driving road, is that it creates chains of link edges that can serve as a shortcut. It makes a path for the router to pass from the walking road to the driving road via the bicycle rental node by taking two link edges in a row. This is currently disallowed by a conditional in StreetTransitLink#traverse, but there has been discussion of removing this check in #2371. Note that the standard linking code that connects transit stops appears to create multiple linkages as well, to multiple nearby streets when they're all within a similar distance. A decision needs to be made about whether nodes can be connected to the street network with multiple link edges; if this is to be maintained, the check needs to be made more subtle to solve #2371.

Note that even if the traverse method forbids traversing two link edges in a row, this will not block dropping off a rented vehicle and exiting the station via a second link edge, because that produces a sequence [ link -> bike dropoff loop edge -> link ]

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days