AutoDJ Timestamp displays GMT

[mixxxbot]

Reported by: david-early
Date: 2012-08-14T05:06:55Z
Status: Fix Released
Importance: Wishlist
Launchpad Issue: lp1036492
Tags: autodj, easy, weekend
Attachments: timestampAsLocal.patch, timeStampAsLocal.patch

---

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.

[mixxxbot]

Commented by: l29634
Date: 2013-04-20T01:39:43Z
Attachments: timestampAsLocal.patch

---

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.

[mixxxbot]

Commented by: daschuer
Date: 2013-04-20T22:01:54Z

---

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

[mixxxbot]

Commented by: l29634
Date: 2013-04-20T22:32:07Z

---

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

[mixxxbot]

Commented by: rryan
Date: 2013-04-20T23:49:17Z

---

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

[mixxxbot]

Commented by: daschuer
Date: 2013-04-21T07:13:36Z

---

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

[mixxxbot]

Commented by: l29634
Date: 2013-04-21T13:36:42Z

---

Hi all,

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

Best regards,

João

[mixxxbot]

Commented by: l29634
Date: 2013-04-22T20:46:53Z

---

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

[mixxxbot]

Commented by: rryan
Date: 2013-04-22T21:24:23Z

---

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 .cpp qt gitorious

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

[mixxxbot]

Commented by: l29634
Date: 2013-04-22T22:22:26Z

---

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

[mixxxbot]

Commented by: l29634
Date: 2013-04-22T22:49:31Z
Attachments: timeStampAsLocal.patch

---

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

[mixxxbot]

Commented by: rryan
Date: 2013-04-23T17:29:36Z

---

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

[mixxxbot]

Commented by: l29634
Date: 2013-04-23T17:35:14Z

---

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 ;)

[mixxxbot]

Issue closed with status Fix Released.