handle duration tags for elevators in the same way as for escalators

Author: tkalvas

application/src/main/java/org/opentripplanner/graph_builder/module/osm/ElevatorProcessor.java

@@ -1,16 +1,16 @@
 package org.opentripplanner.graph_builder.module.osm;
 
 import gnu.trove.list.TLongList;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.OptionalInt;
+import java.util.function.Consumer;
 import org.opentripplanner.framework.i18n.NonLocalizedString;
 import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
 import org.opentripplanner.graph_builder.issue.api.Issue;
-import org.opentripplanner.osm.model.OsmEntity;
 import org.opentripplanner.osm.model.OsmLevel;
 import org.opentripplanner.osm.model.OsmNode;
 import org.opentripplanner.osm.model.OsmWay;
@@ -40,18 +40,25 @@ class ElevatorProcessor {
 
   private static final Logger LOG = LoggerFactory.getLogger(ElevatorProcessor.class);
 
-  private final DataImportIssueStore issueStore;
   private final OsmDatabase osmdb;
   private final VertexGenerator vertexGenerator;
+  private final Consumer<String> osmEntityDurationIssueConsumer;
 
   public ElevatorProcessor(
     DataImportIssueStore issueStore,
     OsmDatabase osmdb,
     VertexGenerator vertexGenerator
   ) {
-    this.issueStore = issueStore;
     this.osmdb = osmdb;
     this.vertexGenerator = vertexGenerator;
+    this.osmEntityDurationIssueConsumer = v ->
+      issueStore.add(
+        Issue.issue(
+          "InvalidDuration",
+          "Duration for osm node {} is not a valid duration: '{}'; the value is ignored.",
+          v
+        )
+      );
   }
 
   public void buildElevatorEdges(Graph graph) {
@@ -93,7 +100,10 @@ public void buildElevatorEdges(Graph graph) {
           levelName
         );
       }
-      int travelTime = parseDuration(node).orElse(-1);
+      long travelTime = node
+        .getDuration(osmEntityDurationIssueConsumer)
+        .map(Duration::toSeconds)
+        .orElse(-1L);
 
       var wheelchair = node.wheelchairAccessibility();
 
@@ -102,7 +112,7 @@ public void buildElevatorEdges(Graph graph) {
         wheelchair,
         !node.isBicycleExplicitlyDenied(),
         levels.length,
-        travelTime
+        (int) travelTime
       );
     } // END elevator edge loop
 
@@ -136,7 +146,10 @@ public void buildElevatorEdges(Graph graph) {
         );
       }
 
-      int travelTime = parseDuration(elevatorWay).orElse(-1);
+      long travelTime = elevatorWay
+        .getDuration(osmEntityDurationIssueConsumer)
+        .map(Duration::toSeconds)
+        .orElse(-1L);
       int levels = nodes.size();
       var wheelchair = elevatorWay.wheelchairAccessibility();
 
@@ -145,7 +158,7 @@ public void buildElevatorEdges(Graph graph) {
         wheelchair,
         !elevatorWay.isBicycleExplicitlyDenied(),
         levels,
-        travelTime
+        (int) travelTime
       );
       LOG.debug("Created elevatorHopEdges for way {}", elevatorWay.getId());
     }
@@ -221,17 +234,4 @@ private boolean isElevatorWay(OsmWay way) {
     // https://www.openstreetmap.org/way/187719215
     return nodeRefs.get(0) != nodeRefs.get(nodeRefs.size() - 1);
   }
-
-  private OptionalInt parseDuration(OsmEntity element) {
-    return element.getTagAsInt("duration", v ->
-      issueStore.add(
-        Issue.issue(
-          "InvalidDuration",
-          "Duration for osm node %d is not a number: '%s'; it's replaced with '-1' (unknown).",
-          element.getId(),
-          v
-        )
-      )
-    );
-  }
 }

application/src/main/java/org/opentripplanner/osm/model/OsmEntity.java

@@ -318,6 +318,10 @@ public Optional<Duration> getTagValueAsDuration(String key, Consumer<String> err
     return Optional.empty();
   }
 
+  public Optional<Duration> getDuration(Consumer<String> errorHandler) {
+    return getTagValueAsDuration("duration", errorHandler);
+  }
+
   /**
    * Some tags are allowed to have values like 55, "true" or "false".
    * <p>

application/src/main/java/org/opentripplanner/osm/model/OsmWay.java

@@ -133,10 +133,6 @@ public boolean isEscalator() {
     return (isTag("highway", "steps") && isOneOfTags("conveying", ESCALATOR_CONVEYING_TAGS));
   }
 
-  public Optional<Duration> getDuration(Consumer<String> errorHandler) {
-    return getTagValueAsDuration("duration", errorHandler);
-  }
-
   public boolean isForwardEscalator() {
     return isEscalator() && "forward".equals(this.getTag("conveying"));
   }

application/src/test/java/org/opentripplanner/street/model/edge/ElevatorHopEdgeTest.java

@@ -1,21 +1,34 @@
 package org.opentripplanner.street.model.edge;
 
+import static com.google.common.truth.Truth.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.opentripplanner.graph_builder.module.osm.moduletests._support.NodeBuilder.node;
 import static org.opentripplanner.street.model._data.StreetModelForTest.intersectionVertex;
 
+import java.util.List;
 import java.util.stream.Stream;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.opentripplanner.framework.geometry.WgsCoordinate;
+import org.opentripplanner.graph_builder.module.osm.OsmModule;
+import org.opentripplanner.graph_builder.module.osm.moduletests._support.TestOsmProvider;
+import org.opentripplanner.osm.model.OsmWay;
+import org.opentripplanner.routing.api.request.StreetMode;
 import org.opentripplanner.routing.api.request.preference.AccessibilityPreferences;
 import org.opentripplanner.routing.api.request.preference.WheelchairPreferences;
+import org.opentripplanner.routing.graph.Graph;
+import org.opentripplanner.service.osminfo.internal.DefaultOsmInfoGraphBuildRepository;
+import org.opentripplanner.service.vehicleparking.internal.DefaultVehicleParkingRepository;
 import org.opentripplanner.street.model.StreetTraversalPermission;
 import org.opentripplanner.street.model.vertex.Vertex;
 import org.opentripplanner.street.search.request.StreetSearchRequest;
 import org.opentripplanner.street.search.state.State;
 import org.opentripplanner.transit.model.basic.Accessibility;
+import org.opentripplanner.transit.model.framework.Deduplicator;
 
 class ElevatorHopEdgeTest {
 
@@ -87,4 +100,27 @@ private State[] traverse(Accessibility wheelchair, StreetSearchRequest req) {
 
     return edge.traverse(state);
   }
+
+  @Test
+  void testDuration() {
+    var way = new OsmWay();
+    way.addTag("duration", "00:01:02");
+    way.addTag("highway", "elevator");
+    var provider = TestOsmProvider.of().addWay(way).build();
+    var graph = new Graph(new Deduplicator());
+    var osmModule = OsmModule.of(
+      provider,
+      graph,
+      new DefaultOsmInfoGraphBuildRepository(),
+      new DefaultVehicleParkingRepository()
+    ).build();
+    osmModule.buildGraph();
+    var edges = graph.getEdgesOfType(ElevatorHopEdge.class);
+    assertThat(edges).hasSize(2);
+    var edge = (ElevatorHopEdge) edges.getFirst();
+    var from = edge.getFromVertex();
+    var req = StreetSearchRequest.of().withMode(StreetMode.WALK);
+    var res = edge.traverse(new State(from, req.build()))[0];
+    assertEquals(62_000, res.getTimeDeltaMilliseconds());
+  }
 }