Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure FIT-to-TCX handles all test files from python-fitparse correctly #13

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

dlenski
Copy link
Contributor

@dlenski dlenski commented Oct 1, 2021

This PR adds dtcooper/python-fitparse as a submodule so we can test against all of its test files. Many of these were not correctly handled by FIT-to-TCX previously.

Then it fixes the resulting issues:

  • Handle optional/missing fields better
  • Handle files which only have local/relative timestamps
  • Readable error for FIT files which contain no activity session message

Also, attempting to parse more of the test files explores more of the branches, and thus increases coverage.

@coveralls
Copy link

coveralls commented Oct 1, 2021

Coverage Status

Coverage increased (+1.7%) to 90.452% when pulling 6cc441b on dlenski:master into 8988a17 on Tigge:main.

…t ALL of its test files

The python-fitparse library is a rich source of test files, many of which
are not correctly handled by FIT-to-TCX currently.
Almost all of the lap fields can be missing (even end-time!); in these
cases we should omit the resulting TCX element entirely, rather than use a
default value, and certainly we shouldn't raise an exception due to being
unable to convert None to float.

Also, some manufacturer and product IDs are not (yet) found in fitparse, and
thus only available in numeric form, which must be coerced to string.

For an example of both of the above problems, see FIT files generated by
recent versions of the Strava Android/iPhone apps.
In some files, perhaps created for non-GPS activities or by non-GPS devices,
all timestamps in laps and trackpoints are expressed as integer offsets.
(Probably an offset in seconds relative to when the device was turned on.)

The only "real" timestamp available is in the 'activity' message field
called 'local_timestamp', which represents the device's time in its *local*
timezone.

To process such files, we must use 'activity.local_timestamp' along with
'activity.timestamp' to calibrate the offset between the integer timestamps
and Unix epoch seconds.  However, even after doing this we *do not know* the
correct timezone for the resulting timestamps.  We emphasize this in the
ISO8601-format timestamps in the TCX output, by not appending 'Z'.

'tests/files/antfs-dump.63.fit' from dtcooper/python-fitparse is a good
example of this issue.  It contains data from a heart-rate monitor device
without GPS.
'tests/files/Workout*.fit' from dtcooper/python-fitparse are good examples
of this.
dlenski added 3 commits April 13, 2023 14:42
This setup.py is just a shim to allow local installation from source
("pip install .") to continue working with setuptools<61.0.0.

setuptools(>=61.0.0) adds standalone support for installation from
pyproject.toml only, with no setup.py shim needed (see
https://drivendata.co/blog/python-packaging-2023)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants