diff --git a/CHANGELOG.md b/CHANGELOG.md index b46c736a57..0c763a8c99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - turn restriction support for restrictions with overlapping and/or multiple via-edges/ways, #3030 - constructor of BaseGraph.Builder uses byte instead of integer count. - KeyValue is now KValue as it holds the value only. Note, the two parameter constructor uses one value for the forward and one for the backward direction (and no longer "key, value") +- sac_scale priority handling for bicycles moved to the bike custom models ### 9.0 [23 Apr 2024] 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 c2be912699..c93d493c45 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 @@ -65,12 +65,6 @@ public WayAccess getAccess(ReaderWay way) { if (!allowedHighways.contains(highwayValue)) return WayAccess.CAN_SKIP; - String sacScale = way.getTag("sac_scale"); - if (sacScale != null) { - if (!isSacScaleAllowed(sacScale)) - return WayAccess.CAN_SKIP; - } - // use the way for pushing if (way.hasTag("bicycle", "dismount")) return WayAccess.WAY; @@ -100,11 +94,6 @@ public WayAccess getAccess(ReaderWay way) { return WayAccess.WAY; } - boolean isSacScaleAllowed(String sacScale) { - // other scales are nearly impossible by an ordinary bike, see http://wiki.openstreetmap.org/wiki/Key:sac_scale - return "hiking".equals(sacScale); - } - @Override public void handleWayTags(int edgeId, EdgeIntAccess edgeIntAccess, ReaderWay way) { WayAccess access = getAccess(way); diff --git a/core/src/main/java/com/graphhopper/routing/util/parsers/MountainBikeAccessParser.java b/core/src/main/java/com/graphhopper/routing/util/parsers/MountainBikeAccessParser.java index 64e855ec30..5893546446 100644 --- a/core/src/main/java/com/graphhopper/routing/util/parsers/MountainBikeAccessParser.java +++ b/core/src/main/java/com/graphhopper/routing/util/parsers/MountainBikeAccessParser.java @@ -19,10 +19,4 @@ protected MountainBikeAccessParser(BooleanEncodedValue accessEnc, BooleanEncoded super(accessEnc, roundaboutEnc); } - @Override - boolean isSacScaleAllowed(String sacScale) { - // other scales are too dangerous even for MTB, see http://wiki.openstreetmap.org/wiki/Key:sac_scale - return "hiking".equals(sacScale) || "mountain_hiking".equals(sacScale) - || "demanding_mountain_hiking".equals(sacScale) || "alpine_hiking".equals(sacScale); - } } diff --git a/core/src/main/resources/com/graphhopper/custom_models/bike.json b/core/src/main/resources/com/graphhopper/custom_models/bike.json index 22f8ba84dd..139568af90 100644 --- a/core/src/main/resources/com/graphhopper/custom_models/bike.json +++ b/core/src/main/resources/com/graphhopper/custom_models/bike.json @@ -1,6 +1,6 @@ // to use this custom model you need to set the following option in the config.yml // graph.elevation.provider: srtm # enables elevation -// graph.encoded_values: bike_priority, mtb_rating, bike_access, roundabout, bike_average_speed, average_slope +// graph.encoded_values: bike_priority, mtb_rating, bike_access, roundabout, bike_average_speed, average_slope, sac_scale // profiles: // - name: bike // custom_model_files: [bike.json, bike_elevation.json] @@ -9,6 +9,7 @@ "priority": [ { "if": "true", "multiply_by": "bike_priority" }, { "if": "mtb_rating > 2", "multiply_by": "0" }, + { "if": "hike_rating > 1", "multiply_by": "0" }, { "if": "!bike_access && (!backward_bike_access || roundabout)", "multiply_by": "0" }, { "else_if": "!bike_access && backward_bike_access", "multiply_by": "0.2" } ], diff --git a/core/src/main/resources/com/graphhopper/custom_models/mtb.json b/core/src/main/resources/com/graphhopper/custom_models/mtb.json index d5297e9964..104a7a0ef3 100644 --- a/core/src/main/resources/com/graphhopper/custom_models/mtb.json +++ b/core/src/main/resources/com/graphhopper/custom_models/mtb.json @@ -1,6 +1,6 @@ // to use this custom model you need to set the following option in the config.yml // graph.elevation.provider: srtm # enables elevation -// graph.encoded_values: mtb_priority, mtb_access, roundabout, mtb_average_speed, average_slope, mtb_rating +// graph.encoded_values: mtb_priority, mtb_access, roundabout, mtb_average_speed, average_slope, mtb_rating, sac_scale // profiles: // - name: mtb // custom_model_files: [mtb.json, bike_elevation.json] @@ -10,6 +10,7 @@ { "if": "true", "multiply_by": "mtb_priority" }, { "if": "mtb_rating > 6", "multiply_by": "0" }, { "if": "mtb_rating > 3", "multiply_by": "0.5" }, + { "if": "hike_rating > 4", "multiply_by": "0" }, { "if": "!mtb_access && (!backward_mtb_access || roundabout)", "multiply_by": "0" }, { "else_if": "!mtb_access && backward_mtb_access", "multiply_by": "0.2" } ], diff --git a/core/src/main/resources/com/graphhopper/custom_models/racingbike.json b/core/src/main/resources/com/graphhopper/custom_models/racingbike.json index fe38f36631..94589d41d6 100644 --- a/core/src/main/resources/com/graphhopper/custom_models/racingbike.json +++ b/core/src/main/resources/com/graphhopper/custom_models/racingbike.json @@ -1,6 +1,6 @@ // to use this custom model you need to set the following option in the config.yml // graph.elevation.provider: srtm # enables elevation -// graph.encoded_values: racingbike_priority, racingbike_access, roundabout, racingbike_average_speed, average_slope, mtb_rating +// graph.encoded_values: racingbike_priority, racingbike_access, roundabout, racingbike_average_speed, average_slope, mtb_rating, sac_scale // profiles: // - name: racingbike // custom_model_files: [racingbike.json, bike_elevation.json] @@ -10,6 +10,7 @@ { "if": "true", "multiply_by": "racingbike_priority" }, { "if": "mtb_rating > 2", "multiply_by": "0" }, { "if": "mtb_rating == 2", "multiply_by": "0.5" }, + { "if": "hike_rating > 1", "multiply_by": "0" }, { "if": "!racingbike_access && (!backward_racingbike_access || roundabout)", "multiply_by": "0" }, { "else_if": "!racingbike_access && backward_racingbike_access", "multiply_by": "0.2" } ], diff --git a/core/src/test/java/com/graphhopper/routing/RoutingAlgorithmWithOSMTest.java b/core/src/test/java/com/graphhopper/routing/RoutingAlgorithmWithOSMTest.java index 7ac0808d0b..c2048db34f 100644 --- a/core/src/test/java/com/graphhopper/routing/RoutingAlgorithmWithOSMTest.java +++ b/core/src/test/java/com/graphhopper/routing/RoutingAlgorithmWithOSMTest.java @@ -710,7 +710,7 @@ private GraphHopper createHopper(String osmFile, Profile... profiles) { setEncodedValuesString("average_slope, max_slope, hike_rating, car_access, car_average_speed, " + "foot_access, foot_priority, foot_average_speed, " + "bike_access, bike_priority, bike_average_speed, foot_network, roundabout, " + - "mtb_access, mtb_priority, mtb_average_speed, " + + "mtb_access, mtb_priority, mtb_average_speed, mtb_rating, " + "racingbike_access, racingbike_priority, racingbike_average_speed"). setGraphHopperLocation(GH_LOCATION); hopper.getRouterConfig().setSimplifyResponse(false); 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 index 7f415ddc62..5c279a04e3 100644 --- a/core/src/test/java/com/graphhopper/routing/util/parsers/AbstractBikeTagParserTester.java +++ b/core/src/test/java/com/graphhopper/routing/util/parsers/AbstractBikeTagParserTester.java @@ -372,18 +372,6 @@ public void testSteps() { assertPriorityAndSpeed(BAD, 2, way); } - @Test - public void testSacScale() { - ReaderWay way = new ReaderWay(1); - way.setTag("highway", "service"); - way.setTag("sac_scale", "hiking"); - // allow - assertTrue(accessParser.getAccess(way).isWay()); - - way.setTag("sac_scale", "alpine_hiking"); - assertTrue(accessParser.getAccess(way).canSkip()); - } - @Test public void testReduceToMaxSpeed() { ReaderWay way = new ReaderWay(12); diff --git a/core/src/test/java/com/graphhopper/routing/util/parsers/BikeCustomModelTest.java b/core/src/test/java/com/graphhopper/routing/util/parsers/BikeCustomModelTest.java index 1249145ac5..bbfb7726a0 100644 --- a/core/src/test/java/com/graphhopper/routing/util/parsers/BikeCustomModelTest.java +++ b/core/src/test/java/com/graphhopper/routing/util/parsers/BikeCustomModelTest.java @@ -25,6 +25,7 @@ public class BikeCustomModelTest { @BeforeEach public void setup() { IntEncodedValue bikeRating = MtbRating.create(); + IntEncodedValue hikeRating = HikeRating.create(); em = new EncodingManager.Builder(). add(VehicleAccess.create("bike")). add(VehicleAccess.create("mtb")). @@ -40,10 +41,12 @@ public void setup() { add(Roundabout.create()). add(Smoothness.create()). add(RoadAccess.create()). - add(bikeRating).build(); + add(bikeRating). + add(hikeRating).build(); parsers = new OSMParsers(). - addWayTagParser(new OSMMtbRatingParser(bikeRating)); + addWayTagParser(new OSMMtbRatingParser(bikeRating)). + addWayTagParser(new OSMHikeRatingParser(hikeRating)); parsers.addWayTagParser(new BikeAccessParser(em, new PMap())); parsers.addWayTagParser(new MountainBikeAccessParser(em, new PMap())); @@ -90,6 +93,7 @@ public void testCustomBike() { way.setTag("sac_scale", "hiking"); edge = createEdge(way); assertEquals(0.9, p.getEdgeToPriorityMapping().get(edge, false), 0.01); + // other scales than hiking are nearly impossible by an ordinary bike, see http://wiki.openstreetmap.org/wiki/Key:sac_scale way.setTag("sac_scale", "mountain_hiking"); edge = createEdge(way); assertEquals(0.0, p.getEdgeToPriorityMapping().get(edge, false), 0.01); @@ -124,9 +128,18 @@ public void testCustomMtbBike() { way.setTag("sac_scale", "hiking"); edge = createEdge(way); assertEquals(1.2, p.getEdgeToPriorityMapping().get(edge, false), 0.01); + way.setTag("sac_scale", "mountain_hiking"); edge = createEdge(way); assertEquals(1.2, p.getEdgeToPriorityMapping().get(edge, false), 0.01); + + way.setTag("sac_scale", "alpine_hiking"); + edge = createEdge(way); + assertEquals(1.2, p.getEdgeToPriorityMapping().get(edge, false), 0.01); + + way.setTag("sac_scale", "demanding_alpine_hiking"); + edge = createEdge(way); + assertEquals(0.0, p.getEdgeToPriorityMapping().get(edge, false), 0.01); } @Test 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 index e0e1435c97..3ede4f3832 100644 --- a/core/src/test/java/com/graphhopper/routing/util/parsers/BikeTagParserTest.java +++ b/core/src/test/java/com/graphhopper/routing/util/parsers/BikeTagParserTest.java @@ -526,28 +526,6 @@ public void testUnchangedRelationShouldNotInfluencePriority() { assertPriorityAndSpeed(AVOID, 18, osmWay, osmRel); } - @Test - @Override - public void testSacScale() { - ReaderWay way = new ReaderWay(1); - way.setTag("highway", "path"); - way.setTag("sac_scale", "hiking"); - assertTrue(accessParser.getAccess(way).isWay()); - - way.setTag("highway", "path"); - way.setTag("sac_scale", "mountain_hiking"); - assertTrue(accessParser.getAccess(way).canSkip()); - - way.setTag("highway", "cycleway"); - way.setTag("sac_scale", "hiking"); - assertTrue(accessParser.getAccess(way).isWay()); - - way.setTag("highway", "cycleway"); - way.setTag("sac_scale", "mountain_hiking"); - // disallow questionable combination as too dangerous - assertTrue(accessParser.getAccess(way).canSkip()); - } - @Test public void testCalcPriority() { ReaderWay osmWay = new ReaderWay(1); diff --git a/core/src/test/java/com/graphhopper/routing/util/parsers/MountainBikeTagParserTest.java b/core/src/test/java/com/graphhopper/routing/util/parsers/MountainBikeTagParserTest.java index 246444384f..1c70662328 100644 --- a/core/src/test/java/com/graphhopper/routing/util/parsers/MountainBikeTagParserTest.java +++ b/core/src/test/java/com/graphhopper/routing/util/parsers/MountainBikeTagParserTest.java @@ -133,25 +133,6 @@ public void testSmoothness() { assertEquals(MIN_SPEED, getSpeedFromFlags(way), 0.01); } - @Test - @Override - public void testSacScale() { - ReaderWay way = new ReaderWay(1); - way.setTag("highway", "service"); - way.setTag("sac_scale", "hiking"); - assertTrue(accessParser.getAccess(way).isWay()); - - way.setTag("highway", "service"); - way.setTag("sac_scale", "mountain_hiking"); - assertTrue(accessParser.getAccess(way).isWay()); - - way.setTag("sac_scale", "alpine_hiking"); - assertTrue(accessParser.getAccess(way).isWay()); - - way.setTag("sac_scale", "demanding_alpine_hiking"); - assertTrue(accessParser.getAccess(way).canSkip()); - } - @Test public void testHandleWayTagsInfluencedByBikeAndMtbRelation() { ReaderWay osmWay = new ReaderWay(1); diff --git a/core/src/test/java/com/graphhopper/routing/util/parsers/RacingBikeTagParserTest.java b/core/src/test/java/com/graphhopper/routing/util/parsers/RacingBikeTagParserTest.java index fe83bc2749..5d0447627b 100644 --- a/core/src/test/java/com/graphhopper/routing/util/parsers/RacingBikeTagParserTest.java +++ b/core/src/test/java/com/graphhopper/routing/util/parsers/RacingBikeTagParserTest.java @@ -109,25 +109,6 @@ public void testTrack() { assertPriorityAndSpeed(VERY_NICE, 20, way); } - @Test - @Override - public void testSacScale() { - ReaderWay way = new ReaderWay(1); - way.setTag("highway", "service"); - way.setTag("sac_scale", "mountain_hiking"); - assertTrue(accessParser.getAccess(way).canSkip()); - - way.setTag("highway", "path"); - way.setTag("sac_scale", "hiking"); - assertTrue(accessParser.getAccess(way).isWay()); - - // This looks to be tagging error: - way.setTag("highway", "cycleway"); - way.setTag("sac_scale", "mountain_hiking"); - // we are cautious and disallow this - assertTrue(accessParser.getAccess(way).canSkip()); - } - @Test public void testGetSpeed() { EdgeIntAccess edgeIntAccess = ArrayEdgeIntAccess.createFromBytes(encodingManager.getBytesForFlags()); diff --git a/web/src/test/java/com/graphhopper/application/resources/RouteResourceCustomModelTest.java b/web/src/test/java/com/graphhopper/application/resources/RouteResourceCustomModelTest.java index eddbf85c69..0a7c419e2e 100644 --- a/web/src/test/java/com/graphhopper/application/resources/RouteResourceCustomModelTest.java +++ b/web/src/test/java/com/graphhopper/application/resources/RouteResourceCustomModelTest.java @@ -66,7 +66,8 @@ private static GraphHopperServerConfiguration createConfig() { putObject("custom_areas.directory", "./src/test/resources/com/graphhopper/application/resources/areas"). putObject("import.osm.ignored_highways", ""). putObject("graph.encoded_values", "max_height, max_weight, max_width, hazmat, toll, surface, track_type, hgv, average_slope, max_slope, bus_access, " + - "car_access, car_average_speed, bike_access, bike_priority, bike_average_speed, road_class, road_access, get_off_bike, roundabout, foot_access, foot_priority, foot_average_speed, country"). + "car_access, car_average_speed, bike_access, bike_priority, bike_average_speed, road_class, road_access, get_off_bike, roundabout, foot_access, " + + "foot_priority, foot_average_speed, country, mtb_rating, hike_rating"). setProfiles(List.of( TestProfiles.constantSpeed("roads", 120), new Profile("car").setCustomModel(TestProfiles.accessAndSpeed("unused", "car").getCustomModel().setDistanceInfluence(70d)),