From 514ac9fb695d699ef9913b810bac810b046abd7b Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Thu, 14 Nov 2024 11:49:14 +0200 Subject: [PATCH 1/4] Limit use of unrecommended shortcuts in car routing --- .../osm/tagmapping/FinlandMapper.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/application/src/main/java/org/opentripplanner/osm/tagmapping/FinlandMapper.java b/application/src/main/java/org/opentripplanner/osm/tagmapping/FinlandMapper.java index 5e413af510a..40132c3dd18 100644 --- a/application/src/main/java/org/opentripplanner/osm/tagmapping/FinlandMapper.java +++ b/application/src/main/java/org/opentripplanner/osm/tagmapping/FinlandMapper.java @@ -1,5 +1,7 @@ package org.opentripplanner.osm.tagmapping; +import java.util.Set; + import static org.opentripplanner.osm.wayproperty.MixinPropertiesBuilder.ofWalkSafety; import static org.opentripplanner.osm.wayproperty.WayPropertiesBuilder.withModes; import static org.opentripplanner.street.model.StreetTraversalPermission.ALL; @@ -25,6 +27,13 @@ * @see OsmTagMapper */ class FinlandMapper extends OsmTagMapper { + private static final Set NOTHROUGH_DRIVING_TAGS = Set.of( + "parking_aisle", + "driveway", + "alley", + "emergency_access", + "drive-through" + ); @Override public void populateProperties(WayPropertySet props) { @@ -219,4 +228,12 @@ public boolean isWalkNoThroughTrafficExplicitlyDisallowed(OsmWithTags way) { String foot = way.getTag("foot"); return isGeneralNoThroughTraffic(way) || doesTagValueDisallowThroughTraffic(foot); } + + @Override + public boolean isMotorVehicleThroughTrafficExplicitlyDisallowed(OsmWithTags way) { + if (super.isMotorVehicleThroughTrafficExplicitlyDisallowed(way)) { + return true; + } + return way.isOneOfTags("service", NOTHROUGH_DRIVING_TAGS); + } } From 9fc622152efb553ffed66a8e916de6a86d867de8 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Fri, 15 Nov 2024 10:01:28 +0200 Subject: [PATCH 2/4] Move constant speed test to Finland test set, add serviceway nothrough test --- .../osm/tagmapping/FinlandMapper.java | 4 +-- .../osm/tagmapping/FinlandMapperTest.java | 35 ++++++++++++++++--- .../osm/tagmapping/OsmTagMapperTest.java | 14 -------- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/osm/tagmapping/FinlandMapper.java b/application/src/main/java/org/opentripplanner/osm/tagmapping/FinlandMapper.java index 40132c3dd18..3048439f377 100644 --- a/application/src/main/java/org/opentripplanner/osm/tagmapping/FinlandMapper.java +++ b/application/src/main/java/org/opentripplanner/osm/tagmapping/FinlandMapper.java @@ -1,7 +1,5 @@ package org.opentripplanner.osm.tagmapping; -import java.util.Set; - import static org.opentripplanner.osm.wayproperty.MixinPropertiesBuilder.ofWalkSafety; import static org.opentripplanner.osm.wayproperty.WayPropertiesBuilder.withModes; import static org.opentripplanner.street.model.StreetTraversalPermission.ALL; @@ -10,6 +8,7 @@ import static org.opentripplanner.street.model.StreetTraversalPermission.PEDESTRIAN; import static org.opentripplanner.street.model.StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE; +import java.util.Set; import org.opentripplanner.framework.functional.FunctionUtils.TriFunction; import org.opentripplanner.osm.model.OsmWithTags; import org.opentripplanner.osm.wayproperty.WayPropertySet; @@ -27,6 +26,7 @@ * @see OsmTagMapper */ class FinlandMapper extends OsmTagMapper { + private static final Set NOTHROUGH_DRIVING_TAGS = Set.of( "parking_aisle", "driveway", diff --git a/application/src/test/java/org/opentripplanner/osm/tagmapping/FinlandMapperTest.java b/application/src/test/java/org/opentripplanner/osm/tagmapping/FinlandMapperTest.java index 8e931e1096a..24b46a001d7 100644 --- a/application/src/test/java/org/opentripplanner/osm/tagmapping/FinlandMapperTest.java +++ b/application/src/test/java/org/opentripplanner/osm/tagmapping/FinlandMapperTest.java @@ -1,10 +1,12 @@ package org.opentripplanner.osm.tagmapping; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.opentripplanner.street.model.StreetTraversalPermission.NONE; import static org.opentripplanner.street.model.StreetTraversalPermission.PEDESTRIAN; import static org.opentripplanner.street.model.StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.opentripplanner.osm.model.OsmWay; import org.opentripplanner.osm.model.OsmWithTags; @@ -13,12 +15,15 @@ public class FinlandMapperTest { - static WayPropertySet wps = new WayPropertySet(); + private WayPropertySet wps; + private OsmTagMapper mapper; static float epsilon = 0.01f; - static { - var source = new FinlandMapper(); - source.populateProperties(wps); + @BeforeEach + public void setup() { + this.wps = new WayPropertySet(); + this.mapper = new FinlandMapper(); + this.mapper.populateProperties(this.wps); } /** @@ -220,4 +225,26 @@ public void testArea() { wayData = wps.getDataForWay(way); assertEquals(wayData.getPermission(), PEDESTRIAN_AND_BICYCLE); } + + @Test + public void constantSpeedCarRouting() { + OsmTagMapper osmTagMapper = new ConstantSpeedFinlandMapper(20f); + + var slowWay = new OsmWithTags(); + slowWay.addTag("highway", "residential"); + assertEquals(20f, osmTagMapper.getCarSpeedForWay(slowWay, true)); + + var fastWay = new OsmWithTags(); + fastWay.addTag("highway", "motorway"); + fastWay.addTag("maxspeed", "120 kmph"); + assertEquals(20f, osmTagMapper.getCarSpeedForWay(fastWay, true)); + } + + @Test + public void serviceNoThroughTraffic() { + var way = new OsmWay(); + way.addTag("highway", "residential"); + way.addTag("service", "driveway"); + assertTrue(mapper.isMotorVehicleThroughTrafficExplicitlyDisallowed(way)); + } } diff --git a/application/src/test/java/org/opentripplanner/osm/tagmapping/OsmTagMapperTest.java b/application/src/test/java/org/opentripplanner/osm/tagmapping/OsmTagMapperTest.java index aaaf5178d5b..c668c1058a1 100644 --- a/application/src/test/java/org/opentripplanner/osm/tagmapping/OsmTagMapperTest.java +++ b/application/src/test/java/org/opentripplanner/osm/tagmapping/OsmTagMapperTest.java @@ -33,20 +33,6 @@ public void isMotorThroughTrafficExplicitlyDisallowed() { ); } - @Test - public void constantSpeedCarRouting() { - OsmTagMapper osmTagMapper = new ConstantSpeedFinlandMapper(20f); - - var slowWay = new OsmWithTags(); - slowWay.addTag("highway", "residential"); - assertEquals(20f, osmTagMapper.getCarSpeedForWay(slowWay, true)); - - var fastWay = new OsmWithTags(); - fastWay.addTag("highway", "motorway"); - fastWay.addTag("maxspeed", "120 kmph"); - assertEquals(20f, osmTagMapper.getCarSpeedForWay(fastWay, true)); - } - @Test public void isBicycleNoThroughTrafficExplicitlyDisallowed() { OsmTagMapper osmTagMapper = new OsmTagMapper(); From 2202ba0d441f862c090d66dfcb6ffa457b940694 Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Fri, 15 Nov 2024 10:39:45 +0200 Subject: [PATCH 3/4] Remove double negation --- .../module/osm/SafetyValueNormalizer.java | 4 +- .../osm/tagmapping/FinlandMapper.java | 4 +- .../osm/tagmapping/OsmTagMapper.java | 4 +- .../osm/tagmapping/OsmTagMapperTest.java | 50 +++++++++---------- 4 files changed, 30 insertions(+), 32 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/SafetyValueNormalizer.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/SafetyValueNormalizer.java index 2f070f272c6..2cb5b01dcc6 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/SafetyValueNormalizer.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/SafetyValueNormalizer.java @@ -86,8 +86,8 @@ void applyWayProperties( boolean motorVehicleNoThrough = tagMapperForWay.isMotorVehicleThroughTrafficExplicitlyDisallowed( way ); - boolean bicycleNoThrough = tagMapperForWay.isBicycleNoThroughTrafficExplicitlyDisallowed(way); - boolean walkNoThrough = tagMapperForWay.isWalkNoThroughTrafficExplicitlyDisallowed(way); + boolean bicycleNoThrough = tagMapperForWay.isBicycleThroughTrafficExplicitlyDisallowed(way); + boolean walkNoThrough = tagMapperForWay.isWalkThroughTrafficExplicitlyDisallowed(way); if (street != null) { double bicycleSafety = wayData.bicycleSafety().forward(); diff --git a/application/src/main/java/org/opentripplanner/osm/tagmapping/FinlandMapper.java b/application/src/main/java/org/opentripplanner/osm/tagmapping/FinlandMapper.java index 3048439f377..e9ac9478552 100644 --- a/application/src/main/java/org/opentripplanner/osm/tagmapping/FinlandMapper.java +++ b/application/src/main/java/org/opentripplanner/osm/tagmapping/FinlandMapper.java @@ -215,7 +215,7 @@ else if (speedLimit <= 16.65f) { } @Override - public boolean isBicycleNoThroughTrafficExplicitlyDisallowed(OsmWithTags way) { + public boolean isBicycleThroughTrafficExplicitlyDisallowed(OsmWithTags way) { String bicycle = way.getTag("bicycle"); return ( isVehicleThroughTrafficExplicitlyDisallowed(way) || @@ -224,7 +224,7 @@ public boolean isBicycleNoThroughTrafficExplicitlyDisallowed(OsmWithTags way) { } @Override - public boolean isWalkNoThroughTrafficExplicitlyDisallowed(OsmWithTags way) { + public boolean isWalkThroughTrafficExplicitlyDisallowed(OsmWithTags way) { String foot = way.getTag("foot"); return isGeneralNoThroughTraffic(way) || doesTagValueDisallowThroughTraffic(foot); } diff --git a/application/src/main/java/org/opentripplanner/osm/tagmapping/OsmTagMapper.java b/application/src/main/java/org/opentripplanner/osm/tagmapping/OsmTagMapper.java index e4c258ccdc4..2df3c22d6a9 100644 --- a/application/src/main/java/org/opentripplanner/osm/tagmapping/OsmTagMapper.java +++ b/application/src/main/java/org/opentripplanner/osm/tagmapping/OsmTagMapper.java @@ -768,7 +768,7 @@ public boolean isMotorVehicleThroughTrafficExplicitlyDisallowed(OsmWithTags way) /** * Returns true if through traffic for bicycle is not allowed. */ - public boolean isBicycleNoThroughTrafficExplicitlyDisallowed(OsmWithTags way) { + public boolean isBicycleThroughTrafficExplicitlyDisallowed(OsmWithTags way) { String bicycle = way.getTag("bicycle"); if (bicycle != null) { return doesTagValueDisallowThroughTraffic(bicycle); @@ -780,7 +780,7 @@ public boolean isBicycleNoThroughTrafficExplicitlyDisallowed(OsmWithTags way) { /** * Returns true if through traffic for walk is not allowed. */ - public boolean isWalkNoThroughTrafficExplicitlyDisallowed(OsmWithTags way) { + public boolean isWalkThroughTrafficExplicitlyDisallowed(OsmWithTags way) { String foot = way.getTag("foot"); if (foot != null) { return doesTagValueDisallowThroughTraffic(foot); diff --git a/application/src/test/java/org/opentripplanner/osm/tagmapping/OsmTagMapperTest.java b/application/src/test/java/org/opentripplanner/osm/tagmapping/OsmTagMapperTest.java index c668c1058a1..b3ee57f9710 100644 --- a/application/src/test/java/org/opentripplanner/osm/tagmapping/OsmTagMapperTest.java +++ b/application/src/test/java/org/opentripplanner/osm/tagmapping/OsmTagMapperTest.java @@ -34,23 +34,21 @@ public void isMotorThroughTrafficExplicitlyDisallowed() { } @Test - public void isBicycleNoThroughTrafficExplicitlyDisallowed() { + public void isBicycleThroughTrafficExplicitlyDisallowed() { OsmTagMapper osmTagMapper = new OsmTagMapper(); assertTrue( - osmTagMapper.isBicycleNoThroughTrafficExplicitlyDisallowed(way("bicycle", "destination")) + osmTagMapper.isBicycleThroughTrafficExplicitlyDisallowed(way("bicycle", "destination")) ); assertTrue( - osmTagMapper.isBicycleNoThroughTrafficExplicitlyDisallowed(way("access", "destination")) + osmTagMapper.isBicycleThroughTrafficExplicitlyDisallowed(way("access", "destination")) ); } @Test - public void isWalkNoThroughTrafficExplicitlyDisallowed() { + public void isWalkThroughTrafficExplicitlyDisallowed() { OsmTagMapper osmTagMapper = new OsmTagMapper(); - assertTrue(osmTagMapper.isWalkNoThroughTrafficExplicitlyDisallowed(way("foot", "destination"))); - assertTrue( - osmTagMapper.isWalkNoThroughTrafficExplicitlyDisallowed(way("access", "destination")) - ); + assertTrue(osmTagMapper.isWalkThroughTrafficExplicitlyDisallowed(way("foot", "destination"))); + assertTrue(osmTagMapper.isWalkThroughTrafficExplicitlyDisallowed(way("access", "destination"))); } @Test @@ -61,8 +59,8 @@ public void testAccessNo() { tags.addTag("access", "no"); assertTrue(osmTagMapper.isMotorVehicleThroughTrafficExplicitlyDisallowed(tags)); - assertTrue(osmTagMapper.isBicycleNoThroughTrafficExplicitlyDisallowed(tags)); - assertTrue(osmTagMapper.isWalkNoThroughTrafficExplicitlyDisallowed(tags)); + assertTrue(osmTagMapper.isBicycleThroughTrafficExplicitlyDisallowed(tags)); + assertTrue(osmTagMapper.isWalkThroughTrafficExplicitlyDisallowed(tags)); } @Test @@ -73,8 +71,8 @@ public void testAccessPrivate() { tags.addTag("access", "private"); assertTrue(osmTagMapper.isMotorVehicleThroughTrafficExplicitlyDisallowed(tags)); - assertTrue(osmTagMapper.isBicycleNoThroughTrafficExplicitlyDisallowed(tags)); - assertTrue(osmTagMapper.isWalkNoThroughTrafficExplicitlyDisallowed(tags)); + assertTrue(osmTagMapper.isBicycleThroughTrafficExplicitlyDisallowed(tags)); + assertTrue(osmTagMapper.isWalkThroughTrafficExplicitlyDisallowed(tags)); } @Test @@ -86,8 +84,8 @@ public void testFootModifier() { tags.addTag("foot", "yes"); assertTrue(osmTagMapper.isMotorVehicleThroughTrafficExplicitlyDisallowed(tags)); - assertTrue(osmTagMapper.isBicycleNoThroughTrafficExplicitlyDisallowed(tags)); - assertFalse(osmTagMapper.isWalkNoThroughTrafficExplicitlyDisallowed(tags)); + assertTrue(osmTagMapper.isBicycleThroughTrafficExplicitlyDisallowed(tags)); + assertFalse(osmTagMapper.isWalkThroughTrafficExplicitlyDisallowed(tags)); } @Test @@ -98,8 +96,8 @@ public void testVehicleDenied() { tags.addTag("vehicle", "destination"); assertTrue(osmTagMapper.isMotorVehicleThroughTrafficExplicitlyDisallowed(tags)); - assertTrue(osmTagMapper.isBicycleNoThroughTrafficExplicitlyDisallowed(tags)); - assertFalse(osmTagMapper.isWalkNoThroughTrafficExplicitlyDisallowed(tags)); + assertTrue(osmTagMapper.isBicycleThroughTrafficExplicitlyDisallowed(tags)); + assertFalse(osmTagMapper.isWalkThroughTrafficExplicitlyDisallowed(tags)); } @Test @@ -111,8 +109,8 @@ public void testVehicleDeniedMotorVehiclePermissive() { tags.addTag("motor_vehicle", "designated"); assertFalse(osmTagMapper.isMotorVehicleThroughTrafficExplicitlyDisallowed(tags)); - assertTrue(osmTagMapper.isBicycleNoThroughTrafficExplicitlyDisallowed(tags)); - assertFalse(osmTagMapper.isWalkNoThroughTrafficExplicitlyDisallowed(tags)); + assertTrue(osmTagMapper.isBicycleThroughTrafficExplicitlyDisallowed(tags)); + assertFalse(osmTagMapper.isWalkThroughTrafficExplicitlyDisallowed(tags)); } @Test @@ -124,8 +122,8 @@ public void testVehicleDeniedBicyclePermissive() { tags.addTag("bicycle", "designated"); assertTrue(osmTagMapper.isMotorVehicleThroughTrafficExplicitlyDisallowed(tags)); - assertFalse(osmTagMapper.isBicycleNoThroughTrafficExplicitlyDisallowed(tags)); - assertFalse(osmTagMapper.isWalkNoThroughTrafficExplicitlyDisallowed(tags)); + assertFalse(osmTagMapper.isBicycleThroughTrafficExplicitlyDisallowed(tags)); + assertFalse(osmTagMapper.isWalkThroughTrafficExplicitlyDisallowed(tags)); } @Test @@ -137,8 +135,8 @@ public void testMotorcycleModifier() { tags.addTag("motor_vehicle", "yes"); assertFalse(osmTagMapper.isMotorVehicleThroughTrafficExplicitlyDisallowed(tags)); - assertTrue(osmTagMapper.isBicycleNoThroughTrafficExplicitlyDisallowed(tags)); - assertTrue(osmTagMapper.isWalkNoThroughTrafficExplicitlyDisallowed(tags)); + assertTrue(osmTagMapper.isBicycleThroughTrafficExplicitlyDisallowed(tags)); + assertTrue(osmTagMapper.isWalkThroughTrafficExplicitlyDisallowed(tags)); } @Test @@ -150,8 +148,8 @@ public void testBicycleModifier() { tags.addTag("bicycle", "yes"); assertTrue(osmTagMapper.isMotorVehicleThroughTrafficExplicitlyDisallowed(tags)); - assertFalse(osmTagMapper.isBicycleNoThroughTrafficExplicitlyDisallowed(tags)); - assertTrue(osmTagMapper.isWalkNoThroughTrafficExplicitlyDisallowed(tags)); + assertFalse(osmTagMapper.isBicycleThroughTrafficExplicitlyDisallowed(tags)); + assertTrue(osmTagMapper.isWalkThroughTrafficExplicitlyDisallowed(tags)); } @Test @@ -163,8 +161,8 @@ public void testBicyclePermissive() { tags.addTag("bicycle", "permissive"); assertTrue(osmTagMapper.isMotorVehicleThroughTrafficExplicitlyDisallowed(tags)); - assertFalse(osmTagMapper.isBicycleNoThroughTrafficExplicitlyDisallowed(tags)); - assertTrue(osmTagMapper.isWalkNoThroughTrafficExplicitlyDisallowed(tags)); + assertFalse(osmTagMapper.isBicycleThroughTrafficExplicitlyDisallowed(tags)); + assertTrue(osmTagMapper.isWalkThroughTrafficExplicitlyDisallowed(tags)); } public OsmWithTags way(String key, String value) { From 48cac2ea33cc823e51c6e3caf38f1ab22914e0ef Mon Sep 17 00:00:00 2001 From: Vesa Meskanen Date: Thu, 21 Nov 2024 13:01:20 +0200 Subject: [PATCH 4/4] Move constant speed test to a dedicated test class --- .../tagmapping/ConstantSpeedMapperTest.java | 23 +++++++++++++++++++ .../osm/tagmapping/FinlandMapperTest.java | 14 ----------- 2 files changed, 23 insertions(+), 14 deletions(-) create mode 100644 application/src/test/java/org/opentripplanner/osm/tagmapping/ConstantSpeedMapperTest.java diff --git a/application/src/test/java/org/opentripplanner/osm/tagmapping/ConstantSpeedMapperTest.java b/application/src/test/java/org/opentripplanner/osm/tagmapping/ConstantSpeedMapperTest.java new file mode 100644 index 00000000000..6256044320d --- /dev/null +++ b/application/src/test/java/org/opentripplanner/osm/tagmapping/ConstantSpeedMapperTest.java @@ -0,0 +1,23 @@ +package org.opentripplanner.osm.tagmapping; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.Test; +import org.opentripplanner.osm.model.OsmWithTags; + +public class ConstantSpeedMapperTest { + + @Test + public void constantSpeedCarRouting() { + OsmTagMapper osmTagMapper = new ConstantSpeedFinlandMapper(20f); + + var slowWay = new OsmWithTags(); + slowWay.addTag("highway", "residential"); + assertEquals(20f, osmTagMapper.getCarSpeedForWay(slowWay, true)); + + var fastWay = new OsmWithTags(); + fastWay.addTag("highway", "motorway"); + fastWay.addTag("maxspeed", "120 kmph"); + assertEquals(20f, osmTagMapper.getCarSpeedForWay(fastWay, true)); + } +} diff --git a/application/src/test/java/org/opentripplanner/osm/tagmapping/FinlandMapperTest.java b/application/src/test/java/org/opentripplanner/osm/tagmapping/FinlandMapperTest.java index 24b46a001d7..f8b07e06bf6 100644 --- a/application/src/test/java/org/opentripplanner/osm/tagmapping/FinlandMapperTest.java +++ b/application/src/test/java/org/opentripplanner/osm/tagmapping/FinlandMapperTest.java @@ -226,20 +226,6 @@ public void testArea() { assertEquals(wayData.getPermission(), PEDESTRIAN_AND_BICYCLE); } - @Test - public void constantSpeedCarRouting() { - OsmTagMapper osmTagMapper = new ConstantSpeedFinlandMapper(20f); - - var slowWay = new OsmWithTags(); - slowWay.addTag("highway", "residential"); - assertEquals(20f, osmTagMapper.getCarSpeedForWay(slowWay, true)); - - var fastWay = new OsmWithTags(); - fastWay.addTag("highway", "motorway"); - fastWay.addTag("maxspeed", "120 kmph"); - assertEquals(20f, osmTagMapper.getCarSpeedForWay(fastWay, true)); - } - @Test public void serviceNoThroughTraffic() { var way = new OsmWay();