Change to not mark past stop times as unavailable when updating trip times #2297
Conversation
If a provided trip update does not start with the first stop time of a trip, all the previous stop times is marked as unavailble. This change that behaviour to instead set the delay for these stop times to zero. This will allow these stop times to be available in routing. This resolves opentripplanner#2295
|
Thanks @johannilsson for the fix to this issue. It seems to me that there are some decisions to be made about overwriting existing predicted values, see discussion in the original ticket #2295. |
| tripUpdate = tripUpdateBuilder.build(); | ||
| updatedTripTimes = timetable.createUpdatedTripTimes(tripUpdate, timeZone, serviceDate); | ||
| assertNotNull(updatedTripTimes); | ||
| // Ensure that times past the updated one is still valid. | ||
| assertEquals(0, updatedTripTimes.getDepartureDelay(0)); | ||
| timetable.setTripTimes(trip_1_1_index, updatedTripTimes); |
There was a problem hiding this comment.
If delay is set to 1 upstream of this stop, shouldn't this be assertEquals(1, ...?
There was a problem hiding this comment.
This check is to ensure that we don't do backward propagation. The delay is applied to second stop index with stop id B here.
There was a problem hiding this comment.
Ah, ok, thanks. I'd suggest changing the comment then - "past the updated one" to me implies a stop downstream of the prediction.
There was a problem hiding this comment.
Agreed, it should probably read "stops upstream of the updated one are still valid, but have no delay"
There was a problem hiding this comment.
Updated the comment as suggested. When I re-read I can see that it caused a confusion too. When I've written "past" I've been referring to the upstream stops. Thanks!
|
As mentioned on #2295 and above, we still need to make decisions about requiring "full trip" updates, propagating delays, etc. for GTFS-RT and probably also SIRI. I'll bring this up on the mailing list. |
|
@abyrd could you add a reference to the mailing list posting, as I could not find the related discussion in neither the Developers nor the Users list. Thank you. |
leonardehrenfried
force-pushed
the
master
branch
2 times, most recently
from
66629f5
to
4bcb63c
Compare
|
In OTP2 we now have Please reopen if that was a mistake. |
If a provided trip update does not start with the first stop time of a trip, all the previous stop times is marked as unavailable.
This change that behaviour to instead set the delay for these stop times to zero. This will allow these stop times to be available in routing.
This resolves #2295