-
Notifications
You must be signed in to change notification settings - Fork 32
Overnight: Reversal on first day of timetable.dat goes wrong #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Same one but without t_data_dump at each route_route. This might not just be the first second of the timetable.dat related but more with departing at midnight...
|
This fixes the calendar date, but doesn't fix the actual routing problem. diff --git a/router.c b/router.c
index 6a0ebbd..69b8bae 100644
--- a/router.c
+++ b/router.c
@@ -530,7 +530,11 @@ bool router_request_reverse(router_t *router, router_request_t *req) {
D router_state_dump (&(states[round][stop]));
req->max_transfers = round;
req->time_cutoff = SEC_TO_RTIME(req->time); // fix units situation -- use durations in seconds o
- req->time = RTIME_TO_SEC(states[round][stop].time - RTIME_ONE_DAY);
+ struct tm origin_tm;
+ rtime_t origin_rtime = epoch_to_rtime (req->time, &origin_tm);
+ origin_tm.tm_min = 0;
+ origin_tm.tm_hour = 0;
+ origin_tm.tm_sec = 0;
+ req->time = RTIME_TO_SEC(states[round][stop].time - RTIME_ONE_DAY) + mktime(&origin_tm);
req->arrive_by = !(req->arrive_by);
// router_request_dump(router, req);
return true; Final patch. |
This is indeed part of the problem, but a deeper problem here is the mixing of internal r(outer)times and epoch times. The epoch time thing seemed like a good idea (especially since I just wanted to test out the date masks) but it's time zone sensitive, and prone to weirdness like this. We should instead store the date and time as separate fields in requests, and only perform math on dates to determine on which day of the internal calendar a request is made. There, in most cases it doesn't matter in which time zone the problem is evaluated because only the relative number of days is important. Daylight saving time problems could be eliminated by evaluating at noon instead of midnight. |
I think you should start thinking on a private structure for the request and the public structure. The private structure could contain the rtime and cal_day, while the public part only does have epoch (or struct tm). |
But epoch time is not "in a timezone". We might as well avoid that trap and do everything timezone-agnostic. C does easily allow a public and private structure: by including the smaller public structure as the first element of the larger private structure, you should be able to use them somewhat interchangeably. |
This is probably partially fixed since we use rtimes now. There is a remaining edge case though: an arrive-by search early on the first day of the schedule may go past midnight into a day where no services are running (its mask is 0). In this case, the search should just fail with no result if the destination cannot be reached. Still, this case should be tested. TODO check that no-trip-found does not crash but fails gracefully. Depart-after searches should not encounter this problem during reversal, since the reversed search should be bounded by the original search's departure time. |
This does imply a "business rule" though: we should always publish timetables including at least yesterday, today, and tomorrow relative to the date of issue. |
If this is a problem, i have no issues at all to just set up as design that On Thu, Sep 26, 2013 at 6:12 PM, Andrew Byrd notifications@github.comwrote:
|
When reversing a request departing at the first possible second in the timetable.dat, the reversal ends up in 1970
The text was updated successfully, but these errors were encountered: