AutoDJ Timestamp displays GMT

[David Early]

Whether this is a bug or an "enhancement request" is somewhat dependent on your view.

Running Mixxx 1.11.0-beta1 on Ubuntu 10.10 (Maverick), Intel core i7, etc (don't think this is really a coding error...more information can be provided if no one can reproduce this)

1) Put songs in AutoDJ queue
2) Exposed "Timestamp" column in AutoDJ view

The time stamp is correct (I checked the database and the timestamp shown in the AutoDJ view is the same as the one in the database), but it would be helpful if this were displayed in local time rather than GMT.

How exactly I would use the timestamp remains to be seen, just figured it would be more useful in local time.

[João Reys Santos (l29634)]

I think I fixed the bug, at least to me, when I add a new item to the Library, it shows the correct time instead of GMT.

[Daniel Schürmann (daschuer)]

Hi João,

Thank you for your patch and adopting this bug!

Did you test your patch?
It does not work for me.
Please mail on mixxx-devel if you have trouble to setup your debug environment.

For fixing this bug, you should read:
http://doc.qt.digia.com/4.7/qdatetime.html#currentDateTime
http://doc.qt.digia.com/4.7/qdatetime.html#toTimeSpec
http://doc.qt.digia.com/4.7/qdatetime.html#toLocalTime

The patch might be not that trivial because
trackdao.h Line 40
const QString LIBRARYTABLE_DATETIMEADDED = "datetime_added";
in some parts of the code this const QString is used in some others the text "datetime_added".
It would be a lot more easy if we have used LIBRARYTABLE_DATETIMEADDED everywhere.
I would be happy if you will fix that for all library strings along with this bug as well.

All other timestamps generated with sqlites CURRENT_TIMESTAMP will suffer the same bug.
But I think they are currently not used. But we have to make sure to use always the same schema
for Database (HD) -> Mixxx data (RAM) -> GUI
UTC -> UTC -> LocalTime or UTC -> LocalTime -> LocalTime
Also you might consider to suffix all Mixxx varaibles carrying Utc with suffix Utc.
Eclipse has a nice refactoring and renaming tool :-)

Kind regards,

Daniel

[João Reys Santos (l29634)]

Hi Daniel,

Yes, I tested it and it seemed to work :/ I live in Portugal and GMT is -1:00h, after adding library items, instead of -1:00 I had the correct time displayed. Did you add a new item do your library or did you check the old items? Because when adding new items it works but it doesn't fix the old ones, is it supposed to?

Best regards,

João

[RJ Skerry-Ryan (rryan)]

Hi all,

Thanks for working on this João! The timestamps in the database are
intentionally stored in GMT. When dealing with times the best practice is
to always use GMT internally but display in the users local time zone.
Because of this, we should not change the actual date as stored in the TIO
or the TrackDAO but instead change BaseSqlTableModel/BaseTrackCache's item
rendering methods to instead just format the time in the local time zone.

Cheers,
RJ

On Sat, Apr 20, 2013 at 6:32 PM, João Reys Santos
<email address hidden>wrote:

> Hi Daniel,
>
> Yes, I tested it and it seemed to work :/ I live in Portugal and GMT is
> -1:00h, after adding library items, instead of -1:00 I had the correct
> time displayed. Did you add a new item do your library or did you check
> the old items? Because when adding new items it works but it doesn't fix
> the old ones, is it supposed to?
>
> Best regards,
>
> João
>
> --
> You received this bug notification because you are a member of Mixxx
> Development Team, which is subscribed to Mixxx.
> https://bugs.launchpad.net/bugs/1036492
>
> Title:
> AutoDJ Timestamp displays GMT
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/mixxx/+bug/1036492/+subscriptions
>

[Daniel Schürmann (daschuer)]

Hi João,

maybe your patch is not including all your changes?

+ QDateTime datetime_added = query.value(query.record().indexOf("datetime_added")).toDateTime().currentDateTime();

This line is only executed if a track object is instantiated. This happens when you load a track too a deck, but not when you browsing to the track tables.
Also, you call a static function of QDateTime.
Your call is equivalent to:
+ QDateTime datetime_added = QDateTime::currentDateTime();

As RJ pointed out this is not the right place to to fix the bug, because we want to have UTC inside the track info object. So i have to revert my idea of "Utc" suffix in favor of a "Loc" suffix for times that are for display purpose.

Kin regards,

Daniel

[João Reys Santos (l29634)]

Hi all,

Ok, I understand the difference and will attempt a fix, thanks for the tips guys :)

Best regards,

João

[João Reys Santos (l29634)]

Hi all,

I'm currently making changes on trackdao.cpp, basesqltablemodel.cpp and basetrackcache.cpp, so I'm no longer inserting the dates in localtime as I was.

The problem is, I already changed all data variables to "toLocalTime()", still I get the times in GMT, so my doubt is, if the "toLocalTime()" function is getting GMT for some reason or I'm doing something wrong (which is very likely).

Best regards,

João

[RJ Skerry-Ryan (rryan)]

Just some debugging hints:

* Verify each step of the process by printing what Qt thinks the date-time is to the console. What does the before and after of calling toLocalTime() do?

* Read the source code to toLocalTime() by looking in the Qt sources -- what does it do that could produce the result you observe in step 1?

^ hint, if you don't have the Qt source code checked out, an easy way to find the code for a class is to google <classname>.cpp qt gitorious

e.g. qt qdatetime.cpp gitorious will get you to a web hosted version of the source for QDateTime

[João Reys Santos (l29634)]

Thanks RJ, will do that (deleted my old post, misread the name, sorry about that :/ ).

[João Reys Santos (l29634)]

Ok after some google research, (thanks again for the tips) I found out that since DateTimes are stored in GMT, I had to implicitly assign the QDateTime(Display porposes only) as UTC and convert it after that.

I'm submitting the fix, I've done some small tests and they went OK.

Best regards,

João

[RJ Skerry-Ryan (rryan)]

I can confirm the patch works. Thanks João -- added to Mixxx 1.11.0 since it's very low risk.

Also -- for future contributions, please set your editor to use tabs, not spaces (and 4-spaces for an indent). :)
Make sure to read the Mixxx coding guidelines:
http://mixxx.org/wiki/doku.php/coding_guidelines

[João Reys Santos (l29634)]

No problem, I've read the guidelines and misunderstood when I read "indents are 4 spaces", I'll keep that in mind for future patches ;)

[Swiftb0y (swiftb0y)]

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/6610