Skip to content
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

Fix some daylight saving time issues by using UTC. #192

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benzea
Copy link

@benzea benzea commented May 3, 2015

Ok, this seems to work now. I have not actually done any search during daylight saving time change, as I cannot properly test it (cannot select the date in the GUI). The one thing I am not entirely sure about is what happens if the connection itself crosses the daylight saving time change. It is definitely an improvement to the current situation.

This commit switches the internal clock of the hafas provider to use
UTC. hafas already provides times that are adjusted for daylight saving
so no further adjustment needs to be done by the parser.
The patch also uses 1970-01-01 as the reference day so that a day switch
during reesult generation won't result in wrong duration displays.

This commit fixes issue smurfy#185.
@smurfy
Copy link
Owner

smurfy commented May 19, 2015

sorry, i did not forgot this PR, i just want to make sure that this problem is not with the other backends as-well.

@smurfy
Copy link
Owner

smurfy commented May 20, 2015

This PR breaks the time on the details page.
Even for today.

@benzea
Copy link
Author

benzea commented May 20, 2015

I am a bit surprised, thought I had that tested ...

Hm, we could just change it to use UTC and a fixed base date for times and leave the addSecs part as is. That should at least work, but I don't understand how it might be failing.

@benzea
Copy link
Author

benzea commented May 20, 2015

Wait, are the datetime objects passed around somewhere, and then a timezone conversion happens in the background? When I hit getJourneyDetails in the fahrplan2 UI, then the times are correct, but obviously they are displayed as UTC (as that is what they are with the patch, even thought that is wrong)

@benzea
Copy link
Author

benzea commented May 20, 2015

With correct I mean, if you assume that the time is really local time and not UTC, then it is correct.

@smurfy
Copy link
Owner

smurfy commented May 21, 2015

I have to take a look, why the date is displayed as is, in the journey overview, but not in the details.

As far as i remember the date difference is about 2 hours more than the "correct" time.
On 25th of october the 00:xx trains in my tests even will be displayed as 03:xx on details.

@benzea
Copy link
Author

benzea commented May 21, 2015

Oh, two hours sounds like the time zone, but 03:XX in oktober sounds weird though (haven't thought it fully through though). Can I properly do a search in october with the "fahrplan2" desktop UI?

The trouble is that we have two timezones in the process as it is impossible to use the phones time zone to store the data:

  • current time zone of the phone
  • UTC (as the patch does) or correct timezone for each backend

I do think that the user will always want to see the local time for the backend. So just using UTC and hiding the fact works, but using the correct local time would also be good (the advantage is that you could generate correct calender entries then).

Then we would need to:

  • Store timezone for each backend
  • correctly convert the hours/minutes to this timezone (not sure how to best do this right now)
  • ensure dates/times are not converted to phones local time for display

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants