Changes triggered by further review comments

ratrun · May 24, 2024 · 7515d65 · 7515d65
1 parent 3ba603d
commit 7515d65
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 13 deletions.
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
@@ -89,12 +89,6 @@ public WayAccess getAccess(ReaderWay way) {
                 if (value.equals("no")) {
                     if (firstRestrictedKey.equals("vehicle"))
                         return WayAccess.WAY;
-                    if (firstRestrictedKey.equals("bicycle") && way.hasTag("vehicle", "no")) {
-                        if (way.hasTag("highway", "cycleway"))
-                            return WayAccess.WAY;  // Tagging error, we follow the highway value
-                        else
-                            return WayAccess.CAN_SKIP;
-                    }
                 }
                 if (restrictedValues.contains(value) && !hasTemporalRestriction(way, firstIndex, restrictionKeys))
                     return WayAccess.CAN_SKIP;

diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/OSMGetOffBikeParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/OSMGetOffBikeParser.java
@@ -32,10 +32,13 @@ public OSMGetOffBikeParser(BooleanEncodedValue getOffBikeEnc, BooleanEncodedValu
     @Override
     public void handleWayTags(int edgeId, EdgeIntAccess edgeIntAccess, ReaderWay way, IntsRef relationFlags) {
         String highway = way.getTag("highway");
+        String vehicle = way.getTag("vehicle") == null ? "" : way.getTag("vehicle");
         boolean notIntended = !way.hasTag("bicycle", INTENDED) &&
                 (GET_OFF_BIKE.contains(highway)
                         || way.hasTag("railway", "platform")
-                        || (way.hasTag("vehicle", "no") && highway != null && !highway.equals("cycleway"))
+                        || !"cycleway".equals(highway) && way.hasTag("vehicle", "no")
+                        || vehicle.contains("forestry")
+                        || vehicle.contains("agricultural")
                         || "path".equals(highway) && way.hasTag("foot", "designated") && !way.hasTag("segregated", "yes"));
         if ("steps".equals(highway) || way.hasTag("bicycle", "dismount") || notIntended) {
             getOffBikeEnc.setBool(false, edgeId, edgeIntAccess, true);

diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/OSMRoadAccessParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/OSMRoadAccessParser.java
@@ -85,7 +85,7 @@ public static List<String> toOSMRestrictions(TransportationMode mode) {
             case VEHICLE:
                 return Arrays.asList("vehicle", "access");
             case BIKE:
-                return Arrays.asList("bicycle", "vehicle", "access");
+                return Arrays.asList("bicycle", "access");
             case CAR:
                 return Arrays.asList("motorcar", "motor_vehicle", "vehicle", "access");
             case MOTORCYCLE:

diff --git a/core/src/test/java/com/graphhopper/routing/util/parsers/AbstractBikeTagParserTester.java b/core/src/test/java/com/graphhopper/routing/util/parsers/AbstractBikeTagParserTester.java
@@ -234,7 +234,7 @@ public void testAccess() {
         way.clearTags();
         way.setTag("highway", "track");
         way.setTag("vehicle", "forestry");
-        assertTrue(accessParser.getAccess(way).canSkip());
+        assertTrue(accessParser.getAccess(way).isWay());
         way.setTag("bicycle", "yes");
         assertTrue(accessParser.getAccess(way).isWay());
     }

diff --git a/core/src/test/java/com/graphhopper/routing/util/parsers/BikeTagParserTest.java b/core/src/test/java/com/graphhopper/routing/util/parsers/BikeTagParserTest.java
@@ -391,9 +391,9 @@ public void testWayAcceptance() {
         way.setTag("vehicle", "no");
         assertTrue(accessParser.getAccess(way).isWay());
 
-        // Sensless tagging: JOSM does create a warning here. We follow the highway tag:
+        // Senseless tagging: JOSM does create a warning here:
         way.setTag("bicycle", "no");
-        assertTrue(accessParser.getAccess(way).isWay());
+        assertTrue(accessParser.getAccess(way).canSkip());
 
         way.setTag("bicycle", "designated");
         assertTrue(accessParser.getAccess(way).isWay());
@@ -426,9 +426,9 @@ public void testWayAcceptance() {
         way.clearTags();
         way.setTag("highway", "track");
         way.setTag("vehicle", "forestry");
-        assertTrue(accessParser.getAccess(way).canSkip());
+        assertTrue(accessParser.getAccess(way).isWay());
         way.setTag("vehicle", "agricultural;forestry");
-        assertTrue(accessParser.getAccess(way).canSkip());
+        assertTrue(accessParser.getAccess(way).isWay());
     }
 
     @Test

diff --git a/core/src/test/java/com/graphhopper/routing/util/parsers/OSMGetOffBikeParserTest.java b/core/src/test/java/com/graphhopper/routing/util/parsers/OSMGetOffBikeParserTest.java
@@ -116,6 +116,13 @@ public void testHandleCommonWayTags() {
         assertFalse(isGetOffBike(way));
         way.setTag("bicycle", "designated");
         assertFalse(isGetOffBike(way));
+
+        way = new ReaderWay(1);
+        way.setTag("highway", "track");
+        way.setTag("vehicle", "forestry");
+        assertTrue(isGetOffBike(way));
+        way.setTag("vehicle", "forestry;agricultural");
+        assertTrue(isGetOffBike(way));
     }
 
     @Test