avoidance of destination only for car, fixes #1831, bug introduced in #…

…1752
graphhopper · Feb 27, 2020 · 1f2084f · 1f2084f
1 parent 3a163c1
commit 1f2084f
Show file tree

Hide file tree

Showing 8 changed files with 43 additions and 16 deletions.
diff --git a/core/src/main/java/com/graphhopper/routing/util/BikeCommonFlagEncoder.java b/core/src/main/java/com/graphhopper/routing/util/BikeCommonFlagEncoder.java
@@ -19,6 +19,7 @@
 
 import com.graphhopper.reader.ReaderWay;
 import com.graphhopper.routing.profiles.*;
+import com.graphhopper.routing.util.spatialrules.TransportationMode;
 import com.graphhopper.routing.weighting.PriorityWeighting;
 import com.graphhopper.storage.IntsRef;
 import com.graphhopper.util.Helper;
@@ -195,6 +196,11 @@ protected BikeCommonFlagEncoder(int speedBits, double speedFactor, int maxTurnCo
         setAvoidSpeedLimit(71);
     }
 
+    @Override
+    public TransportationMode getTransportationMode() {
+        return TransportationMode.BICYCLE;
+    }
+
     @Override
     public int getVersion() {
         return 3;

diff --git a/core/src/main/java/com/graphhopper/routing/util/CarFlagEncoder.java b/core/src/main/java/com/graphhopper/routing/util/CarFlagEncoder.java
@@ -17,10 +17,10 @@
  */
 package com.graphhopper.routing.util;
 
-import com.graphhopper.reader.OSMTurnRelation;
 import com.graphhopper.reader.ReaderWay;
 import com.graphhopper.routing.profiles.EncodedValue;
 import com.graphhopper.routing.profiles.UnsignedDecimalEncodedValue;
+import com.graphhopper.routing.util.spatialrules.TransportationMode;
 import com.graphhopper.storage.IntsRef;
 import com.graphhopper.util.Helper;
 import com.graphhopper.util.PMap;
@@ -144,6 +144,10 @@ public CarFlagEncoder(int speedBits, double speedFactor, int maxTurnCosts) {
         speedDefault = defaultSpeedMap.get("secondary");
     }
 
+    public TransportationMode getTransportationMode() {
+        return TransportationMode.MOTOR_VEHICLE;
+    }
+
     @Override
     public int getVersion() {
         return 2;

diff --git a/core/src/main/java/com/graphhopper/routing/util/FlagEncoder.java b/core/src/main/java/com/graphhopper/routing/util/FlagEncoder.java
@@ -20,6 +20,7 @@
 import com.graphhopper.routing.profiles.BooleanEncodedValue;
 import com.graphhopper.routing.profiles.DecimalEncodedValue;
 import com.graphhopper.routing.profiles.EncodedValueLookup;
+import com.graphhopper.routing.util.spatialrules.TransportationMode;
 
 /**
  * This class provides methods to define how a value (like speed or direction) converts to a flag
@@ -35,6 +36,8 @@ public interface FlagEncoder extends EncodedValueLookup {
      */
     int getVersion();
 
+    TransportationMode getTransportationMode();
+
     /**
      * @return the maximum speed in km/h
      */

diff --git a/core/src/main/java/com/graphhopper/routing/util/FootFlagEncoder.java b/core/src/main/java/com/graphhopper/routing/util/FootFlagEncoder.java
@@ -19,6 +19,7 @@
 
 import com.graphhopper.reader.ReaderWay;
 import com.graphhopper.routing.profiles.*;
+import com.graphhopper.routing.util.spatialrules.TransportationMode;
 import com.graphhopper.routing.weighting.PriorityWeighting;
 import com.graphhopper.storage.IntsRef;
 import com.graphhopper.util.PMap;
@@ -143,6 +144,10 @@ public FootFlagEncoder(int speedBits, double speedFactor) {
         speedDefault = MEAN_SPEED;
     }
 
+    public TransportationMode getTransportationMode() {
+        return TransportationMode.FOOT;
+    }
+
     @Override
     public int getVersion() {
         return 5;

diff --git a/core/src/main/java/com/graphhopper/routing/util/spatialrules/TransportationMode.java b/core/src/main/java/com/graphhopper/routing/util/spatialrules/TransportationMode.java
@@ -27,9 +27,7 @@
  */
 public enum TransportationMode {
 
-    MOTOR_VEHICLE(0),
-    BICYCLE(1),
-    FOOT(2);
+    OTHER(0), MOTOR_VEHICLE(1), BICYCLE(2), FOOT(3);
 
     private final int value;
 

diff --git a/core/src/main/java/com/graphhopper/routing/weighting/FastestWeighting.java b/core/src/main/java/com/graphhopper/routing/weighting/FastestWeighting.java
@@ -21,6 +21,7 @@
 import com.graphhopper.routing.profiles.RoadAccess;
 import com.graphhopper.routing.util.FlagEncoder;
 import com.graphhopper.routing.util.HintsMap;
+import com.graphhopper.routing.util.spatialrules.TransportationMode;
 import com.graphhopper.util.EdgeIteratorState;
 import com.graphhopper.util.PMap;
 import com.graphhopper.util.Parameters.Routing;
@@ -45,7 +46,7 @@ public class FastestWeighting extends AbstractWeighting {
     private final double maxSpeed;
     private final EnumEncodedValue<RoadAccess> roadAccessEnc;
     // this factor puts a penalty on roads with a "destination"-only access, see #733
-    private final double roadAccessPenalty;
+    private final double destinationPenalty;
 
     public FastestWeighting(FlagEncoder encoder) {
         this(encoder, new HintsMap(0));
@@ -65,14 +66,13 @@ public FastestWeighting(FlagEncoder encoder, PMap map, TurnCostProvider turnCost
         headingPenaltyMillis = Math.round(headingPenalty * 1000);
         maxSpeed = encoder.getMaxSpeed() / SPEED_CONV;
 
-        if (encoder.hasEncodedValue(RoadAccess.KEY)) {
-            // ensure that we do not need to change getMinWeight, i.e. road_access_factor >= 1
-            roadAccessPenalty = checkBounds("road_access_factor", map.getDouble("road_access_factor", 10), 1, 10);
-            roadAccessEnc = encoder.getEnumEncodedValue(RoadAccess.KEY, RoadAccess.class);
-        } else {
-            roadAccessPenalty = 0;
-            roadAccessEnc = null;
-        }
+        if (!encoder.hasEncodedValue(RoadAccess.KEY))
+            throw new IllegalArgumentException("road_access is not available but expected for FastestWeighting");
+
+        // ensure that we do not need to change getMinWeight, i.e. road_access_factor >= 1
+        double defaultFactor = encoder.getTransportationMode() == TransportationMode.MOTOR_VEHICLE ? 10 : 1;
+        destinationPenalty = checkBounds("road_access_destination_factor", map.getDouble("road_access_destination_factor", defaultFactor), 1, 10);
+        roadAccessEnc = destinationPenalty > 1 ? encoder.getEnumEncodedValue(RoadAccess.KEY, RoadAccess.class) : null;
     }
 
     @Override
@@ -88,9 +88,11 @@ public double calcEdgeWeight(EdgeIteratorState edgeState, boolean reverse) {
 
         double time = edgeState.getDistance() / speed * SPEED_CONV;
 
-        if (roadAccessEnc != null && edgeState.get(roadAccessEnc) == RoadAccess.DESTINATION)
-            time *= roadAccessPenalty;
-
+        if (roadAccessEnc != null) {
+            RoadAccess access = edgeState.get(roadAccessEnc);
+            if (access == RoadAccess.DESTINATION)
+                time *= destinationPenalty;
+        }
         // add direction penalties at start/stop/via points
         boolean unfavoredEdge = edgeState.get(EdgeIteratorState.UNFAVORED_EDGE);
         if (unfavoredEdge)

diff --git a/core/src/test/java/com/graphhopper/routing/util/CarFlagEncoderTest.java b/core/src/test/java/com/graphhopper/routing/util/CarFlagEncoderTest.java
@@ -221,16 +221,20 @@ public void testDestinationTag() {
         IntsRef relFlags = em.createRelationFlags();
 
         FastestWeighting weighting = new FastestWeighting(encoder);
+        FastestWeighting bikeWeighting = new FastestWeighting(em.getEncoder("bike"));
+
         ReaderWay way = new ReaderWay(1);
         way.setTag("highway", "secondary");
         EncodingManager.AcceptWay acceptWay = new EncodingManager.AcceptWay();
         assertTrue(em.acceptWay(way, acceptWay));
         IntsRef edgeFlags = em.handleWayTags(way, acceptWay, relFlags);
         assertEquals(60, weighting.calcEdgeWeight(GHUtility.createMockedEdgeIteratorState(1000, edgeFlags), false), 0.1);
+        assertEquals(200, bikeWeighting.calcEdgeWeight(GHUtility.createMockedEdgeIteratorState(1000, edgeFlags), false), 0.1);
 
         way.setTag("vehicle", "destination");
         edgeFlags = em.handleWayTags(way, acceptWay, relFlags);
         assertEquals(600, weighting.calcEdgeWeight(GHUtility.createMockedEdgeIteratorState(1000, edgeFlags), false), 0.1);
+        assertEquals(200, bikeWeighting.calcEdgeWeight(GHUtility.createMockedEdgeIteratorState(1000, edgeFlags), false), 0.1);
     }
 
     @Test

diff --git a/core/src/test/java/com/graphhopper/routing/util/EncodingManagerTest.java b/core/src/test/java/com/graphhopper/routing/util/EncodingManagerTest.java
@@ -23,6 +23,7 @@
 import com.graphhopper.routing.profiles.EnumEncodedValue;
 import com.graphhopper.routing.profiles.Roundabout;
 import com.graphhopper.routing.profiles.RouteNetwork;
+import com.graphhopper.routing.util.spatialrules.TransportationMode;
 import com.graphhopper.storage.IntsRef;
 import org.junit.Test;
 
@@ -83,6 +84,10 @@ public void testWrongEncoders() {
     @Test
     public void testToDetailsStringIncludesEncoderVersionNumber() {
         FlagEncoder encoder = new AbstractFlagEncoder(1, 2.0, 0) {
+            public TransportationMode getTransportationMode() {
+                return TransportationMode.BICYCLE;
+            }
+
             @Override
             public int getVersion() {
                 return 10;