diff --git a/application/src/main/java/org/opentripplanner/updater/trip/TripTimesUpdater.java b/application/src/main/java/org/opentripplanner/updater/trip/TripTimesUpdater.java index 96e2f2d1ba5..2eb430de8a0 100644 --- a/application/src/main/java/org/opentripplanner/updater/trip/TripTimesUpdater.java +++ b/application/src/main/java/org/opentripplanner/updater/trip/TripTimesUpdater.java @@ -14,6 +14,7 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import javax.annotation.Nullable; import org.opentripplanner.model.Timetable; import org.opentripplanner.model.TimetableSnapshot; import org.opentripplanner.model.TripTimesPatch; @@ -105,7 +106,9 @@ public static Result createUpdatedTripTimesFromGTFS GtfsRealtime.TripUpdate.StopTimeUpdate update = updates.next(); int numStops = newTimes.getNumStops(); + @Nullable Integer delay = null; + @Nullable Integer firstUpdatedIndex = null; final long today = ServiceDateUtils @@ -150,11 +153,22 @@ public static Result createUpdatedTripTimesFromGTFS newTimes.setNoData(i); } else { // Else the status is SCHEDULED, update times as needed. - if (update.hasArrival()) { + GtfsRealtime.TripUpdate.StopTimeEvent arrival = update.hasArrival() + ? update.getArrival() + : null; + GtfsRealtime.TripUpdate.StopTimeEvent departure = update.hasDeparture() + ? update.getDeparture() + : null; + + // This extra variable is necessary if the departure is specified but the arrival isn't. + // We want to propagate the arrival delay from the previous stop, even if the departure + // delay at this stop is different. + var previousDelay = delay; + + if (arrival != null) { if (firstUpdatedIndex == null) { firstUpdatedIndex = i; } - GtfsRealtime.TripUpdate.StopTimeEvent arrival = update.getArrival(); if (arrival.hasDelay()) { delay = arrival.getDelay(); if (arrival.hasTime()) { @@ -173,15 +187,12 @@ public static Result createUpdatedTripTimesFromGTFS ); return Result.failure(new UpdateError(feedScopedTripId, INVALID_ARRIVAL_TIME, i)); } - } else if (delay != null) { - newTimes.updateArrivalDelay(i, delay); } - if (update.hasDeparture()) { + if (departure != null) { if (firstUpdatedIndex == null) { firstUpdatedIndex = i; } - GtfsRealtime.TripUpdate.StopTimeEvent departure = update.getDeparture(); if (departure.hasDelay()) { delay = departure.getDelay(); if (departure.hasTime()) { @@ -200,8 +211,38 @@ public static Result createUpdatedTripTimesFromGTFS ); return Result.failure(new UpdateError(feedScopedTripId, INVALID_DEPARTURE_TIME, i)); } - } else if (delay != null) { - newTimes.updateDepartureDelay(i, delay); + } + + // propagate arrival and departure times, taking care not to cause negative dwells / hops + if (arrival == null) { + // propagate the delay from the previous stop + if (previousDelay != null) { + newTimes.updateArrivalDelay(i, previousDelay); + } + // if the arrival time is later than the departure time, set it to the departure time + if (departure != null && newTimes.getArrivalTime(i) > newTimes.getDepartureTime(i)) { + newTimes.updateArrivalTime(i, newTimes.getDepartureTime(i)); + } + } + + previousDelay = newTimes.getArrivalDelay(i); + if (departure == null) { + if (previousDelay < 0) { + // if the bus is early, only propagate if it is not a timepoint, otherwise assume that + // the bus will wait until the scheduled time + if (newTimes.isTimepoint(i)) { + newTimes.updateDepartureDelay(i, 0); + } else { + newTimes.updateDepartureDelay(i, previousDelay); + } + } else { + // the bus is late, depart as soon as it can after the scheduled time + newTimes.updateDepartureTime( + i, + Math.max(newTimes.getArrivalTime(i), newTimes.getScheduledDepartureTime(i)) + ); + } + delay = newTimes.getDepartureDelay(i); } } diff --git a/application/src/test/java/org/opentripplanner/updater/trip/TripTimesUpdaterTest.java b/application/src/test/java/org/opentripplanner/updater/trip/TripTimesUpdaterTest.java index 54519155c25..981d6a54a54 100644 --- a/application/src/test/java/org/opentripplanner/updater/trip/TripTimesUpdaterTest.java +++ b/application/src/test/java/org/opentripplanner/updater/trip/TripTimesUpdaterTest.java @@ -314,7 +314,7 @@ public void fixIncoherentTimes() { stopTimeUpdateBuilder.setStopSequence(1); stopTimeUpdateBuilder.setScheduleRelationship(StopTimeUpdate.ScheduleRelationship.SCHEDULED); var stopTimeEventBuilder = stopTimeUpdateBuilder.getArrivalBuilder(); - stopTimeEventBuilder.setDelay(0); + stopTimeEventBuilder.setDelay(900); stopTimeUpdateBuilder = tripUpdateBuilder.addStopTimeUpdateBuilder(1); stopTimeUpdateBuilder.setStopSequence(2); stopTimeUpdateBuilder.setScheduleRelationship(StopTimeUpdate.ScheduleRelationship.SCHEDULED); @@ -482,7 +482,8 @@ public void testUpdateWithRequiredNoDataDelayPropagationWhenItsNotRequired() { assertEquals(0, updatedTripTimes.getArrivalDelay(1)); assertEquals(0, updatedTripTimes.getDepartureDelay(1)); assertEquals(-100, updatedTripTimes.getArrivalDelay(2)); - assertEquals(-100, updatedTripTimes.getDepartureDelay(2)); + // the stop is a timepoint so the bus will wait until the scheduled time + assertEquals(0, updatedTripTimes.getDepartureDelay(2)); assertTrue(updatedTripTimes.getDepartureTime(1) < updatedTripTimes.getArrivalTime(2)); // REQUIRED_NO_DATA propagation type should always set NO_DATA flags' @@ -522,7 +523,8 @@ public void testUpdateWithRequiredNoDataDelayPropagationWhenItsRequired() { assertEquals(-700, updatedTripTimes.getArrivalDelay(1)); assertEquals(-700, updatedTripTimes.getDepartureDelay(1)); assertEquals(-700, updatedTripTimes.getArrivalDelay(2)); - assertEquals(-700, updatedTripTimes.getDepartureDelay(2)); + // the stop is a timepoint so the bus will wait until the scheduled time + assertEquals(0, updatedTripTimes.getDepartureDelay(2)); assertTrue(updatedTripTimes.getDepartureTime(1) < updatedTripTimes.getArrivalTime(2)); // REQUIRED_NO_DATA propagation type should always set NO_DATA flags' @@ -556,13 +558,17 @@ public void testUpdateWithRequiredNoDataDelayPropagationOnArrivalTime() { SERVICE_DATE, BackwardsDelayPropagationType.REQUIRED_NO_DATA ); - // if arrival time is not defined but departure time is not and the arrival time is greater - // than to departure time on a stop, we should not try to fix it by default because the spec - // only allows you to drop all estimates for a stop when it's passed according to schedule - assertTrue(result.isFailure()); - - result.ifFailure(err -> { - assertEquals(err.errorType(), NEGATIVE_DWELL_TIME); + // the spec does not require you to supply both arrival and departure on a StopTimeUpdate + // it says: + // either arrival or departure must be provided within a StopTimeUpdate - both fields cannot be + // empty + // therefore the processing should succeed even if only one of them is given + assertTrue(result.isSuccess()); + result.ifSuccess(p -> { + var updatedTripTimes = p.getTripTimes(); + assertNotNull(updatedTripTimes); + assertEquals(15, updatedTripTimes.getArrivalDelay(2)); + assertEquals(15, updatedTripTimes.getDepartureDelay(2)); }); } @@ -596,7 +602,8 @@ public void testUpdateWithRequiredDelayPropagationWhenItsRequired() { assertEquals(-700, updatedTripTimes.getArrivalDelay(1)); assertEquals(-700, updatedTripTimes.getDepartureDelay(1)); assertEquals(-700, updatedTripTimes.getArrivalDelay(2)); - assertEquals(-700, updatedTripTimes.getDepartureDelay(2)); + // the stop is a timepoint so the bus will wait until the scheduled time + assertEquals(0, updatedTripTimes.getDepartureDelay(2)); assertTrue(updatedTripTimes.getDepartureTime(1) < updatedTripTimes.getArrivalTime(2)); // REQUIRED propagation type should never set NO_DATA flags' diff --git a/application/src/test/java/org/opentripplanner/updater/trip/TripUpdateBuilder.java b/application/src/test/java/org/opentripplanner/updater/trip/TripUpdateBuilder.java index e8218edfc1f..5df6c202d9c 100644 --- a/application/src/test/java/org/opentripplanner/updater/trip/TripUpdateBuilder.java +++ b/application/src/test/java/org/opentripplanner/updater/trip/TripUpdateBuilder.java @@ -65,6 +65,30 @@ public TripUpdateBuilder addDelayedStopTime(int stopSequence, int delay) { return addStopTime(null, -1, stopSequence, delay, delay, DEFAULT_SCHEDULE_RELATIONSHIP, null); } + public TripUpdateBuilder addDelayedArrivalStopTime(int stopSequence, int arrivalDelay) { + return addStopTime( + null, + -1, + stopSequence, + arrivalDelay, + NO_DELAY, + DEFAULT_SCHEDULE_RELATIONSHIP, + null + ); + } + + public TripUpdateBuilder addDelayedDepartureStopTime(int stopSequence, int departureDelay) { + return addStopTime( + null, + -1, + stopSequence, + NO_DELAY, + departureDelay, + DEFAULT_SCHEDULE_RELATIONSHIP, + null + ); + } + public TripUpdateBuilder addDelayedStopTime( int stopSequence, int arrivalDelay, @@ -166,6 +190,14 @@ private TripUpdateBuilder addStopTime( departureBuilder.setDelay(departureDelay); } + if (!arrivalBuilder.hasTime() && !arrivalBuilder.hasDelay()) { + stopTimeUpdateBuilder.clearArrival(); + } + + if (!departureBuilder.hasTime() && !departureBuilder.hasDelay()) { + stopTimeUpdateBuilder.clearDeparture(); + } + return this; } diff --git a/application/src/test/java/org/opentripplanner/updater/trip/moduletests/delay/DelayedTest.java b/application/src/test/java/org/opentripplanner/updater/trip/moduletests/delay/DelayedTest.java index f17ad90f560..592171566af 100644 --- a/application/src/test/java/org/opentripplanner/updater/trip/moduletests/delay/DelayedTest.java +++ b/application/src/test/java/org/opentripplanner/updater/trip/moduletests/delay/DelayedTest.java @@ -127,4 +127,48 @@ void complexDelay() { env.getRealtimeTimetable(TRIP_2_ID) ); } + + @Test + void delayedAfterNextStopDeparture() { + var tripInput = TripInput + .of(TRIP_2_ID) + .addStop(STOP_A1, "0:00:00", "0:00:00") + // 5-minute dwell + .addStop(STOP_B1, "0:05:00", "0:10:00") + .addStop(STOP_C1, "0:15:00", "0:16:00") + .addStop(STOP_D1, "0:20:00", "0:20:00") + .build(); + var env = RealtimeTestEnvironment.of().addTrip(tripInput).build(); + + var tripUpdate = new TripUpdateBuilder(TRIP_2_ID, SERVICE_DATE, SCHEDULED, TIME_ZONE) + .addDelayedStopTime(0, 0) + .addDelayedArrivalStopTime(1, 900) // 00:20 arr + .addDelayedStopTime(2, 540) // 00:24 arr / 00:25 dep + .addDelayedDepartureStopTime(3, 420) // 00:27 dep + .build(); + + assertSuccess(env.applyTripUpdate(tripUpdate)); + + var snapshot = env.getTimetableSnapshot(); + + var trip2 = env.getTransitService().getTrip(id(TRIP_2_ID)); + var originalTripPattern = env.getTransitService().findPattern(trip2); + + var originalTimetableScheduled = snapshot.resolve(originalTripPattern, null); + + final int originalTripIndexScheduled = originalTimetableScheduled.getTripIndex(TRIP_2_ID); + var originalTripTimesScheduled = originalTimetableScheduled.getTripTimes( + originalTripIndexScheduled + ); + assertEquals(RealTimeState.SCHEDULED, originalTripTimesScheduled.getRealTimeState()); + + assertEquals( + "SCHEDULED | A1 0:00 0:00 | B1 0:05 0:10 | C1 0:15 0:16 | D1 0:20 0:20", + env.getScheduledTimetable(TRIP_2_ID) + ); + assertEquals( + "UPDATED | A1 0:00 0:00 | B1 0:20 0:20 | C1 0:24 0:25 | D1 0:27 0:27", + env.getRealtimeTimetable(TRIP_2_ID) + ); + } }