From 82ae896090563a33b00c52ba586f277453e90662 Mon Sep 17 00:00:00 2001 From: ratrun Date: Sat, 14 Sep 2024 19:30:53 +0200 Subject: [PATCH] Move sac_scale handling for bicycles to bike custom models --- CHANGELOG.md | 1 + .../util/parsers/BikeCommonAccessParser.java | 11 ---------- .../parsers/MountainBikeAccessParser.java | 6 ----- .../com/graphhopper/custom_models/bike.json | 3 ++- .../com/graphhopper/custom_models/mtb.json | 3 ++- .../graphhopper/custom_models/racingbike.json | 3 ++- .../routing/RoutingAlgorithmWithOSMTest.java | 2 +- .../parsers/AbstractBikeTagParserTester.java | 12 ---------- .../util/parsers/BikeCustomModelTest.java | 17 ++++++++++++-- .../util/parsers/BikeTagParserTest.java | 22 ------------------- .../parsers/MountainBikeTagParserTest.java | 19 ---------------- .../util/parsers/RacingBikeTagParserTest.java | 19 ---------------- .../RouteResourceCustomModelTest.java | 3 ++- 13 files changed, 25 insertions(+), 96 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b46c736a578..0c763a8c997 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 c2be9126993..c93d493c454 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 64e855ec308..58935464464 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 22f8ba84dd7..139568af904 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 d5297e99641..104a7a0ef37 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 fe38f366312..94589d41d6a 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 7ac0808d0bf..c2048db34f5 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 7f415ddc62a..5c279a04e3f 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 1249145ac58..bbfb7726a0e 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 e0e1435c978..3ede4f38325 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 246444384f7..1c70662328a 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 fe83bc27499..5d0447627b0 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 eddbf85c691..0a7c419e2ed 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)),