mfdz / OpenTripPlanner

MFDZ (HSLdevcom/)OpenTripPlanner clone supporting Carpooling.
http://www.opentripplanner.org
Other
5 stars 4 forks source link

Fixing unroutable locations #49

Open leonardehrenfried opened 3 years ago

leonardehrenfried commented 3 years ago

In Herrenberg and the surrounding regions there are a number of locations that are unroutable for cars:

URL prod URL dev :heavy_check_mark:
URL prod URL dev :heavy_check_mark:
URL prod URL dev :x:

It’s not entirely clear why that is but a frequent source for unroutable locations are “islands”: small clusters of roads that OTP cannot route to or out of because they are not connected to the road network, while being reachable by bike or for pedestrians. A cause for this can be incorrectly mapped roads, gates or bollards. In any case, whilst it isn’t clear that the above locations contain mapping errors, even if they do, it should not lead to unroutable locations but rather to sub-optimal routing results.

Related to #46

leonardehrenfried commented 3 years ago

Helsinki reported that they have a similar problem and made a PR for OTP1: https://github.com/opentripplanner/OpenTripPlanner/pull/2676

They are planning to port it to OTP2 but cannot really say when that happens.

flaktack commented 3 years ago

The routing issues seems to be caused by three things:

flaktack commented 3 years ago

I've pushed a branch with all the related fixes: feature/routing-test, which contains the drive-only linking fixes, no-thru traffic render and island pruning for car only (not bicycle).

These are the linked trip-plans using the debug client:

This trip-plan also only works in one direction -- we working on creating explicit test to see why the two tests fail in one direction only, and fixing it if the reason is clear.

leonardehrenfried commented 3 years ago

@hbruch What is the expected routing result when you want to drive to Marktplatz? As far as I can see its surrounded by either pedestrian areas or no-traffic areas.

Screenshot from 2021-06-03 12-39-19 Screenshot from 2021-06-03 12-39-09

Should the result just snap to the nearest drivable road?

hbruch commented 3 years ago

Should be park and walk

leonardehrenfried commented 3 years ago

Just to clarify: in the car tile in DT you want to see the car being parked in the street and then walk? I don't think this is how it works right now. The driving leg is always the latest one.

leonardehrenfried commented 3 years ago

Screenshot from 2021-06-03 12-58-58

Or do you mean something like this?

hbruch commented 3 years ago

In the car tile, but neverthless parked in a garage and the walk. However, I wonder if there isn‘t a better parking than the one at the station, eg Bronntor?

leonardehrenfried commented 3 years ago

There is but the ParkAPI updater is not complete yet: https://github.com/mfdz/OpenTripPlanner/issues/55

leonardehrenfried commented 3 years ago

Sorry to belabour the point, but you want Park & Ride results in the car tile?

This is currently not possible as the mode is CAR and not CAR_PARK.

hbruch commented 3 years ago

I‘m aware, that technically car_park returns park & walk and park & transit. But from a user perspective, car and walk is not park & ride.

As a user, I want to see car & walk in the car tile, car & transit in the park & ride tile

leonardehrenfried commented 3 years ago

Not sure I can immediately come up with a good implementation strategy. Can you?

flaktack commented 3 years ago

For the CAR tile depending on whether to is inside a no-car-zone (some polygon) one of two request may be sent: CAR_PARK or CAR (without any TRANSIT modes).

For the P+R tile CAR_PARK,TRANSIT (any TRANSIT type works, there just has to be something) could be modified to not return directMode = CAR_TO_PARK results: https://github.com/mfdz/OpenTripPlanner/blob/d2687d9c0a2f3f197e520ffdb8d5281baa15d5ee/src/main/java/org/opentripplanner/api/parameter/QualifiedModeSet.java#L145-L149

directMode = transitModes.isEmpty() ? StreetMode.CAR_TO_PARK : null

Would this work?

leonardehrenfried commented 3 years ago

Estimation: 0/2/1 days

leonardehrenfried commented 3 years ago

I merged #88 and this improves the situation but I also found an even harder case: https://www.openstreetmap.org/directions?engine=graphhopper_car&route=48.60748%2C8.87760%3B48.60500%2C8.87627

This doesn't work in OTP but the Graphhopper one is also incorrect as it goes through a no-thru traffic way.

The gate is also ignored by OTP.

leonardehrenfried commented 3 years ago

After deployment to dev you can now route to Herrenberg Marktplatz: https://dev.stadtnavi.eu/reiseplan/Ehningen%3A%3A48.6592556%2C8.9405097/Markplatz%2C%20Marktplatz%201%2C%2071083%20Herrenberg%3A%3A48.5963346%2C8.8699234/car?time=1623166926

But as Holger already said: we actually want to suggest parking the car in a parking lot.

leonardehrenfried commented 3 years ago

I've also updated the location ins the description. The ones in Herrenberg are fixed but the Ludwigsburg one is not.

leonardehrenfried commented 3 years ago

@flaktack If you have the capacity, can you please look at the last remaining unroutable car location?

https://api.dev.stadtnavi.eu/?module=planner&fromPlace=48.89323798324758%2C9.17582631111145&toPlace=48.89219576968702%2C9.184058010578156&time=10%3A50am&date=06-25-2021&mode=CAR&arriveBy=false&wheelchair=false&debugItineraryFilter=false&locale=en&itinIndex=0

If you move the destination needle on top of the station, car routing stops working.

flaktack commented 3 years ago

That seems to be caused by this elevator: https://www.openstreetmap.org/way/301633786 https://api.dev.stadtnavi.eu/?module=planner&fromPlace=48.89228218077439%2C9.18465346097946&toPlace=48.89204763604696%2C9.184771478176117&time=10%3A50am&date=06-25-2021&mode=CAR&arriveBy=false&wheelchair=false&debugItineraryFilter=false&locale=en&itinIndex=0

image

We're looking into what the options are.

leonardehrenfried commented 3 years ago

Well spotted!

I see a couple of problems with this:

flaktack commented 3 years ago

When matching OSMSpecificer-s the AND is lenient and so the best match for highway=elevator is: https://github.com/mfdz/OpenTripPlanner/blob/da5cc644868c6607b885c5dc386ec1cc744d4036/src/main/java/org/opentripplanner/graph_builder/module/osm/GermanyWayPropertySetSource.java#L57-L58

If the permissions for the way are ALL then the pruning removes the elevator as expected.

I'm not sure what the best approach here would be.

leonardehrenfried commented 3 years ago

Sorry, completely missed this. What if you just set a permission to BICYCLE_AND_WALK for highway=elevator?

That would be correct IMO, since this tag is intented for indoor mapping of elevators.

leonardehrenfried commented 3 years ago

On the other hand we should really remove those islands.

flaktack commented 3 years ago

We went down a few paths, but the solution will be to:

The current pruning logic doesn't removing CAR islands, only pedestrian ones.