From b5c6c1b9995eed5d0951aba07e2eb85c5c261ca8 Mon Sep 17 00:00:00 2001 From: Andi Date: Thu, 27 Jul 2023 11:28:56 +0200 Subject: [PATCH] Use same definition for ferries for road environment and speed parsers (#2853) --- .../java/com/graphhopper/reader/osm/OSMReader.java | 3 ++- .../routing/util/FerrySpeedCalculator.java | 14 ++++++++------ .../util/parsers/AbstractAverageSpeedParser.java | 6 ------ .../util/parsers/BikeCommonAccessParser.java | 5 ++--- .../util/parsers/BikeCommonAverageSpeedParser.java | 2 +- .../util/parsers/BikeCommonPriorityParser.java | 5 ++--- .../routing/util/parsers/CarAccessParser.java | 5 ++--- .../util/parsers/CarAverageSpeedParser.java | 2 +- .../routing/util/parsers/FootAccessParser.java | 4 ++-- .../util/parsers/FootAverageSpeedParser.java | 2 +- .../routing/util/parsers/FootPriorityParser.java | 5 ++--- .../util/parsers/OSMRoadEnvironmentParser.java | 5 ++--- .../util/parsers/WheelchairAverageSpeedParser.java | 2 +- .../util/parsers/OSMRoadEnvironmentParserTest.java | 9 ++++++++- 14 files changed, 34 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/com/graphhopper/reader/osm/OSMReader.java b/core/src/main/java/com/graphhopper/reader/osm/OSMReader.java index 0b523987307..91b0951c032 100644 --- a/core/src/main/java/com/graphhopper/reader/osm/OSMReader.java +++ b/core/src/main/java/com/graphhopper/reader/osm/OSMReader.java @@ -37,6 +37,7 @@ import com.graphhopper.routing.ev.State; import com.graphhopper.routing.util.AreaIndex; import com.graphhopper.routing.util.CustomArea; +import com.graphhopper.routing.util.FerrySpeedCalculator; import com.graphhopper.routing.util.OSMParsers; import com.graphhopper.routing.util.countryrules.CountryRule; import com.graphhopper.routing.util.countryrules.CountryRuleFactory; @@ -223,7 +224,7 @@ protected boolean isCalculateWayDistance(ReaderWay way) { } private boolean isFerry(ReaderWay way) { - return way.hasTag("route", "ferry", "shuttle_train"); + return FerrySpeedCalculator.isFerry(way); } /** diff --git a/core/src/main/java/com/graphhopper/routing/util/FerrySpeedCalculator.java b/core/src/main/java/com/graphhopper/routing/util/FerrySpeedCalculator.java index a13393ccd45..24542990599 100644 --- a/core/src/main/java/com/graphhopper/routing/util/FerrySpeedCalculator.java +++ b/core/src/main/java/com/graphhopper/routing/util/FerrySpeedCalculator.java @@ -6,17 +6,19 @@ import com.graphhopper.routing.util.parsers.TagParser; import com.graphhopper.storage.IntsRef; -import java.util.Arrays; -import java.util.Collection; - public class FerrySpeedCalculator implements TagParser { - public static final Collection FERRIES = Arrays.asList("shuttle_train", "ferry"); - private DecimalEncodedValue ferrySpeedEnc; + private final DecimalEncodedValue ferrySpeedEnc; public FerrySpeedCalculator(DecimalEncodedValue ferrySpeedEnc) { this.ferrySpeedEnc = ferrySpeedEnc; } + public static boolean isFerry(ReaderWay way) { + return way.hasTag("route", "ferry") && !way.hasTag("ferry", "no") || + // TODO shuttle_train is sometimes also used in relations, e.g. https://www.openstreetmap.org/relation/1932780 + way.hasTag("route", "shuttle_train") && !way.hasTag("shuttle_train", "no"); + } + static double getSpeed(ReaderWay way) { // todo: We currently face two problems related to ferry speeds: // 1) We cannot account for waiting times for short ferries (when we do the ferry speed is slower than the slowest we can store) @@ -54,7 +56,7 @@ public static double minmax(double speed, DecimalEncodedValue avgSpeedEnc) { @Override public void handleWayTags(int edgeId, EdgeIntAccess edgeIntAccess, ReaderWay way, IntsRef relationFlags) { - if (!way.hasTag("highway") && way.hasTag("route", FERRIES)) { + if (isFerry(way)) { double ferrySpeed = minmax(getSpeed(way), ferrySpeedEnc); ferrySpeedEnc.setDecimal(false, edgeId, edgeIntAccess, ferrySpeed); } diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/AbstractAverageSpeedParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/AbstractAverageSpeedParser.java index a39b4deeaae..2749b4b19b7 100644 --- a/core/src/main/java/com/graphhopper/routing/util/parsers/AbstractAverageSpeedParser.java +++ b/core/src/main/java/com/graphhopper/routing/util/parsers/AbstractAverageSpeedParser.java @@ -3,19 +3,13 @@ import com.graphhopper.reader.ReaderWay; import com.graphhopper.routing.ev.DecimalEncodedValue; import com.graphhopper.routing.ev.EdgeIntAccess; -import com.graphhopper.routing.util.FerrySpeedCalculator; import com.graphhopper.routing.util.parsers.helpers.OSMValueExtractor; import com.graphhopper.storage.IntsRef; -import java.util.HashSet; -import java.util.Set; - -import static com.graphhopper.routing.util.FerrySpeedCalculator.FERRIES; public abstract class AbstractAverageSpeedParser implements TagParser { // http://wiki.openstreetmap.org/wiki/Mapfeatures#Barrier protected final DecimalEncodedValue avgSpeedEnc; - protected final Set ferries = new HashSet<>(FERRIES); protected final DecimalEncodedValue ferrySpeedEnc; protected AbstractAverageSpeedParser(DecimalEncodedValue speedEnc, DecimalEncodedValue ferrySpeedEnc) { diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/BikeCommonAccessParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/BikeCommonAccessParser.java index e8f1f472b18..685f073cc85 100644 --- a/core/src/main/java/com/graphhopper/routing/util/parsers/BikeCommonAccessParser.java +++ b/core/src/main/java/com/graphhopper/routing/util/parsers/BikeCommonAccessParser.java @@ -3,13 +3,12 @@ import com.graphhopper.reader.ReaderWay; import com.graphhopper.routing.ev.BooleanEncodedValue; import com.graphhopper.routing.ev.EdgeIntAccess; +import com.graphhopper.routing.util.FerrySpeedCalculator; import com.graphhopper.routing.util.TransportationMode; import com.graphhopper.routing.util.WayAccess; import java.util.*; -import static com.graphhopper.routing.util.FerrySpeedCalculator.FERRIES; - public abstract class BikeCommonAccessParser extends AbstractAccessParser implements TagParser { private static final Set OPP_LANES = new HashSet<>(Arrays.asList("opposite", "opposite_lane", "opposite_track")); @@ -43,7 +42,7 @@ public WayAccess getAccess(ReaderWay way) { if (highwayValue == null) { WayAccess access = WayAccess.CAN_SKIP; - if (way.hasTag("route", FERRIES)) { + if (FerrySpeedCalculator.isFerry(way)) { // if bike is NOT explicitly tagged allow bike but only if foot is not specified either String bikeTag = way.getTag("bicycle"); if (bikeTag == null && !way.hasTag("foot") || intendedValues.contains(bikeTag)) diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/BikeCommonAverageSpeedParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/BikeCommonAverageSpeedParser.java index d59d57b8a16..4a86ac0950b 100644 --- a/core/src/main/java/com/graphhopper/routing/util/parsers/BikeCommonAverageSpeedParser.java +++ b/core/src/main/java/com/graphhopper/routing/util/parsers/BikeCommonAverageSpeedParser.java @@ -133,7 +133,7 @@ protected BikeCommonAverageSpeedParser(DecimalEncodedValue speedEnc, EnumEncoded public void handleWayTags(int edgeId, EdgeIntAccess edgeIntAccess, ReaderWay way) { String highwayValue = way.getTag("highway"); if (highwayValue == null) { - if (way.hasTag("route", ferries)) { + if (FerrySpeedCalculator.isFerry(way)) { double ferrySpeed = FerrySpeedCalculator.minmax(ferrySpeedEnc.getDecimal(false, edgeId, edgeIntAccess), avgSpeedEnc); setSpeed(false, edgeId, edgeIntAccess, ferrySpeed); if (avgSpeedEnc.isStoreTwoDirections()) diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/BikeCommonPriorityParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/BikeCommonPriorityParser.java index efad34d3c04..add8fa9489d 100644 --- a/core/src/main/java/com/graphhopper/routing/util/parsers/BikeCommonPriorityParser.java +++ b/core/src/main/java/com/graphhopper/routing/util/parsers/BikeCommonPriorityParser.java @@ -5,13 +5,13 @@ import com.graphhopper.routing.ev.EdgeIntAccess; import com.graphhopper.routing.ev.EnumEncodedValue; import com.graphhopper.routing.ev.RouteNetwork; +import com.graphhopper.routing.util.FerrySpeedCalculator; import com.graphhopper.routing.util.PriorityCode; import com.graphhopper.storage.IntsRef; import java.util.*; import static com.graphhopper.routing.ev.RouteNetwork.*; -import static com.graphhopper.routing.util.FerrySpeedCalculator.FERRIES; import static com.graphhopper.routing.util.PriorityCode.*; import static com.graphhopper.routing.util.parsers.AbstractAccessParser.INTENDED; import static com.graphhopper.routing.util.parsers.AbstractAverageSpeedParser.getMaxSpeed; @@ -27,7 +27,6 @@ public abstract class BikeCommonPriorityParser implements TagParser { protected final Set preferHighwayTags = new HashSet<>(); protected final Map avoidHighwayTags = new HashMap<>(); protected final Set unpavedSurfaceTags = new HashSet<>(); - protected final Set ferries = new HashSet<>(FERRIES); protected final Set intendedValues = new HashSet<>(INTENDED); protected final DecimalEncodedValue avgSpeedEnc; @@ -90,7 +89,7 @@ public void handleWayTags(int edgeId, EdgeIntAccess edgeIntAccess, ReaderWay way String highwayValue = way.getTag("highway"); Integer priorityFromRelation = routeMap.get(bikeRouteEnc.getEnum(false, edgeId, edgeIntAccess)); if (highwayValue == null) { - if (way.hasTag("route", ferries)) { + if (FerrySpeedCalculator.isFerry(way)) { priorityFromRelation = SLIGHT_AVOID.getValue(); } else { return; diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/CarAccessParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/CarAccessParser.java index fbef9e0a4ef..e4832e5b170 100644 --- a/core/src/main/java/com/graphhopper/routing/util/parsers/CarAccessParser.java +++ b/core/src/main/java/com/graphhopper/routing/util/parsers/CarAccessParser.java @@ -19,14 +19,13 @@ import com.graphhopper.reader.ReaderWay; import com.graphhopper.routing.ev.*; +import com.graphhopper.routing.util.FerrySpeedCalculator; import com.graphhopper.routing.util.TransportationMode; import com.graphhopper.routing.util.WayAccess; import com.graphhopper.util.PMap; import java.util.*; -import static com.graphhopper.routing.util.FerrySpeedCalculator.FERRIES; - public class CarAccessParser extends AbstractAccessParser implements TagParser { protected final Set trackTypeValues = new HashSet<>(); @@ -82,7 +81,7 @@ public WayAccess getAccess(ReaderWay way) { String highwayValue = way.getTag("highway"); String firstValue = way.getFirstPriorityTag(restrictions); if (highwayValue == null) { - if (way.hasTag("route", FERRIES)) { + if (FerrySpeedCalculator.isFerry(way)) { if (restrictedValues.contains(firstValue)) return WayAccess.CAN_SKIP; if (intendedValues.contains(firstValue) || diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/CarAverageSpeedParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/CarAverageSpeedParser.java index 7661202fa64..8fdaf16d00e 100644 --- a/core/src/main/java/com/graphhopper/routing/util/parsers/CarAverageSpeedParser.java +++ b/core/src/main/java/com/graphhopper/routing/util/parsers/CarAverageSpeedParser.java @@ -122,7 +122,7 @@ protected double getSpeed(ReaderWay way) { @Override public void handleWayTags(int edgeId, EdgeIntAccess edgeIntAccess, ReaderWay way) { - if (way.hasTag("route", ferries)) { + if (FerrySpeedCalculator.isFerry(way)) { double ferrySpeed = FerrySpeedCalculator.minmax(ferrySpeedEnc.getDecimal(false, edgeId, edgeIntAccess), avgSpeedEnc); setSpeed(false, edgeId, edgeIntAccess, ferrySpeed); if (avgSpeedEnc.isStoreTwoDirections()) diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/FootAccessParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/FootAccessParser.java index 4f049abccf6..f52b64325b9 100644 --- a/core/src/main/java/com/graphhopper/routing/util/parsers/FootAccessParser.java +++ b/core/src/main/java/com/graphhopper/routing/util/parsers/FootAccessParser.java @@ -19,6 +19,7 @@ import com.graphhopper.reader.ReaderWay; import com.graphhopper.routing.ev.*; +import com.graphhopper.routing.util.FerrySpeedCalculator; import com.graphhopper.routing.util.TransportationMode; import com.graphhopper.routing.util.WayAccess; import com.graphhopper.util.PMap; @@ -26,7 +27,6 @@ import java.util.*; import static com.graphhopper.routing.ev.RouteNetwork.*; -import static com.graphhopper.routing.util.FerrySpeedCalculator.FERRIES; import static com.graphhopper.routing.util.PriorityCode.UNCHANGED; public class FootAccessParser extends AbstractAccessParser implements TagParser { @@ -98,7 +98,7 @@ public WayAccess getAccess(ReaderWay way) { if (highwayValue == null) { WayAccess acceptPotentially = WayAccess.CAN_SKIP; - if (way.hasTag("route", FERRIES)) { + if (FerrySpeedCalculator.isFerry(way)) { String footTag = way.getTag("foot"); if (footTag == null || intendedValues.contains(footTag)) acceptPotentially = WayAccess.FERRY; diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/FootAverageSpeedParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/FootAverageSpeedParser.java index 85c55ef8f9a..9dd68d0e23d 100644 --- a/core/src/main/java/com/graphhopper/routing/util/parsers/FootAverageSpeedParser.java +++ b/core/src/main/java/com/graphhopper/routing/util/parsers/FootAverageSpeedParser.java @@ -34,7 +34,7 @@ protected FootAverageSpeedParser(DecimalEncodedValue speedEnc, DecimalEncodedVal public void handleWayTags(int edgeId, EdgeIntAccess edgeIntAccess, ReaderWay way) { String highwayValue = way.getTag("highway"); if (highwayValue == null) { - if (way.hasTag("route", ferries)) { + if (FerrySpeedCalculator.isFerry(way)) { double ferrySpeed = FerrySpeedCalculator.minmax(ferrySpeedEnc.getDecimal(false, edgeId, edgeIntAccess), avgSpeedEnc); setSpeed(false, edgeId, edgeIntAccess, ferrySpeed); if (avgSpeedEnc.isStoreTwoDirections()) diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/FootPriorityParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/FootPriorityParser.java index bcfb743201c..fe7cd54bed2 100644 --- a/core/src/main/java/com/graphhopper/routing/util/parsers/FootPriorityParser.java +++ b/core/src/main/java/com/graphhopper/routing/util/parsers/FootPriorityParser.java @@ -2,6 +2,7 @@ import com.graphhopper.reader.ReaderWay; import com.graphhopper.routing.ev.*; +import com.graphhopper.routing.util.FerrySpeedCalculator; import com.graphhopper.routing.util.PriorityCode; import com.graphhopper.storage.IntsRef; import com.graphhopper.util.PMap; @@ -9,14 +10,12 @@ import java.util.*; import static com.graphhopper.routing.ev.RouteNetwork.*; -import static com.graphhopper.routing.util.FerrySpeedCalculator.FERRIES; import static com.graphhopper.routing.util.PriorityCode.*; import static com.graphhopper.routing.util.parsers.AbstractAccessParser.INTENDED; import static com.graphhopper.routing.util.parsers.AbstractAverageSpeedParser.getMaxSpeed; import static com.graphhopper.routing.util.parsers.AbstractAverageSpeedParser.isValidSpeed; public class FootPriorityParser implements TagParser { - final Set ferries = new HashSet<>(FERRIES); final Set intendedValues = new HashSet<>(INTENDED); final Set safeHighwayTags = new HashSet<>(); final Set avoidHighwayTags = new HashSet<>(); @@ -76,7 +75,7 @@ public void handleWayTags(int edgeId, EdgeIntAccess edgeIntAccess, ReaderWay way String highwayValue = way.getTag("highway"); Integer priorityFromRelation = routeMap.get(footRouteEnc.getEnum(false, edgeId, edgeIntAccess)); if (highwayValue == null) { - if (way.hasTag("route", ferries)) + if (FerrySpeedCalculator.isFerry(way)) priorityWayEncoder.setDecimal(false, edgeId, edgeIntAccess, PriorityCode.getValue(handlePriority(way, priorityFromRelation))); } else { priorityWayEncoder.setDecimal(false, edgeId, edgeIntAccess, PriorityCode.getValue(handlePriority(way, priorityFromRelation))); diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/OSMRoadEnvironmentParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/OSMRoadEnvironmentParser.java index 3e17a30fafe..4299c545adf 100644 --- a/core/src/main/java/com/graphhopper/routing/util/parsers/OSMRoadEnvironmentParser.java +++ b/core/src/main/java/com/graphhopper/routing/util/parsers/OSMRoadEnvironmentParser.java @@ -21,6 +21,7 @@ import com.graphhopper.routing.ev.EnumEncodedValue; import com.graphhopper.routing.ev.EdgeIntAccess; import com.graphhopper.routing.ev.RoadEnvironment; +import com.graphhopper.routing.util.FerrySpeedCalculator; import com.graphhopper.storage.IntsRef; import java.util.Collections; @@ -40,9 +41,7 @@ public OSMRoadEnvironmentParser(EnumEncodedValue roadEnvEnc) { @Override public void handleWayTags(int edgeId, EdgeIntAccess edgeIntAccess, ReaderWay readerWay, IntsRef relationFlags) { RoadEnvironment roadEnvironment = OTHER; - if ((readerWay.hasTag("route", "ferry") && !readerWay.hasTag("ferry", "no")) || - // TODO shuttle_train is sometimes also used in relations, e.g. https://www.openstreetmap.org/relation/1932780 - readerWay.hasTag("route", "shuttle_train") && !readerWay.hasTag("shuttle_train", "no")) + if (FerrySpeedCalculator.isFerry(readerWay)) roadEnvironment = FERRY; else if (readerWay.hasTag("bridge") && !readerWay.hasTag("bridge", "no")) roadEnvironment = BRIDGE; diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/WheelchairAverageSpeedParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/WheelchairAverageSpeedParser.java index 5e48f70427a..702fb54f790 100644 --- a/core/src/main/java/com/graphhopper/routing/util/parsers/WheelchairAverageSpeedParser.java +++ b/core/src/main/java/com/graphhopper/routing/util/parsers/WheelchairAverageSpeedParser.java @@ -21,7 +21,7 @@ protected WheelchairAverageSpeedParser(DecimalEncodedValue speedEnc, DecimalEnco public void handleWayTags(int edgeId, EdgeIntAccess edgeIntAccess, ReaderWay way) { String highwayValue = way.getTag("highway"); if (highwayValue == null) { - if (way.hasTag("route", ferries)) { + if (FerrySpeedCalculator.isFerry(way)) { double ferrySpeed = FerrySpeedCalculator.minmax(ferrySpeedEnc.getDecimal(false, edgeId, edgeIntAccess), avgSpeedEnc); setSpeed(false, edgeId, edgeIntAccess, ferrySpeed); setSpeed(true, edgeId, edgeIntAccess, ferrySpeed); diff --git a/core/src/test/java/com/graphhopper/routing/util/parsers/OSMRoadEnvironmentParserTest.java b/core/src/test/java/com/graphhopper/routing/util/parsers/OSMRoadEnvironmentParserTest.java index 0bc294af446..730332d2c73 100644 --- a/core/src/test/java/com/graphhopper/routing/util/parsers/OSMRoadEnvironmentParserTest.java +++ b/core/src/test/java/com/graphhopper/routing/util/parsers/OSMRoadEnvironmentParserTest.java @@ -39,6 +39,13 @@ void ferry() { parser.handleWayTags(edgeId, edgeIntAccess, way, new IntsRef(2)); RoadEnvironment roadEnvironment = roadEnvironmentEnc.getEnum(false, edgeId, edgeIntAccess); assertEquals(RoadEnvironment.FERRY, roadEnvironment); + + way = new ReaderWay(1); + way.setTag("highway", "footway"); + way.setTag("route", "ferry"); + parser.handleWayTags(edgeId, edgeIntAccess = new ArrayEdgeIntAccess(1), way, new IntsRef(2)); + roadEnvironment = roadEnvironmentEnc.getEnum(false, edgeId, edgeIntAccess); + assertEquals(RoadEnvironment.FERRY, roadEnvironment); } -} \ No newline at end of file +}