Skip to content

Normalize missing FIT lap bounds and grade-adjusted speed units - #49

Merged
tabishm52 merged 2 commits into
tabishm52:mainfrom
shkyyy18:fix/fit-summary-unit-conversions
Oct 3, 2026
Merged

tabishm52 merged 2 commits into
tabishm52:mainfrom
shkyyy18:fix/fit-summary-unit-conversions

Conversation

@shkyyy18

@shkyyy18 shkyyy18 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Bug

The unbounded garmin-fit-sdk dependency currently resolves to 21.217.0. Its profile exposes five lap fields and one session field that are missing from the unit-conversion tables:

  • Lap nec_lat, nec_long, swc_lat, swc_long stay in semicircles rather than degrees.
  • Lap/session avg_grade_adjusted_speed stays in m/s rather than km/h.

This also causes the existing test_table_covers_installed_profile tests for lap and session to fail. For example, a synthetic lap parsed with include_all_columns=True returns nec_lat=536870912 instead of 45.0, and avg_grade_adjusted_speed=10.0 instead of 36.0.

Fix

Add the missing entries to the existing tables. No new dependencies, default output columns or public APIs; existing vertical-rate exceptions and raw FIT-native parsing remain unchanged.

Tests

  • Full suite passed on Python 3.12 with garmin-fit-sdk 21.217.0 and pandas 3.0.6.
  • All configured pre-commit hooks passed: Ruff checks/formatting, Pyright and pytest.

Prepared with AI assistance; all stated tests were actually run using synthetic FIT data. Tests on this Windows machine used Python UTF-8 mode because existing README tests read UTF-8 text with the platform-default encoding.

shkyyy18 and others added 2 commits October 1, 2026 01:07
test_table_covers_installed_profile already pins these entries against the
installed profile, and existing tests cover the lap and raw conversion paths.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tabishm52

Copy link
Copy Markdown
Owner

Thanks for the fix! I pushed a small follow-up: sorted the new entries into the existing table order, bumped the SDK version in the module docstring, and dropped the new tests and README note. test_table_covers_installed_profile already pins these entries against the installed profile, and the existing laps/raw-parsing tests cover the conversion path.

@tabishm52
tabishm52 merged commit 5bc2e6d into tabishm52:main Oct 3, 2026
4 checks passed
@tabishm52 tabishm52 mentioned this pull request Oct 3, 2026
2 tasks done
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