From 47300f981eb08b2c73125a3937eb77165834f799 Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Tue, 19 Nov 2024 15:47:12 +0200 Subject: [PATCH 01/17] use duration tag from osm as escalator traversal duration --- .../module/osm/EscalatorProcessor.java | 13 ++++-- .../org/opentripplanner/osm/model/OsmWay.java | 15 +++++++ .../street/model/edge/EscalatorEdge.java | 15 +++++-- .../utils/time/DurationUtils.java | 43 +++++++++++++++++++ .../utils/time/DurationUtilsTest.java | 18 ++++++++ 5 files changed, 96 insertions(+), 8 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java index 75e0965d82f..ff439a37f59 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java @@ -27,30 +27,35 @@ public void buildEscalatorEdge(OsmWay escalatorWay, double length) { .boxed() .toList(); + Long duration = escalatorWay.getDurationSeconds(); for (int i = 0; i < nodes.size() - 1; i++) { if (escalatorWay.isForwardEscalator()) { EscalatorEdge.createEscalatorEdge( intersectionNodes.get(nodes.get(i)), intersectionNodes.get(nodes.get(i + 1)), - length + length, + duration ); } else if (escalatorWay.isBackwardEscalator()) { EscalatorEdge.createEscalatorEdge( intersectionNodes.get(nodes.get(i + 1)), intersectionNodes.get(nodes.get(i)), - length + length, + duration ); } else { EscalatorEdge.createEscalatorEdge( intersectionNodes.get(nodes.get(i)), intersectionNodes.get(nodes.get(i + 1)), - length + length, + duration ); EscalatorEdge.createEscalatorEdge( intersectionNodes.get(nodes.get(i + 1)), intersectionNodes.get(nodes.get(i)), - length + length, + duration ); } } 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 7b5fbe56748..24423024311 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java @@ -2,9 +2,11 @@ import gnu.trove.list.TLongList; import gnu.trove.list.array.TLongArrayList; +import java.time.format.DateTimeParseException; import java.util.Set; import org.opentripplanner.graph_builder.module.osm.StreetTraversalPermissionPair; import org.opentripplanner.street.model.StreetTraversalPermission; +import org.opentripplanner.utils.time.DurationUtils; public class OsmWay extends OsmWithTags { @@ -130,6 +132,19 @@ public boolean isEscalator() { return (isTag("highway", "steps") && isOneOfTags("conveying", ESCALATOR_CONVEYING_TAGS)); } + public Long getDurationSeconds() { + var duration = getTag("duration"); + if (duration != null) { + try { + return DurationUtils.parseClockDuration(duration).getSeconds(); + } catch (DateTimeParseException e) { + // For malformed duration tags, just pretend they weren't there. + return null; + } + } + return null; + } + public boolean isForwardEscalator() { return isEscalator() && "forward".equals(this.getTag("conveying")); } diff --git a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java index 20fae657c78..35c2db5cbc2 100644 --- a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java +++ b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java @@ -17,10 +17,12 @@ public class EscalatorEdge extends Edge { private static final double HORIZONTAL_SPEED = 0.45; private static final LocalizedString NAME = new LocalizedString("name.escalator"); private final double length; + private final Long duration; - private EscalatorEdge(Vertex v1, Vertex v2, double length) { + private EscalatorEdge(Vertex v1, Vertex v2, double length, Long duration) { super(v1, v2); this.length = length; + this.duration = duration; } @Override @@ -28,7 +30,12 @@ public State[] traverse(State s0) { // Only allow traversal by walking if (s0.currentMode() == TraverseMode.WALK && !s0.getRequest().wheelchair()) { var s1 = s0.edit(this); - var time = getDistanceMeters() / HORIZONTAL_SPEED; + double time; + if (duration == null) { + time = getDistanceMeters() / HORIZONTAL_SPEED; + } else { + time = duration; + } s1.incrementWeight(s0.getPreferences().walk().escalatorReluctance() * time); s1.incrementTimeInSeconds((int) Math.round(time)); s1.incrementWalkDistance(getDistanceMeters()); @@ -51,7 +58,7 @@ public I18NString getName() { return NAME; } - public static EscalatorEdge createEscalatorEdge(Vertex from, Vertex to, double length) { - return connectToGraph(new EscalatorEdge(from, to, length)); + public static EscalatorEdge createEscalatorEdge(Vertex from, Vertex to, double length, Long duration) { + return connectToGraph(new EscalatorEdge(from, to, length, duration)); } } diff --git a/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java b/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java index d73faecee03..10f04700f3f 100644 --- a/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java +++ b/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java @@ -11,6 +11,7 @@ import java.util.Optional; import java.util.regex.Pattern; import java.util.stream.Collectors; +import org.opentripplanner.utils.lang.StringUtils; /** * This class extend the Java {@link Duration} with utility functionality to parse and convert @@ -123,6 +124,48 @@ public static Duration duration(String duration) { } } + /** + * Parse a duration string in format hh:mm:ss. + * @param duration string in format hh:mm:ss + * @return Duration + * @throws DateTimeParseException on bad input + */ + public static Duration parseClockDuration(String duration) { + int colonCount = (int) duration.chars().filter(ch -> ch == ':').count(); + if (colonCount <= 2) { + try { + int i, j; + long hours, minutes = 0, seconds = 0; + switch (colonCount) { + case 0: + hours = Long.parseLong(duration); + break; + case 1: + i = duration.indexOf(':'); + hours = Long.parseLong(duration.substring(0, i)); + minutes = Long.parseLong(duration.substring(i + 1)); + break; + default: + //case 2: + i = duration.indexOf(':'); + j = duration.indexOf(':', i + 1); + hours = Long.parseLong(duration.substring(0, i)); + minutes = Long.parseLong(duration.substring(i + 1, j)); + seconds = Long.parseLong(duration.substring(j + 1)); + break; + } + if (hours >= 0 && minutes >= 0 && minutes < 60 && seconds >= 0 && seconds < 60) { + return Duration.ofHours(hours) + .plus(Duration.ofMinutes(minutes)) + .plus(Duration.ofSeconds(seconds)); + } + } catch (NumberFormatException e) { + // fallthrough + } + } + throw new DateTimeParseException("bad clock duration", duration, 0); + } + /** * This is used to parse a string which may be a number {@code NNNN}(number of seconds) or a * duration with format {@code NhNmNs}, where {@code N} is a decimal number and diff --git a/utils/src/test/java/org/opentripplanner/utils/time/DurationUtilsTest.java b/utils/src/test/java/org/opentripplanner/utils/time/DurationUtilsTest.java index ef2e0f50901..1e0839091cb 100644 --- a/utils/src/test/java/org/opentripplanner/utils/time/DurationUtilsTest.java +++ b/utils/src/test/java/org/opentripplanner/utils/time/DurationUtilsTest.java @@ -30,6 +30,8 @@ public class DurationUtilsTest { private final Duration D5m = Duration.ofMinutes(5); private final Duration D9s = Duration.ofSeconds(9); private final Duration D3d5m9s = D3d.plus(D5m).plus(D9s); + private final Duration D2h5m = D2h.plus(D5m); + private final Duration D2h5m9s = D2h.plus(D5m).plus(D9s); private final int I9h31m = durationSec(9, 31, 0); private final int I9h36m55s = durationSec(9, 36, 55); private final int I13h33m57s = durationSec(13, 33, 57); @@ -91,6 +93,22 @@ public void duration() { assertEquals(-D9s.toSeconds(), DurationUtils.duration("-9", ChronoUnit.SECONDS).toSeconds()); } + @Test + public void parseClockDuration() { + assertEquals(D2h, DurationUtils.parseClockDuration("2")); + assertEquals(D2h5m, DurationUtils.parseClockDuration("02:05")); + assertEquals(D2h5m9s, DurationUtils.parseClockDuration("02:05:09")); + assertThrows( + DateTimeParseException.class, + () -> DurationUtils.parseClockDuration("02:65:09")); + assertThrows( + DateTimeParseException.class, + () -> DurationUtils.parseClockDuration("02:05:09:00")); + assertThrows( + DateTimeParseException.class, + () -> DurationUtils.parseClockDuration("02:x5:09")); + } + @Test public void parseSecondsOrDuration() { assertEquals(D9s, DurationUtils.parseSecondsOrDuration("9s").orElseThrow()); From d9ead12868cc132284260423c69b6bfdb7582ee0 Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Fri, 22 Nov 2024 12:08:22 +0200 Subject: [PATCH 02/17] make default escalator speed a configuration parameter --- .../module/osm/EscalatorProcessor.java | 2 +- .../org/opentripplanner/osm/model/OsmWay.java | 8 +- .../preference/EscalatorPreferences.java | 74 +++++++++++++++++++ .../request/preference/StreetPreferences.java | 16 ++++ .../routerequest/RouteRequestConfig.java | 11 +++ .../street/model/edge/EscalatorEdge.java | 17 +++-- .../street/model/edge/EscalatorEdgeTest.java | 23 ++++-- doc/user/RouteRequest.md | 1 + .../utils/time/DurationUtils.java | 5 +- .../utils/time/DurationUtilsTest.java | 11 +-- 10 files changed, 141 insertions(+), 27 deletions(-) create mode 100644 application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java index ff439a37f59..488fc96a0b3 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java @@ -27,7 +27,7 @@ public void buildEscalatorEdge(OsmWay escalatorWay, double length) { .boxed() .toList(); - Long duration = escalatorWay.getDurationSeconds(); + Integer duration = escalatorWay.getDurationSeconds(); for (int i = 0; i < nodes.size() - 1; i++) { if (escalatorWay.isForwardEscalator()) { EscalatorEdge.createEscalatorEdge( 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 24423024311..0819ab57b59 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java @@ -132,11 +132,15 @@ public boolean isEscalator() { return (isTag("highway", "steps") && isOneOfTags("conveying", ESCALATOR_CONVEYING_TAGS)); } - public Long getDurationSeconds() { + public Integer getDurationSeconds() { var duration = getTag("duration"); if (duration != null) { try { - return DurationUtils.parseClockDuration(duration).getSeconds(); + long seconds = DurationUtils.parseClockDuration(duration).getSeconds(); + if (seconds < 0 || seconds > Integer.MAX_VALUE) { + return null; + } + return (int) seconds; } catch (DateTimeParseException e) { // For malformed duration tags, just pretend they weren't there. return null; diff --git a/application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java b/application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java new file mode 100644 index 00000000000..af4cbda39ef --- /dev/null +++ b/application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java @@ -0,0 +1,74 @@ +package org.opentripplanner.routing.api.request.preference; + +import java.io.Serializable; +import java.util.function.Consumer; + +public class EscalatorPreferences implements Serializable { + + public static final EscalatorPreferences DEFAULT = new EscalatorPreferences(); + + private final double horizontalSpeed; + + /* A quick internet search gives escalator speed range of 0.3-0.6 m/s and angle of 30 degrees. + * Using the angle of 30 degrees and a speed of 0.5 m/s gives a horizontal component + * of approx. 0.43 m/s */ + private static final double HORIZONTAL_SPEED = 0.45; + + private EscalatorPreferences() { + this.horizontalSpeed = HORIZONTAL_SPEED; + } + + private EscalatorPreferences(Builder builder) { + this.horizontalSpeed = builder.horizontalSpeed; + } + + public static Builder of() { + return DEFAULT.copyOf(); + } + + public Builder copyOf() { + return new Builder(this); + } + + public double horizontalSpeed() { + return horizontalSpeed; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + EscalatorPreferences that = (EscalatorPreferences) o; + return horizontalSpeed == that.horizontalSpeed; + } + + public static class Builder { + + private final EscalatorPreferences original; + private double horizontalSpeed; + + public Builder(EscalatorPreferences original) { + this.original = original; + this.horizontalSpeed = original.horizontalSpeed; + } + + public EscalatorPreferences original() { + return original; + } + + public Builder withHorizontalSpeed(double horizontalSpeed) { + this.horizontalSpeed = horizontalSpeed; + return this; + } + + public Builder apply(Consumer body) { + body.accept(this); + return this; + } + + public EscalatorPreferences build() { + var value = new EscalatorPreferences(this); + return original.equals(value) ? original : value; + } + } +} diff --git a/application/src/main/java/org/opentripplanner/routing/api/request/preference/StreetPreferences.java b/application/src/main/java/org/opentripplanner/routing/api/request/preference/StreetPreferences.java index bb526ceaa31..e20ed82f9a5 100644 --- a/application/src/main/java/org/opentripplanner/routing/api/request/preference/StreetPreferences.java +++ b/application/src/main/java/org/opentripplanner/routing/api/request/preference/StreetPreferences.java @@ -30,6 +30,7 @@ public final class StreetPreferences implements Serializable { private final double turnReluctance; private final DrivingDirection drivingDirection; private final ElevatorPreferences elevator; + private final EscalatorPreferences escalator; private final AccessEgressPreferences accessEgress; private final IntersectionTraversalModel intersectionTraversalModel; private final DurationForEnum maxDirectDuration; @@ -39,6 +40,7 @@ private StreetPreferences() { this.turnReluctance = 1.0; this.drivingDirection = DrivingDirection.RIGHT; this.elevator = ElevatorPreferences.DEFAULT; + this.escalator = EscalatorPreferences.DEFAULT; this.accessEgress = AccessEgressPreferences.DEFAULT; this.intersectionTraversalModel = IntersectionTraversalModel.SIMPLE; this.maxDirectDuration = durationForStreetModeOf(ofHours(4)); @@ -49,6 +51,7 @@ private StreetPreferences(Builder builder) { this.turnReluctance = Units.reluctance(builder.turnReluctance); this.drivingDirection = requireNonNull(builder.drivingDirection); this.elevator = requireNonNull(builder.elevator); + this.escalator = requireNonNull(builder.escalator); this.accessEgress = requireNonNull(builder.accessEgress); this.intersectionTraversalModel = requireNonNull(builder.intersectionTraversalModel); this.maxDirectDuration = requireNonNull(builder.maxDirectDuration); @@ -78,6 +81,10 @@ public ElevatorPreferences elevator() { return elevator; } + public EscalatorPreferences escalator() { + return escalator; + } + /** Preferences for access/egress routing */ public AccessEgressPreferences accessEgress() { return accessEgress; @@ -110,6 +117,7 @@ public boolean equals(Object o) { DoubleUtils.doubleEquals(that.turnReluctance, turnReluctance) && drivingDirection == that.drivingDirection && elevator.equals(that.elevator) && + escalator.equals(that.escalator) && routingTimeout.equals(that.routingTimeout) && intersectionTraversalModel == that.intersectionTraversalModel && maxDirectDuration.equals(that.maxDirectDuration) && @@ -138,6 +146,7 @@ public String toString() { .addEnum("drivingDirection", drivingDirection, DEFAULT.drivingDirection) .addDuration("routingTimeout", routingTimeout, DEFAULT.routingTimeout()) .addObj("elevator", elevator, DEFAULT.elevator) + .addObj("escalator", escalator, DEFAULT.escalator) .addObj( "intersectionTraversalModel", intersectionTraversalModel, @@ -154,6 +163,7 @@ public static class Builder { private double turnReluctance; private DrivingDirection drivingDirection; private ElevatorPreferences elevator; + private EscalatorPreferences escalator; private IntersectionTraversalModel intersectionTraversalModel; private DurationForEnum maxDirectDuration; private Duration routingTimeout; @@ -164,6 +174,7 @@ public Builder(StreetPreferences original) { this.turnReluctance = original.turnReluctance; this.drivingDirection = original.drivingDirection; this.elevator = original.elevator; + this.escalator = original.escalator; this.intersectionTraversalModel = original.intersectionTraversalModel; this.accessEgress = original.accessEgress; this.maxDirectDuration = original.maxDirectDuration; @@ -189,6 +200,11 @@ public Builder withElevator(Consumer body) { return this; } + public Builder withEscalator(Consumer body) { + this.escalator = escalator.copyOf().apply(body).build(); + return this; + } + public Builder withAccessEgress(Consumer body) { this.accessEgress = accessEgress.copyOf().apply(body).build(); return this; diff --git a/application/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java b/application/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java index d05aee96ccf..9c344afc44c 100644 --- a/application/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java +++ b/application/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java @@ -6,6 +6,7 @@ import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_3; import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_4; import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_5; +import static org.opentripplanner.standalone.config.framework.json.OtpVersion.V2_7; import static org.opentripplanner.standalone.config.routerequest.ItineraryFiltersConfig.mapItineraryFilterParams; import static org.opentripplanner.standalone.config.routerequest.TransferConfig.mapTransferPreferences; import static org.opentripplanner.standalone.config.routerequest.TriangleOptimizationConfig.mapOptimizationTriangle; @@ -459,6 +460,16 @@ private static void mapStreetPreferences(NodeAdapter c, StreetPreferences.Builde .asInt(dftElevator.hopTime()) ); }) + .withEscalator(escalator -> { + var dftEscalator = dft.escalator(); + escalator.withHorizontalSpeed( + c + .of("escalatorSpeed") + .since(V2_7) + .summary("How fast does an escalator move horizontally?") + .asDouble(dftEscalator.horizontalSpeed()) + ); + }) .withAccessEgress(accessEgress -> { var dftAccessEgress = dft.accessEgress(); accessEgress diff --git a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java index 35c2db5cbc2..6f9e9c74b2f 100644 --- a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java +++ b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java @@ -11,15 +11,11 @@ /** Represents an escalator. An escalator edge can only be traversed by walking */ public class EscalatorEdge extends Edge { - /* A quick internet search gives escalator speed range of 0.3-0.6 m/s and angle of 30 degrees. - * Using the angle of 30 degrees and a speed of 0.5 m/s gives a horizontal component - * of approx. 0.43 m/s */ - private static final double HORIZONTAL_SPEED = 0.45; private static final LocalizedString NAME = new LocalizedString("name.escalator"); private final double length; - private final Long duration; + private final Integer duration; - private EscalatorEdge(Vertex v1, Vertex v2, double length, Long duration) { + private EscalatorEdge(Vertex v1, Vertex v2, double length, Integer duration) { super(v1, v2); this.length = length; this.duration = duration; @@ -32,7 +28,7 @@ public State[] traverse(State s0) { var s1 = s0.edit(this); double time; if (duration == null) { - time = getDistanceMeters() / HORIZONTAL_SPEED; + time = getDistanceMeters() / s0.getPreferences().street().escalator().horizontalSpeed(); } else { time = duration; } @@ -58,7 +54,12 @@ public I18NString getName() { return NAME; } - public static EscalatorEdge createEscalatorEdge(Vertex from, Vertex to, double length, Long duration) { + public static EscalatorEdge createEscalatorEdge( + Vertex from, + Vertex to, + double length, + Integer duration + ) { return connectToGraph(new EscalatorEdge(from, to, length, duration)); } } diff --git a/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java b/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java index 1cfff635c45..9e0b4f7d297 100644 --- a/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java +++ b/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java @@ -27,20 +27,29 @@ static Stream args() { @ParameterizedTest(name = "escalatorReluctance of {0} should lead to traversal costs of {1}") @MethodSource("args") void testWalking(double escalatorReluctance, double expectedWeight) { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 45); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 45, null); var req = StreetSearchRequest .of() .withPreferences(p -> p.withWalk(w -> w.withEscalatorReluctance(escalatorReluctance))) .withMode(StreetMode.WALK); var res = edge.traverse(new State(from, req.build()))[0]; - assertEquals(res.weight, expectedWeight); - assertEquals(res.getTimeDeltaSeconds(), 100); + assertEquals(expectedWeight, res.weight); + assertEquals(100, res.getTimeDeltaSeconds()); + } + + @Test + void testDuration() { + // If duration is given, length does not affect timeDeltaSeconds, only duration does. + var edge = EscalatorEdge.createEscalatorEdge(from, to, 45, 60); + var req = StreetSearchRequest.of().withMode(StreetMode.WALK); + var res = edge.traverse(new State(from, req.build()))[0]; + assertEquals(60, res.getTimeDeltaSeconds()); } @Test void testCycling() { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 10); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, null); var req = StreetSearchRequest.of().withMode(StreetMode.BIKE); var res = edge.traverse(new State(from, req.build())); assertThat(res).isEmpty(); @@ -48,7 +57,7 @@ void testCycling() { @Test void testWheelchair() { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 10); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, null); var req = StreetSearchRequest.of().withMode(StreetMode.WALK).withWheelchair(true); var res = edge.traverse(new State(from, req.build())); assertThat(res).isEmpty(); @@ -56,14 +65,14 @@ void testWheelchair() { @Test void name() { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 10); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, null); assertEquals("Rolltreppe", edge.getName().toString(Locale.GERMANY)); assertEquals("escalator", edge.getName().toString(Locale.ENGLISH)); } @Test void geometry() { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 10); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, null); assertThat(edge.getGeometry().getCoordinates()).isNotEmpty(); } } diff --git a/doc/user/RouteRequest.md b/doc/user/RouteRequest.md index ea3d0d12c74..c0c5d2bb590 100644 --- a/doc/user/RouteRequest.md +++ b/doc/user/RouteRequest.md @@ -23,6 +23,7 @@ and in the [transferRequests in build-config.json](BuildConfiguration.md#transfe | elevatorBoardTime | `integer` | How long does it take to get on an elevator, on average. | *Optional* | `90` | 2.0 | | elevatorHopCost | `integer` | What is the cost of travelling one floor on an elevator? | *Optional* | `20` | 2.0 | | elevatorHopTime | `integer` | How long does it take to advance one floor on an elevator? | *Optional* | `20` | 2.0 | +| escalatorSpeed | `double` | How fast does an escalator move horizontally? | *Optional* | `0.45` | 2.7 | | geoidElevation | `boolean` | If true, the Graph's ellipsoidToGeoidDifference is applied to all elevations returned by this query. | *Optional* | `false` | 2.0 | | ignoreRealtimeUpdates | `boolean` | When true, real-time updates are ignored during this search. | *Optional* | `false` | 2.0 | | [intersectionTraversalModel](#rd_intersectionTraversalModel) | `enum` | The model that computes the costs of turns. | *Optional* | `"simple"` | 2.2 | diff --git a/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java b/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java index 10f04700f3f..e1c0b557855 100644 --- a/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java +++ b/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java @@ -146,7 +146,7 @@ public static Duration parseClockDuration(String duration) { minutes = Long.parseLong(duration.substring(i + 1)); break; default: - //case 2: + //case 2: i = duration.indexOf(':'); j = duration.indexOf(':', i + 1); hours = Long.parseLong(duration.substring(0, i)); @@ -155,7 +155,8 @@ public static Duration parseClockDuration(String duration) { break; } if (hours >= 0 && minutes >= 0 && minutes < 60 && seconds >= 0 && seconds < 60) { - return Duration.ofHours(hours) + return Duration + .ofHours(hours) .plus(Duration.ofMinutes(minutes)) .plus(Duration.ofSeconds(seconds)); } diff --git a/utils/src/test/java/org/opentripplanner/utils/time/DurationUtilsTest.java b/utils/src/test/java/org/opentripplanner/utils/time/DurationUtilsTest.java index 1e0839091cb..15fbd6e5027 100644 --- a/utils/src/test/java/org/opentripplanner/utils/time/DurationUtilsTest.java +++ b/utils/src/test/java/org/opentripplanner/utils/time/DurationUtilsTest.java @@ -98,15 +98,12 @@ public void parseClockDuration() { assertEquals(D2h, DurationUtils.parseClockDuration("2")); assertEquals(D2h5m, DurationUtils.parseClockDuration("02:05")); assertEquals(D2h5m9s, DurationUtils.parseClockDuration("02:05:09")); + assertThrows(DateTimeParseException.class, () -> DurationUtils.parseClockDuration("02:65:09")); assertThrows( DateTimeParseException.class, - () -> DurationUtils.parseClockDuration("02:65:09")); - assertThrows( - DateTimeParseException.class, - () -> DurationUtils.parseClockDuration("02:05:09:00")); - assertThrows( - DateTimeParseException.class, - () -> DurationUtils.parseClockDuration("02:x5:09")); + () -> DurationUtils.parseClockDuration("02:05:09:00") + ); + assertThrows(DateTimeParseException.class, () -> DurationUtils.parseClockDuration("02:x5:09")); } @Test From 8028990ba63ac01bef999087f0e98bde5fb0c683 Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Mon, 25 Nov 2024 10:31:38 +0200 Subject: [PATCH 03/17] test the added duration tag in OsmWay --- .../java/org/opentripplanner/osm/model/OsmWayTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) 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 9ac9457a9ec..a659a767ff4 100644 --- a/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java +++ b/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java @@ -1,6 +1,8 @@ package org.opentripplanner.osm.model; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.Test; @@ -172,6 +174,13 @@ void escalator() { escalator.addTag("conveying", "yes"); assertTrue(escalator.isEscalator()); + assertNull(escalator.getDurationSeconds()); + + escalator.addTag("duration", "00:00:61"); + assertNull(escalator.getDurationSeconds()); + escalator.addTag("duration", "00:01:01"); + assertEquals(61, escalator.getDurationSeconds()); + escalator.addTag("conveying", "whoknows?"); assertFalse(escalator.isEscalator()); } From aa3de3a455b7c2120268b391999a29596395fc09 Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Mon, 25 Nov 2024 14:10:48 +0200 Subject: [PATCH 04/17] ignore escalator duration tags if the speed are nonsense. include duration in debug client edge popup. --- .../graph_builder/module/osm/EscalatorProcessor.java | 11 +++++++++++ .../inspector/vector/edge/EdgePropertyMapper.java | 5 ++++- .../street/model/edge/EscalatorEdge.java | 4 ++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java index 488fc96a0b3..b97458d642a 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java @@ -14,6 +14,11 @@ class EscalatorProcessor { private final Map intersectionNodes; + // If an escalator is tagged as moving less than 5 cm/s, or more than 5 m/s, + // assume it's an error and ignore it. + private static final double SLOW_ESCALATOR_ERROR_CUTOFF = 0.05; + private static final double FAST_ESCALATOR_ERROR_CUTOFF = 5.0; + public EscalatorProcessor(Map intersectionNodes) { this.intersectionNodes = intersectionNodes; } @@ -28,6 +33,12 @@ public void buildEscalatorEdge(OsmWay escalatorWay, double length) { .toList(); Integer duration = escalatorWay.getDurationSeconds(); + if (duration != null) { + double speed = length / duration; + if (speed < SLOW_ESCALATOR_ERROR_CUTOFF || speed > FAST_ESCALATOR_ERROR_CUTOFF) { + duration = null; + } + } for (int i = 0; i < nodes.size() - 1; i++) { if (escalatorWay.isForwardEscalator()) { EscalatorEdge.createEscalatorEdge( diff --git a/application/src/main/java/org/opentripplanner/inspector/vector/edge/EdgePropertyMapper.java b/application/src/main/java/org/opentripplanner/inspector/vector/edge/EdgePropertyMapper.java index 30763edca9e..13956af99c4 100644 --- a/application/src/main/java/org/opentripplanner/inspector/vector/edge/EdgePropertyMapper.java +++ b/application/src/main/java/org/opentripplanner/inspector/vector/edge/EdgePropertyMapper.java @@ -22,7 +22,10 @@ protected Collection map(Edge input) { List properties = switch (input) { case StreetEdge e -> mapStreetEdge(e); - case EscalatorEdge e -> List.of(kv("distance", e.getDistanceMeters())); + case EscalatorEdge e -> List.of( + kv("distance", e.getDistanceMeters()), + kv("duration", e.getDuration()) + ); default -> List.of(); }; return ListUtils.combine(baseProps, properties); diff --git a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java index 6f9e9c74b2f..2e69793ccf1 100644 --- a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java +++ b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java @@ -49,6 +49,10 @@ public double getDistanceMeters() { return length; } + public Integer getDuration() { + return duration; + } + @Override public I18NString getName() { return NAME; From 456619446b332854f46f8337598847cef845accd Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Mon, 25 Nov 2024 14:29:13 +0200 Subject: [PATCH 05/17] use Optional instead of Integer which can be null --- .../module/osm/EscalatorProcessor.java | 10 ++++++---- .../java/org/opentripplanner/osm/model/OsmWay.java | 14 ++++++-------- .../street/model/edge/EscalatorEdge.java | 14 ++++++++------ 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java index b97458d642a..1d9640f0a6f 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java @@ -1,8 +1,10 @@ package org.opentripplanner.graph_builder.module.osm; +import java.time.Duration; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Optional; import org.opentripplanner.osm.model.OsmWay; import org.opentripplanner.street.model.edge.EscalatorEdge; import org.opentripplanner.street.model.vertex.IntersectionVertex; @@ -32,11 +34,11 @@ public void buildEscalatorEdge(OsmWay escalatorWay, double length) { .boxed() .toList(); - Integer duration = escalatorWay.getDurationSeconds(); - if (duration != null) { - double speed = length / duration; + Optional duration = escalatorWay.getDuration(); + if (duration.isPresent()) { + double speed = length / duration.get().toSeconds(); if (speed < SLOW_ESCALATOR_ERROR_CUTOFF || speed > FAST_ESCALATOR_ERROR_CUTOFF) { - duration = null; + duration = Optional.empty(); } } for (int i = 0; i < nodes.size() - 1; i++) { 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 0819ab57b59..e80a3ab6499 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java @@ -2,7 +2,9 @@ import gnu.trove.list.TLongList; import gnu.trove.list.array.TLongArrayList; +import java.time.Duration; import java.time.format.DateTimeParseException; +import java.util.Optional; import java.util.Set; import org.opentripplanner.graph_builder.module.osm.StreetTraversalPermissionPair; import org.opentripplanner.street.model.StreetTraversalPermission; @@ -132,21 +134,17 @@ public boolean isEscalator() { return (isTag("highway", "steps") && isOneOfTags("conveying", ESCALATOR_CONVEYING_TAGS)); } - public Integer getDurationSeconds() { + public Optional getDuration() { var duration = getTag("duration"); if (duration != null) { try { - long seconds = DurationUtils.parseClockDuration(duration).getSeconds(); - if (seconds < 0 || seconds > Integer.MAX_VALUE) { - return null; - } - return (int) seconds; + return Optional.of(DurationUtils.parseClockDuration(duration)); } catch (DateTimeParseException e) { // For malformed duration tags, just pretend they weren't there. - return null; + return Optional.empty(); } } - return null; + return Optional.empty(); } public boolean isForwardEscalator() { diff --git a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java index 2e69793ccf1..825d642373b 100644 --- a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java +++ b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java @@ -1,5 +1,7 @@ package org.opentripplanner.street.model.edge; +import java.time.Duration; +import java.util.Optional; import org.locationtech.jts.geom.LineString; import org.opentripplanner.framework.geometry.GeometryUtils; import org.opentripplanner.framework.i18n.I18NString; @@ -13,9 +15,9 @@ public class EscalatorEdge extends Edge { private static final LocalizedString NAME = new LocalizedString("name.escalator"); private final double length; - private final Integer duration; + private final Optional duration; - private EscalatorEdge(Vertex v1, Vertex v2, double length, Integer duration) { + private EscalatorEdge(Vertex v1, Vertex v2, double length, Optional duration) { super(v1, v2); this.length = length; this.duration = duration; @@ -27,10 +29,10 @@ public State[] traverse(State s0) { if (s0.currentMode() == TraverseMode.WALK && !s0.getRequest().wheelchair()) { var s1 = s0.edit(this); double time; - if (duration == null) { + if (duration.isEmpty()) { time = getDistanceMeters() / s0.getPreferences().street().escalator().horizontalSpeed(); } else { - time = duration; + time = duration.get().toSeconds(); } s1.incrementWeight(s0.getPreferences().walk().escalatorReluctance() * time); s1.incrementTimeInSeconds((int) Math.round(time)); @@ -49,7 +51,7 @@ public double getDistanceMeters() { return length; } - public Integer getDuration() { + public Optional getDuration() { return duration; } @@ -62,7 +64,7 @@ public static EscalatorEdge createEscalatorEdge( Vertex from, Vertex to, double length, - Integer duration + Optional duration ) { return connectToGraph(new EscalatorEdge(from, to, length, duration)); } From 3127cf35b0d8f1169034d9a86cdf571183792b3b Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Mon, 25 Nov 2024 14:34:54 +0200 Subject: [PATCH 06/17] test changes included --- .../org/opentripplanner/osm/model/OsmWayTest.java | 8 +++++--- .../street/model/edge/EscalatorEdgeTest.java | 14 ++++++++------ 2 files changed, 13 insertions(+), 9 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 a659a767ff4..869e579e9aa 100644 --- a/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java +++ b/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java @@ -5,6 +5,8 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.time.Duration; +import java.util.Optional; import org.junit.jupiter.api.Test; import org.opentripplanner.osm.wayproperty.specifier.WayTestData; @@ -174,12 +176,12 @@ void escalator() { escalator.addTag("conveying", "yes"); assertTrue(escalator.isEscalator()); - assertNull(escalator.getDurationSeconds()); + assertEquals(Optional.empty(), escalator.getDuration()); escalator.addTag("duration", "00:00:61"); - assertNull(escalator.getDurationSeconds()); + assertEquals(Optional.empty(), escalator.getDuration()); escalator.addTag("duration", "00:01:01"); - assertEquals(61, escalator.getDurationSeconds()); + assertEquals(Optional.of(Duration.ofSeconds(61)), escalator.getDuration()); escalator.addTag("conveying", "whoknows?"); assertFalse(escalator.isEscalator()); diff --git a/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java b/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java index 9e0b4f7d297..601aeb0bc27 100644 --- a/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java +++ b/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java @@ -3,7 +3,9 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import java.time.Duration; import java.util.Locale; +import java.util.Optional; import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -27,7 +29,7 @@ static Stream args() { @ParameterizedTest(name = "escalatorReluctance of {0} should lead to traversal costs of {1}") @MethodSource("args") void testWalking(double escalatorReluctance, double expectedWeight) { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 45, null); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 45, Optional.empty()); var req = StreetSearchRequest .of() .withPreferences(p -> p.withWalk(w -> w.withEscalatorReluctance(escalatorReluctance))) @@ -41,7 +43,7 @@ void testWalking(double escalatorReluctance, double expectedWeight) { @Test void testDuration() { // If duration is given, length does not affect timeDeltaSeconds, only duration does. - var edge = EscalatorEdge.createEscalatorEdge(from, to, 45, 60); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 45, Optional.of(Duration.ofSeconds(60))); var req = StreetSearchRequest.of().withMode(StreetMode.WALK); var res = edge.traverse(new State(from, req.build()))[0]; assertEquals(60, res.getTimeDeltaSeconds()); @@ -49,7 +51,7 @@ void testDuration() { @Test void testCycling() { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, null); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, Optional.empty()); var req = StreetSearchRequest.of().withMode(StreetMode.BIKE); var res = edge.traverse(new State(from, req.build())); assertThat(res).isEmpty(); @@ -57,7 +59,7 @@ void testCycling() { @Test void testWheelchair() { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, null); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, Optional.empty()); var req = StreetSearchRequest.of().withMode(StreetMode.WALK).withWheelchair(true); var res = edge.traverse(new State(from, req.build())); assertThat(res).isEmpty(); @@ -65,14 +67,14 @@ void testWheelchair() { @Test void name() { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, null); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, Optional.empty()); assertEquals("Rolltreppe", edge.getName().toString(Locale.GERMANY)); assertEquals("escalator", edge.getName().toString(Locale.ENGLISH)); } @Test void geometry() { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, null); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, Optional.empty()); assertThat(edge.getGeometry().getCoordinates()).isNotEmpty(); } } From 1e5aca122c89934b9a9b5382c0bb5575ff54aa1d Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Tue, 26 Nov 2024 14:43:15 +0200 Subject: [PATCH 07/17] use Optional upwards only --- .../module/osm/EscalatorProcessor.java | 8 ++++---- .../street/model/edge/EscalatorEdge.java | 13 +++++++------ .../street/model/edge/EscalatorEdgeTest.java | 13 ++++++------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java index 1d9640f0a6f..22a91305f22 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java @@ -47,28 +47,28 @@ public void buildEscalatorEdge(OsmWay escalatorWay, double length) { intersectionNodes.get(nodes.get(i)), intersectionNodes.get(nodes.get(i + 1)), length, - duration + duration.orElse(null) ); } else if (escalatorWay.isBackwardEscalator()) { EscalatorEdge.createEscalatorEdge( intersectionNodes.get(nodes.get(i + 1)), intersectionNodes.get(nodes.get(i)), length, - duration + duration.orElse(null) ); } else { EscalatorEdge.createEscalatorEdge( intersectionNodes.get(nodes.get(i)), intersectionNodes.get(nodes.get(i + 1)), length, - duration + duration.orElse(null) ); EscalatorEdge.createEscalatorEdge( intersectionNodes.get(nodes.get(i + 1)), intersectionNodes.get(nodes.get(i)), length, - duration + duration.orElse(null) ); } } diff --git a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java index 825d642373b..7dab48dc3e2 100644 --- a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java +++ b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java @@ -2,6 +2,7 @@ import java.time.Duration; import java.util.Optional; +import javax.annotation.Nullable; import org.locationtech.jts.geom.LineString; import org.opentripplanner.framework.geometry.GeometryUtils; import org.opentripplanner.framework.i18n.I18NString; @@ -15,9 +16,9 @@ public class EscalatorEdge extends Edge { private static final LocalizedString NAME = new LocalizedString("name.escalator"); private final double length; - private final Optional duration; + private final Duration duration; - private EscalatorEdge(Vertex v1, Vertex v2, double length, Optional duration) { + private EscalatorEdge(Vertex v1, Vertex v2, double length, Duration duration) { super(v1, v2); this.length = length; this.duration = duration; @@ -29,10 +30,10 @@ public State[] traverse(State s0) { if (s0.currentMode() == TraverseMode.WALK && !s0.getRequest().wheelchair()) { var s1 = s0.edit(this); double time; - if (duration.isEmpty()) { + if (duration == null) { time = getDistanceMeters() / s0.getPreferences().street().escalator().horizontalSpeed(); } else { - time = duration.get().toSeconds(); + time = duration.toSeconds(); } s1.incrementWeight(s0.getPreferences().walk().escalatorReluctance() * time); s1.incrementTimeInSeconds((int) Math.round(time)); @@ -52,7 +53,7 @@ public double getDistanceMeters() { } public Optional getDuration() { - return duration; + return Optional.ofNullable(duration); } @Override @@ -64,7 +65,7 @@ public static EscalatorEdge createEscalatorEdge( Vertex from, Vertex to, double length, - Optional duration + @Nullable Duration duration ) { return connectToGraph(new EscalatorEdge(from, to, length, duration)); } diff --git a/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java b/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java index 601aeb0bc27..25199e373a0 100644 --- a/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java +++ b/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java @@ -5,7 +5,6 @@ import java.time.Duration; import java.util.Locale; -import java.util.Optional; import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -29,7 +28,7 @@ static Stream args() { @ParameterizedTest(name = "escalatorReluctance of {0} should lead to traversal costs of {1}") @MethodSource("args") void testWalking(double escalatorReluctance, double expectedWeight) { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 45, Optional.empty()); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 45, null); var req = StreetSearchRequest .of() .withPreferences(p -> p.withWalk(w -> w.withEscalatorReluctance(escalatorReluctance))) @@ -43,7 +42,7 @@ void testWalking(double escalatorReluctance, double expectedWeight) { @Test void testDuration() { // If duration is given, length does not affect timeDeltaSeconds, only duration does. - var edge = EscalatorEdge.createEscalatorEdge(from, to, 45, Optional.of(Duration.ofSeconds(60))); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 45, Duration.ofSeconds(60)); var req = StreetSearchRequest.of().withMode(StreetMode.WALK); var res = edge.traverse(new State(from, req.build()))[0]; assertEquals(60, res.getTimeDeltaSeconds()); @@ -51,7 +50,7 @@ void testDuration() { @Test void testCycling() { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, Optional.empty()); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, null); var req = StreetSearchRequest.of().withMode(StreetMode.BIKE); var res = edge.traverse(new State(from, req.build())); assertThat(res).isEmpty(); @@ -59,7 +58,7 @@ void testCycling() { @Test void testWheelchair() { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, Optional.empty()); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, null); var req = StreetSearchRequest.of().withMode(StreetMode.WALK).withWheelchair(true); var res = edge.traverse(new State(from, req.build())); assertThat(res).isEmpty(); @@ -67,14 +66,14 @@ void testWheelchair() { @Test void name() { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, Optional.empty()); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, null); assertEquals("Rolltreppe", edge.getName().toString(Locale.GERMANY)); assertEquals("escalator", edge.getName().toString(Locale.ENGLISH)); } @Test void geometry() { - var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, Optional.empty()); + var edge = EscalatorEdge.createEscalatorEdge(from, to, 10, null); assertThat(edge.getGeometry().getCoordinates()).isNotEmpty(); } } From 627b3fd84aec882a5f68d09271941e2f1508f588 Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Thu, 28 Nov 2024 15:26:31 +0200 Subject: [PATCH 08/17] add bad duration tags to issue store, better testing of duration parsing --- .../module/osm/EscalatorProcessor.java | 28 ++++++- .../graph_builder/module/osm/OsmModule.java | 5 +- .../org/opentripplanner/osm/model/OsmWay.java | 14 +--- .../osm/model/OsmWithTags.java | 66 +++++++++++++++++ .../preference/EscalatorPreferences.java | 74 ------------------- .../request/preference/StreetPreferences.java | 16 ---- .../request/preference/WalkPreferences.java | 33 +++++++-- .../routerequest/RouteRequestConfig.java | 18 ++--- .../street/model/edge/EscalatorEdge.java | 6 +- .../opentripplanner/osm/model/OsmWayTest.java | 8 +- .../osm/model/OsmWithTagsTest.java | 23 ++++++ .../standalone/config/router-config.json | 3 +- doc/user/RouteRequest.md | 14 +++- doc/user/RouterConfiguration.md | 3 +- .../utils/time/DurationUtils.java | 43 ----------- .../utils/time/DurationUtilsTest.java | 15 ---- 16 files changed, 179 insertions(+), 190 deletions(-) delete mode 100644 application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java index 22a91305f22..c8f4e0e0042 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java @@ -5,6 +5,8 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore; +import org.opentripplanner.graph_builder.issue.api.Issue; import org.opentripplanner.osm.model.OsmWay; import org.opentripplanner.street.model.edge.EscalatorEdge; import org.opentripplanner.street.model.vertex.IntersectionVertex; @@ -15,14 +17,19 @@ class EscalatorProcessor { private final Map intersectionNodes; + private final DataImportIssueStore issueStore; // If an escalator is tagged as moving less than 5 cm/s, or more than 5 m/s, // assume it's an error and ignore it. private static final double SLOW_ESCALATOR_ERROR_CUTOFF = 0.05; private static final double FAST_ESCALATOR_ERROR_CUTOFF = 5.0; - public EscalatorProcessor(Map intersectionNodes) { + public EscalatorProcessor( + Map intersectionNodes, + DataImportIssueStore issueStore + ) { this.intersectionNodes = intersectionNodes; + this.issueStore = issueStore; } public void buildEscalatorEdge(OsmWay escalatorWay, double length) { @@ -34,11 +41,28 @@ public void buildEscalatorEdge(OsmWay escalatorWay, double length) { .boxed() .toList(); - Optional duration = escalatorWay.getDuration(); + Optional duration = escalatorWay.getDuration(v -> + issueStore.add( + Issue.issue( + "InvalidDuration", + "Duration for osm node %d is not a valid duration: '%s'; it's replaced with 'Optional.empty()' (unknown).", + escalatorWay.getId(), + v + ) + ) + ); if (duration.isPresent()) { double speed = length / duration.get().toSeconds(); if (speed < SLOW_ESCALATOR_ERROR_CUTOFF || speed > FAST_ESCALATOR_ERROR_CUTOFF) { duration = Optional.empty(); + issueStore.add( + Issue.issue( + "InvalidDuration", + "Duration for osm node {} makes implied speed {} be outside acceptable range.", + escalatorWay.getId(), + speed + ) + ); } } for (int i = 0; i < nodes.size() - 1; i++) { diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java index 08d23087a45..e4a08b40177 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/OsmModule.java @@ -247,7 +247,10 @@ private void buildBasicGraph() { long wayCount = osmdb.getWays().size(); ProgressTracker progress = ProgressTracker.track("Build street graph", 5_000, wayCount); LOG.info(progress.startMessage()); - var escalatorProcessor = new EscalatorProcessor(vertexGenerator.intersectionNodes()); + var escalatorProcessor = new EscalatorProcessor( + vertexGenerator.intersectionNodes(), + issueStore + ); WAY:for (OsmWay way : osmdb.getWays()) { WayProperties wayData = way.getOsmProvider().getWayPropertySet().getDataForWay(way); 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 e80a3ab6499..0cc19363a0a 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java @@ -6,6 +6,7 @@ import java.time.format.DateTimeParseException; import java.util.Optional; import java.util.Set; +import java.util.function.Consumer; import org.opentripplanner.graph_builder.module.osm.StreetTraversalPermissionPair; import org.opentripplanner.street.model.StreetTraversalPermission; import org.opentripplanner.utils.time.DurationUtils; @@ -134,17 +135,8 @@ public boolean isEscalator() { return (isTag("highway", "steps") && isOneOfTags("conveying", ESCALATOR_CONVEYING_TAGS)); } - public Optional getDuration() { - var duration = getTag("duration"); - if (duration != null) { - try { - return Optional.of(DurationUtils.parseClockDuration(duration)); - } catch (DateTimeParseException e) { - // For malformed duration tags, just pretend they weren't there. - return Optional.empty(); - } - } - return Optional.empty(); + public Optional getDuration(Consumer errorHandler) { + return getTagAsDuration("duration", errorHandler); } public boolean isForwardEscalator() { 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 67f737e4c79..bb3e1056f2f 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java @@ -1,5 +1,9 @@ package org.opentripplanner.osm.model; +import java.time.Duration; +import java.time.LocalTime; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeParseException; import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -221,6 +225,68 @@ public OptionalInt getTagAsInt(String tag, Consumer errorHandler) { return OptionalInt.empty(); } + /** + * Parse an OSM duration tag, which is one of: + * mm + * hh:mm + * hh:mm:ss + * and where the leading value is not limited to any maximum. + * @param duration string in format mm, hh:mm, or hh:mm:ss + * @return Duration + * @throws DateTimeParseException on bad input + */ + public static Duration parseOsmDuration(String duration) { + int colonCount = (int) duration.chars().filter(ch -> ch == ':').count(); + if (colonCount <= 2) { + try { + int i, j; + long hours, minutes, seconds; + switch (colonCount) { + case 0: + minutes = Long.parseLong(duration); + if (minutes >= 0) { + return Duration.ofMinutes(minutes); + } + break; + case 1: + i = duration.indexOf(':'); + hours = Long.parseLong(duration.substring(0, i)); + minutes = Long.parseLong(duration.substring(i + 1)); + if (hours >= 0 && minutes >= 0 && minutes < 60) { + return Duration.ofHours(hours).plusMinutes(minutes); + } + break; + default: + //case 2: + i = duration.indexOf(':'); + j = duration.indexOf(':', i + 1); + hours = Long.parseLong(duration.substring(0, i)); + minutes = Long.parseLong(duration.substring(i + 1, j)); + seconds = Long.parseLong(duration.substring(j + 1)); + if (hours >= 0 && minutes >= 0 && minutes < 60 && seconds >= 0 && seconds < 60) { + return Duration.ofHours(hours).plusMinutes(minutes).plusSeconds(seconds); + } + break; + } + } catch (NumberFormatException e) { + // fallthrough + } + } + throw new DateTimeParseException("bad clock duration", duration, 0); + } + + public Optional getTagAsDuration(String tag, Consumer errorHandler) { + String value = getTag(tag); + if (value != null) { + try { + return Optional.of(parseOsmDuration(value)); + } catch (DateTimeParseException e) { + errorHandler.accept(value); + } + } + return Optional.empty(); + } + /** * Some tags are allowed to have values like 55, "true" or "false". *

diff --git a/application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java b/application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java deleted file mode 100644 index af4cbda39ef..00000000000 --- a/application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java +++ /dev/null @@ -1,74 +0,0 @@ -package org.opentripplanner.routing.api.request.preference; - -import java.io.Serializable; -import java.util.function.Consumer; - -public class EscalatorPreferences implements Serializable { - - public static final EscalatorPreferences DEFAULT = new EscalatorPreferences(); - - private final double horizontalSpeed; - - /* A quick internet search gives escalator speed range of 0.3-0.6 m/s and angle of 30 degrees. - * Using the angle of 30 degrees and a speed of 0.5 m/s gives a horizontal component - * of approx. 0.43 m/s */ - private static final double HORIZONTAL_SPEED = 0.45; - - private EscalatorPreferences() { - this.horizontalSpeed = HORIZONTAL_SPEED; - } - - private EscalatorPreferences(Builder builder) { - this.horizontalSpeed = builder.horizontalSpeed; - } - - public static Builder of() { - return DEFAULT.copyOf(); - } - - public Builder copyOf() { - return new Builder(this); - } - - public double horizontalSpeed() { - return horizontalSpeed; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - EscalatorPreferences that = (EscalatorPreferences) o; - return horizontalSpeed == that.horizontalSpeed; - } - - public static class Builder { - - private final EscalatorPreferences original; - private double horizontalSpeed; - - public Builder(EscalatorPreferences original) { - this.original = original; - this.horizontalSpeed = original.horizontalSpeed; - } - - public EscalatorPreferences original() { - return original; - } - - public Builder withHorizontalSpeed(double horizontalSpeed) { - this.horizontalSpeed = horizontalSpeed; - return this; - } - - public Builder apply(Consumer body) { - body.accept(this); - return this; - } - - public EscalatorPreferences build() { - var value = new EscalatorPreferences(this); - return original.equals(value) ? original : value; - } - } -} diff --git a/application/src/main/java/org/opentripplanner/routing/api/request/preference/StreetPreferences.java b/application/src/main/java/org/opentripplanner/routing/api/request/preference/StreetPreferences.java index e20ed82f9a5..bb526ceaa31 100644 --- a/application/src/main/java/org/opentripplanner/routing/api/request/preference/StreetPreferences.java +++ b/application/src/main/java/org/opentripplanner/routing/api/request/preference/StreetPreferences.java @@ -30,7 +30,6 @@ public final class StreetPreferences implements Serializable { private final double turnReluctance; private final DrivingDirection drivingDirection; private final ElevatorPreferences elevator; - private final EscalatorPreferences escalator; private final AccessEgressPreferences accessEgress; private final IntersectionTraversalModel intersectionTraversalModel; private final DurationForEnum maxDirectDuration; @@ -40,7 +39,6 @@ private StreetPreferences() { this.turnReluctance = 1.0; this.drivingDirection = DrivingDirection.RIGHT; this.elevator = ElevatorPreferences.DEFAULT; - this.escalator = EscalatorPreferences.DEFAULT; this.accessEgress = AccessEgressPreferences.DEFAULT; this.intersectionTraversalModel = IntersectionTraversalModel.SIMPLE; this.maxDirectDuration = durationForStreetModeOf(ofHours(4)); @@ -51,7 +49,6 @@ private StreetPreferences(Builder builder) { this.turnReluctance = Units.reluctance(builder.turnReluctance); this.drivingDirection = requireNonNull(builder.drivingDirection); this.elevator = requireNonNull(builder.elevator); - this.escalator = requireNonNull(builder.escalator); this.accessEgress = requireNonNull(builder.accessEgress); this.intersectionTraversalModel = requireNonNull(builder.intersectionTraversalModel); this.maxDirectDuration = requireNonNull(builder.maxDirectDuration); @@ -81,10 +78,6 @@ public ElevatorPreferences elevator() { return elevator; } - public EscalatorPreferences escalator() { - return escalator; - } - /** Preferences for access/egress routing */ public AccessEgressPreferences accessEgress() { return accessEgress; @@ -117,7 +110,6 @@ public boolean equals(Object o) { DoubleUtils.doubleEquals(that.turnReluctance, turnReluctance) && drivingDirection == that.drivingDirection && elevator.equals(that.elevator) && - escalator.equals(that.escalator) && routingTimeout.equals(that.routingTimeout) && intersectionTraversalModel == that.intersectionTraversalModel && maxDirectDuration.equals(that.maxDirectDuration) && @@ -146,7 +138,6 @@ public String toString() { .addEnum("drivingDirection", drivingDirection, DEFAULT.drivingDirection) .addDuration("routingTimeout", routingTimeout, DEFAULT.routingTimeout()) .addObj("elevator", elevator, DEFAULT.elevator) - .addObj("escalator", escalator, DEFAULT.escalator) .addObj( "intersectionTraversalModel", intersectionTraversalModel, @@ -163,7 +154,6 @@ public static class Builder { private double turnReluctance; private DrivingDirection drivingDirection; private ElevatorPreferences elevator; - private EscalatorPreferences escalator; private IntersectionTraversalModel intersectionTraversalModel; private DurationForEnum maxDirectDuration; private Duration routingTimeout; @@ -174,7 +164,6 @@ public Builder(StreetPreferences original) { this.turnReluctance = original.turnReluctance; this.drivingDirection = original.drivingDirection; this.elevator = original.elevator; - this.escalator = original.escalator; this.intersectionTraversalModel = original.intersectionTraversalModel; this.accessEgress = original.accessEgress; this.maxDirectDuration = original.maxDirectDuration; @@ -200,11 +189,6 @@ public Builder withElevator(Consumer body) { return this; } - public Builder withEscalator(Consumer body) { - this.escalator = escalator.copyOf().apply(body).build(); - return this; - } - public Builder withAccessEgress(Consumer body) { this.accessEgress = accessEgress.copyOf().apply(body).build(); return this; diff --git a/application/src/main/java/org/opentripplanner/routing/api/request/preference/WalkPreferences.java b/application/src/main/java/org/opentripplanner/routing/api/request/preference/WalkPreferences.java index 4a5969049ba..87dcc320c83 100644 --- a/application/src/main/java/org/opentripplanner/routing/api/request/preference/WalkPreferences.java +++ b/application/src/main/java/org/opentripplanner/routing/api/request/preference/WalkPreferences.java @@ -30,6 +30,7 @@ public final class WalkPreferences implements Serializable { private final double safetyFactor; private final double escalatorReluctance; + private final double escalatorSpeed; private WalkPreferences() { this.speed = 1.33; @@ -39,6 +40,7 @@ private WalkPreferences() { this.stairsTimeFactor = 3.0; this.safetyFactor = 1.0; this.escalatorReluctance = 1.5; + this.escalatorSpeed = 0.45; } private WalkPreferences(Builder builder) { @@ -49,6 +51,7 @@ private WalkPreferences(Builder builder) { this.stairsTimeFactor = Units.reluctance(builder.stairsTimeFactor); this.safetyFactor = Units.reluctance(builder.safetyFactor); this.escalatorReluctance = Units.reluctance(builder.escalatorReluctance); + this.escalatorSpeed = Units.speed(builder.escalatorSpeed); } public static Builder of() { @@ -108,6 +111,14 @@ public double safetyFactor() { return safetyFactor; } + public double escalatorReluctance() { + return escalatorReluctance; + } + + public double escalatorSpeed() { + return escalatorSpeed; + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -120,7 +131,8 @@ public boolean equals(Object o) { doubleEquals(that.stairsReluctance, stairsReluctance) && doubleEquals(that.stairsTimeFactor, stairsTimeFactor) && doubleEquals(that.safetyFactor, safetyFactor) && - doubleEquals(that.escalatorReluctance, escalatorReluctance) + doubleEquals(that.escalatorReluctance, escalatorReluctance) && + doubleEquals(that.escalatorSpeed, escalatorSpeed) ); } @@ -133,7 +145,8 @@ public int hashCode() { stairsReluctance, stairsTimeFactor, safetyFactor, - escalatorReluctance + escalatorReluctance, + escalatorSpeed ); } @@ -148,13 +161,10 @@ public String toString() { .addNum("stairsTimeFactor", stairsTimeFactor, DEFAULT.stairsTimeFactor) .addNum("safetyFactor", safetyFactor, DEFAULT.safetyFactor) .addNum("escalatorReluctance", escalatorReluctance, DEFAULT.escalatorReluctance) + .addNum("escalatorSpeed", escalatorSpeed, DEFAULT.escalatorSpeed) .toString(); } - public double escalatorReluctance() { - return escalatorReluctance; - } - public static class Builder { private final WalkPreferences original; @@ -166,6 +176,7 @@ public static class Builder { private double safetyFactor; private double escalatorReluctance; + private double escalatorSpeed; public Builder(WalkPreferences original) { this.original = original; @@ -176,6 +187,7 @@ public Builder(WalkPreferences original) { this.stairsTimeFactor = original.stairsTimeFactor; this.safetyFactor = original.safetyFactor; this.escalatorReluctance = original.escalatorReluctance; + this.escalatorSpeed = original.escalatorSpeed; } public WalkPreferences original() { @@ -251,6 +263,15 @@ public Builder withEscalatorReluctance(double escalatorReluctance) { return this; } + public double escalatorSpeed() { + return escalatorSpeed; + } + + public Builder withEscalatorSpeed(double escalatorSpeed) { + this.escalatorSpeed = escalatorSpeed; + return this; + } + public Builder apply(Consumer body) { body.accept(this); return this; diff --git a/application/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java b/application/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java index 9c344afc44c..9f0e0ba17b4 100644 --- a/application/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java +++ b/application/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java @@ -460,16 +460,6 @@ private static void mapStreetPreferences(NodeAdapter c, StreetPreferences.Builde .asInt(dftElevator.hopTime()) ); }) - .withEscalator(escalator -> { - var dftEscalator = dft.escalator(); - escalator.withHorizontalSpeed( - c - .of("escalatorSpeed") - .since(V2_7) - .summary("How fast does an escalator move horizontally?") - .asDouble(dftEscalator.horizontalSpeed()) - ); - }) .withAccessEgress(accessEgress -> { var dftAccessEgress = dft.accessEgress(); accessEgress @@ -828,6 +818,14 @@ private static void mapWalkPreferences(NodeAdapter root, WalkPreferences.Builder "A multiplier for how bad being in an escalator is compared to being in transit for equal lengths of time" ) .asDouble(dft.escalatorReluctance()) + ) + .withEscalatorSpeed( + c + .of("escalatorSpeed") + .since(V2_7) + .summary("How fast does an escalator move horizontally?") + .description("Horizontal speed of escalator in m/s.") + .asDouble(dft.escalatorSpeed()) ); } } diff --git a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java index 7dab48dc3e2..fdfd4ee44b4 100644 --- a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java +++ b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java @@ -31,7 +31,7 @@ public State[] traverse(State s0) { var s1 = s0.edit(this); double time; if (duration == null) { - time = getDistanceMeters() / s0.getPreferences().street().escalator().horizontalSpeed(); + time = getDistanceMeters() / s0.getPreferences().walk().escalatorSpeed(); } else { time = duration.toSeconds(); } @@ -52,6 +52,10 @@ public double getDistanceMeters() { return length; } + /** + * Parsed content of duration tag in OSM, if any. Not a calculated value. + * @return Duration, or empty + */ public Optional getDuration() { return Optional.ofNullable(duration); } 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 869e579e9aa..277cda7ac9b 100644 --- a/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java +++ b/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java @@ -7,6 +7,7 @@ import java.time.Duration; import java.util.Optional; +import java.util.function.Consumer; import org.junit.jupiter.api.Test; import org.opentripplanner.osm.wayproperty.specifier.WayTestData; @@ -176,13 +177,6 @@ void escalator() { escalator.addTag("conveying", "yes"); assertTrue(escalator.isEscalator()); - assertEquals(Optional.empty(), escalator.getDuration()); - - escalator.addTag("duration", "00:00:61"); - assertEquals(Optional.empty(), escalator.getDuration()); - escalator.addTag("duration", "00:01:01"); - assertEquals(Optional.of(Duration.ofSeconds(61)), escalator.getDuration()); - escalator.addTag("conveying", "whoknows?"); assertFalse(escalator.isEscalator()); } diff --git a/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java b/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java index a89f8612040..c379fc11b02 100644 --- a/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java +++ b/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java @@ -5,9 +5,11 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.time.Duration; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.OptionalInt; import java.util.Set; import org.junit.jupiter.api.Test; @@ -298,4 +300,25 @@ void parseIntOrBoolean(String value, OptionalInt expected) { var maybeInt = way.parseIntOrBoolean(key, i -> {}); assertEquals(expected, maybeInt); } + + private static List parseTagAsDurationCases() { + return List.of( + Arguments.of("00:11", Optional.of(Duration.ofMinutes(11))), + Arguments.of("11", Optional.of(Duration.ofMinutes(11))), + Arguments.of("1:22:33", Optional.of(Duration.ofHours(1).plusMinutes(22).plusSeconds(33))), + Arguments.of("22:60", Optional.empty()), + Arguments.of("10:61:40", Optional.empty()), + Arguments.of("10:59:60", Optional.empty()) + ); + } + + @ParameterizedTest + @MethodSource("parseTagAsDurationCases") + void parseTagAsDuration(String value, Optional expected) { + var way = new OsmWithTags(); + var key = "duration"; + way.addTag(key, value); + var duration = way.getTagAsDuration(key, i -> {}); + assertEquals(expected, duration); + } } diff --git a/application/src/test/resources/standalone/config/router-config.json b/application/src/test/resources/standalone/config/router-config.json index f1eac1cb41e..5539ec4de65 100644 --- a/application/src/test/resources/standalone/config/router-config.json +++ b/application/src/test/resources/standalone/config/router-config.json @@ -75,7 +75,8 @@ "reluctance": 4.0, "stairsReluctance": 1.65, "boardCost": 600, - "escalatorReluctance": 1.5 + "escalatorReluctance": 1.5, + "escalatorSpeed": 0.45 }, "waitReluctance": 1.0, "otherThanPreferredRoutesPenalty": 300, diff --git a/doc/user/RouteRequest.md b/doc/user/RouteRequest.md index c0c5d2bb590..c6cdef82c25 100644 --- a/doc/user/RouteRequest.md +++ b/doc/user/RouteRequest.md @@ -23,7 +23,6 @@ and in the [transferRequests in build-config.json](BuildConfiguration.md#transfe | elevatorBoardTime | `integer` | How long does it take to get on an elevator, on average. | *Optional* | `90` | 2.0 | | elevatorHopCost | `integer` | What is the cost of travelling one floor on an elevator? | *Optional* | `20` | 2.0 | | elevatorHopTime | `integer` | How long does it take to advance one floor on an elevator? | *Optional* | `20` | 2.0 | -| escalatorSpeed | `double` | How fast does an escalator move horizontally? | *Optional* | `0.45` | 2.7 | | geoidElevation | `boolean` | If true, the Graph's ellipsoidToGeoidDifference is applied to all elevations returned by this query. | *Optional* | `false` | 2.0 | | ignoreRealtimeUpdates | `boolean` | When true, real-time updates are ignored during this search. | *Optional* | `false` | 2.0 | | [intersectionTraversalModel](#rd_intersectionTraversalModel) | `enum` | The model that computes the costs of turns. | *Optional* | `"simple"` | 2.2 | @@ -157,6 +156,7 @@ and in the [transferRequests in build-config.json](BuildConfiguration.md#transfe | walk | `object` | Walking preferences. | *Optional* | | 2.5 | |    boardCost | `integer` | Prevents unnecessary transfers by adding a cost for boarding a vehicle. This is the cost that is used when boarding while walking. | *Optional* | `600` | 2.0 | |    escalatorReluctance | `double` | A multiplier for how bad being in an escalator is compared to being in transit for equal lengths of time | *Optional* | `1.5` | 2.4 | +|    [escalatorSpeed](#rd_walk_escalatorSpeed) | `double` | How fast does an escalator move horizontally? | *Optional* | `0.45` | 2.7 | |    [reluctance](#rd_walk_reluctance) | `double` | A multiplier for how bad walking is, compared to being in transit for equal lengths of time. | *Optional* | `2.0` | 2.0 | |    [safetyFactor](#rd_walk_safetyFactor) | `double` | Factor for how much the walk safety is considered in routing. | *Optional* | `1.0` | 2.2 | |    speed | `double` | The user's walking speed in meters/second. | *Optional* | `1.33` | 2.0 | @@ -1072,6 +1072,15 @@ The ids of the routes that incur an extra cost when being used. Format: `FeedId: How much cost is added is configured in `unpreferredCost`. +

escalatorSpeed

+ +**Since version:** `2.7` ∙ **Type:** `double` ∙ **Cardinality:** `Optional` ∙ **Default value:** `0.45` +**Path:** /routingDefaults/walk + +How fast does an escalator move horizontally? + +Horizontal speed of escalator in m/s. +

reluctance

**Since version:** `2.0` ∙ **Type:** `double` ∙ **Cardinality:** `Optional` ∙ **Default value:** `2.0` @@ -1215,7 +1224,8 @@ include stairs as a last result. "reluctance" : 4.0, "stairsReluctance" : 1.65, "boardCost" : 600, - "escalatorReluctance" : 1.5 + "escalatorReluctance" : 1.5, + "escalatorSpeed" : 0.45 }, "waitReluctance" : 1.0, "otherThanPreferredRoutesPenalty" : 300, diff --git a/doc/user/RouterConfiguration.md b/doc/user/RouterConfiguration.md index 6dbd1174397..043e4f2f485 100644 --- a/doc/user/RouterConfiguration.md +++ b/doc/user/RouterConfiguration.md @@ -528,7 +528,8 @@ Used to group requests when monitoring OTP. "reluctance" : 4.0, "stairsReluctance" : 1.65, "boardCost" : 600, - "escalatorReluctance" : 1.5 + "escalatorReluctance" : 1.5, + "escalatorSpeed" : 0.45 }, "waitReluctance" : 1.0, "otherThanPreferredRoutesPenalty" : 300, diff --git a/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java b/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java index e1c0b557855..78a6ae16885 100644 --- a/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java +++ b/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java @@ -124,49 +124,6 @@ public static Duration duration(String duration) { } } - /** - * Parse a duration string in format hh:mm:ss. - * @param duration string in format hh:mm:ss - * @return Duration - * @throws DateTimeParseException on bad input - */ - public static Duration parseClockDuration(String duration) { - int colonCount = (int) duration.chars().filter(ch -> ch == ':').count(); - if (colonCount <= 2) { - try { - int i, j; - long hours, minutes = 0, seconds = 0; - switch (colonCount) { - case 0: - hours = Long.parseLong(duration); - break; - case 1: - i = duration.indexOf(':'); - hours = Long.parseLong(duration.substring(0, i)); - minutes = Long.parseLong(duration.substring(i + 1)); - break; - default: - //case 2: - i = duration.indexOf(':'); - j = duration.indexOf(':', i + 1); - hours = Long.parseLong(duration.substring(0, i)); - minutes = Long.parseLong(duration.substring(i + 1, j)); - seconds = Long.parseLong(duration.substring(j + 1)); - break; - } - if (hours >= 0 && minutes >= 0 && minutes < 60 && seconds >= 0 && seconds < 60) { - return Duration - .ofHours(hours) - .plus(Duration.ofMinutes(minutes)) - .plus(Duration.ofSeconds(seconds)); - } - } catch (NumberFormatException e) { - // fallthrough - } - } - throw new DateTimeParseException("bad clock duration", duration, 0); - } - /** * This is used to parse a string which may be a number {@code NNNN}(number of seconds) or a * duration with format {@code NhNmNs}, where {@code N} is a decimal number and diff --git a/utils/src/test/java/org/opentripplanner/utils/time/DurationUtilsTest.java b/utils/src/test/java/org/opentripplanner/utils/time/DurationUtilsTest.java index 15fbd6e5027..ef2e0f50901 100644 --- a/utils/src/test/java/org/opentripplanner/utils/time/DurationUtilsTest.java +++ b/utils/src/test/java/org/opentripplanner/utils/time/DurationUtilsTest.java @@ -30,8 +30,6 @@ public class DurationUtilsTest { private final Duration D5m = Duration.ofMinutes(5); private final Duration D9s = Duration.ofSeconds(9); private final Duration D3d5m9s = D3d.plus(D5m).plus(D9s); - private final Duration D2h5m = D2h.plus(D5m); - private final Duration D2h5m9s = D2h.plus(D5m).plus(D9s); private final int I9h31m = durationSec(9, 31, 0); private final int I9h36m55s = durationSec(9, 36, 55); private final int I13h33m57s = durationSec(13, 33, 57); @@ -93,19 +91,6 @@ public void duration() { assertEquals(-D9s.toSeconds(), DurationUtils.duration("-9", ChronoUnit.SECONDS).toSeconds()); } - @Test - public void parseClockDuration() { - assertEquals(D2h, DurationUtils.parseClockDuration("2")); - assertEquals(D2h5m, DurationUtils.parseClockDuration("02:05")); - assertEquals(D2h5m9s, DurationUtils.parseClockDuration("02:05:09")); - assertThrows(DateTimeParseException.class, () -> DurationUtils.parseClockDuration("02:65:09")); - assertThrows( - DateTimeParseException.class, - () -> DurationUtils.parseClockDuration("02:05:09:00") - ); - assertThrows(DateTimeParseException.class, () -> DurationUtils.parseClockDuration("02:x5:09")); - } - @Test public void parseSecondsOrDuration() { assertEquals(D9s, DurationUtils.parseSecondsOrDuration("9s").orElseThrow()); From dccc6d3ddc1b2c23c6ee10f086fef37cdd3addab Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Thu, 28 Nov 2024 16:11:23 +0200 Subject: [PATCH 09/17] add tests for the osm duration tag cases where the leading unit is larger than localdate would allow --- .../java/org/opentripplanner/osm/model/OsmWithTagsTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java b/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java index c379fc11b02..a5dc168d3fc 100644 --- a/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java +++ b/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java @@ -306,6 +306,9 @@ private static List parseTagAsDurationCases() { Arguments.of("00:11", Optional.of(Duration.ofMinutes(11))), Arguments.of("11", Optional.of(Duration.ofMinutes(11))), Arguments.of("1:22:33", Optional.of(Duration.ofHours(1).plusMinutes(22).plusSeconds(33))), + Arguments.of("82", Optional.of(Duration.ofMinutes(82))), + Arguments.of("25:00", Optional.of(Duration.ofHours(25))), + Arguments.of("25:00:00", Optional.of(Duration.ofHours(25))), Arguments.of("22:60", Optional.empty()), Arguments.of("10:61:40", Optional.empty()), Arguments.of("10:59:60", Optional.empty()) From d345a03465f54c8ab912f1c5611eacb2d77341a9 Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Fri, 29 Nov 2024 09:45:11 +0200 Subject: [PATCH 10/17] explain the duration parser better, more tests --- .../osm/model/OsmWithTags.java | 24 +++++++++++++------ .../osm/model/OsmWithTagsTest.java | 6 ++++- 2 files changed, 22 insertions(+), 8 deletions(-) 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 bb3e1056f2f..f5ead1da4d5 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java @@ -1,8 +1,6 @@ package org.opentripplanner.osm.model; import java.time.Duration; -import java.time.LocalTime; -import java.time.format.DateTimeFormatter; import java.time.format.DateTimeParseException; import java.util.Arrays; import java.util.HashMap; @@ -236,34 +234,46 @@ public OptionalInt getTagAsInt(String tag, Consumer errorHandler) { * @throws DateTimeParseException on bad input */ public static Duration parseOsmDuration(String duration) { + // Unfortunately DateFormatParserBuilder doesn't quite do enough for this case. + // It has the capability for expressing optional parts, so it could express hh(:mm(:ss)?)? + // but it cannot express (hh:)?mm(:ss)? where the existence of (:ss) implies the existence + // of (hh:). Even if it did, it would not be able to handle the cases where hours are + // greater than 23 or (if there is no hours part at all) minutes are greater than 59, which + // are both allowed by the spec and exist in OSM data. Durations are not LocalTimes after + // all, in parsing a LocalTime it makes sense and is correct that hours cannot be more than + // 23 or minutes more than 59, but in durations if you have capped the largest unit, it is + // reasonable for the amount of the largest unit to be as large as it needs to be. int colonCount = (int) duration.chars().filter(ch -> ch == ':').count(); if (colonCount <= 2) { try { int i, j; long hours, minutes, seconds; + // The first :-separated element can be any width, and has no maximum. It still has + // to be non-negative. The following elements must be 2 characters wide, non-negative, + // and less than 60. switch (colonCount) { - case 0: + case 0: // case "m" minutes = Long.parseLong(duration); if (minutes >= 0) { return Duration.ofMinutes(minutes); } break; - case 1: + case 1: // case "h:mm" i = duration.indexOf(':'); hours = Long.parseLong(duration.substring(0, i)); minutes = Long.parseLong(duration.substring(i + 1)); - if (hours >= 0 && minutes >= 0 && minutes < 60) { + if (duration.length() - i == 3 && hours >= 0 && minutes >= 0 && minutes < 60) { return Duration.ofHours(hours).plusMinutes(minutes); } break; - default: + default: // case "h:mm:ss" //case 2: i = duration.indexOf(':'); j = duration.indexOf(':', i + 1); hours = Long.parseLong(duration.substring(0, i)); minutes = Long.parseLong(duration.substring(i + 1, j)); seconds = Long.parseLong(duration.substring(j + 1)); - if (hours >= 0 && minutes >= 0 && minutes < 60 && seconds >= 0 && seconds < 60) { + if (j - i == 3 && duration.length() - j == 3 && hours >= 0 && minutes >= 0 && minutes < 60 && seconds >= 0 && seconds < 60) { return Duration.ofHours(hours).plusMinutes(minutes).plusSeconds(seconds); } break; diff --git a/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java b/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java index a5dc168d3fc..3fe49ded403 100644 --- a/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java +++ b/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java @@ -311,7 +311,11 @@ private static List parseTagAsDurationCases() { Arguments.of("25:00:00", Optional.of(Duration.ofHours(25))), Arguments.of("22:60", Optional.empty()), Arguments.of("10:61:40", Optional.empty()), - Arguments.of("10:59:60", Optional.empty()) + Arguments.of("10:59:60", Optional.empty()), + Arguments.of("1:12:34", Optional.of(Duration.ofHours(1).plusMinutes(12).plusSeconds(34))), + Arguments.of("1:2:34", Optional.empty()), + Arguments.of("1:12:3", Optional.empty()), + Arguments.of("1:2", Optional.empty()) ); } From c161378b8cf478ecbcb387c978735d1077224dfc Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Fri, 29 Nov 2024 09:47:57 +0200 Subject: [PATCH 11/17] prettier reformatting --- .../org/opentripplanner/osm/model/OsmWithTags.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) 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 f5ead1da4d5..74d1ffd25f5 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java @@ -273,7 +273,15 @@ public static Duration parseOsmDuration(String duration) { hours = Long.parseLong(duration.substring(0, i)); minutes = Long.parseLong(duration.substring(i + 1, j)); seconds = Long.parseLong(duration.substring(j + 1)); - if (j - i == 3 && duration.length() - j == 3 && hours >= 0 && minutes >= 0 && minutes < 60 && seconds >= 0 && seconds < 60) { + if ( + j - i == 3 && + duration.length() - j == 3 && + hours >= 0 && + minutes >= 0 && + minutes < 60 && + seconds >= 0 && + seconds < 60 + ) { return Duration.ofHours(hours).plusMinutes(minutes).plusSeconds(seconds); } break; From 21e183f70d33b6a5f812ec688232667835713cc5 Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Mon, 2 Dec 2024 13:49:13 +0200 Subject: [PATCH 12/17] fix showing duration for escalator edges in debug client --- .../inspector/vector/edge/EdgePropertyMapper.java | 2 +- .../main/java/org/opentripplanner/utils/time/DurationUtils.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/inspector/vector/edge/EdgePropertyMapper.java b/application/src/main/java/org/opentripplanner/inspector/vector/edge/EdgePropertyMapper.java index 13956af99c4..83b677a62ce 100644 --- a/application/src/main/java/org/opentripplanner/inspector/vector/edge/EdgePropertyMapper.java +++ b/application/src/main/java/org/opentripplanner/inspector/vector/edge/EdgePropertyMapper.java @@ -24,7 +24,7 @@ protected Collection map(Edge input) { case StreetEdge e -> mapStreetEdge(e); case EscalatorEdge e -> List.of( kv("distance", e.getDistanceMeters()), - kv("duration", e.getDuration()) + kv("duration", e.getDuration().map(d -> d.toString()).orElse(null)) ); default -> List.of(); }; diff --git a/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java b/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java index 78a6ae16885..d73faecee03 100644 --- a/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java +++ b/utils/src/main/java/org/opentripplanner/utils/time/DurationUtils.java @@ -11,7 +11,6 @@ import java.util.Optional; import java.util.regex.Pattern; import java.util.stream.Collectors; -import org.opentripplanner.utils.lang.StringUtils; /** * This class extend the Java {@link Duration} with utility functionality to parse and convert From 6f5b5df790faef7e95c33677b8dac3501969767d Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Mon, 9 Dec 2024 12:45:54 +0200 Subject: [PATCH 13/17] put escalator preferences in a sub object in walk preferences --- .../module/osm/EscalatorProcessor.java | 6 +- .../org/opentripplanner/osm/model/OsmWay.java | 4 +- .../osm/model/OsmWithTags.java | 54 +++++---- .../preference/EscalatorPreferences.java | 109 ++++++++++++++++++ .../request/preference/WalkPreferences.java | 50 +++----- .../routerequest/RouteRequestConfig.java | 45 +++++--- .../street/model/edge/EscalatorEdge.java | 4 +- .../opentripplanner/osm/model/OsmWayTest.java | 5 - .../osm/model/OsmWithTagsTest.java | 2 +- .../preference/WalkPreferencesTest.java | 16 +-- .../street/model/edge/EscalatorEdgeTest.java | 4 +- .../standalone/config/router-config.json | 6 +- doc/user/RouteRequest.md | 29 ++--- doc/user/RouterConfiguration.md | 6 +- 14 files changed, 228 insertions(+), 112 deletions(-) create mode 100644 application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java diff --git a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java index c8f4e0e0042..40695e7c67c 100644 --- a/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java +++ b/application/src/main/java/org/opentripplanner/graph_builder/module/osm/EscalatorProcessor.java @@ -45,8 +45,8 @@ public void buildEscalatorEdge(OsmWay escalatorWay, double length) { issueStore.add( Issue.issue( "InvalidDuration", - "Duration for osm node %d is not a valid duration: '%s'; it's replaced with 'Optional.empty()' (unknown).", - escalatorWay.getId(), + "Duration for osm node {} is not a valid duration: '{}'; the value is ignored.", + escalatorWay.url(), v ) ) @@ -59,7 +59,7 @@ public void buildEscalatorEdge(OsmWay escalatorWay, double length) { Issue.issue( "InvalidDuration", "Duration for osm node {} makes implied speed {} be outside acceptable range.", - escalatorWay.getId(), + escalatorWay.url(), speed ) ); 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 0cc19363a0a..a620545f521 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWay.java @@ -3,13 +3,11 @@ import gnu.trove.list.TLongList; import gnu.trove.list.array.TLongArrayList; import java.time.Duration; -import java.time.format.DateTimeParseException; import java.util.Optional; import java.util.Set; import java.util.function.Consumer; import org.opentripplanner.graph_builder.module.osm.StreetTraversalPermissionPair; import org.opentripplanner.street.model.StreetTraversalPermission; -import org.opentripplanner.utils.time.DurationUtils; public class OsmWay extends OsmWithTags { @@ -136,7 +134,7 @@ public boolean isEscalator() { } public Optional getDuration(Consumer errorHandler) { - return getTagAsDuration("duration", errorHandler); + return getTagValueAsDuration("duration", errorHandler); } public boolean isForwardEscalator() { 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 74d1ffd25f5..ab50fb0ec1b 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java @@ -229,36 +229,43 @@ public OptionalInt getTagAsInt(String tag, Consumer errorHandler) { * hh:mm * hh:mm:ss * and where the leading value is not limited to any maximum. + * See OSM wiki definition + * of duration. + * * @param duration string in format mm, hh:mm, or hh:mm:ss * @return Duration * @throws DateTimeParseException on bad input */ public static Duration parseOsmDuration(String duration) { - // Unfortunately DateFormatParserBuilder doesn't quite do enough for this case. - // It has the capability for expressing optional parts, so it could express hh(:mm(:ss)?)? - // but it cannot express (hh:)?mm(:ss)? where the existence of (:ss) implies the existence - // of (hh:). Even if it did, it would not be able to handle the cases where hours are - // greater than 23 or (if there is no hours part at all) minutes are greater than 59, which - // are both allowed by the spec and exist in OSM data. Durations are not LocalTimes after - // all, in parsing a LocalTime it makes sense and is correct that hours cannot be more than - // 23 or minutes more than 59, but in durations if you have capped the largest unit, it is - // reasonable for the amount of the largest unit to be as large as it needs to be. + /* + * Unfortunately DateFormatParserBuilder doesn't quite do enough for this case. + * It has the capability for expressing optional parts, so it could express hh(:mm(:ss)?)? + * but it cannot express (hh:)?mm(:ss)? where the existence of (:ss) implies the existence + * of (hh:). Even if it did, it would not be able to handle the cases where hours are + * greater than 23 or (if there is no hours part at all) minutes are greater than 59, which + * are both allowed by the spec and exist in OSM data. Durations are not LocalTimes after + * all, in parsing a LocalTime it makes sense and is correct that hours cannot be more than + * 23 or minutes more than 59, but in durations if you have capped the largest unit, it is + * reasonable for the amount of the largest unit to be as large as it needs to be. + */ int colonCount = (int) duration.chars().filter(ch -> ch == ':').count(); if (colonCount <= 2) { try { int i, j; long hours, minutes, seconds; - // The first :-separated element can be any width, and has no maximum. It still has - // to be non-negative. The following elements must be 2 characters wide, non-negative, - // and less than 60. + /* + * The first :-separated element can be any width, and has no maximum. It still has + * to be non-negative. The following elements must be 2 characters wide, non-negative, + * and less than 60. + */ switch (colonCount) { - case 0: // case "m" + case 0: /* case "m" */ minutes = Long.parseLong(duration); if (minutes >= 0) { return Duration.ofMinutes(minutes); } break; - case 1: // case "h:mm" + case 1: /* case "h:mm" */ i = duration.indexOf(':'); hours = Long.parseLong(duration.substring(0, i)); minutes = Long.parseLong(duration.substring(i + 1)); @@ -266,8 +273,7 @@ public static Duration parseOsmDuration(String duration) { return Duration.ofHours(hours).plusMinutes(minutes); } break; - default: // case "h:mm:ss" - //case 2: + default: /* case "h:mm:ss" */ i = duration.indexOf(':'); j = duration.indexOf(':', i + 1); hours = Long.parseLong(duration.substring(0, i)); @@ -287,14 +293,22 @@ public static Duration parseOsmDuration(String duration) { break; } } catch (NumberFormatException e) { - // fallthrough + /* fallthrough */ } } - throw new DateTimeParseException("bad clock duration", duration, 0); + throw new DateTimeParseException("Bad OSM duration", duration, 0); } - public Optional getTagAsDuration(String tag, Consumer errorHandler) { - String value = getTag(tag); + /** + * Gets a tag's value, assumes it is an OSM wiki spesified duration, parses and returns it. + * If parsing fails, calls the error handler. + * + * @param key + * @param errorHandler + * @return parsed Duration, or empty + */ + public Optional getTagValueAsDuration(String key, Consumer errorHandler) { + String value = getTag(key); if (value != null) { try { return Optional.of(parseOsmDuration(value)); diff --git a/application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java b/application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java new file mode 100644 index 00000000000..9a770d99fda --- /dev/null +++ b/application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java @@ -0,0 +1,109 @@ +package org.opentripplanner.routing.api.request.preference; + +import static org.opentripplanner.utils.lang.DoubleUtils.doubleEquals; + +import java.io.Serializable; +import java.util.Objects; +import java.util.function.Consumer; +import org.opentripplanner.utils.tostring.ToStringBuilder; + +public class EscalatorPreferences implements Serializable { + + public static final EscalatorPreferences DEFAULT = new EscalatorPreferences(); + + private final double reluctance; + private final double speed; + + private EscalatorPreferences() { + this.reluctance = 1.5; + this.speed = 0.45; + } + + private EscalatorPreferences(Builder builder) { + reluctance = builder.reluctance; + speed = builder.speed; + } + + public static Builder of() { + return new Builder(DEFAULT); + } + + public Builder copyOf() { + return new Builder(this); + } + + public double reluctance() { + return reluctance; + } + + public double speed() { + return speed; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + EscalatorPreferences that = (EscalatorPreferences) o; + return (doubleEquals(that.reluctance, reluctance) && doubleEquals(that.speed, speed)); + } + + @Override + public int hashCode() { + return Objects.hash(speed, reluctance); + } + + @Override + public String toString() { + return ToStringBuilder + .of(EscalatorPreferences.class) + .addNum("speed", speed, DEFAULT.speed) + .addNum("reluctance", reluctance, DEFAULT.reluctance) + .toString(); + } + + public static class Builder { + + private final EscalatorPreferences original; + private double reluctance; + private double speed; + + public Builder(EscalatorPreferences original) { + this.original = original; + this.reluctance = original.reluctance; + this.speed = original.speed; + } + + public EscalatorPreferences original() { + return original; + } + + public double speed() { + return speed; + } + + public Builder withSpeed(double speed) { + this.speed = speed; + return this; + } + + public double reluctance() { + return reluctance; + } + + public Builder withReluctance(double reluctance) { + this.reluctance = reluctance; + return this; + } + + public Builder apply(Consumer body) { + body.accept(this); + return this; + } + + public EscalatorPreferences build() { + var newObj = new EscalatorPreferences(this); + return original.equals(newObj) ? original : newObj; + } + } +} diff --git a/application/src/main/java/org/opentripplanner/routing/api/request/preference/WalkPreferences.java b/application/src/main/java/org/opentripplanner/routing/api/request/preference/WalkPreferences.java index 87dcc320c83..5bc0a120d32 100644 --- a/application/src/main/java/org/opentripplanner/routing/api/request/preference/WalkPreferences.java +++ b/application/src/main/java/org/opentripplanner/routing/api/request/preference/WalkPreferences.java @@ -1,6 +1,7 @@ package org.opentripplanner.routing.api.request.preference; import static org.opentripplanner.utils.lang.DoubleUtils.doubleEquals; +import static org.opentripplanner.utils.lang.ObjectUtils.ifNotNull; import java.io.Serializable; import java.util.Objects; @@ -29,8 +30,7 @@ public final class WalkPreferences implements Serializable { private final double stairsTimeFactor; private final double safetyFactor; - private final double escalatorReluctance; - private final double escalatorSpeed; + private final EscalatorPreferences escalator; private WalkPreferences() { this.speed = 1.33; @@ -39,8 +39,7 @@ private WalkPreferences() { this.stairsReluctance = 2.0; this.stairsTimeFactor = 3.0; this.safetyFactor = 1.0; - this.escalatorReluctance = 1.5; - this.escalatorSpeed = 0.45; + this.escalator = EscalatorPreferences.DEFAULT; } private WalkPreferences(Builder builder) { @@ -50,8 +49,7 @@ private WalkPreferences(Builder builder) { this.stairsReluctance = Units.reluctance(builder.stairsReluctance); this.stairsTimeFactor = Units.reluctance(builder.stairsTimeFactor); this.safetyFactor = Units.reluctance(builder.safetyFactor); - this.escalatorReluctance = Units.reluctance(builder.escalatorReluctance); - this.escalatorSpeed = Units.speed(builder.escalatorSpeed); + this.escalator = builder.escalator; } public static Builder of() { @@ -111,12 +109,8 @@ public double safetyFactor() { return safetyFactor; } - public double escalatorReluctance() { - return escalatorReluctance; - } - - public double escalatorSpeed() { - return escalatorSpeed; + public EscalatorPreferences escalator() { + return escalator; } @Override @@ -131,8 +125,7 @@ public boolean equals(Object o) { doubleEquals(that.stairsReluctance, stairsReluctance) && doubleEquals(that.stairsTimeFactor, stairsTimeFactor) && doubleEquals(that.safetyFactor, safetyFactor) && - doubleEquals(that.escalatorReluctance, escalatorReluctance) && - doubleEquals(that.escalatorSpeed, escalatorSpeed) + escalator.equals(that.escalator) ); } @@ -145,8 +138,7 @@ public int hashCode() { stairsReluctance, stairsTimeFactor, safetyFactor, - escalatorReluctance, - escalatorSpeed + escalator ); } @@ -160,8 +152,7 @@ public String toString() { .addNum("stairsReluctance", stairsReluctance, DEFAULT.stairsReluctance) .addNum("stairsTimeFactor", stairsTimeFactor, DEFAULT.stairsTimeFactor) .addNum("safetyFactor", safetyFactor, DEFAULT.safetyFactor) - .addNum("escalatorReluctance", escalatorReluctance, DEFAULT.escalatorReluctance) - .addNum("escalatorSpeed", escalatorSpeed, DEFAULT.escalatorSpeed) + .addObj("escalator", escalator, DEFAULT.escalator) .toString(); } @@ -175,8 +166,7 @@ public static class Builder { private double stairsTimeFactor; private double safetyFactor; - private double escalatorReluctance; - private double escalatorSpeed; + private EscalatorPreferences escalator; public Builder(WalkPreferences original) { this.original = original; @@ -186,8 +176,7 @@ public Builder(WalkPreferences original) { this.stairsReluctance = original.stairsReluctance; this.stairsTimeFactor = original.stairsTimeFactor; this.safetyFactor = original.safetyFactor; - this.escalatorReluctance = original.escalatorReluctance; - this.escalatorSpeed = original.escalatorSpeed; + this.escalator = original.escalator; } public WalkPreferences original() { @@ -254,21 +243,12 @@ public Builder withSafetyFactor(double safetyFactor) { return this; } - public double escalatorReluctance() { - return escalatorReluctance; - } - - public Builder withEscalatorReluctance(double escalatorReluctance) { - this.escalatorReluctance = escalatorReluctance; - return this; - } - - public double escalatorSpeed() { - return escalatorSpeed; + public EscalatorPreferences escalator() { + return escalator; } - public Builder withEscalatorSpeed(double escalatorSpeed) { - this.escalatorSpeed = escalatorSpeed; + public Builder withEscalator(Consumer body) { + this.escalator = ifNotNull(this.escalator, original.escalator).copyOf().apply(body).build(); return this; } diff --git a/application/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java b/application/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java index 9f0e0ba17b4..454ab29a68c 100644 --- a/application/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java +++ b/application/src/main/java/org/opentripplanner/standalone/config/routerequest/RouteRequestConfig.java @@ -26,6 +26,7 @@ import org.opentripplanner.routing.api.request.preference.AccessEgressPreferences; import org.opentripplanner.routing.api.request.preference.BikePreferences; import org.opentripplanner.routing.api.request.preference.CarPreferences; +import org.opentripplanner.routing.api.request.preference.EscalatorPreferences; import org.opentripplanner.routing.api.request.preference.RoutingPreferences; import org.opentripplanner.routing.api.request.preference.ScooterPreferences; import org.opentripplanner.routing.api.request.preference.StreetPreferences; @@ -737,6 +738,32 @@ private static void mapSystemPreferences(NodeAdapter c, SystemPreferences.Builde } } + private static void mapEscalatorPreferences( + NodeAdapter root, + EscalatorPreferences.Builder escalator + ) { + var dft = escalator.original(); + NodeAdapter c = root.of("escalator").since(V2_7).summary("Escalator preferences.").asObject(); + escalator + .withReluctance( + c + .of("reluctance") + .since(V2_4) + .summary( + "A multiplier for how bad being in an escalator is compared to being in transit for equal lengths of time" + ) + .asDouble(dft.reluctance()) + ) + .withSpeed( + c + .of("speed") + .since(V2_7) + .summary("How fast does an escalator move horizontally?") + .description("Horizontal speed of escalator in m/s.") + .asDouble(dft.speed()) + ); + } + private static void mapWalkPreferences(NodeAdapter root, WalkPreferences.Builder walk) { var dft = walk.original(); NodeAdapter c = root.of("walk").since(V2_5).summary("Walking preferences.").asObject(); @@ -810,22 +837,6 @@ private static void mapWalkPreferences(NodeAdapter root, WalkPreferences.Builder ) .asDouble(dft.safetyFactor()) ) - .withEscalatorReluctance( - c - .of("escalatorReluctance") - .since(V2_4) - .summary( - "A multiplier for how bad being in an escalator is compared to being in transit for equal lengths of time" - ) - .asDouble(dft.escalatorReluctance()) - ) - .withEscalatorSpeed( - c - .of("escalatorSpeed") - .since(V2_7) - .summary("How fast does an escalator move horizontally?") - .description("Horizontal speed of escalator in m/s.") - .asDouble(dft.escalatorSpeed()) - ); + .withEscalator(escalator -> mapEscalatorPreferences(c, escalator)); } } diff --git a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java index fdfd4ee44b4..d44b67568d6 100644 --- a/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java +++ b/application/src/main/java/org/opentripplanner/street/model/edge/EscalatorEdge.java @@ -31,11 +31,11 @@ public State[] traverse(State s0) { var s1 = s0.edit(this); double time; if (duration == null) { - time = getDistanceMeters() / s0.getPreferences().walk().escalatorSpeed(); + time = getDistanceMeters() / s0.getPreferences().walk().escalator().speed(); } else { time = duration.toSeconds(); } - s1.incrementWeight(s0.getPreferences().walk().escalatorReluctance() * time); + s1.incrementWeight(s0.getPreferences().walk().escalator().reluctance() * time); s1.incrementTimeInSeconds((int) Math.round(time)); s1.incrementWalkDistance(getDistanceMeters()); return s1.makeStateArray(); 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 277cda7ac9b..9ac9457a9ec 100644 --- a/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java +++ b/application/src/test/java/org/opentripplanner/osm/model/OsmWayTest.java @@ -1,13 +1,8 @@ package org.opentripplanner.osm.model; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.time.Duration; -import java.util.Optional; -import java.util.function.Consumer; import org.junit.jupiter.api.Test; import org.opentripplanner.osm.wayproperty.specifier.WayTestData; diff --git a/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java b/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java index 3fe49ded403..84b74b8f655 100644 --- a/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java +++ b/application/src/test/java/org/opentripplanner/osm/model/OsmWithTagsTest.java @@ -325,7 +325,7 @@ void parseTagAsDuration(String value, Optional expected) { var way = new OsmWithTags(); var key = "duration"; way.addTag(key, value); - var duration = way.getTagAsDuration(key, i -> {}); + var duration = way.getTagValueAsDuration(key, i -> {}); assertEquals(expected, duration); } } diff --git a/application/src/test/java/org/opentripplanner/routing/api/request/preference/WalkPreferencesTest.java b/application/src/test/java/org/opentripplanner/routing/api/request/preference/WalkPreferencesTest.java index a61a0558bb1..785b130ca7a 100644 --- a/application/src/test/java/org/opentripplanner/routing/api/request/preference/WalkPreferencesTest.java +++ b/application/src/test/java/org/opentripplanner/routing/api/request/preference/WalkPreferencesTest.java @@ -88,7 +88,7 @@ void testEqualsAndHashCodeWithNewlyConstructedPreferences() { .withReluctance(sameReluctance) .withStairsReluctance(sameStairsReluctance) .withSafetyFactor(sameSafetyFactor) - .withEscalatorReluctance(sameEscalatorReluctance) + .withEscalator(escalator -> escalator.withReluctance(sameEscalatorReluctance)) .withBoardCost(sameBoardCost) .build(); var secondEqual = WalkPreferences @@ -97,7 +97,7 @@ void testEqualsAndHashCodeWithNewlyConstructedPreferences() { .withReluctance(sameReluctance) .withStairsReluctance(sameStairsReluctance) .withSafetyFactor(sameSafetyFactor) - .withEscalatorReluctance(sameEscalatorReluctance) + .withEscalator(escalator -> escalator.withReluctance(sameEscalatorReluctance)) .withBoardCost(sameBoardCost) .build(); assertEqualsAndHashCode(firstEqual, secondEqual); @@ -110,7 +110,7 @@ void testEqualsAndHashCodeWithNewlyConstructedPreferences() { .withReluctance(sameReluctance) .withStairsReluctance(sameStairsReluctance) .withSafetyFactor(sameSafetyFactor) - .withEscalatorReluctance(sameEscalatorReluctance) + .withEscalator(escalator -> escalator.withReluctance(sameEscalatorReluctance)) .withBoardCost(sameBoardCost) .build(); assertNotEqualsAndHashCode(firstEqual, differentSpeedPreferences); @@ -123,7 +123,7 @@ void testEqualsAndHashCodeWithNewlyConstructedPreferences() { .withReluctance(notSameReluctance) .withStairsReluctance(sameStairsReluctance) .withSafetyFactor(sameSafetyFactor) - .withEscalatorReluctance(sameEscalatorReluctance) + .withEscalator(escalator -> escalator.withReluctance(sameEscalatorReluctance)) .withBoardCost(sameBoardCost) .build(); assertNotEqualsAndHashCode(firstEqual, differentReluctancePreferences); @@ -136,7 +136,7 @@ void testEqualsAndHashCodeWithNewlyConstructedPreferences() { .withReluctance(sameReluctance) .withStairsReluctance(notSameStairsReluctance) .withSafetyFactor(sameSafetyFactor) - .withEscalatorReluctance(sameEscalatorReluctance) + .withEscalator(escalator -> escalator.withReluctance(sameEscalatorReluctance)) .withBoardCost(sameBoardCost) .build(); assertNotEqualsAndHashCode(firstEqual, differentStairsReluctancePreferences); @@ -149,7 +149,7 @@ void testEqualsAndHashCodeWithNewlyConstructedPreferences() { .withReluctance(sameReluctance) .withStairsReluctance(sameStairsReluctance) .withSafetyFactor(notSameSafetyFactor) - .withEscalatorReluctance(sameEscalatorReluctance) + .withEscalator(escalator -> escalator.withReluctance(sameEscalatorReluctance)) .withBoardCost(sameBoardCost) .build(); assertNotEqualsAndHashCode(firstEqual, differentSafetyFactorPreferences); @@ -162,7 +162,7 @@ void testEqualsAndHashCodeWithNewlyConstructedPreferences() { .withReluctance(sameReluctance) .withStairsReluctance(sameStairsReluctance) .withSafetyFactor(sameSafetyFactor) - .withEscalatorReluctance(notSameEscalatorReluctance) + .withEscalator(escalator -> escalator.withReluctance(notSameEscalatorReluctance)) .withBoardCost(sameBoardCost) .build(); assertNotEqualsAndHashCode(firstEqual, differentEscalatorReluctancePreferences); @@ -175,7 +175,7 @@ void testEqualsAndHashCodeWithNewlyConstructedPreferences() { .withReluctance(sameReluctance) .withStairsReluctance(sameStairsReluctance) .withSafetyFactor(sameSafetyFactor) - .withEscalatorReluctance(sameEscalatorReluctance) + .withEscalator(escalator -> escalator.withReluctance(sameEscalatorReluctance)) .withBoardCost(notSameBoardCost) .build(); assertNotEqualsAndHashCode(firstEqual, differentBoardCostPreferences); diff --git a/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java b/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java index 25199e373a0..23ae7a567f8 100644 --- a/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java +++ b/application/src/test/java/org/opentripplanner/street/model/edge/EscalatorEdgeTest.java @@ -31,7 +31,9 @@ void testWalking(double escalatorReluctance, double expectedWeight) { var edge = EscalatorEdge.createEscalatorEdge(from, to, 45, null); var req = StreetSearchRequest .of() - .withPreferences(p -> p.withWalk(w -> w.withEscalatorReluctance(escalatorReluctance))) + .withPreferences(p -> + p.withWalk(w -> w.withEscalator(escalator -> escalator.withReluctance(escalatorReluctance))) + ) .withMode(StreetMode.WALK); var res = edge.traverse(new State(from, req.build()))[0]; diff --git a/application/src/test/resources/standalone/config/router-config.json b/application/src/test/resources/standalone/config/router-config.json index 5539ec4de65..e3f5b0caad2 100644 --- a/application/src/test/resources/standalone/config/router-config.json +++ b/application/src/test/resources/standalone/config/router-config.json @@ -75,8 +75,10 @@ "reluctance": 4.0, "stairsReluctance": 1.65, "boardCost": 600, - "escalatorReluctance": 1.5, - "escalatorSpeed": 0.45 + "escalator": { + "reluctance": 1.5, + "speed": 0.45 + } }, "waitReluctance": 1.0, "otherThanPreferredRoutesPenalty": 300, diff --git a/doc/user/RouteRequest.md b/doc/user/RouteRequest.md index c6cdef82c25..5b428ff9175 100644 --- a/doc/user/RouteRequest.md +++ b/doc/user/RouteRequest.md @@ -155,13 +155,14 @@ and in the [transferRequests in build-config.json](BuildConfiguration.md#transfe |    [routes](#rd_unpreferred_routes) | `feed-scoped-id[]` | The ids of the routes that incur an extra cost when being used. Format: `FeedId:RouteId` | *Optional* | | 2.2 | | walk | `object` | Walking preferences. | *Optional* | | 2.5 | |    boardCost | `integer` | Prevents unnecessary transfers by adding a cost for boarding a vehicle. This is the cost that is used when boarding while walking. | *Optional* | `600` | 2.0 | -|    escalatorReluctance | `double` | A multiplier for how bad being in an escalator is compared to being in transit for equal lengths of time | *Optional* | `1.5` | 2.4 | -|    [escalatorSpeed](#rd_walk_escalatorSpeed) | `double` | How fast does an escalator move horizontally? | *Optional* | `0.45` | 2.7 | |    [reluctance](#rd_walk_reluctance) | `double` | A multiplier for how bad walking is, compared to being in transit for equal lengths of time. | *Optional* | `2.0` | 2.0 | |    [safetyFactor](#rd_walk_safetyFactor) | `double` | Factor for how much the walk safety is considered in routing. | *Optional* | `1.0` | 2.2 | |    speed | `double` | The user's walking speed in meters/second. | *Optional* | `1.33` | 2.0 | |    stairsReluctance | `double` | Used instead of walkReluctance for stairs. | *Optional* | `2.0` | 2.0 | |    [stairsTimeFactor](#rd_walk_stairsTimeFactor) | `double` | How much more time does it take to walk a flight of stairs compared to walking a similar horizontal length. | *Optional* | `3.0` | 2.1 | +|    escalator | `object` | Escalator preferences. | *Optional* | | 2.7 | +|       reluctance | `double` | A multiplier for how bad being in an escalator is compared to being in transit for equal lengths of time | *Optional* | `1.5` | 2.4 | +|       [speed](#rd_walk_escalator_speed) | `double` | How fast does an escalator move horizontally? | *Optional* | `0.45` | 2.7 | | wheelchairAccessibility | `object` | See [Wheelchair Accessibility](Accessibility.md) | *Optional* | | 2.2 | |    enabled | `boolean` | Enable wheelchair accessibility. | *Optional* | `false` | 2.0 | |    inaccessibleStreetReluctance | `double` | The factor to multiply the cost of traversing a street edge that is not wheelchair-accessible. | *Optional* | `25.0` | 2.2 | @@ -1072,15 +1073,6 @@ The ids of the routes that incur an extra cost when being used. Format: `FeedId: How much cost is added is configured in `unpreferredCost`. -

escalatorSpeed

- -**Since version:** `2.7` ∙ **Type:** `double` ∙ **Cardinality:** `Optional` ∙ **Default value:** `0.45` -**Path:** /routingDefaults/walk - -How fast does an escalator move horizontally? - -Horizontal speed of escalator in m/s. -

reluctance

**Since version:** `2.0` ∙ **Type:** `double` ∙ **Cardinality:** `Optional` ∙ **Default value:** `2.0` @@ -1115,6 +1107,15 @@ Default value is based on: Fujiyama, T., & Tyler, N. (2010). Predicting the walk speed of pedestrians on stairs. Transportation Planning and Technology, 33(2), 177–202. +

speed

+ +**Since version:** `2.7` ∙ **Type:** `double` ∙ **Cardinality:** `Optional` ∙ **Default value:** `0.45` +**Path:** /routingDefaults/walk/escalator + +How fast does an escalator move horizontally? + +Horizontal speed of escalator in m/s. +

maxSlope

**Since version:** `2.0` ∙ **Type:** `double` ∙ **Cardinality:** `Optional` ∙ **Default value:** `0.083` @@ -1224,8 +1225,10 @@ include stairs as a last result. "reluctance" : 4.0, "stairsReluctance" : 1.65, "boardCost" : 600, - "escalatorReluctance" : 1.5, - "escalatorSpeed" : 0.45 + "escalator" : { + "reluctance" : 1.5, + "speed" : 0.45 + } }, "waitReluctance" : 1.0, "otherThanPreferredRoutesPenalty" : 300, diff --git a/doc/user/RouterConfiguration.md b/doc/user/RouterConfiguration.md index 043e4f2f485..7dae97fd74c 100644 --- a/doc/user/RouterConfiguration.md +++ b/doc/user/RouterConfiguration.md @@ -528,8 +528,10 @@ Used to group requests when monitoring OTP. "reluctance" : 4.0, "stairsReluctance" : 1.65, "boardCost" : 600, - "escalatorReluctance" : 1.5, - "escalatorSpeed" : 0.45 + "escalator" : { + "reluctance" : 1.5, + "speed" : 0.45 + } }, "waitReluctance" : 1.0, "otherThanPreferredRoutesPenalty" : 300, From 1e64a995b777320e15a22e044eded8d50645fe12 Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Mon, 9 Dec 2024 13:19:02 +0200 Subject: [PATCH 14/17] prettier wanted less spaces --- .../java/org/opentripplanner/osm/model/OsmWithTags.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 ab50fb0ec1b..0c44d892524 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java @@ -259,13 +259,13 @@ public static Duration parseOsmDuration(String duration) { * and less than 60. */ switch (colonCount) { - case 0: /* case "m" */ + case 0:/* case "m" */ minutes = Long.parseLong(duration); if (minutes >= 0) { return Duration.ofMinutes(minutes); } break; - case 1: /* case "h:mm" */ + case 1:/* case "h:mm" */ i = duration.indexOf(':'); hours = Long.parseLong(duration.substring(0, i)); minutes = Long.parseLong(duration.substring(i + 1)); @@ -273,7 +273,7 @@ public static Duration parseOsmDuration(String duration) { return Duration.ofHours(hours).plusMinutes(minutes); } break; - default: /* case "h:mm:ss" */ + default:/* case "h:mm:ss" */ i = duration.indexOf(':'); j = duration.indexOf(':', i + 1); hours = Long.parseLong(duration.substring(0, i)); From 9c8e928399d3e82c58816d71760df9b1640b154f Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Mon, 9 Dec 2024 13:43:30 +0200 Subject: [PATCH 15/17] comment formatting --- .../osm/model/OsmWithTags.java | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) 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 0c44d892524..d6ed30be092 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java @@ -237,35 +237,33 @@ public OptionalInt getTagAsInt(String tag, Consumer errorHandler) { * @throws DateTimeParseException on bad input */ public static Duration parseOsmDuration(String duration) { - /* - * Unfortunately DateFormatParserBuilder doesn't quite do enough for this case. - * It has the capability for expressing optional parts, so it could express hh(:mm(:ss)?)? - * but it cannot express (hh:)?mm(:ss)? where the existence of (:ss) implies the existence - * of (hh:). Even if it did, it would not be able to handle the cases where hours are - * greater than 23 or (if there is no hours part at all) minutes are greater than 59, which - * are both allowed by the spec and exist in OSM data. Durations are not LocalTimes after - * all, in parsing a LocalTime it makes sense and is correct that hours cannot be more than - * 23 or minutes more than 59, but in durations if you have capped the largest unit, it is - * reasonable for the amount of the largest unit to be as large as it needs to be. - */ + // Unfortunately DateFormatParserBuilder doesn't quite do enough for this case. + // It has the capability for expressing optional parts, so it could express hh(:mm(:ss)?)? + // but it cannot express (hh:)?mm(:ss)? where the existence of (:ss) implies the existence + // of (hh:). Even if it did, it would not be able to handle the cases where hours are + // greater than 23 or (if there is no hours part at all) minutes are greater than 59, which + // are both allowed by the spec and exist in OSM data. Durations are not LocalTimes after + // all, in parsing a LocalTime it makes sense and is correct that hours cannot be more than + // 23 or minutes more than 59, but in durations if you have capped the largest unit, it is + // reasonable for the amount of the largest unit to be as large as it needs to be. int colonCount = (int) duration.chars().filter(ch -> ch == ':').count(); if (colonCount <= 2) { try { int i, j; long hours, minutes, seconds; - /* - * The first :-separated element can be any width, and has no maximum. It still has - * to be non-negative. The following elements must be 2 characters wide, non-negative, - * and less than 60. - */ + // The first :-separated element can be any width, and has no maximum. It still has + // to be non-negative. The following elements must be 2 characters wide, non-negative, + // and less than 60. switch (colonCount) { - case 0:/* case "m" */ + // case "m" + case 0: minutes = Long.parseLong(duration); if (minutes >= 0) { return Duration.ofMinutes(minutes); } break; - case 1:/* case "h:mm" */ + // case "h:mm" + case 1: i = duration.indexOf(':'); hours = Long.parseLong(duration.substring(0, i)); minutes = Long.parseLong(duration.substring(i + 1)); @@ -273,7 +271,8 @@ public static Duration parseOsmDuration(String duration) { return Duration.ofHours(hours).plusMinutes(minutes); } break; - default:/* case "h:mm:ss" */ + // case "h:mm:ss" + default: i = duration.indexOf(':'); j = duration.indexOf(':', i + 1); hours = Long.parseLong(duration.substring(0, i)); @@ -293,7 +292,7 @@ public static Duration parseOsmDuration(String duration) { break; } } catch (NumberFormatException e) { - /* fallthrough */ + // fallthrough } } throw new DateTimeParseException("Bad OSM duration", duration, 0); From 68b9c61ac442e449bda595d11e36f259834e306a Mon Sep 17 00:00:00 2001 From: Teemu Kalvas Date: Mon, 9 Dec 2024 13:50:08 +0200 Subject: [PATCH 16/17] revert explanation of default escalator speed --- .../api/request/preference/EscalatorPreferences.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java b/application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java index 9a770d99fda..3c150b43417 100644 --- a/application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java +++ b/application/src/main/java/org/opentripplanner/routing/api/request/preference/EscalatorPreferences.java @@ -14,9 +14,14 @@ public class EscalatorPreferences implements Serializable { private final double reluctance; private final double speed; + /* Using the angle of 30 degrees and a speed of 0.5 m/s gives a horizontal component + * of approx. 0.43 m/s. This is typical of short escalators like those in shopping + * malls. */ + private static final double HORIZONTAL_SPEED = 0.45; + private EscalatorPreferences() { this.reluctance = 1.5; - this.speed = 0.45; + this.speed = HORIZONTAL_SPEED; } private EscalatorPreferences(Builder builder) { From ffffb13b54cc7785d7563f79698af84100d6deb6 Mon Sep 17 00:00:00 2001 From: Joel Lappalainen Date: Mon, 9 Dec 2024 14:04:15 +0200 Subject: [PATCH 17/17] Update application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java Co-authored-by: Leonard Ehrenfried --- .../main/java/org/opentripplanner/osm/model/OsmWithTags.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d6ed30be092..3f47d4454bd 100644 --- a/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java +++ b/application/src/main/java/org/opentripplanner/osm/model/OsmWithTags.java @@ -299,7 +299,7 @@ public static Duration parseOsmDuration(String duration) { } /** - * Gets a tag's value, assumes it is an OSM wiki spesified duration, parses and returns it. + * Gets a tag's value, assumes it is an OSM wiki specified duration, parses and returns it. * If parsing fails, calls the error handler. * * @param key