Merge pull request #6302 from entur/fix_time_penalty_on_car_mode_defa…

…ults Add default penalty to all car API modes
opentripplanner · Dec 17, 2024 · cbdb640 · cbdb640
2 parents e9f5941 + 6dc9a4e
commit cbdb640
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 22 deletions.
diff --git a/...main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java b/...main/java/org/opentripplanner/routing/api/request/preference/AccessEgressPreferences.java
@@ -21,21 +21,7 @@
  */
 public final class AccessEgressPreferences implements Serializable {
 
-  private static final TimeAndCostPenalty DEFAULT_PENALTY = TimeAndCostPenalty.of(
-    TimePenalty.of(ofMinutes(20), 2f),
-    1.5
-  );
-  private static final TimeAndCostPenalty FLEX_DEFAULT_PENALTY = TimeAndCostPenalty.of(
-    TimePenalty.of(ofMinutes(10), 1.3f),
-    1.3
-  );
-  private static final TimeAndCostPenaltyForEnum<StreetMode> DEFAULT_TIME_AND_COST = TimeAndCostPenaltyForEnum
-    .of(StreetMode.class)
-    .with(StreetMode.CAR_TO_PARK, DEFAULT_PENALTY)
-    .with(StreetMode.CAR_HAILING, DEFAULT_PENALTY)
-    .with(StreetMode.CAR_RENTAL, DEFAULT_PENALTY)
-    .with(StreetMode.FLEXIBLE, FLEX_DEFAULT_PENALTY)
-    .build();
+  private static final TimeAndCostPenaltyForEnum<StreetMode> DEFAULT_TIME_AND_COST = createDefaultCarPenalty();
 
   public static final AccessEgressPreferences DEFAULT = new AccessEgressPreferences();
 
@@ -159,4 +145,21 @@ AccessEgressPreferences build() {
   private static DurationForEnum<StreetMode> durationForStreetModeOf(Duration defaultValue) {
     return DurationForEnum.of(StreetMode.class).withDefault(defaultValue).build();
   }
+
+  private static TimeAndCostPenaltyForEnum<StreetMode> createDefaultCarPenalty() {
+    var penaltyBuilder = TimeAndCostPenaltyForEnum.of(StreetMode.class);
+
+    var flexDefaultPenalty = TimeAndCostPenalty.of(TimePenalty.of(ofMinutes(10), 1.3f), 1.3);
+    penaltyBuilder.with(StreetMode.FLEXIBLE, flexDefaultPenalty);
+
+    // Add penalty to all car variants with access and/or egress.
+    var carPenalty = TimeAndCostPenalty.of(TimePenalty.of(ofMinutes(20), 2f), 1.5);
+    for (var it : StreetMode.values()) {
+      if (it.includesDriving() && (it.accessAllowed() || it.egressAllowed())) {
+        penaltyBuilder.with(it, carPenalty);
+      }
+    }
+
+    return penaltyBuilder.build();
+  }
 }
diff --git a/application/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql b/application/src/main/resources/org/opentripplanner/apis/transmodel/schema.graphql
@@ -775,7 +775,7 @@ type QueryType {
   "Input type for executing a travel search for a trip between two locations. Returns trip patterns describing suggested alternatives for the trip."
   trip(
     "Time and cost penalty on access/egress modes."
-    accessEgressPenalty: [PenaltyForStreetMode!] = [{streetMode : car_park, timePenalty : "20m + 2.0 t", costFactor : 1.5}, {streetMode : car_rental, timePenalty : "20m + 2.0 t", costFactor : 1.5}, {streetMode : flexible, timePenalty : "10m + 1.30 t", costFactor : 1.3}],
+    accessEgressPenalty: [PenaltyForStreetMode!] = [{streetMode : car, timePenalty : "20m + 2.0 t", costFactor : 1.5}, {streetMode : car_park, timePenalty : "20m + 2.0 t", costFactor : 1.5}, {streetMode : car_pickup, timePenalty : "20m + 2.0 t", costFactor : 1.5}, {streetMode : car_rental, timePenalty : "20m + 2.0 t", costFactor : 1.5}, {streetMode : flexible, timePenalty : "10m + 1.30 t", costFactor : 1.3}],
     "The alightSlack is the minimum extra time after exiting a public transport vehicle. This is the default value used, if not overridden by the 'alightSlackList'."
     alightSlackDefault: Int = 0,
     "List of alightSlack for a given set of modes. Defaults: []"

diff --git a/...c/test/java/org/opentripplanner/routing/api/request/preference/StreetPreferencesTest.java b/...c/test/java/org/opentripplanner/routing/api/request/preference/StreetPreferencesTest.java
@@ -23,7 +23,7 @@ class StreetPreferencesTest {
   private static final int ELEVATOR_BOARD_TIME = (int) Duration.ofMinutes(2).toSeconds();
   private static final IntersectionTraversalModel INTERSECTION_TRAVERSAL_MODEL =
     IntersectionTraversalModel.CONSTANT;
-  private static final TimeAndCostPenalty CAR_PENALTY = TimeAndCostPenalty.of(
+  private static final TimeAndCostPenalty CAR_TO_PARK_PENALTY = TimeAndCostPenalty.of(
     TimePenalty.of("2m + 1.5t"),
     3.5
   );
@@ -34,7 +34,7 @@ class StreetPreferencesTest {
     .withTurnReluctance(TURN_RELUCTANCE)
     .withElevator(it -> it.withBoardTime(ELEVATOR_BOARD_TIME))
     .withIntersectionTraversalModel(INTERSECTION_TRAVERSAL_MODEL)
-    .withAccessEgress(it -> it.withPenalty(Map.of(StreetMode.CAR_TO_PARK, CAR_PENALTY)))
+    .withAccessEgress(it -> it.withPenalty(Map.of(StreetMode.CAR_TO_PARK, CAR_TO_PARK_PENALTY)))
     .withAccessEgress(it -> it.withMaxDuration(MAX_ACCESS_EGRESS, Map.of()))
     .withMaxDirectDuration(MAX_DIRECT, Map.of())
     .withRoutingTimeout(ROUTING_TIMEOUT)
@@ -56,7 +56,10 @@ void accessEgressPenalty() {
       TimeAndCostPenalty.ZERO,
       subject.accessEgress().penalty().valueOf(StreetMode.WALK)
     );
-    assertEquals(CAR_PENALTY, subject.accessEgress().penalty().valueOf(StreetMode.CAR_TO_PARK));
+    assertEquals(
+      CAR_TO_PARK_PENALTY,
+      subject.accessEgress().penalty().valueOf(StreetMode.CAR_TO_PARK)
+    );
   }
 
   @Test
@@ -109,9 +112,14 @@ void testToString() {
       "routingTimeout: 3s, " +
       "elevator: ElevatorPreferences{boardTime: 2m}, " +
       "intersectionTraversalModel: CONSTANT, " +
-      "accessEgress: AccessEgressPreferences{penalty: TimeAndCostPenaltyForEnum{CAR_TO_PARK: " +
-      CAR_PENALTY +
-      ", CAR_RENTAL: (timePenalty: 20m + 2.0 t, costFactor: 1.50), CAR_HAILING: (timePenalty: 20m + 2.0 t, costFactor: 1.50), " +
+      "accessEgress: AccessEgressPreferences{penalty: TimeAndCostPenaltyForEnum{" +
+      "CAR: (timePenalty: 20m + 2.0 t, costFactor: 1.50), " +
+      "CAR_TO_PARK: " +
+      CAR_TO_PARK_PENALTY +
+      ", " +
+      "CAR_PICKUP: (timePenalty: 20m + 2.0 t, costFactor: 1.50), " +
+      "CAR_RENTAL: (timePenalty: 20m + 2.0 t, costFactor: 1.50), " +
+      "CAR_HAILING: (timePenalty: 20m + 2.0 t, costFactor: 1.50), " +
       "FLEXIBLE: (timePenalty: 10m + 1.30 t, costFactor: 1.30)}, " +
       "maxDuration: DurationForStreetMode{default:5m}" +
       "}, " +

diff --git a/doc/user/RouteRequest.md b/doc/user/RouteRequest.md
@@ -447,7 +447,9 @@ performance will be better.
 
 The default values are
 
+- `car` = (timePenalty: 20m + 2.0 t, costFactor: 1.50)
 - `car-to-park` = (timePenalty: 20m + 2.0 t, costFactor: 1.50)
+- `car-pickup` = (timePenalty: 20m + 2.0 t, costFactor: 1.50)
 - `car-rental` = (timePenalty: 20m + 2.0 t, costFactor: 1.50)
 - `car-hailing` = (timePenalty: 20m + 2.0 t, costFactor: 1.50)
 - `flexible` = (timePenalty: 10m + 1.30 t, costFactor: 1.30)