Clean up FerrySpeedCalculator, use exact edge distance

[easbar]

Work for #2526.

The estimated_distance tag is now called beeline_distance and represents the straight line distance between the tower nodes of the edge, not the straight line distance between the first and last nodes of the OSM way as estimated_distance did (at some point). We can also use this for the bendiness calculation without applyWayTags (see #2469). The actual length of the road/edge is now stored in an artificial tag road_distance and we get the full point list of the edge by point_list.

More comments inline.

[easbar]

longSpeed and shortSpeed were never used, because the distance was never missing (see below). At least when this code was used with our OSMReader. Also note that longSpeed and shortSpeed made no sense anyway for some encoders, because they exceeded maxSpeed (and this was not accounted for here). Even worse: maxSpeed is used in some (but not all) cases to cap the speed.

[easbar]

estimatedLength == null can never happen.

[easbar]

I understand this now: Before #2448, #2457, cb38af6, estimatedLength == null could happen when the first/last OSM node of a ferry way was missing:

graphhopper/core/src/main/java/com/graphhopper/reader/osm/OSMReader.java

Lines 206 to 212 in f95397b

	GHPoint estimatedCenter = null;
	if (!Double.isNaN(firstLat) && !Double.isNaN(firstLon) && !Double.isNaN(lastLat) && !Double.isNaN(lastLon)) {
	double estimatedDist = distCalc.calcDist(firstLat, firstLon, lastLat, lastLon);
	// Add artificial tag for the estimated distance
	way.setTag("estimated_distance", estimatedDist);
	estimatedCenter = new GHPoint((firstLat + lastLat) / 2, (firstLon + lastLon) / 2);
	}

And the reason we have to consider the distance of the entire way is that 1) the duration tag refers to the duration of the entire way and 2) ferry ways sometimes intersect and are thus split into several edges for which we cannot directly use the duration tag.

[karussell]

Thanks for this cleanup and the previous work with the new tests :)

[easbar]

Thanks for taking a look. I think we can merge here, but this is probably not finished yet. I still don't like how the FerrySpeedCalculator rounds the speed and caps the speed to min/max speed, because this is not part of the speed modelling/calculation, but only something that needs to be done because of our limited speed storage. We should have this in the same place where we do similar stuff (see the different setSpeed methods of the encoders. I know the reason this is here is that FerrySpeedCalculator was inside AbstractFlagEncoder at some point.

[easbar]

lengthy ferries should be faster than short trip ferry

I still think this is something we might have to do. If the duration tag is missing we probably should apply a faster speed for long ferries than for short ones. Especially because the unknownSpeed is very slow (5km/h). There is probably no ferry that crosses the sea at 5km/h. But so far we only applied this long/shortSpeed when the distance was missing (which never happened anyway), not when the duration was missing, or at least we only distinguished between "very slow/unknownSpeed and "as slow as possible"/speedFactor / 2 in this case.

We probably need to figure out how often the duration is missing and if yes if it is sometimes missing also for longer ferries. I can imagine that mappers skip the duration tag for short ferries that just cross a river, but for longer distance ferries this seems like a mapping error and we could even print a warning, report this to OSM and/or skip the ferry in this case. I opened #2532 for this.

[easbar]

We probably need to figure out how often the duration is missing and if yes if it is sometimes missing also for longer ferries.

It is in fact missing in very many cases and also for longer ferries like this one: https://www.openstreetmap.org/way/999670971. More precisely, around 21400 of 24400 (87%) ferries are missing the duration tag and around 10% of them are longer than 20km. Simply falling back to the unknownSpeed=5km/h is thus not a good idea. Here is the route. The average speed is 6km/h (unknownSpeed=5, but rounded to 6, because of speedFactor=2). It takes more than eleven days when really this ferry only needs around two (35km/h).

[easbar]

There is another problem: Some ferry ways share nodes with each other, for example 'Burgeo - Ramea' and 'Grey River - Ramea' here: https://www.openstreetmap.org/way/168781977. This means that since #2448 we use the duration tag of the OSM way, even though they only represent a part of the ferry way. In this case we fall back to the unknown (or minimum) speed and since typically these edges are rather short it's probably not so bad, but still ugly. -> fixed here: 82c16db