search

GoldenCheetah / GoldenCheetah

Performance Software for Cyclists, Runners, Triathletes and Coaches
http://goldencheetah.org/
GNU General Public License v2.0
1.84k stars 447 forks source link

Importing Garmin multisport fit files produces a single activity #3211

Closed thebaron06 closed 4 years ago

thebaron06 commented 5 years ago

Importing a multisport fit file produced by a Garmin device causes GC to create only a single activity that has a wrong Sport Tag. It gets the tag of the last extracted session. For Triathlons this is a run activity. This in turn leads to wrong analysis/metrics/...

This was already implemented and merged, see https://github.com/GoldenCheetah/GoldenCheetah/pull/1397.

The expected behavior is that GC splits the sessions included in a single multisport file into several activities. Best case would be that there is a mechanism that allows to link them together.

amtriathlon commented 5 years ago

IIRC that change was reverted due to its dependence on session header position, see https://github.com/GoldenCheetah/GoldenCheetah/commit/47d19d6f19c2062d21b0048c6b948c4b6c226363 PS: Current import functionality requires user manipulation as documented in: https://github.com/GoldenCheetah/GoldenCheetah/wiki/FAQ-RUNNING-&-SWIMMING#how-to-deal-with-multisport-activities

amtriathlon commented 5 years ago

Here you have a more detailed description of the problems with the code in #1397 and the reason it was reverted: https://github.com/GoldenCheetah/GoldenCheetah/issues/1424#issuecomment-118408146

thebaron06 commented 5 years ago

Oh, should have read the spec first. -.- Thanks for the hints.

But, if we already know that Garmin files are working like this wouldn't it be a good idea to support at least files from them? We already know the vendor/product of the file. We can easily add a check in there to support only Garmin created files for this feature. I do think this would fit almost all use-cases as the following statistics for sports watches used at ironman kona shows [1] [2]. I think it is needless to mention that kona is one of the most important event for any triathlete in the world.

2017 Garmin 1869 Polar 91 Suunto 20 Timex 13 Tomtom 9 Smart Watches 5

2016 Garmin – 1673 Polar – 85 Suunto – 47 Tom Tom – 24 Timex – 16 Fitbit – 1

[EDIT]: I had a quick overview of the fit file spec (Rev. 2.2, D00001309 FIT File Types Description Rev 2.2.pdf) and at page 52, Figure 9-2 it seems that - as with interleaved file format - session entries appear too in the non-interleaved format. @Joern-R is that true for the o_synce files too?

thebaron06 commented 5 years ago

@amtriathlon can you tell me if there is any plan on what functionality should be supported for such a feature? I had a quick look at the fit file spec and the activity spec mentioned earlier and I also had a look at the SDK code provided by Garmin. What if we replace the custom variant recently used and import the Garmin (thisisant) SDK code to parse fit files. IMHO the license would be easy to fulfill and the project's usage and support for fit files already implies to agree with their license, so why not use their code to deal with such file? It would also make it easier to maintain and/or upgrade fit file {en|de}coding. If I would work out such a solution and try to figure out on how to best add support for multisport files and how to split the sessions in either ways, would that be accepted? Alternatively - of course - we can implement the multisport support on basis of the recent fit file code. @Joern-R can you provide the O_SYNCE multisport files you mentioned here for testing purposes?

amtriathlon commented 5 years ago

@amtriathlon can you tell me if there is any plan on what functionality should be supported for such a feature?

Basically it doesn't break any existing functionality. Previous solution to this issue didn't allow the import of multi session fit files with non-interleaved session headers.

I had a quick look at the fit file spec and the activity spec mentioned earlier and I also had a look at the SDK code provided by Garmin. What if we replace the custom variant recently used and import the Garmin (thisisant) SDK code to parse fit files. IMHO the license would be easy to fulfill and the project's usage and support for fit files already implies to agree with their license, so why not use their code to deal with such file? It would also make it easier to maintain and/or upgrade fit file {en|de}coding. If I would work out such a solution and try to figure out on how to best add support for multisport files and how to split the sessions in either ways, would that be accepted?

The SDK source code could not be redistributed in an open source project when GC was developed, here is an explicit answer from some years ago: https://www.thisisant.com/forum/viewthread/759

Alternatively - of course - we can implement the multisport support on basis of the recent fit file code.

We are open for this, but the FIT importer is such a critical component that the less changes and more testing we do, the better.

BTW, these are just my opinions, I am not the GC project leader.

amtriathlon commented 4 years ago

I've added some test files from https://www.dcrainmaker.com/2013/05/navi2coach-cycling-computer.html for testing.

amtriathlon commented 4 years ago

After looking in more details to this problem and doing some tests my preliminary conclusion is the simple approach of #1397, even when a good starting point, has severe limitations in the current version: there are too much loss of information: device information, all XData series and HRM for swims, to name just a few. A complete solution will require several changes and I think we should postpone for the next version.

Importing a multisport fit file produced by a Garmin device causes GC to create only a single activity that has a wrong Sport Tag. It gets the tag of the last extracted session.

This is easy to fix and worth including in v3.5: if the Sport tag changes due to more than one Session record with different Sports we can safely tag the activity as a Multisport one and this can be used to avoid stats distorsions and as indication to the user to split it using Split Activity Wizzard.

amtriathlon commented 4 years ago

The wrong sport part is fixed, lets keep this open for the automatic split functionality still pending, in the meantime it can be done manually using the Split Activity Wizard as suggested in https://github.com/GoldenCheetah/GoldenCheetah/wiki/FAQ-RUNNING-&-SWIMMING#how-to-deal-with-multisport-activities

thebaron06 commented 4 years ago

I'm glad to have that much feedback, bit recently I've been busy, so this will have to wait. Looking forward to continue work in a couple of days again.

liversedge commented 4 years ago

Is this something you want to work on still - it is now in the backlog for v3.6...

thebaron06 commented 4 years ago

3317 feels pretty complete to me. I just wanted to wait for some review/comments on that before removing the WIP state.

There are two small things that might be added:

The subsport thing is easy, but I do not know where the wrong altitude comes from.

It is also worth mentioning it again that this patchset does change the default sport/subsport for files w/ an unknown sport/subsport tag since this has an impact on all metrics calculated. This might be removed but personally I would prefer not to take anything into account we are not sure about the fact it really belongs to some sport.

liversedge commented 4 years ago

Thanks, I will defer to Ale on this.