Use same definition for ferries for road environment and speed parsers (

graphhopper#2853)
ratrun · Jul 27, 2023 · b5c6c1b · b5c6c1b
1 parent b31a44b
commit b5c6c1b
Show file tree

Hide file tree

Showing 14 changed files with 34 additions and 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
@@ -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
@@ -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<String> 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
@@ -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<String> 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
@@ -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<String> 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
@@ -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
@@ -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<String> preferHighwayTags = new HashSet<>();
     protected final Map<String, PriorityCode> avoidHighwayTags = new HashMap<>();
     protected final Set<String> unpavedSurfaceTags = new HashSet<>();
-    protected final Set<String> ferries = new HashSet<>(FERRIES);
     protected final Set<String> 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
@@ -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<String> 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
@@ -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
@@ -19,14 +19,14 @@
 
 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.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
@@ -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
@@ -2,21 +2,20 @@
 
 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;
 
 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<String> ferries = new HashSet<>(FERRIES);
     final Set<String> intendedValues = new HashSet<>(INTENDED);
     final Set<String> safeHighwayTags = new HashSet<>();
     final Set<String> 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
@@ -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<RoadEnvironment> 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
@@ -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
@@ -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);
     }
 
-}
+}