smurfy / fahrplan

QT Application for Public transportation
GNU General Public License v2.0
58 stars 33 forks source link

[Ubuntu] [bahn.de] Shows trains which do not depart on the current day of the week #185

Open nikwen opened 9 years ago

nikwen commented 9 years ago

Lately, I was on a journey from Nuremberg (Germany) to Munich on a Sunday and the app displayed a train which, according to the app, was supposed to depart at 12:10 h. However, I found out the hard way that it only departs on Saturdays.

Result: Had to wait one hour in Nuremberg for the next train.

leppa commented 9 years ago

If you go to bahn.de website and search for the same connection there, does it return train at 12:10?

We show the information as provided by the service. So if bahn.de returns that train in the list of journeys, there's nothing we can do about that.

Unless Fahrplan, somehow, sent wrong date / time to bahn.de.

nikwen commented 9 years ago

@leppa Thanks you very much for your reply.

The bahn.de website doesn't display the train. I'll try to find another case so that you can reproduce the bug.

leppa commented 9 years ago

I can see there's a train from Nuremberg to Munich at 13:10 on Sundays. That's exactly one hour difference. Did your issue happen this Sunday (05.04) or the Sunday before (29.03)? I have a feeling that it might be somehow related to daylight saving time change.

nikwen commented 9 years ago

@leppa I had exactly the same thing in mind as it happened on Sunday, 29th March.

I'll send you a screenshot I took back then if you want one.

nikwen commented 9 years ago

Here is proof that it is in fact related to the daylight saving time change.

This is the screenshot I took on the day before the journey (some personal information removed):

fahrplan

This is a screenshot I took right now from the bahn.de website:

bahn de website

Comparing the train numbers clearly shows that the time was displayed incorrectly in the application.

I'm not sure how it looked the day of the journey itself as I mainly used the screenshots I took to save bandwidth.

leppa commented 9 years ago

Thanks for the screenshot. Maybe, changing the phone date will reproduce the issue. Some of us will take a look at this issue.

I can also see that train comments are cut on the right. @mzanetti, can you enable word wrapping on the label?

nikwen commented 9 years ago

Thanks. :)

Give me a moment and you'll have a PR with word wrapping. ;)

nikwen commented 9 years ago

Another thought: If changing the date back doesn't help, one can always look at trains after the next daylight saving time change.

nikwen commented 9 years ago

The pull request for word wrapping: https://github.com/smurfy/fahrplan/pull/186 :)

benzea commented 9 years ago

Hey, had the same issue, and I would say the reason might be the toTime function in the parser (which means that changing the date on the phone only changes some aspects).

nikwen, the first train you took, was it a 4:XX long train ride? Looking at the code I would expect that any duration that lasts for more than 2 hours will be displayed an hour too long if the phones date is on the change to summer time. For all other connection the trains date is used, so it will always be wrong.

All that means that hafas actually returns the hours and minutes on the day with daylight saving time already accounted for. It would probably be enough to modify the toTime functions of the hafas parser to use QDateTime::setTime() with the QTime of the calculated hours/minutes. i.e. the change below:

    return tmpDateTime.addSecs(((hours * 60) + minutes) * 60);

becomes

    return tmpDateTime.setTime(QTime(hours, minutes);

A small improvement might be to use a QTime in the first place to store the duration instead of relying on a QDateTime object there.

benzea commented 9 years ago

Reproducing the issue is trivial. Just do a search on 25th of October, and all trains will be displayed an hour early (well, after the first 2:59 o'clock).

EDIT: It looks like Fahrplan uses the phones current timezone for all calculations. So the effect is visible when your local timezone changes daylight saving and it not when Europe/Berlin changes. That is likely only an issue if you try to store the times for other uses (e.g. by inserting them into the calendar).

nikwen commented 9 years ago

@benzea No, the 5:XX hours which were displayed were correct. The journey would have lasted from 10:XX to 15:XX (opposed to 09:XX to 14:XX which the app displayed).

I took the screenshot on the day before the journey.

benzea commented 9 years ago

On So, 2015-04-26 at 05:15 -0700, Niklas Wenzel wrote:

@benzea No, the 5:XX hours which were displayed were correct. The journey would have lasted from 10:XX to 15:XX (opposed to 09:XX to 14:XX which the app displayed).

I took the screenshot on the day before the journey.

Makes sense then. Any search on the day will display wrong durations.

Two more things:

Edited: Right, QTime canot handle date changes … well, I created a patch that should fix the issue. Is not properly tested though.

smurfy commented 9 years ago

@benzea thanks for looking into this, feel free to create a PR once finished and tested. Main thing to test is if it still works if you have a journey > 24h