adriangibbons / php-fit-file-analysis

A PHP class for analysing FIT files created by Garmin GPS devices
MIT License
126 stars 49 forks source link

Wrong timestamp #33

Closed JacobDelcroix closed 9 years ago

JacobDelcroix commented 9 years ago

When decoding a .fit file with your lib I don't get real timestamp.

From your lib : 1995-10-03 From Strava : 2015-10-01 (good one) Timestamp : 812681141

Power, cadence, etc... are fine but I can't figure out why timestamp is wrong.

Do you have any clues ? I can send you the file if needed.

adriangibbons commented 9 years ago

Hi, I think "wrong" is a bit strong but I understand the confusion.

FIT timestamps are seconds since UTC 00:00:00 Dec 31 1989 (source FIT SDK) UNIX timestamps are seconds since UTC 00:00:00 Jan 01 1970

Hence when you convert a FIT timestamp without compensating for the difference it will give you an unexpected value. This was the decision of the owners of the FIT standard.

In the demos, you will see how I have compensated for the difference.

https://github.com/adriangibbons/php-fit-file-analysis/blob/master/demo/mountain-biking.php#L58

// FIT timestamps are seconds since UTC 00:00:00 Dec 31 1989, source FIT SDK
$date = new DateTime('1989-12-31', new DateTimeZone('UTC'));
$date_s = $date->getTimestamp() + $pFFA->data_mesgs['session']['start_time'];

Further down in the code for the mountain biking demo, I use the Google Timezone API to take into account the timezone offset.

Have a look at this quick example using the PHP Sandbox

The question I see is whether we should add the option to convert timestamps into UNIX ones. I guess we should and by default they should be converted to UNIX timestamps as this is what people are expecting to see?

JacobDelcroix commented 9 years ago

Thanks for your reply. Yes I think we should convert to UNIX timestamps too because almost everyone expect it. Or maybe this part should be more explicit in your documentation.

Regards.

adriangibbons commented 9 years ago

Ok, it should be relatively straightforward but I'll need a few days to do it properly and update the README.md etc as work is very busy

On 6 Oct 2015, at 4:06 PM, Jacob Delcroix notifications@github.com wrote:

Thanks for your reply. Yes I think we should convert to UNIX timestamps too because almost everyone expect it. Or maybe this part should be more explicit in your documentation.

Regards.

— Reply to this email directly or view it on GitHub.

adriangibbons commented 9 years ago

timestamp is a common field and always field_number 253

4.7 Common Fields (Field#, Field Name, Field Type) Certain fields are common across all FIT messages. These fields have a reserved field number that is also common across all FIT messages (including in the manufacturer specific space). 4.7.2 Timestamp (Field # = 253, timestamp, date_time)

image

In addition, there are messages with fields besides timestamp with the date_time or local_date_time type:

image

For these fields, 631,065,600 will need to be added to their values. This is the number of seconds difference between FIT and UNIX timestamps. It represents a difference of ~20 years.

adriangibbons commented 9 years ago

@JacobDelcroix I have made changes so that by default, timestamps are now in Unix time. Please could you test it out?

An option can be passed to continue using Garmin FIT timestamps:

$options = [
    // false by default!
    'garmin_timestamps' => true
];
$pFFA = new adriangibbons\phpFITFileAnalysis('my_fit_file.fit', $options);

I updated the README.md, demos and tests, as well as the src.

Cheers

JacobDelcroix commented 9 years ago

@adriangibbons I will give it a try this week, thanks. But I think you should let garmin timestamp by default and add option for UNIX timestamp because if someone update your lib, his script will be "broken".

adriangibbons commented 9 years ago

I hear you, but I think it is better long-term if the class defaults to Unix time rather than people having to select it as an option. I sense that there are not thousands of users of this project out there so hopefully the pain of this change can be tolerated - the easy fix is to set the garmin_timestamps option to true