Patch: garmin_fit: warn instead of fail on unsupported protocol version

[frief]

Files conforming to newer protocol versions should be (at least partly) decodable with programs that support older versions. Successfully converted Garmin Edge 520 FW 12.20 fit files protocol version 2.0 to gpx with lat, lon, ele, time, speed, wpt.

Thanks for gpsbabel!

0001-garmin_fit-warn-instead-of-fail-on-unsupported-proto.patch.txt

[robertlipe]

This seems very dangerous as fit has changed in incompatible ways before. I think I'd rather raise the level than turn off the error.

To do this, please modify your patch to include a representative (trimmed is preferable) .fit file for reference/track/ and an entry in test.d/fit.test so we can test this going forward.

Thanx, RJL

On Mon, Oct 2, 2017 at 11:49 AM, frief notifications@github.com wrote:

Files conforming to newer protocol versions should be (at least partly) decodable with programs that support older versions. Successfully converted Garmin Edge 520 FW 12.20 fit files protocol version 2.0 to gpx with lat, lon, ele, time, speed, wpt.

Thanks for gpsbabel!

0001-garmin_fit-warn-instead-of-fail-on-unsupported-proto.patch.txt https://github.com/gpsbabel/gpsbabel/files/1349616/0001-garmin_fit-warn-instead-of-fail-on-unsupported-proto.patch.txt

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/gpsbabel/gpsbabel/issues/90, or mute the thread https://github.com/notifications/unsubscribe-auth/AIUh71POAWHb4VpV_FWnuqAkkiJW-vnfks5soRQogaJpZM4Pq7Tu .

[frief]

Thanks, please find the updated patch with the requested changes here https://gist.github.com/frief/fc7fffb0b855d94df5ce645a5db4374b The .fit file includes laps and was recorded with heartrate, power (with cadence), speed and Shimano Di2 gear shifting data. (Just drop a note if not ok)

[xmedeko]

I vote for this bug since more and more devices generates FIT 2.0.

[tormet]

Hi, I have some code ready to support FIT 2.0. At least in the way that it read all the data we already get for FIT version 1 files. For the tests I still have to record a track again. The one I have, started at my front door. I want to avoid this. Then I have to do a last code review by myself and then I can do the pull request. This week I am very short of time. But I hope I get the things done before the weekend.

In some way I understand your reasoning with the warning. If the old code can handle the data (because it does not use "special things" from version 2.0) you get a result. If it cannot handle the data while converting it, it is the same as it give you the error that it does not support version 2.0. But if it gives you a warning you also have the possibility that it will write wrong data. Who knows this exactly, even if I do not expect it in practice. And having code that write wrong data is something one wants to avoid.

For me e.g. the old code throws an error while converting data because the Wahoo Element Bolt uses so called developer fields which are not handled in the current implementation.

I hope this helps a little bit.

[xmedeko]

@tormet It's great you have a patch and thanks for the analysis that developer fields may cause a crash.

You are generally right about dangers reading unsupported version. I think the idea from @robertlipe solves it - some input (command line) parameter like "force", "ignore version", etc would solve it. Then it would be up to the user to take the risk of unsupported version or not. It may be implemented even with your patch. Who know, maybe tomorrow the new version of FIT will be released ...

[tormet]

@xmedeko I agree concerning the idea of a command line parameter like "force", "ignore version" ....

If it is only a flag for the garmin_fit format the implementation seems to be easy. If it should be a global flag for "everything" one has to figure out where it should have a meaning.

[robertlipe]

Fixed in #163