Distance issue clean#322
Open
Carlixgonzam wants to merge 8 commits into
Open
Conversation
* 3.6.6 prerelease = false * Fix multi-sport activity import (tcgoetz#289) Multi-sport activities (triathlon / brick sessions) are stored by Garmin as a single FIT file containing multiple `session` messages plus child sessions for transitions. Previously the importer wrote every session under the bare parent activity_id, so the last session silently overwrote all the others and laps/records/HR zones/devices ended up attached to the wrong activity (or dropped entirely when the hr_zones_timer processing order created empty rows that blocked the real lap writes). This change detects multi-sport files by checking `len(fit_file.session) > 1` and, when detected: - creates one activity row per session with a suffixed id `<parent>_<n>` (e.g. 22638574127_1 ... _5 for swim→T1→run→T2→swim) - derives each session's `stop_time` from `start_time + total_elapsed_time` (Garmin reuses the parent start_time as every child's `timestamp`, which would otherwise result in stop_time <= start_time) - partitions laps by session using each session's `num_laps` and rewrites `lap_num` to be session-local (0..N per session), so every child has its own contiguous lap sequence - partitions records (GPS trackpoints) by matching their timestamp against each session's time window - remaps session/lap HR-zone writes to the correct child activity_id - associates device entries with every child activity `_write_lap_entry` was also switched to `ActivityLaps.s_insert_or_update` so lap data correctly coalesces with the hr_zones_timer rows that the generic message dispatcher creates earlier in the import; the same `stop_time` correction applies when the file is multi-sport. Single-session FIT files go through the exact same code path as before. Extends and supersedes the single-session subset fixed in tcgoetz#310 (same root cause for `stop_time`, lap-ordering race, and the add-vs-insert-or-update issue). Fixes tcgoetz#289 --------- Co-authored-by: Tom Goetz <tcgoetz@gmail.com> Co-authored-by: Tom Goetz <15090876+tcgoetz@users.noreply.github.com>
* Add PostgreSQL backend support
garmin_connect_config_manager.get_db_params builds a DbParams for
db.type='postgresql', reading user/password/host/port/database from
the JSON config. GarminConnectConfig.json.example shows the new keys.
Also fix two pre-existing sqlite-only constructs surfaced by the
postgres backend:
- activities_db._create_sport_view / _create_course_view: use SQL '='
and single-quoted literals (sqlite tolerates '==' and double-quoted
strings; postgres reads "foo" as an identifier).
- summarydb.summary_base.__create_weeks_months_years_selectable: the
per-period view selected sweat_loss_avg twice. Postgres rejects
duplicate output columns. Second slot was meant to be sweat_loss.
* Fix import-side type issues exposed by postgres backend
Four bugs surfaced when running `--import` against the new postgres
backend. Sqlite was permissive enough to mask each one; postgres is
not. Fixed in place — sqlite and mysql users see no behaviour change.
- Device.serial_number is declared `Integer` but Garmin's Descent Mk3i
family reports serials > 2**31 (e.g. 3476167836). The hard-coded
fallback `unknown_device_serial_number = 9999999999` would also
overflow on a strict 32-bit backend. Switch to BigInteger on the
primary key and on the matching FK columns in DeviceInfo / File and
on activities.device_serial_number / activity_laps.device_serial_number.
- monitoring_info.file_id was `Integer`, but the value passed in is the
string id from `File.id` (which itself is `String`). Sqlite quietly
coerced "269351985837" to a numeric value; postgres rejects the
string-to-int4 cast outright. Switch monitoring_info.file_id to
`String` so the FK shape matches `File.id`.
- monitoring_fit_file_processor.__unpack_tuple only unpacked when
type(value) is tuple, but fitfile yields per-activity-type arrays as
plain `list`. The list bled through to a `Float` column and the
failure poisoned the SQLAlchemy session for every subsequent insert
in the same file. Accept both list and tuple via isinstance().
* Plug remaining import-side gaps + include today in --latest
Three small bugs hit while running --import + --download --latest
against the postgres backend; bundled here because they share the same
"sqlite tolerated, postgres does not" pattern.
- scripts/garmindb_cli.py __get_date_and_days set days = (today - date).days,
which omits today. With a one-day-ago anchor that pulled yesterday and
the day before but silently skipped today. Bumping both --latest and
--all branches by 1 includes today; when upstream has nothing yet the
API returns empty and the importer is a no-op for that day.
- garmindb/fit_file_processor._write_device_info_entry treated a serial
of None as "unset" (and synthesised an id from parent + device_type)
but kept the literal 0 that fitfile emits for an invalid/unset serial.
Multiple local sensors per FIT then collide on a shared 0 PK, the
failure poisons the session, and the rest of the file is dropped.
Treat 0 the same as None on SourceType.local.
- garmindb/garmindb/garmin_db.Device.local_device_serial_number returned
a string ('%s%06d' % ...); the value lands in an Integer column. Wrap
in int() so postgres does not have to coerce.
* Update garmindb_cli.py
---------
Co-authored-by: no84by <silvius@duck.com>
Co-authored-by: Tom Goetz <15090876+tcgoetz@users.noreply.github.com>
replaced the s_exists + add pattern in _write_lap_entry with s_insert_or_update(ignore_none=True) so that existing lap rows are updated on re-import rather than silently skipped. This preserves legitimate zero-valued fields such as distance=0.0 for rest laps, and adds the missing min_temperature mapping to both _write_lap_entry and _write_session_entry
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
while investigating the issue, I noticed that min_temperature was defined in ActivitiesCommon and therefore available in both ActivityLaps and Activities, but it was never populated from the FIT file.
this PR adds the missing mapping so the value is persisted whenever it is present in the FIT activity data.
Background
issue #317 originally reported incorrect lap persistence for some FIT activity imports. While tracing the import pipeline and reviewing how lap data is written, I found that min_temperature was already part of the database schema but was omitted from both _write_lap_entry and _write_session_entry.
As a result, even when the FIT file contained this information, it was never stored in the database.
Changes
This PR adds the missing field mapping in two places:
_write_lap_entry
'min_temperature': message_fields.get('min_temperature'),
_write_session_entry
'min_temperature': message_fields.get('min_temperature'),
The field is inserted alongside the existing temperature mappings (max_temperature and avg_temperature), keeping the implementation consistent with the rest of the activity import code.
During the investigation I also noticed that FIT length messages are currently ignored because _write_length_entry is inherited from FitFileProcessor and is effectively a no-op.
Supporting pool swim length data would require introducing a dedicated ActivityLengths table and a database schema migration, which is a separate feature and intentionally outside the scope of this PR.
Verification
Confirmed that min_temperature is now persisted for both activity_laps and activities whenever the FIT message contains the field.
Verified that the resulting diff against develop only contains the two new mappings.
Checked that the modified file parses correctly and introduces no new lint warnings.