propershark / timetable_cpp

Purveyor of schedule information for transit agencies via GTFS feeds and WAMP procedures.
1 stars 0 forks source link

Calls to visits_between don't honor the time range #11

Closed elliottwilliams closed 7 years ago

elliottwilliams commented 7 years ago

I'm seeing things like:

Received call to `visits_between`:
        stop:   BUS064
        start:  20170319 23:17:33
        end:    20170320 00:17:33
        count:  10
Responded in 1.838ms with:
[["20170319 09:11:00","20170319 09:11:00","1B","to CityBus Center"],["20170319 10:11:00","20170319 10:11:00","1B","to CityBus Center"],["20170319 11:11:00","20170319 11:11:00","1B","to CityBus Center"],["20170319 12:11:00","20170319 12:11:00","1B","to CityBus Center"],["20170319 13:11:00","20170319 13:11:00","1B","to CityBus Center"],["20170319 14:11:00","20170319 14:11:00","1B","to CityBus Center"],["20170319 15:11:00","20170319 15:11:00","1B","to CityBus Center"],["20170319 16:11:00","20170319 16:11:00","1B","to CityBus Center"],["20170319 17:11:00","20170319 17:11:00","1B","to CityBus Center"],["20170319 18:11:00","20170319 18:11:00","1B","to CityBus Center"]]

Note that the returned stop times are for the morning previous day. Maybe this has to do with requesting past midnight?

faultyserver commented 7 years ago

Hmmm, interesting that I hadn't noticed this before, but it's a fairly obvious bug in the cop-out-of-a-temporary-implementation I'm using to push Visits onto the result list.

The exposure of this bug (which your example hits pretty squarely) exploits 3 caveats of that implementation:

I haven't fully looked into this yet, but I'm fairly certain that the results you're getting are actually for the next station in the visit list. Essentially, there are no visits after 23:17:33 for the BUS064, and the visit list sees 20170320 00:17:33 as occurring before the start time. Because of that, the upper bound that visits_between uses to determine when to stop iterating is never met, and instead the iteration continues onto the next station.

I'm currently working on implementing support for requests that span multiple dates, so I believe this will be covered by that work. You can track the date_iteration branch if you'd like to follow along.

faultyserver commented 7 years ago

I think 52890f6a fixes this, but I don't want to close until I've tested it more and finished the implementation.

At a minimum, you should now be getting results with the correct date on them when you make a call to visits_between (the other visits_* calls haven't been updated yet), so this issue should at least be more obvious, if not resolved.

elliottwilliams commented 7 years ago

Thanks for working on this. 52890f6a seems to introduce a new bug with the date iterator (look at the last result):

Received call to `visits_between`:
        stop:   BUS313SW
        start:  20170320 23:13:45
        end:    20170321 00:13:45
        count:  10
20170320
Iterating date from 20170320 to 20170321
20170321
Responded in 5.745ms with:
[["20170320 23:30:00","20170320 23:30:00","1B","to CityBus Center"],["20170320 23:32:00","20170320 23:32:00","4B","to Campus & CityBus Center"],["20170320 24:30:00","20170320 24:30:00","1B","to CityBus Center"]]

It looks like times past midnight are showing up as hours > 23 on the same date, rather than starting with hour 0 on the next date.

faultyserver commented 7 years ago

Indeed. This is due to the way that GTFS deals with stop_times that pass the midnight boundary. In short, it defines visits above 24 hours rather than wrapping to 0 on the next day so that trip times can more easily be calculated. You can get a little more information from the stop_times.txt reference section on the arrival_time field.

I'm currently working on a proper DateTime implementation that will deal with these kinds of things, and will hopefully make the iteration implementation much cleaner as well. I'll have an initial commit up soon.

faultyserver commented 7 years ago

I believe 46bd10e fully implements #10 for visits_between and fixes this bug. @elliottwilliams can you test this and let me know if you hit any new issues? I'll be implementing the other visits_* calls shortly.

faultyserver commented 7 years ago

This should be solved with the merging of #12.

elliottwilliams commented 7 years ago

FWIW, I've been using Timetable near midnight recently and haven't seen any issues. Thanks!