From a4f955fe210ef24de81ab00a296ae9957e0fa7f8 Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Sun, 6 Oct 2024 20:09:27 +0100 Subject: [PATCH 01/12] improve area handling --- .../graph_builder/module/osm/OsmDatabase.java | 5 +- .../openstreetmap/model/OSMWay.java | 6 ++- .../openstreetmap/model/OSMWithTags.java | 9 +++- .../openstreetmap/model/OSMWayTest.java | 51 +++++++++++++++++++ 4 files changed, 65 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java b/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java index 60525b3cb6c..a52b4824e10 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java @@ -254,10 +254,7 @@ public void addWay(OSMWay way) { applyLevelsForWay(way); /* An area can be specified as such, or be one by default as an amenity */ - if ( - (way.isArea() || way.isParkAndRide() || way.isBikeParking() || way.isBoardingArea()) && - way.getNodeRefs().size() > 2 - ) { + if (way.isArea()) { // this is an area that's a simple polygon. So we can just add it straight // to the areas, if it's not part of a relation. if (!areaWayIds.contains(wayId)) { diff --git a/src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java b/src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java index b1e90044bf2..f5eb115e4ff 100644 --- a/src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java +++ b/src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java @@ -138,7 +138,11 @@ public boolean isBackwardEscalator() { } public boolean isArea() { - return isTag("area", "yes"); + return ( + !isTag("area", "no") && + (isTag("area", "yes") || isParking() || isBikeParking() || isBoardingArea()) && + getNodeRefs().size() > 2 + ); } /** diff --git a/src/main/java/org/opentripplanner/openstreetmap/model/OSMWithTags.java b/src/main/java/org/opentripplanner/openstreetmap/model/OSMWithTags.java index b53739bf6a1..2b228f2c537 100644 --- a/src/main/java/org/opentripplanner/openstreetmap/model/OSMWithTags.java +++ b/src/main/java/org/opentripplanner/openstreetmap/model/OSMWithTags.java @@ -423,6 +423,13 @@ public boolean isPedestrianExplicitlyAllowed() { return doesTagAllowAccess("foot"); } + /** + * @return True if this node / area is a parking. + */ + public boolean isParking() { + return isTag("amenity", "parking"); + } + /** * @return True if this node / area is a park and ride. */ @@ -430,7 +437,7 @@ public boolean isParkAndRide() { String parkingType = getTag("parking"); String parkAndRide = getTag("park_ride"); return ( - isTag("amenity", "parking") && + isParking() && ( (parkingType != null && parkingType.contains("park_and_ride")) || (parkAndRide != null && !parkAndRide.equalsIgnoreCase("no")) diff --git a/src/test/java/org/opentripplanner/openstreetmap/model/OSMWayTest.java b/src/test/java/org/opentripplanner/openstreetmap/model/OSMWayTest.java index c0af4cf2701..c6c4bd49651 100644 --- a/src/test/java/org/opentripplanner/openstreetmap/model/OSMWayTest.java +++ b/src/test/java/org/opentripplanner/openstreetmap/model/OSMWayTest.java @@ -17,6 +17,48 @@ void testIsBicycleDismountForced() { assertTrue(way.isBicycleDismountForced()); } + @Test + void testAreaMustContain3Nodes() { + OSMWay way = new OSMWay(); + way.addTag("area", "yes"); + assertFalse(way.isArea()); + way.addNodeRef(1); + assertFalse(way.isArea()); + way.addNodeRef(2); + assertFalse(way.isArea()); + way.addNodeRef(3); + assertTrue(way.isArea()); + way.addNodeRef(4); + assertTrue(way.isArea()); + } + + @Test + void testAreaTags() { + OSMWay platform = getClosedPolygon(); + platform.addTag("public_transport", "platform"); + assertTrue(platform.isArea()); + platform.addTag("area", "no"); + assertFalse(platform.isArea()); + + OSMWay roundabout = getClosedPolygon(); + roundabout.addTag("highway", "roundabout"); + assertFalse(roundabout.isArea()); + + OSMWay pedestrian = getClosedPolygon(); + pedestrian.addTag("highway", "pedestrian"); + assertFalse(pedestrian.isArea()); + pedestrian.addTag("area", "yes"); + assertTrue(pedestrian.isArea()); + + OSMWay parking = getClosedPolygon(); + parking.addTag("amenity", "parking"); + assertTrue(parking.isArea()); + + OSMWay bikeParking = getClosedPolygon(); + bikeParking.addTag("amenity", "bicycle_parking"); + assertTrue(bikeParking.isArea()); + } + @Test void testIsSteps() { OSMWay way = new OSMWay(); @@ -125,4 +167,13 @@ void escalator() { escalator.addTag("conveying", "whoknows?"); assertFalse(escalator.isEscalator()); } + + private OSMWay getClosedPolygon() { + var way = new OSMWay(); + way.addNodeRef(1); + way.addNodeRef(2); + way.addNodeRef(3); + way.addNodeRef(1); + return way; + } } From 93543b174e78883f8e5bfdff46bf7750165c8063 Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Sun, 6 Oct 2024 20:52:05 +0100 Subject: [PATCH 02/12] add indoor tags for area detection --- .../openstreetmap/model/OSMWay.java | 8 ++++++- .../openstreetmap/model/OSMWayTest.java | 24 ++++++++++++------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java b/src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java index f5eb115e4ff..299c1c6c545 100644 --- a/src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java +++ b/src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java @@ -140,11 +140,17 @@ public boolean isBackwardEscalator() { public boolean isArea() { return ( !isTag("area", "no") && - (isTag("area", "yes") || isParking() || isBikeParking() || isBoardingArea()) && + ( + isTag("area", "yes") || isParking() || isBikeParking() || isBoardingArea() || isIndoorArea() + ) && getNodeRefs().size() > 2 ); } + public boolean isIndoorArea() { + return isTag("indoor", "room") || isTag("indoor", "area") || isTag("indoor", "corridor"); + } + /** * Given a set of {@code permissions} check if it can really be applied to both directions * of the way and return the permissions for both cases. diff --git a/src/test/java/org/opentripplanner/openstreetmap/model/OSMWayTest.java b/src/test/java/org/opentripplanner/openstreetmap/model/OSMWayTest.java index c6c4bd49651..c35e2d216af 100644 --- a/src/test/java/org/opentripplanner/openstreetmap/model/OSMWayTest.java +++ b/src/test/java/org/opentripplanner/openstreetmap/model/OSMWayTest.java @@ -34,11 +34,11 @@ void testAreaMustContain3Nodes() { @Test void testAreaTags() { - OSMWay platform = getClosedPolygon(); - platform.addTag("public_transport", "platform"); - assertTrue(platform.isArea()); - platform.addTag("area", "no"); - assertFalse(platform.isArea()); + OSMWay way1 = getClosedPolygon(); + way1.addTag("public_transport", "platform"); + assertTrue(way1.isArea()); + way1.addTag("area", "no"); + assertFalse(way1.isArea()); OSMWay roundabout = getClosedPolygon(); roundabout.addTag("highway", "roundabout"); @@ -50,13 +50,21 @@ void testAreaTags() { pedestrian.addTag("area", "yes"); assertTrue(pedestrian.isArea()); - OSMWay parking = getClosedPolygon(); - parking.addTag("amenity", "parking"); - assertTrue(parking.isArea()); + OSMWay indoorArea = getClosedPolygon(); + indoorArea.addTag("indoor", "area"); + assertTrue(indoorArea.isArea()); OSMWay bikeParking = getClosedPolygon(); bikeParking.addTag("amenity", "bicycle_parking"); assertTrue(bikeParking.isArea()); + + OSMWay corridor = getClosedPolygon(); + corridor.addTag("indoor", "corridor"); + assertTrue(corridor.isArea()); + + OSMWay door = getClosedPolygon(); + door.addTag("indoor", "door"); + assertFalse(door.isArea()); } @Test From cd16137ecfd96aabb584a78ff14de9719858e85d Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Sun, 6 Oct 2024 20:58:16 +0100 Subject: [PATCH 03/12] add properties for indoor tags --- doc/user/osm/Default.md | 2 ++ doc/user/osm/Finland.md | 2 ++ doc/user/osm/Germany.md | 2 ++ doc/user/osm/UK.md | 2 ++ .../openstreetmap/tagmapping/DefaultMapper.java | 2 ++ .../openstreetmap/tagmapping/DefaultMapperTest.java | 9 +++++++++ .../openstreetmap/wayproperty/specifier/WayTestData.java | 6 ++++++ 7 files changed, 25 insertions(+) diff --git a/doc/user/osm/Default.md b/doc/user/osm/Default.md index 814420b791f..1373b499579 100644 --- a/doc/user/osm/Default.md +++ b/doc/user/osm/Default.md @@ -35,6 +35,8 @@ Lower safety values make an OSM way more desirable and higher values less desira | `public_transport=platform` | `PEDESTRIAN` | | | | `railway=platform` | `PEDESTRIAN` | | | | `footway=sidewalk; highway=footway` | `PEDESTRIAN` | | | +| `indoor=area` | `PEDESTRIAN` | | | +| `indoor=corridor` | `PEDESTRIAN` | | | | `mtb:scale=1` | `PEDESTRIAN` | | | | `mtb:scale=2` | `PEDESTRIAN` | | | | `mtb:scale=0` | `PEDESTRIAN_AND_BICYCLE` | | | diff --git a/doc/user/osm/Finland.md b/doc/user/osm/Finland.md index 8a60b5f0b13..820e47c46e0 100644 --- a/doc/user/osm/Finland.md +++ b/doc/user/osm/Finland.md @@ -79,6 +79,8 @@ Lower safety values make an OSM way more desirable and higher values less desira | `public_transport=platform` | `PEDESTRIAN` | | | | `railway=platform` | `PEDESTRIAN` | | | | `footway=sidewalk; highway=footway` | `PEDESTRIAN` | | | +| `indoor=area` | `PEDESTRIAN` | | | +| `indoor=corridor` | `PEDESTRIAN` | | | | `mtb:scale=1` | `PEDESTRIAN` | | | | `mtb:scale=2` | `PEDESTRIAN` | | | | `mtb:scale=0` | `PEDESTRIAN_AND_BICYCLE` | | | diff --git a/doc/user/osm/Germany.md b/doc/user/osm/Germany.md index 922aa3af836..f422022d4f2 100644 --- a/doc/user/osm/Germany.md +++ b/doc/user/osm/Germany.md @@ -44,6 +44,8 @@ Lower safety values make an OSM way more desirable and higher values less desira | `public_transport=platform` | `PEDESTRIAN` | | | | `railway=platform` | `PEDESTRIAN` | | | | `footway=sidewalk; highway=footway` | `PEDESTRIAN` | | | +| `indoor=area` | `PEDESTRIAN` | | | +| `indoor=corridor` | `PEDESTRIAN` | | | | `mtb:scale=1` | `PEDESTRIAN` | | | | `mtb:scale=2` | `PEDESTRIAN` | | | | `mtb:scale=0` | `PEDESTRIAN_AND_BICYCLE` | | | diff --git a/doc/user/osm/UK.md b/doc/user/osm/UK.md index 4a640caf95c..d5e401e40cc 100644 --- a/doc/user/osm/UK.md +++ b/doc/user/osm/UK.md @@ -49,6 +49,8 @@ Lower safety values make an OSM way more desirable and higher values less desira | `public_transport=platform` | `PEDESTRIAN` | | | | `railway=platform` | `PEDESTRIAN` | | | | `footway=sidewalk; highway=footway` | `PEDESTRIAN` | | | +| `indoor=area` | `PEDESTRIAN` | | | +| `indoor=corridor` | `PEDESTRIAN` | | | | `mtb:scale=1` | `PEDESTRIAN` | | | | `mtb:scale=2` | `PEDESTRIAN` | | | | `mtb:scale=0` | `PEDESTRIAN_AND_BICYCLE` | | | diff --git a/src/main/java/org/opentripplanner/openstreetmap/tagmapping/DefaultMapper.java b/src/main/java/org/opentripplanner/openstreetmap/tagmapping/DefaultMapper.java index c5d1c0d1582..fe498333aac 100644 --- a/src/main/java/org/opentripplanner/openstreetmap/tagmapping/DefaultMapper.java +++ b/src/main/java/org/opentripplanner/openstreetmap/tagmapping/DefaultMapper.java @@ -67,6 +67,8 @@ public void populateProperties(WayPropertySet props) { props.setProperties("public_transport=platform", pedestrianWayProperties); props.setProperties("railway=platform", pedestrianWayProperties); props.setProperties("footway=sidewalk;highway=footway", pedestrianWayProperties); + props.setProperties("indoor=area", pedestrianWayProperties); + props.setProperties("indoor=corridor", pedestrianWayProperties); props.setProperties("mtb:scale=1", pedestrianWayProperties); props.setProperties("mtb:scale=2", pedestrianWayProperties); diff --git a/src/test/java/org/opentripplanner/openstreetmap/tagmapping/DefaultMapperTest.java b/src/test/java/org/opentripplanner/openstreetmap/tagmapping/DefaultMapperTest.java index 2a8988dda61..8de9661f547 100644 --- a/src/test/java/org/opentripplanner/openstreetmap/tagmapping/DefaultMapperTest.java +++ b/src/test/java/org/opentripplanner/openstreetmap/tagmapping/DefaultMapperTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.opentripplanner.street.model.StreetTraversalPermission.ALL; +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; @@ -121,6 +122,14 @@ void stairs() { assertEquals(PEDESTRIAN, props.getPermission()); } + @Test + void indoor() { + var corridor = wps.getDataForWay(WayTestData.indoor("corridor")); + assertEquals(PEDESTRIAN, corridor.getPermission()); + var area = wps.getDataForWay(WayTestData.indoor("area")); + assertEquals(PEDESTRIAN, area.getPermission()); + } + @Test void footDiscouraged() { var regular = WayTestData.pedestrianTunnel(); diff --git a/src/test/java/org/opentripplanner/openstreetmap/wayproperty/specifier/WayTestData.java b/src/test/java/org/opentripplanner/openstreetmap/wayproperty/specifier/WayTestData.java index b09f690f794..7ca0901561a 100644 --- a/src/test/java/org/opentripplanner/openstreetmap/wayproperty/specifier/WayTestData.java +++ b/src/test/java/org/opentripplanner/openstreetmap/wayproperty/specifier/WayTestData.java @@ -218,4 +218,10 @@ public static OSMWithTags zooPlatform() { way.addTag("usage", "tourism"); return way; } + + public static OSMWithTags indoor(String value) { + var way = new OSMWithTags(); + way.addTag("indoor", value); + return way; + } } From b929da86ff32e8a5086e3f2457d898c9bd524959 Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Mon, 7 Oct 2024 11:48:22 +0100 Subject: [PATCH 04/12] update comment --- .../graph_builder/module/osm/OsmDatabase.java | 1 - .../java/org/opentripplanner/openstreetmap/model/OSMWay.java | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java b/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java index a52b4824e10..5fe0280270c 100644 --- a/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java +++ b/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java @@ -253,7 +253,6 @@ public void addWay(OSMWay way) { applyLevelsForWay(way); - /* An area can be specified as such, or be one by default as an amenity */ if (way.isArea()) { // this is an area that's a simple polygon. So we can just add it straight // to the areas, if it's not part of a relation. diff --git a/src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java b/src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java index 299c1c6c545..f2bd687495d 100644 --- a/src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java +++ b/src/main/java/org/opentripplanner/openstreetmap/model/OSMWay.java @@ -137,6 +137,11 @@ public boolean isBackwardEscalator() { return isEscalator() && "backward".equals(this.getTag("conveying")); } + /** + * Returns true if the way is considered an area. + * + * An area can be specified as such, or be one by default as an amenity. + */ public boolean isArea() { return ( !isTag("area", "no") && From a7952e392976d933d576d7078a834ae0eaf4aa2a Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Mon, 7 Oct 2024 11:52:58 +0100 Subject: [PATCH 05/12] use more descriptive name --- .../openstreetmap/model/OSMWayTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/opentripplanner/openstreetmap/model/OSMWayTest.java b/src/test/java/org/opentripplanner/openstreetmap/model/OSMWayTest.java index c35e2d216af..0aeaf36ff49 100644 --- a/src/test/java/org/opentripplanner/openstreetmap/model/OSMWayTest.java +++ b/src/test/java/org/opentripplanner/openstreetmap/model/OSMWayTest.java @@ -34,11 +34,11 @@ void testAreaMustContain3Nodes() { @Test void testAreaTags() { - OSMWay way1 = getClosedPolygon(); - way1.addTag("public_transport", "platform"); - assertTrue(way1.isArea()); - way1.addTag("area", "no"); - assertFalse(way1.isArea()); + OSMWay platform = getClosedPolygon(); + platform.addTag("public_transport", "platform"); + assertTrue(platform.isArea()); + platform.addTag("area", "no"); + assertFalse(platform.isArea()); OSMWay roundabout = getClosedPolygon(); roundabout.addTag("highway", "roundabout"); From 953d894762989d84422ed5e9db11f2bceddf3a6a Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Tue, 8 Oct 2024 10:11:21 +0100 Subject: [PATCH 06/12] make indoor areas routable --- .../opentripplanner/openstreetmap/model/OSMWithTags.java | 6 +++++- .../openstreetmap/model/OSMWithTagsTest.java | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opentripplanner/openstreetmap/model/OSMWithTags.java b/src/main/java/org/opentripplanner/openstreetmap/model/OSMWithTags.java index 2b228f2c537..23511f83022 100644 --- a/src/main/java/org/opentripplanner/openstreetmap/model/OSMWithTags.java +++ b/src/main/java/org/opentripplanner/openstreetmap/model/OSMWithTags.java @@ -549,7 +549,7 @@ public void setOsmProvider(OsmProvider provider) { public boolean isRoutable() { if (isOneOfTags("highway", NON_ROUTABLE_HIGHWAYS)) { return false; - } else if (hasTag("highway") || isPlatform()) { + } else if (hasTag("highway") || isPlatform() || isIndoorRoutable()) { if (isGeneralAccessDenied()) { // There are exceptions. return ( @@ -566,6 +566,10 @@ public boolean isRoutable() { return false; } + public boolean isIndoorRoutable() { + return isTag("indoor", "area") || isTag("indoor", "corridor"); + } + /** * Is this a link to another road, like a highway ramp. */ diff --git a/src/test/java/org/opentripplanner/openstreetmap/model/OSMWithTagsTest.java b/src/test/java/org/opentripplanner/openstreetmap/model/OSMWithTagsTest.java index 3b50d0bff0d..cbc7635113c 100644 --- a/src/test/java/org/opentripplanner/openstreetmap/model/OSMWithTagsTest.java +++ b/src/test/java/org/opentripplanner/openstreetmap/model/OSMWithTagsTest.java @@ -212,6 +212,8 @@ void isWheelchairAccessible() { @Test void isRoutable() { assertFalse(WayTestData.zooPlatform().isRoutable()); + assertTrue(WayTestData.indoor("area").isRoutable()); + assertFalse(WayTestData.indoor("room").isRoutable()); } @Test From d3f7d709f8d068f76b6cd900df1d19288f9d2b65 Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Tue, 15 Oct 2024 10:41:09 +0100 Subject: [PATCH 07/12] use isOneOfTags for testing OSM objects against multiple values --- .../main/java/org/opentripplanner/osm/model/OsmWay.java | 9 ++++++++- .../java/org/opentripplanner/osm/model/OsmWithTags.java | 7 ++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java b/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java index 710d41425e7..0bd7b5a8975 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java @@ -14,6 +14,13 @@ public class OsmWay extends OsmWithTags { "backward", "reversible" ); + + private static final Set INDOOR_AREA_VALUES = Set.of( + "room", + "corridor", + "area" + ); + private final TLongList nodes = new TLongArrayList(); public void addNodeRef(long nodeRef) { @@ -153,7 +160,7 @@ public boolean isArea() { } public boolean isIndoorArea() { - return isTag("indoor", "room") || isTag("indoor", "area") || isTag("indoor", "corridor"); + return isOneOfTags("indoor", INDOOR_AREA_VALUES); } /** diff --git a/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java b/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java index cbc77a10fbf..28dd1db9fae 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java @@ -44,6 +44,11 @@ public class OsmWithTags { "bus_guideway", "escape" ); + + private static final Set INDOOR_ROUTABLE_VALUES = Set.of( + "corridor", + "area" + ); private static final Set LEVEL_TAGS = Set.of("level", "layer"); private static final Set DEFAULT_LEVEL = Set.of("0"); @@ -557,7 +562,7 @@ public boolean isRoutable() { } public boolean isIndoorRoutable() { - return isTag("indoor", "area") || isTag("indoor", "corridor"); + return isOneOfTags("indoor", INDOOR_ROUTABLE_VALUES); } /** From 6dbaf214a78e2479ae8241df04bf8a56e4364794 Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Tue, 15 Oct 2024 10:47:37 +0100 Subject: [PATCH 08/12] renamed isArea to isAreaWay as the logic has changed --- .../opentripplanner/graph_builder/module/osm/OsmDatabase.java | 2 +- .../src/main/java/org/opentripplanner/osm/model/OsmWay.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java index cd396c53cd3..a560b96b2ab 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java @@ -253,7 +253,7 @@ public void addWay(OsmWay way) { applyLevelsForWay(way); - if (way.isArea()) { + if (way.isAreaWay()) { // this is an area that's a simple polygon. So we can just add it straight // to the areas, if it's not part of a relation. if (!areaWayIds.contains(wayId)) { diff --git a/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java b/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java index 0bd7b5a8975..31a0d8d9442 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java @@ -149,7 +149,7 @@ public boolean isBackwardEscalator() { * * An area can be specified as such, or be one by default as an amenity. */ - public boolean isArea() { + public boolean isAreaWay() { return ( !isTag("area", "no") && ( From 32a97b7fdd4f3dc5c7cc1c920f08b1cb26661dd1 Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Tue, 15 Oct 2024 11:14:28 +0100 Subject: [PATCH 09/12] only check for routable areas instead of areas in general --- .../graph_builder/module/osm/OsmDatabase.java | 2 +- .../org/opentripplanner/osm/model/OsmWay.java | 20 +++++++------------ .../osm/model/OsmWithTags.java | 7 ++----- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java index a560b96b2ab..e6727190b54 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmDatabase.java @@ -253,7 +253,7 @@ public void addWay(OsmWay way) { applyLevelsForWay(way); - if (way.isAreaWay()) { + if (way.isRoutableArea()) { // this is an area that's a simple polygon. So we can just add it straight // to the areas, if it's not part of a relation. if (!areaWayIds.contains(wayId)) { diff --git a/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java b/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java index 31a0d8d9442..7b5fbe56748 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java @@ -14,13 +14,7 @@ public class OsmWay extends OsmWithTags { "backward", "reversible" ); - - private static final Set INDOOR_AREA_VALUES = Set.of( - "room", - "corridor", - "area" - ); - + private final TLongList nodes = new TLongArrayList(); public void addNodeRef(long nodeRef) { @@ -149,20 +143,20 @@ public boolean isBackwardEscalator() { * * An area can be specified as such, or be one by default as an amenity. */ - public boolean isAreaWay() { + public boolean isRoutableArea() { return ( !isTag("area", "no") && ( - isTag("area", "yes") || isParking() || isBikeParking() || isBoardingArea() || isIndoorArea() + isTag("area", "yes") || + isParking() || + isBikeParking() || + isBoardingArea() || + isIndoorRoutable() ) && getNodeRefs().size() > 2 ); } - public boolean isIndoorArea() { - return isOneOfTags("indoor", INDOOR_AREA_VALUES); - } - /** * Given a set of {@code permissions} check if it can really be applied to both directions * of the way and return the permissions for both cases. diff --git a/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java b/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java index 28dd1db9fae..cbaf4652b34 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java @@ -44,11 +44,8 @@ public class OsmWithTags { "bus_guideway", "escape" ); - - private static final Set INDOOR_ROUTABLE_VALUES = Set.of( - "corridor", - "area" - ); + + private static final Set INDOOR_ROUTABLE_VALUES = Set.of("corridor", "area"); private static final Set LEVEL_TAGS = Set.of("level", "layer"); private static final Set DEFAULT_LEVEL = Set.of("0"); From a8a7829672cdff551a80d468f34b6f85038a84af Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Tue, 15 Oct 2024 12:07:09 +0100 Subject: [PATCH 10/12] fix test cases --- .../opentripplanner/osm/model/OsmWayTest.java | 48 +++++++++---------- .../wayproperty/specifier/WayTestData.java | 4 +- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java b/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java index 16a7498f94b..9ac9457a9ec 100644 --- a/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java +++ b/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java @@ -19,52 +19,52 @@ void testIsBicycleDismountForced() { @Test void testAreaMustContain3Nodes() { - OSMWay way = new OSMWay(); + OsmWay way = new OsmWay(); way.addTag("area", "yes"); - assertFalse(way.isArea()); + assertFalse(way.isRoutableArea()); way.addNodeRef(1); - assertFalse(way.isArea()); + assertFalse(way.isRoutableArea()); way.addNodeRef(2); - assertFalse(way.isArea()); + assertFalse(way.isRoutableArea()); way.addNodeRef(3); - assertTrue(way.isArea()); + assertTrue(way.isRoutableArea()); way.addNodeRef(4); - assertTrue(way.isArea()); + assertTrue(way.isRoutableArea()); } @Test void testAreaTags() { - OSMWay platform = getClosedPolygon(); + OsmWay platform = getClosedPolygon(); platform.addTag("public_transport", "platform"); - assertTrue(platform.isArea()); + assertTrue(platform.isRoutableArea()); platform.addTag("area", "no"); - assertFalse(platform.isArea()); + assertFalse(platform.isRoutableArea()); - OSMWay roundabout = getClosedPolygon(); + OsmWay roundabout = getClosedPolygon(); roundabout.addTag("highway", "roundabout"); - assertFalse(roundabout.isArea()); + assertFalse(roundabout.isRoutableArea()); - OSMWay pedestrian = getClosedPolygon(); + OsmWay pedestrian = getClosedPolygon(); pedestrian.addTag("highway", "pedestrian"); - assertFalse(pedestrian.isArea()); + assertFalse(pedestrian.isRoutableArea()); pedestrian.addTag("area", "yes"); - assertTrue(pedestrian.isArea()); + assertTrue(pedestrian.isRoutableArea()); - OSMWay indoorArea = getClosedPolygon(); + OsmWay indoorArea = getClosedPolygon(); indoorArea.addTag("indoor", "area"); - assertTrue(indoorArea.isArea()); + assertTrue(indoorArea.isRoutableArea()); - OSMWay bikeParking = getClosedPolygon(); + OsmWay bikeParking = getClosedPolygon(); bikeParking.addTag("amenity", "bicycle_parking"); - assertTrue(bikeParking.isArea()); + assertTrue(bikeParking.isRoutableArea()); - OSMWay corridor = getClosedPolygon(); + OsmWay corridor = getClosedPolygon(); corridor.addTag("indoor", "corridor"); - assertTrue(corridor.isArea()); + assertTrue(corridor.isRoutableArea()); - OSMWay door = getClosedPolygon(); + OsmWay door = getClosedPolygon(); door.addTag("indoor", "door"); - assertFalse(door.isArea()); + assertFalse(door.isRoutableArea()); } @Test @@ -176,8 +176,8 @@ void escalator() { assertFalse(escalator.isEscalator()); } - private OSMWay getClosedPolygon() { - var way = new OSMWay(); + private OsmWay getClosedPolygon() { + var way = new OsmWay(); way.addNodeRef(1); way.addNodeRef(2); way.addNodeRef(3); diff --git a/application/src/test/java/org/opentripplanner/osm/wayproperty/specifier/WayTestData.java b/application/src/test/java/org/opentripplanner/osm/wayproperty/specifier/WayTestData.java index 8238a44a9f4..e075955f6c4 100644 --- a/application/src/test/java/org/opentripplanner/osm/wayproperty/specifier/WayTestData.java +++ b/application/src/test/java/org/opentripplanner/osm/wayproperty/specifier/WayTestData.java @@ -219,8 +219,8 @@ public static OsmWithTags zooPlatform() { return way; } - public static OSMWithTags indoor(String value) { - var way = new OSMWithTags(); + public static OsmWithTags indoor(String value) { + var way = new OsmWithTags(); way.addTag("indoor", value); return way; } From 5cc750a9e67580435866d9b70eb6cae9a46f0231 Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Tue, 15 Oct 2024 15:14:19 +0100 Subject: [PATCH 11/12] remove unused imports --- .../org/opentripplanner/osm/tagmapping/DefaultMapperTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/application/src/test/java/org/opentripplanner/osm/tagmapping/DefaultMapperTest.java b/application/src/test/java/org/opentripplanner/osm/tagmapping/DefaultMapperTest.java index 7837c216c31..31073f87e92 100644 --- a/application/src/test/java/org/opentripplanner/osm/tagmapping/DefaultMapperTest.java +++ b/application/src/test/java/org/opentripplanner/osm/tagmapping/DefaultMapperTest.java @@ -4,7 +4,6 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.opentripplanner.street.model.StreetTraversalPermission.ALL; -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; From 0d826d7ae96c5508a966aa5878c03c9864a6d9c4 Mon Sep 17 00:00:00 2001 From: Michael Tsang Date: Thu, 24 Oct 2024 15:44:27 +0100 Subject: [PATCH 12/12] only enable pedestrian indoor routing in the UK --- .../osm/tagmapping/DefaultMapper.java | 2 -- .../osm/tagmapping/UKMapper.java | 6 +++++ .../osm/tagmapping/DefaultMapperTest.java | 8 ------ .../osm/tagmapping/UKMapperTest.java | 26 +++++++++++++++++++ doc/user/osm/Default.md | 2 -- doc/user/osm/Finland.md | 2 -- doc/user/osm/Germany.md | 2 -- doc/user/osm/UK.md | 4 +-- 8 files changed, 34 insertions(+), 18 deletions(-) create mode 100644 application/src/test/java/org/opentripplanner/osm/tagmapping/UKMapperTest.java diff --git a/application/src/main/java/org/opentripplanner/osm/tagmapping/DefaultMapper.java b/application/src/main/java/org/opentripplanner/osm/tagmapping/DefaultMapper.java index 532b2569d45..faa666c750a 100644 --- a/application/src/main/java/org/opentripplanner/osm/tagmapping/DefaultMapper.java +++ b/application/src/main/java/org/opentripplanner/osm/tagmapping/DefaultMapper.java @@ -67,8 +67,6 @@ public void populateProperties(WayPropertySet props) { props.setProperties("public_transport=platform", pedestrianWayProperties); props.setProperties("railway=platform", pedestrianWayProperties); props.setProperties("footway=sidewalk;highway=footway", pedestrianWayProperties); - props.setProperties("indoor=area", pedestrianWayProperties); - props.setProperties("indoor=corridor", pedestrianWayProperties); props.setProperties("mtb:scale=1", pedestrianWayProperties); props.setProperties("mtb:scale=2", pedestrianWayProperties); diff --git a/application/src/main/java/org/opentripplanner/osm/tagmapping/UKMapper.java b/application/src/main/java/org/opentripplanner/osm/tagmapping/UKMapper.java index bef0be4101b..08531ce051d 100644 --- a/application/src/main/java/org/opentripplanner/osm/tagmapping/UKMapper.java +++ b/application/src/main/java/org/opentripplanner/osm/tagmapping/UKMapper.java @@ -2,7 +2,9 @@ import static org.opentripplanner.osm.wayproperty.WayPropertiesBuilder.withModes; import static org.opentripplanner.street.model.StreetTraversalPermission.ALL; +import static org.opentripplanner.street.model.StreetTraversalPermission.PEDESTRIAN; +import org.opentripplanner.osm.wayproperty.WayProperties; import org.opentripplanner.osm.wayproperty.WayPropertySet; /** @@ -73,6 +75,10 @@ public void populateProperties(WayPropertySet props) { props.setCarSpeed("highway=secondary_link", 13.4f); // ~= 30mph props.setCarSpeed("highway=tertiary", 15.7f); // ~= 35mph + WayProperties pedestrianWayProperties = withModes(PEDESTRIAN).build(); + props.setProperties("indoor=area", pedestrianWayProperties); + props.setProperties("indoor=corridor", pedestrianWayProperties); + // Read the rest from the default set new DefaultMapper().populateProperties(props); } diff --git a/application/src/test/java/org/opentripplanner/osm/tagmapping/DefaultMapperTest.java b/application/src/test/java/org/opentripplanner/osm/tagmapping/DefaultMapperTest.java index 31073f87e92..87e23acbf12 100644 --- a/application/src/test/java/org/opentripplanner/osm/tagmapping/DefaultMapperTest.java +++ b/application/src/test/java/org/opentripplanner/osm/tagmapping/DefaultMapperTest.java @@ -122,14 +122,6 @@ void stairs() { assertEquals(PEDESTRIAN, props.getPermission()); } - @Test - void indoor() { - var corridor = wps.getDataForWay(WayTestData.indoor("corridor")); - assertEquals(PEDESTRIAN, corridor.getPermission()); - var area = wps.getDataForWay(WayTestData.indoor("area")); - assertEquals(PEDESTRIAN, area.getPermission()); - } - @Test void footDiscouraged() { var regular = WayTestData.pedestrianTunnel(); diff --git a/application/src/test/java/org/opentripplanner/osm/tagmapping/UKMapperTest.java b/application/src/test/java/org/opentripplanner/osm/tagmapping/UKMapperTest.java new file mode 100644 index 00000000000..3e9e46618fd --- /dev/null +++ b/application/src/test/java/org/opentripplanner/osm/tagmapping/UKMapperTest.java @@ -0,0 +1,26 @@ +package org.opentripplanner.osm.tagmapping; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.opentripplanner.street.model.StreetTraversalPermission.PEDESTRIAN; + +import org.junit.jupiter.api.Test; +import org.opentripplanner.osm.wayproperty.WayPropertySet; +import org.opentripplanner.osm.wayproperty.specifier.WayTestData; + +public class UKMapperTest { + + static WayPropertySet wps = new WayPropertySet(); + + static { + var source = new UKMapper(); + source.populateProperties(wps); + } + + @Test + void indoor() { + var corridor = wps.getDataForWay(WayTestData.indoor("corridor")); + assertEquals(PEDESTRIAN, corridor.getPermission()); + var area = wps.getDataForWay(WayTestData.indoor("area")); + assertEquals(PEDESTRIAN, area.getPermission()); + } +} diff --git a/doc/user/osm/Default.md b/doc/user/osm/Default.md index 1373b499579..814420b791f 100644 --- a/doc/user/osm/Default.md +++ b/doc/user/osm/Default.md @@ -35,8 +35,6 @@ Lower safety values make an OSM way more desirable and higher values less desira | `public_transport=platform` | `PEDESTRIAN` | | | | `railway=platform` | `PEDESTRIAN` | | | | `footway=sidewalk; highway=footway` | `PEDESTRIAN` | | | -| `indoor=area` | `PEDESTRIAN` | | | -| `indoor=corridor` | `PEDESTRIAN` | | | | `mtb:scale=1` | `PEDESTRIAN` | | | | `mtb:scale=2` | `PEDESTRIAN` | | | | `mtb:scale=0` | `PEDESTRIAN_AND_BICYCLE` | | | diff --git a/doc/user/osm/Finland.md b/doc/user/osm/Finland.md index 820e47c46e0..8a60b5f0b13 100644 --- a/doc/user/osm/Finland.md +++ b/doc/user/osm/Finland.md @@ -79,8 +79,6 @@ Lower safety values make an OSM way more desirable and higher values less desira | `public_transport=platform` | `PEDESTRIAN` | | | | `railway=platform` | `PEDESTRIAN` | | | | `footway=sidewalk; highway=footway` | `PEDESTRIAN` | | | -| `indoor=area` | `PEDESTRIAN` | | | -| `indoor=corridor` | `PEDESTRIAN` | | | | `mtb:scale=1` | `PEDESTRIAN` | | | | `mtb:scale=2` | `PEDESTRIAN` | | | | `mtb:scale=0` | `PEDESTRIAN_AND_BICYCLE` | | | diff --git a/doc/user/osm/Germany.md b/doc/user/osm/Germany.md index f422022d4f2..922aa3af836 100644 --- a/doc/user/osm/Germany.md +++ b/doc/user/osm/Germany.md @@ -44,8 +44,6 @@ Lower safety values make an OSM way more desirable and higher values less desira | `public_transport=platform` | `PEDESTRIAN` | | | | `railway=platform` | `PEDESTRIAN` | | | | `footway=sidewalk; highway=footway` | `PEDESTRIAN` | | | -| `indoor=area` | `PEDESTRIAN` | | | -| `indoor=corridor` | `PEDESTRIAN` | | | | `mtb:scale=1` | `PEDESTRIAN` | | | | `mtb:scale=2` | `PEDESTRIAN` | | | | `mtb:scale=0` | `PEDESTRIAN_AND_BICYCLE` | | | diff --git a/doc/user/osm/UK.md b/doc/user/osm/UK.md index d5e401e40cc..34c4d1c1778 100644 --- a/doc/user/osm/UK.md +++ b/doc/user/osm/UK.md @@ -38,6 +38,8 @@ Lower safety values make an OSM way more desirable and higher values less desira | `highway=trunk_link; cycleway=opposite_track` | `ALL` | forward: 2.06
back: 0.85 | | | `highway=trunk; bicycle=designated` | `ALL` | 7.25 | | | `highway=trunk_link; bicycle=designated` | `ALL` | 2.0 | | +| `indoor=area` | `PEDESTRIAN` | | | +| `indoor=corridor` | `PEDESTRIAN` | | | | `mtb:scale=3` | `NONE` | | | | `mtb:scale=4` | `NONE` | | | | `mtb:scale=5` | `NONE` | | | @@ -49,8 +51,6 @@ Lower safety values make an OSM way more desirable and higher values less desira | `public_transport=platform` | `PEDESTRIAN` | | | | `railway=platform` | `PEDESTRIAN` | | | | `footway=sidewalk; highway=footway` | `PEDESTRIAN` | | | -| `indoor=area` | `PEDESTRIAN` | | | -| `indoor=corridor` | `PEDESTRIAN` | | | | `mtb:scale=1` | `PEDESTRIAN` | | | | `mtb:scale=2` | `PEDESTRIAN` | | | | `mtb:scale=0` | `PEDESTRIAN_AND_BICYCLE` | | |