Convert transferSlack configuration to duration

docs/RouteRequest.md

@@ -15,9 +15,9 @@ and in the [transferRequests in build-config.json](BuildConfiguration.md#transfe
 
 | Config Parameter                                                                                             |          Type          | Summary                                                                                                                                        |  Req./Opt. | Default Value    | Since |
 |--------------------------------------------------------------------------------------------------------------|:----------------------:|------------------------------------------------------------------------------------------------------------------------------------------------|:----------:|------------------|:-----:|
-| [alightSlack](#rd_alightSlack)                                                                               |       `duration`       | The minimum extra time after exiting a public transport vehicle.                                                                               | *Optional* | `"PT0S"`         |  2.0  |
+| [alightSlack](#rd_alightSlack)                                                                               |       `duration`       | The time safety margin when alighting from a vehicle.                                                                                          | *Optional* | `"PT0S"`         |  2.0  |
 | arriveBy                                                                                                     |        `boolean`       | Whether the trip should depart or arrive at the specified date and time.                                                                       | *Optional* | `false`          |  2.0  |
-| [boardSlack](#rd_boardSlack)                                                                                 |       `duration`       | The boardSlack is the minimum extra time to board a public transport vehicle.                                                                  | *Optional* | `"PT0S"`         |  2.0  |
+| [boardSlack](#rd_boardSlack)                                                                                 |       `duration`       | The time safety margin when boarding a vehicle.                                                                                                | *Optional* | `"PT0S"`         |  2.0  |
 | [drivingDirection](#rd_drivingDirection)                                                                     |         `enum`         | The driving direction to use in the intersection traversal calculation                                                                         | *Optional* | `"right"`        |  2.2  |
 | elevatorBoardCost                                                                                            |        `integer`       | What is the cost of boarding a elevator?                                                                                                       | *Optional* | `90`             |  2.0  |
 | elevatorBoardTime                                                                                            |        `integer`       | How long does it take to get on an elevator, on average.                                                                                       | *Optional* | `90`             |  2.0  |
@@ -38,7 +38,7 @@ and in the [transferRequests in build-config.json](BuildConfiguration.md#transfe
 | [searchWindow](#rd_searchWindow)                                                                             |       `duration`       | The duration of the search-window.                                                                                                             | *Optional* |                  |  2.0  |
 | [streetRoutingTimeout](#rd_streetRoutingTimeout)                                                             |       `duration`       | The maximum time a street routing request is allowed to take before returning the results.                                                     | *Optional* | `"PT5S"`         |  2.2  |
 | [transferPenalty](#rd_transferPenalty)                                                                       |        `integer`       | An additional penalty added to boardings after the first.                                                                                      | *Optional* | `0`              |  2.0  |
-| [transferSlack](#rd_transferSlack)                                                                           |        `integer`       | The extra time needed to make a safe transfer in seconds.                                                                                      | *Optional* | `120`            |  2.0  |
+| [transferSlack](#rd_transferSlack)                                                                           |       `duration`       | The extra time needed to make a safe transfer.                                                                                                 | *Optional* | `"PT2M"`         |  2.0  |
 | turnReluctance                                                                                               |        `double`        | Multiplicative factor on expected turning time.                                                                                                | *Optional* | `1.0`            |  2.0  |
 | [unpreferredCost](#rd_unpreferredCost)                                                                       | `cost-linear-function` | A cost function used to calculate penalty for an unpreferred route.                                                                            | *Optional* | `"0s + 1.00 t"`  |  2.2  |
 | waitReluctance                                                                                               |        `double`        | How much worse is waiting for a transit vehicle than being on a transit vehicle, as a multiplier.                                              | *Optional* | `1.0`            |  2.0  |
@@ -192,22 +192,34 @@ and in the [transferRequests in build-config.json](BuildConfiguration.md#transfe
 **Since version:** `2.0` ∙ **Type:** `duration` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"PT0S"`   
 **Path:** /routingDefaults 
 
-The minimum extra time after exiting a public transport vehicle.
+The time safety margin when alighting from a vehicle.
+
+This time slack is added to arrival time of the vehicle before any transfer or onward travel.
+
+The sole reason for this is to avoid missed connections when there are minor schedule variations. This
+parameter is intended to be set by agencies not individual users. In general it is better to use
+`boardSlack` - see its documentation for details. However, for specific modes, like airplane and
+subway, that need more time than others, this is also configurable per mode with `alightSlackForMode`.
 
-The slack is added to the time when going from the transit vehicle to the stop.
 
 <h3 id="rd_boardSlack">boardSlack</h3>
 
 **Since version:** `2.0` ∙ **Type:** `duration` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"PT0S"`   
 **Path:** /routingDefaults 
 
-The boardSlack is the minimum extra time to board a public transport vehicle.
+The time safety margin when boarding a vehicle.
 
-The board time is added to the time when going from the stop (offboard) to onboard a transit
-vehicle.
+The board slack is added to the passenger's arrival time at a stop before boarding evaluating which
+vehicles can be boarded.
 
-This is the same as the `transferSlack`, except that this also applies to to the first
-transit leg in the trip. This is the default value used, if not overridden by the `boardSlackList`.
+The sole reason for this is to avoid missed connections when there are minor schedule variations. This
+parameter is intended to be set by agencies not individual users. For specific modes, like airplane and
+subway, that need more time than others, this is also configurable per mode with `boardSlackForMode`.
+
+Agencies can use this parameter to ensure that the trip planner does not instruct passengers to arrive
+at the last second.
+This slack is added at every boarding including the first vehicle and transfers except for in-seat
+transfers and guaranteed transfers.
 
 
 <h3 id="rd_drivingDirection">drivingDirection</h3>
@@ -351,15 +363,17 @@ significant time or walking will still be taken.
 
 <h3 id="rd_transferSlack">transferSlack</h3>
 
-**Since version:** `2.0` ∙ **Type:** `integer` ∙ **Cardinality:** `Optional` ∙ **Default value:** `120`   
+**Since version:** `2.0` ∙ **Type:** `duration` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"PT2M"`   
 **Path:** /routingDefaults 
 
-The extra time needed to make a safe transfer in seconds.
+The extra time needed to make a safe transfer.
+
+The extra buffer time/safety margin added to transfers to make sure the connection is safe, time
+wise. We recommend allowing the end-user to set this, and use `board-/alight-slack` to enforce
+agency policies. This time is in addition to how long it might take to walk, board and alight.
 
-An expected transfer time in seconds that specifies the amount of time that must pass
-between exiting one public transport vehicle and boarding another. This time is in
-addition to time it might take to walk between stops plus `boardSlack` and
-`alightSlack`.
+It is useful for passengers on long distance travel, and people with mobility issues, but can be set
+close to zero for everyday commuters and short distance searches in high transit frequency areas.
 
 
 <h3 id="rd_unpreferredCost">unpreferredCost</h3>
@@ -1181,7 +1195,7 @@ include stairs as a last result.
     },
     "waitReluctance" : 1.0,
     "otherThanPreferredRoutesPenalty" : 300,
-    "transferSlack" : 120,
+    "transferSlack" : "2m",
     "boardSlackForMode" : {
       "AIRPLANE" : "35m"
     },

docs/RouterConfiguration.md

@@ -532,7 +532,7 @@ Used to group requests when monitoring OTP.
     },
     "waitReluctance" : 1.0,
     "otherThanPreferredRoutesPenalty" : 300,
-    "transferSlack" : 120,
+    "transferSlack" : "2m",
     "boardSlackForMode" : {
       "AIRPLANE" : "35m"
     },

docs/examples/entur/router-config.json

@@ -52,7 +52,7 @@
     },
     "waitReluctance": 1.0,
     "otherThanPreferredRoutesPenalty": 300,
-    "transferSlack": 120,
+    "transferSlack": "2m",
     // Default slack for any mode is 0 (zero)
     "boardSlackForMode": {
       "AIRPLANE" : "2100s"

docs/examples/ibi/portland/router-config.json

@@ -3,7 +3,7 @@
     "maxJourneyDuration": "6h",
     "boardSlack": "0s",
     "alightSlack": "0s",
-    "transferSlack": 180,
+    "transferSlack": "3m",
     "waitReluctance": 0.9,
     "walk": {
       "reluctance": 1.75,

docs/examples/skanetrafiken/router-config.json

@@ -3,7 +3,7 @@
     "itineraryFilters": {
       "filterItinerariesWithSameFirstOrLastTrip": true
     },
-    "transferSlack": 180,
+    "transferSlack": "3m",
     "waitReluctance": 0.175,
     "walk": {
       "reluctance": 5

src/ext/java/org/opentripplanner/ext/restapi/resources/RequestToPreferencesMapper.java

@@ -151,7 +151,7 @@ private void mapTransfer(BoardAndAlightSlack boardAndAlightSlack) {
             "Invalid parameters: 'minTransferTime' must be greater than or equal to board slack plus alight slack"
           );
         }
-        transfer.withSlack(req.minTransferTime - boardAndAlightSlack.value);
+        transfer.withSlackSec(req.minTransferTime - boardAndAlightSlack.value);
       }
 
       setIfNotNull(req.nonpreferredTransferPenalty, transfer::withNonpreferredCost);

src/main/java/org/opentripplanner/apis/gtfs/mapping/routerequest/LegacyRouteRequestMapper.java

@@ -164,7 +164,7 @@ public static RouteRequest toRouteRequest(
       });
       preferences.withTransfer(tx -> {
         callWith.argument("transferPenalty", tx::withCost);
-        callWith.argument("minTransferTime", tx::withSlack);
+        callWith.argument("minTransferTime", tx::withSlackSec);
         callWith.argument("waitReluctance", tx::withWaitReluctance);
         callWith.argument("maxTransfers", tx::withMaxTransfers);
         callWith.argument("nonpreferredTransferPenalty", tx::withNonpreferredCost);

src/main/java/org/opentripplanner/apis/gtfs/mapping/routerequest/TransitPreferencesMapper.java

@@ -73,7 +73,7 @@ static void setTransitPreferences(
       var slack = transfer.getGraphQLSlack();
       if (slack != null) {
         transferPreferences.withSlack(
-          (int) DurationUtils.requireNonNegativeMedium(slack, "transfer slack").toSeconds()
+          DurationUtils.requireNonNegativeMedium(slack, "transfer slack")
         );
       }
       var maxTransfers = transfer.getGraphQLMaximumTransfers();

src/main/java/org/opentripplanner/apis/transmodel/mapping/PreferencesMapper.java

@@ -26,7 +26,7 @@ static void mapPreferences(
     preferences.withBike(bike -> mapBikePreferences(bike, callWith));
     preferences.withCar(car -> mapCarPreferences(car, callWith));
     preferences.withScooter(scooter -> mapScooterPreferences(scooter, callWith));
-    preferences.withTransfer(transfer -> mapTransferPreferences(transfer, environment, callWith));
+    preferences.withTransfer(transfer -> mapTransferPreferences(transfer, callWith));
     preferences.withTransit(transit -> mapTransitPreferences(transit, environment, callWith));
     preferences.withItineraryFilter(itineraryFilter ->
       mapItineraryFilterPreferences(itineraryFilter, environment, callWith)

src/main/java/org/opentripplanner/apis/transmodel/mapping/preferences/TransferPreferencesMapper.java

@@ -1,21 +1,19 @@
 package org.opentripplanner.apis.transmodel.mapping.preferences;
 
-import graphql.schema.DataFetchingEnvironment;
 import org.opentripplanner.apis.transmodel.support.DataFetcherDecorator;
 import org.opentripplanner.routing.api.request.preference.TransferPreferences;
 
 public class TransferPreferencesMapper {
 
   public static void mapTransferPreferences(
     TransferPreferences.Builder transfer,
-    DataFetchingEnvironment environment,
     DataFetcherDecorator callWith
   ) {
     callWith.argument("transferPenalty", transfer::withCost);
 
     // 'minimumTransferTime' is deprecated, that's why we are mapping 'slack' twice.
-    callWith.argument("minimumTransferTime", transfer::withSlack);
-    callWith.argument("transferSlack", transfer::withSlack);
+    callWith.argument("minimumTransferTime", transfer::withSlackSec);
+    callWith.argument("transferSlack", transfer::withSlackSec);
 
     callWith.argument("waitReluctance", transfer::withWaitReluctance);
     callWith.argument("maximumTransfers", transfer::withMaxTransfers);

src/main/java/org/opentripplanner/apis/transmodel/model/DefaultRouteRequestType.java

@@ -296,7 +296,7 @@ private GraphQLObjectType createGraphQLType() {
             "A global minimum transfer time (in seconds) that specifies the minimum amount of time that must pass between exiting one transit vehicle and boarding another."
           )
           .type(Scalars.GraphQLInt)
-          .dataFetcher(env -> preferences.transfer().slack())
+          .dataFetcher(env -> preferences.transfer().slack().toSeconds())
           .build()
       )
       .field(

src/main/java/org/opentripplanner/apis/transmodel/model/plan/TripQuery.java

@@ -440,7 +440,7 @@ Normally this is when the search is performed (now), plus a small grace period t
             "This time is in addition to time it might take to walk between stops."
           )
           .type(Scalars.GraphQLInt)
-          .defaultValue(preferences.transfer().slack())
+          .defaultValue(preferences.transfer().slack().toSeconds())
           .build()
       )
       .argument(

src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitData.java

@@ -109,7 +109,7 @@ public RaptorRoutingRequestTransitData(
 
     this.slackProvider =
       new SlackProvider(
-        request.preferences().transfer().slack(),
+        (int) request.preferences().transfer().slack().toSeconds(),
         request.preferences().transit().boardSlack(),
         request.preferences().transit().alightSlack()
       );

src/main/java/org/opentripplanner/routing/api/request/preference/TransferPreferences.java

@@ -4,10 +4,12 @@
 import static org.opentripplanner.framework.lang.DoubleUtils.doubleEquals;
 
 import java.io.Serializable;
+import java.time.Duration;
 import java.util.Objects;
 import java.util.function.Consumer;
 import org.opentripplanner.framework.model.Cost;
 import org.opentripplanner.framework.model.Units;
+import org.opentripplanner.framework.time.DurationUtils;
 import org.opentripplanner.framework.tostring.ToStringBuilder;
 import org.opentripplanner.routing.algorithm.transferoptimization.api.TransferOptimizationParameters;
 
@@ -23,7 +25,7 @@ public final class TransferPreferences implements Serializable {
   private static final int MAX_NUMBER_OF_TRANSFERS = 30;
 
   private final Cost cost;
-  private final int slack;
+  private final Duration slack;
   private final double waitReluctance;
   private final int maxTransfers;
   private final int maxAdditionalTransfers;
@@ -32,7 +34,7 @@ public final class TransferPreferences implements Serializable {
 
   private TransferPreferences() {
     this.cost = Cost.ZERO;
-    this.slack = 120;
+    this.slack = Duration.ofMinutes(2);
     this.waitReluctance = 1.0;
     this.maxTransfers = 12;
     this.maxAdditionalTransfers = 5;
@@ -42,7 +44,7 @@ private TransferPreferences() {
 
   private TransferPreferences(Builder builder) {
     this.cost = builder.cost;
-    this.slack = Units.duration(builder.slack);
+    this.slack = DurationUtils.requireNonNegative(builder.slack);
     this.waitReluctance = Units.reluctance(builder.waitReluctance);
     this.maxTransfers = Units.count(builder.maxTransfers, MAX_NUMBER_OF_TRANSFERS);
     this.maxAdditionalTransfers =
@@ -78,18 +80,18 @@ public int cost() {
   }
 
   /**
-   * A global minimum transfer time (in seconds) that specifies the minimum amount of time that must
+   * A global minimum transfer time that specifies the minimum amount of time that must
    * pass between exiting one transit vehicle and boarding another. This time is in addition to time
    * it might take to walk between transit stops, the {@link TransitPreferences#alightSlack()}, and the {@link
-   * TransitPreferences#boardSlack()}. This time should also be overridden by specific transfer timing information in
-   * transfers.txt
+   * TransitPreferences#boardSlack()}.
+   * This time can also be overridden by specific transfer timing information in transfers.txt
    * <p>
-   * This only apply to transfer between two trips, it does not apply when boarding the first
+   * This only applies to transfer between two trips, it does not apply when boarding the first
    * transit.
    * <p>
-   * Unit is seconds. Default value is 2 minutes.
+   * Default value is 2 minutes.
    */
-  public int slack() {
+  public Duration slack() {
     return slack;
   }
 
@@ -156,7 +158,7 @@ public boolean equals(Object o) {
     TransferPreferences that = (TransferPreferences) o;
     return (
       cost.equals(that.cost) &&
-      slack == that.slack &&
+      slack.equals(that.slack) &&
       doubleEquals(that.waitReluctance, waitReluctance) &&
       maxTransfers == that.maxTransfers &&
       maxAdditionalTransfers == that.maxAdditionalTransfers &&
@@ -183,7 +185,7 @@ public String toString() {
     return ToStringBuilder
       .of(TransferPreferences.class)
       .addObj("cost", cost, DEFAULT.cost)
-      .addNum("slack", slack, DEFAULT.slack)
+      .addDuration("slack", slack, DEFAULT.slack)
       .addNum("waitReluctance", waitReluctance, DEFAULT.waitReluctance)
       .addNum("maxTransfers", maxTransfers, DEFAULT.maxTransfers)
       .addNum("maxAdditionalTransfers", maxAdditionalTransfers, DEFAULT.maxAdditionalTransfers)
@@ -196,7 +198,7 @@ public static class Builder {
 
     private final TransferPreferences original;
     private Cost cost;
-    private int slack;
+    private Duration slack;
     private Integer maxTransfers;
     private Integer maxAdditionalTransfers;
     private double waitReluctance;
@@ -223,7 +225,11 @@ public Builder withCost(int cost) {
       return this;
     }
 
-    public Builder withSlack(int slack) {
+    public Builder withSlackSec(Number seconds) {
+      return withSlack(Duration.ofSeconds(seconds.longValue()));
+    }
+
+    public Builder withSlack(Duration slack) {
       this.slack = slack;
       return this;
     }