From a178b9d1236b73e76160892d73ad059e84434c3a Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Sun, 8 Dec 2024 22:30:48 +0100 Subject: [PATCH 1/2] Set bogus name to false when updating a StreetEdge's name --- .../opentripplanner/street/model/edge/Edge.java | 12 +++++++----- .../street/model/edge/StreetEdge.java | 14 +++++++++----- .../module/osm/naming/SidewalkNamerTest.java | 1 + .../mapping/StatesToWalkStepsMapperTest.java | 6 +++--- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/street/model/edge/Edge.java b/application/src/main/java/org/opentripplanner/street/model/edge/Edge.java index 31bff5b1e15..8e4126b4e59 100644 --- a/application/src/main/java/org/opentripplanner/street/model/edge/Edge.java +++ b/application/src/main/java/org/opentripplanner/street/model/edge/Edge.java @@ -145,15 +145,17 @@ public String getDefaultName() { */ public abstract I18NString getName(); - // TODO Add comments about what a "bogus name" is. + /** + * Returns true if this edge has a generated name that is derived from its properties, + * like "path", "stairs" or "tunnel". + *

+ * Returns false if the field reflects the real world name, like "Fifth Avenue", + * "Hauptstraße" or "Øvre Holmegate". + */ public boolean hasBogusName() { return false; } - // The next few functions used to live in EdgeNarrative, which has now been - // removed - // @author mattwigway - public LineString getGeometry() { return null; } diff --git a/application/src/main/java/org/opentripplanner/street/model/edge/StreetEdge.java b/application/src/main/java/org/opentripplanner/street/model/edge/StreetEdge.java index ee24e87f07c..468da359e0a 100644 --- a/application/src/main/java/org/opentripplanner/street/model/edge/StreetEdge.java +++ b/application/src/main/java/org/opentripplanner/street/model/edge/StreetEdge.java @@ -9,9 +9,6 @@ import java.util.List; import java.util.Objects; import java.util.Optional; -import java.util.Set; -import java.util.stream.Stream; -import javax.annotation.Nullable; import org.locationtech.jts.geom.LineString; import org.locationtech.jts.geom.impl.PackedCoordinateSequence; import org.opentripplanner.framework.geometry.CompactLineStringUtils; @@ -82,14 +79,14 @@ public class StreetEdge /** * bicycleSafetyWeight = length * bicycleSafetyFactor. For example, a 100m street with a safety - * factor of 2.0 will be considered in term of safety cost as the same as a 200m street with a + * factor of 2.0 will be considered in terms of safety cost as the same as a 200m street with a * safety factor of 1.0. */ private float bicycleSafetyFactor; /** * walkSafetyFactor = length * walkSafetyFactor. For example, a 100m street with a safety - * factor of 2.0 will be considered in term of safety cost as the same as a 200m street with a + * factor of 2.0 will be considered in terms of safety cost as the same as a 200m street with a * safety factor of 1.0. */ private float walkSafetyFactor; @@ -446,8 +443,15 @@ public I18NString getName() { return this.name; } + /** + * Update the name of the edge after it has been constructed. This method also sets the bogusName + * property to false, indicating to the code that maps from edges to steps that this is a real + * street name. + * @see Edge#hasBogusName() + */ public void setName(I18NString name) { this.name = name; + this.flags = BitSetUtils.set(flags, HASBOGUSNAME_FLAG_INDEX, false); } public boolean hasBogusName() { diff --git a/application/src/test/java/org/opentripplanner/graph_builder/module/osm/naming/SidewalkNamerTest.java b/application/src/test/java/org/opentripplanner/graph_builder/module/osm/naming/SidewalkNamerTest.java index 465eeb32ba2..cbfb7b76c6f 100644 --- a/application/src/test/java/org/opentripplanner/graph_builder/module/osm/naming/SidewalkNamerTest.java +++ b/application/src/test/java/org/opentripplanner/graph_builder/module/osm/naming/SidewalkNamerTest.java @@ -58,6 +58,7 @@ void postprocess() { assertNotEquals(sidewalk.edge.getName(), pryorStreet.edge.getName()); builder.postProcess(new SidewalkNamer()); assertEquals(sidewalk.edge.getName(), pryorStreet.edge.getName()); + assertFalse(sidewalk.edge.hasBogusName()); } private static class ModelBuilder { diff --git a/application/src/test/java/org/opentripplanner/routing/algorithm/mapping/StatesToWalkStepsMapperTest.java b/application/src/test/java/org/opentripplanner/routing/algorithm/mapping/StatesToWalkStepsMapperTest.java index 6e41f6ddf2b..de9fe21718a 100644 --- a/application/src/test/java/org/opentripplanner/routing/algorithm/mapping/StatesToWalkStepsMapperTest.java +++ b/application/src/test/java/org/opentripplanner/routing/algorithm/mapping/StatesToWalkStepsMapperTest.java @@ -42,7 +42,7 @@ void enterStation() { var walkSteps = buildWalkSteps(builder); assertEquals(2, walkSteps.size()); var enter = walkSteps.get(1); - assertEquals(enter.getRelativeDirection(), ENTER_STATION); + assertEquals(ENTER_STATION, enter.getRelativeDirection()); } @Test @@ -54,7 +54,7 @@ void exitStation() { var walkSteps = buildWalkSteps(builder); assertEquals(3, walkSteps.size()); var enter = walkSteps.get(2); - assertEquals(enter.getRelativeDirection(), EXIT_STATION); + assertEquals(EXIT_STATION, enter.getRelativeDirection()); } @Test @@ -64,7 +64,7 @@ void signpostedPathway() { var walkSteps = buildWalkSteps(builder); assertEquals(2, walkSteps.size()); var step = walkSteps.get(1); - assertEquals(step.getRelativeDirection(), FOLLOW_SIGNS); + assertEquals(FOLLOW_SIGNS, step.getRelativeDirection()); assertEquals(sign, step.getDirectionText().toString()); } From f0dd4f2d9457ae3b976c2da714568df0e528a094 Mon Sep 17 00:00:00 2001 From: Leonard Ehrenfried Date: Mon, 9 Dec 2024 11:36:54 +0100 Subject: [PATCH 2/2] Add test for name setting --- .../street/model/edge/StreetEdgeTest.java | 68 ++++++++++++------- 1 file changed, 44 insertions(+), 24 deletions(-) diff --git a/application/src/test/java/org/opentripplanner/street/model/edge/StreetEdgeTest.java b/application/src/test/java/org/opentripplanner/street/model/edge/StreetEdgeTest.java index 7de7b0c7ce6..ef2bacb91dd 100644 --- a/application/src/test/java/org/opentripplanner/street/model/edge/StreetEdgeTest.java +++ b/application/src/test/java/org/opentripplanner/street/model/edge/StreetEdgeTest.java @@ -6,6 +6,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.opentripplanner.street.model.StreetTraversalPermission.ALL; import static org.opentripplanner.street.model._data.StreetModelForTest.intersectionVertex; import static org.opentripplanner.street.model._data.StreetModelForTest.streetEdge; import static org.opentripplanner.street.model._data.StreetModelForTest.streetEdgeBuilder; @@ -17,6 +18,8 @@ import org.locationtech.jts.geom.GeometryFactory; import org.locationtech.jts.geom.LineString; import org.locationtech.jts.geom.impl.PackedCoordinateSequence; +import org.opentripplanner.framework.geometry.GeometryUtils; +import org.opentripplanner.framework.i18n.I18NString; import org.opentripplanner.routing.api.request.StreetMode; import org.opentripplanner.routing.core.VehicleRoutingOptimizeType; import org.opentripplanner.routing.util.ElevationUtils; @@ -43,7 +46,7 @@ public class StreetEdgeTest { private StreetSearchRequest proto; @BeforeEach - public void before() { + void before() { v0 = intersectionVertex("maple_0th", 0.0, 0.0); // label, X, Y v1 = intersectionVertex("maple_1st", 2.0, 2.0); v2 = intersectionVertex("maple_2nd", 2.0, 1.0); @@ -64,9 +67,9 @@ public void before() { } @Test - public void testInAndOutAngles() { + void testInAndOutAngles() { // An edge heading straight West - StreetEdge e1 = streetEdge(v1, v2, 1.0, StreetTraversalPermission.ALL); + StreetEdge e1 = streetEdge(v1, v2, 1.0, ALL); // Edge has same first and last angle. assertEquals(90, e1.getInAngle()); @@ -77,7 +80,7 @@ public void testInAndOutAngles() { StreetVertex v = intersectionVertex("test2", 2.0, 2.0); // Second edge, heading straight North - StreetEdge e2 = streetEdge(u, v, 1.0, StreetTraversalPermission.ALL); + StreetEdge e2 = streetEdge(u, v, 1.0, ALL); // 180 degrees could be expressed as 180 or -180. Our implementation happens to use -180. assertEquals(180, Math.abs(e2.getInAngle())); @@ -85,10 +88,8 @@ public void testInAndOutAngles() { } @Test - public void testTraverseAsPedestrian() { - StreetEdge e1 = streetEdgeBuilder(v1, v2, 100.0, StreetTraversalPermission.ALL) - .withCarSpeed(10.0f) - .buildAndConnect(); + void testTraverseAsPedestrian() { + StreetEdge e1 = streetEdgeBuilder(v1, v2, 100.0, ALL).withCarSpeed(10.0f).buildAndConnect(); StreetSearchRequest options = StreetSearchRequest .copyOf(proto) @@ -106,10 +107,8 @@ public void testTraverseAsPedestrian() { } @Test - public void testTraverseAsCar() { - StreetEdge e1 = streetEdgeBuilder(v1, v2, 100.0, StreetTraversalPermission.ALL) - .withCarSpeed(10.0f) - .buildAndConnect(); + void testTraverseAsCar() { + StreetEdge e1 = streetEdgeBuilder(v1, v2, 100.0, ALL).withCarSpeed(10.0f).buildAndConnect(); State s0 = new State(v1, StreetSearchRequest.copyOf(proto).withMode(StreetMode.CAR).build()); State s1 = e1.traverse(s0)[0]; @@ -122,8 +121,8 @@ public void testTraverseAsCar() { } @Test - public void testModeSetCanTraverse() { - StreetEdge e = streetEdge(v1, v2, 1.0, StreetTraversalPermission.ALL); + void testModeSetCanTraverse() { + StreetEdge e = streetEdge(v1, v2, 1.0, ALL); TraverseModeSet modes = TraverseModeSet.allModes(); assertTrue(e.canTraverse(modes)); @@ -146,7 +145,7 @@ public void testModeSetCanTraverse() { * correctly during turn cost computation. 4. Traffic light wait time is taken into account. */ @Test - public void testTraverseModeSwitchBike() { + void testTraverseModeSwitchBike() { var vWithTrafficLight = new LabelledIntersectionVertex("maple_1st", 2.0, 2.0, false, true); StreetEdge e0 = streetEdge(v0, vWithTrafficLight, 50.0, StreetTraversalPermission.PEDESTRIAN); StreetEdge e1 = streetEdge( @@ -183,7 +182,7 @@ public void testTraverseModeSwitchBike() { * the bike walking speed on the walking speed. 4. Traffic light wait time is taken into account. */ @Test - public void testTraverseModeSwitchWalk() { + void testTraverseModeSwitchWalk() { var vWithTrafficLight = new LabelledIntersectionVertex("maple_1st", 2.0, 2.0, false, true); StreetEdge e0 = streetEdge( v0, @@ -214,7 +213,7 @@ public void testTraverseModeSwitchWalk() { * Test the bike switching penalty feature, both its cost penalty and its separate time penalty. */ @Test - public void testBikeSwitch() { + void testBikeSwitch() { StreetEdge e0 = streetEdge(v0, v1, 0.0, StreetTraversalPermission.PEDESTRIAN); StreetEdge e1 = streetEdge(v1, v2, 0.0, StreetTraversalPermission.BICYCLE); StreetEdge e2 = streetEdge(v2, v0, 0.0, StreetTraversalPermission.PEDESTRIAN_AND_BICYCLE); @@ -271,9 +270,9 @@ public void testBikeSwitch() { } @Test - public void testTurnRestriction() { - StreetEdge e0 = streetEdge(v0, v1, 50.0, StreetTraversalPermission.ALL); - StreetEdge e1 = streetEdge(v1, v2, 18.4, StreetTraversalPermission.ALL); + void testTurnRestriction() { + StreetEdge e0 = streetEdge(v0, v1, 50.0, ALL); + StreetEdge e1 = streetEdge(v1, v2, 18.4, ALL); StreetSearchRequestBuilder streetSearchRequestBuilder = StreetSearchRequest.copyOf(proto); streetSearchRequestBuilder.withArriveBy(true); StreetSearchRequest request = streetSearchRequestBuilder.withMode(StreetMode.WALK).build(); @@ -285,13 +284,13 @@ public void testTurnRestriction() { } @Test - public void testElevationProfile() { + void testElevationProfile() { var elevationProfile = new PackedCoordinateSequence.Double( new double[] { 0, 10, 50, 12 }, 2, 0 ); - var edge = streetEdge(v0, v1, 50.0, StreetTraversalPermission.ALL); + var edge = streetEdge(v0, v1, 50.0, ALL); StreetElevationExtensionBuilder .of(edge) .withElevationProfile(elevationProfile) @@ -306,7 +305,7 @@ public void testElevationProfile() { } @Test - public void testBikeOptimizeTriangle() { + void testBikeOptimizeTriangle() { // This test does not depend on the setup method - and can probably be simplified Coordinate c1 = new Coordinate(-122.575033, 45.456773); @@ -326,7 +325,7 @@ public void testBikeOptimizeTriangle() { .withGeometry(geometry) .withName("Test Lane") .withMeterLength(length) - .withPermission(StreetTraversalPermission.ALL) + .withPermission(ALL) .withBack(false) // a safe street .withBicycleSafetyFactor(0.74f) @@ -400,4 +399,25 @@ public void testBikeOptimizeTriangle() { double expectedWeight = timeWeight * 0.33 + slopeWeight * 0.33 + safetyWeight * 0.34; assertEquals(expectedWeight, result.getWeight(), DELTA); } + + @Test + void setName() { + var path = I18NString.of("path"); + var edge = new StreetEdgeBuilder<>() + .withFromVertex(v0) + .withToVertex(v1) + .withPermission(ALL) + .withGeometry(GeometryUtils.makeLineString(v0.getCoordinate(), v1.getCoordinate())) + .withName(path) + .withBogusName(true) + .buildAndConnect(); + + assertEquals(path, edge.getName()); + assertTrue(edge.hasBogusName()); + + var mainStreet = I18NString.of("Main Street"); + edge.setName(mainStreet); + assertEquals(mainStreet, edge.getName()); + assertFalse(edge.hasBogusName()); + } }