Repository navigation
Read Garmin FIT activities into the geospatial formats (#451) - #530
Conversation
3cc9178 to
255a808
Compare
KnifeLemon
left a comment
There was a problem hiding this comment.
@SeoJCalive 님, 꼼꼼한 PR 감사해요. 리더가 헤더·파일 CRC, 압축 타임스탬프 헤더(롤오버 포함), 개발자 필드 건너뛰기, 무효값, semicircle→도, 1989-12-31 기준 시각을 프로토콜대로 처리하고 있어요. 생성한 테스트 파일과 변형 테스트로 하나하나 확인해 주셔서 리뷰하기 편했어요. 스키마 47도 main의 46 다음 번호가 맞고, 새 문구 9개도 16개 언어 파일에 모두 들어 있어요.
먼저 질문에 답할게요.
- 필드 번호: Profile에서 번호·스케일·이름만 가져오고 SDK 코드나 파일을 넣지 않았다면 괜찮아요. "공개된 프로토콜 설명으로 직접 작성"에 해당해요.
- KML에서 높이가 일부 없을 때: 지금처럼
LineString에coordTimes로 써 주세요.gx:coord는 경도·위도·높이 세 값이 정해진 형식이라 두 값으로 쓰면 안 돼요. - GPX 알림: 시작 시간이 첫 점과 다르다는 이유로는 알림을 띄우지 말아 주세요. 점마다 시간이 남아 있어서 실제로 잃는 건 거의 없는데, 실제 시계 파일마다 뜨면 소음이 돼요. 점 시간이 하나도 없는데 세션 시작 시간만 있을 때처럼 시작 시간이 정말 사라지는 경우에만 지금 알림을 남기면 돼요. FIT 전용 문구는 필요 없어요.
Gpx_only_reports_a_different_or_unrepresentable_summary_start도 거기에 맞춰 주세요.
고칠 점이에요.
FitFormat.Read의 session(18) 개수에 제한이 없어요. 레코드는 2,000,000개 제한이 있는데activity.Sessions는 메시지마다 하나씩 늘어나요. 필드가 0개인 session 정의 뒤에 1바이트짜리 data 메시지만 반복한 파일을 만들어 보니, 16 MiB 파일에서 세션 16,777,216개, 메모리 835 MiB, 12.7초가 걸렸어요. 64 MiB 상한 파일이면 3 GB가 넘어요.ToDataset이 쓰는 건 가장 이른 시작 시간과 종목 목록(최대 256개)뿐이라서, 읽는 동안 이 두 가지만 모아 두면 리스트가 필요 없어요. 아니면 세션에도 개수 제한을 걸어 주세요. 이 경우를 막는 테스트도 하나 넣어 주세요.ReadNumber가 float32·float64의 NaN·Infinity를 값으로 돌려줘요.lat is < -90 or > 90범위 검사는 NaN을 거르지 못해요. 그래서 위치 필드를 float로 정의한 깨진 파일은 GPX에<trkpt lat="NaN" lon="22.5">를 그대로 쓰고, CSV에도NaN이 들어가요.double.IsFinite가 아닌 값은 무효값(null)으로 처리해 주세요.
이 두 가지와 GPX 알림만 정리되면 머지할 수 있어요.
255a808 to
636116e
Compare
…tice only when the start is lost Review on #530: - Sessions are no longer kept one by one: the reader keeps the earliest start and the sports in file order, so a file of empty session messages can't grow memory (16 MiB: 95 MB peak, 0.96 s). - float32/float64 NaN and infinities are invalid values, like the base type's invalid value. - The GPX notice shows only when no point has a time and the activity has a start; a start that differs from the first point's time no longer raises it.
|
@KnifeLemon 님, 리뷰와 답변 감사해요. 말씀하신 세 가지를 새 커밋(636116e)으로 고쳤고, 그사이 들어온 main(#550) 위로도 올렸어요.
새로 넣은 두 테스트와 바꾼 GPX 테스트는 고친 곳을 하나씩 되돌리면 각각 실패하는 것도 확인했어요. 다시 봐 주세요! |
|
@KnifeLemon 님, 리더를 다시 점검하다가 비슷한 문제를 두 가지 더 찾아서 지금 고치고 있어요.
곧 커밋 하나로 올릴게요. 그때까지 머지는 잠시만 기다려 주세요 |
…file width Found while checking the reader again after the review on #530: - A record without a single value is counted toward the record limit but not kept, and the FIT CSV rows are made as they are written. CsvFormat.WriteRows writes row by row to a file beside the target and replaces it only once every row is written; an existing read-only or kept file stays as it was. 2M records: CSV peak 1.4 GB -> 415 MB. - Fields are used only when the file defines them as integers of the FIT Profile width, so a float32 altitude of 3e38 or a uint32 heart rate no longer reaches the output.
|
@KnifeLemon 님, 리더를 다시 점검하다가 비슷한 문제 두 가지를 더 찾아서 새 커밋(3f3e65d)으로 고쳤어요.
실제 기기 파일 세 개(Garmin Fenix 2, Wahoo ELEMNT, Coros)는 CSV·GPX·KML·GeoJSON 출력이 고치기 전과 바이트까지 같아요. 새 테스트 세 개는 고친 곳을 되돌리면 각각 실패하는 것도 확인했어요. |
KnifeLemon
left a comment
There was a problem hiding this comment.
@SeoJCalive 님, 새 커밋 3f3e65d5만 다시 봤어요. 값이 하나도 없는 레코드를 세기만 하고 저장하지 않는 것, CSV를 한 줄씩 만들어 쓰는 것, 공용 CsvFormat.WriteRows를 대상 옆 임시 파일에 다 쓴 다음에만 바꾸는 것 모두 좋아요. 다른 호출부(데이터 → CSV, vCard·iCalendar → CSV, GeoCSV)도 출력 바이트(UTF-8 BOM, CRLF, 따옴표)가 그대로이고, 빈 표도 예전처럼 BOM만 있는 파일이 나와요. 임시 파일이 같은 폴더라 교체는 같은 볼륨 안의 이름 바꾸기이고, 감시 폴더는 점으로 시작하는 이름과 .tmp를 건너뛰어서 다시 변환될 일도 없어요. 실패하면 있던 파일이 그대로 남는다는 것도 테스트로 확인돼 있고요. Windows에서 FIT/CSV/Data/Geo 관련 테스트 556개가 통과했어요. 공용 CSV 변경은 작고 FIT 메모리 때문에 필요한 거라 이 PR에 그대로 두셔도 돼요.
작은 것 세 가지만 봐 주세요.
src/Filee.Engines/Office/Sheets/CsvFormat.cs:96:catch안의File.Delete(partial)가 실패하면(Windows에서 백신이 막 쓴 파일을 잠깐 잡고 있을 때 등) 원래 예외 대신 삭제 오류가 올라가고 임시 파일도 남아요.try { File.Delete(partial); } catch (Exception e) when (e is IOException or UnauthorizedAccessException) { }로 감싸 주세요. 81행의 임시 이름도JobQueue.ReplaceOriginal처럼.{이름}.{8자리}.filee-tmp로 맞춰 주시면 좋겠어요. 지금 형식은 이름이 38자 늘어서, 아주 긴 출력 이름(구성 요소 255자 한도 근처)은 예전엔 되던 게 실패할 수 있어요.src/Filee.Engines/Geo/FitFormat.cs:239: 리더는 고도만 있는 레코드를 남기는데, CSV의HasRow(343행)는 그런 행을 빼요. 그래서 주석("어떤 출력에도 보탬이 없는 레코드는 남기지 않는다")과 달리 고도만 있는 레코드는 여전히 전부 메모리에 남아요. 리더의 조건을HasRow와 같게 맞추면WriteCsv의Any(HasRow)/Where(HasRow)도Records.Count == 0확인 하나로 줄일 수 있어요.src/Filee.Engines/Geo/FitFormat.cs:200: base type의 폭이 프로필 폭과 다르면 값을 버리는데, FIT은 정의 메시지의 base type으로 값을 읽어요. 그래서 다른 기기나 앱이 심박을 uint16으로, 고도를 uint32로 쓴 파일도 올바른 FIT이고, 지금은 그런 파일의 해당 열이 조용히 비어요. 막으려는 건 프로필 범위를 넘는 값이니 폭이 아니라 값으로 걸러 주세요. 예를 들어 프로필 타입의 최댓값(심박 uint8이면 254)을 넘으면 버리는 식이에요. 그러면 3e38 고도나 40억 심박은 똑같이 막고 정상 값은 남길 수 있어요.
#541(독일어)이 방금 머지돼서 이 PR의 새 문자열 키가 de.json에 없는데, 그건 저희가 채워서 올릴게요. 위 세 가지가 반영되고 CI가 통과하면 머지할 수 있어요.
A FIT activity becomes one track whose points keep their time and height: GPX, KML/KMZ (gx:Track, or LineString + coordTimes when some heights are missing), GeoJSON (coordTimes), Shapefile, and a nine-column CSV of the records (XLSX and JSON through it). FIT is read only, by Filee's own reader; damaged, chained, non-activity and position-less files are refused with their own messages. Settings schema 47 adds .fit to the geospatial donut of existing libraries.
…tice only when the start is lost Review on #530: - Sessions are no longer kept one by one: the reader keeps the earliest start and the sports in file order, so a file of empty session messages can't grow memory (16 MiB: 95 MB peak, 0.96 s). - float32/float64 NaN and infinities are invalid values, like the base type's invalid value. - The GPX notice shows only when no point has a time and the activity has a start; a start that differs from the first point's time no longer raises it.
…file width Found while checking the reader again after the review on #530: - A record without a single value is counted toward the record limit but not kept, and the FIT CSV rows are made as they are written. CsvFormat.WriteRows writes row by row to a file beside the target and replaces it only once every row is written; an existing read-only or kept file stays as it was. 2M records: CSV peak 1.4 GB -> 415 MB. - Fields are used only when the file defines them as integers of the FIT Profile width, so a float32 altitude of 3e38 or a uint32 heart rate no longer reaches the output.
…t uses
- Read each record, session and file_id field in the base type its definition
gives and use it only when it is a whole number within its Profile type's
range (short of the invalid value), so wider, signed or float definitions
of valid values are kept while out-of-range values, fractions and the sint32
invalid position written in another type are dropped.
- Keep a record only when a CSV row or a track uses it (a height alone is
counted toward the limit but not kept); WriteCsv only checks for no records.
- CsvFormat.WriteRows: name the partial file like JobQueue does
(.{name}.{8 hex}.filee-tmp) and let a failed cleanup never replace the
original error.
- Add the FIT strings to de.json.
3f3e65d to
a99f4f1
Compare
|
@KnifeLemon 님, 짚어 주신 세 가지를 새 커밋(a99f4f1)으로 고쳤어요. 그사이 머지된 독일어(#541) 위로도 올렸고, CI가 통과하도록
실제 기기 파일 세 개(Garmin Fenix 2, Wahoo ELEMNT, Coros)의 CSV·GPX·KML·GeoJSON 출력은 고치기 전과 바이트까지 같고, 레코드 200만 개 파일의 메모리도 지난번과 같은 수준이에요(CSV 82 / 385 / 414 MB). 임시 이름은 |
KnifeLemon
left a comment
There was a problem hiding this comment.
@SeoJCalive 님, a99f4f1 확인했어요. 세 가지 모두 말씀드린 대로예요. 임시 파일을 지우지 못해도 원래 예외가 그대로 올라가고 이름도 JobQueue와 같은 형식이 됐어요. 리더가 레코드를 남기는 조건도 HasRow와 같아져서 고도만 있는 레코드는 더 남지 않아요. 값 범위로 거르는 방식은 uint16 심박이나 uint32 고도는 살리고 3e38 고도, 40억 심박, 72.5 심박은 막아서 좋아요. de.json에 FIT 문구까지 넣어 주셔서 감사해요. Windows에서 FIT·CSV·Office·Geo 테스트와 Core 테스트가 모두 통과했어요. CI의 Windows build는 러너 문제(첫 실행은 시간 제한, 다시 실행은 .NET 설치 다운로드 오류)로 끝나지 못해서 저희가 다시 돌리고 있어요. CI가 통과하면 머지할게요.
What does this change?
Closes #451. Garmin FIT activities (
.fit, from a watch or bike computer) can now be converted to GPX, KML/KMZ, GeoJSON, Shapefile and CSV (and through CSV to XLSX and JSON), as planned in the issue.Opened as a draft for the questions below.
Questions for you
Profile.xlsx) and added no SDK code or files. Is that fine for "written from the published protocol description"?gx:Trackwith a<when>per point". When some points have no height, I write aLineStringwith the times incoordTimesinstead, so as not to make up an altitude forgx:coord. Or should it stay agx:Trackand write those points'gx:coordwith two values?Also decided this way (tell me if you'd prefer otherwise):
typeandtime, the names GPX uses.0for the missing ones.Screenshots
Linux under Xvfb with no window manager: the donut background is white and the toast sits top-left instead of bottom-right.
A check, not a Filee feature: Filee's GPX and CSV output for three real files, read back and plotted (track from
<trkpt>/<time>, height from<ele>, heart rate from the CSV):How was it tested?
dotnet format --verify-no-changes; Release build adds no warnings.dotnet teston main (e492ec2): Core 609/609, CLI 29/29, Engines 1708 passed / 7 failed. The failures are not from this change (details below). 38 new tests; mutation checks make them fail.Details: what each format keeps
recordmessages, in file order. A single position becomes a point. Laps and pauses don't split the track.session) astype; the start (the earliest session start, else the first record time) astime.<time>and<ele>on the points that have them, and the sport as<type>. The track has no start time field, so when the start differs from the first point's time (or the first point has none), the existing GPX notice shows.gx:Track(a<when>and agx:coordper point) when every point has a time and a height; otherwise aLineStringwith the point times in acoordTimesDataentry.LineStringwith acoordTimesproperty (nullwhere a point has no time).PolyLineZwhen every point has a height; when only some do, all heights are dropped with a notice. Point times are left out;typeandtimeare.dbfcolumns.time(UTC),longitude,latitude,elevation(m),heart_rate(bpm),cadence(rpm),power(W),speed(m/s),distance(m). Records without a position are kept, so an indoor ride converts to CSV.file_id; not an activity (courses, settings, monitoring…); no positions (map formats only, pointing to CSV/XLSX/JSON); no records.Details: implementation
Geo/FitFormat.cs(new: reader, dataset, CSV);GeoModel.cs(an optional time list onGeoLineString, kept through coordinate mapping, andGeoDataset.SparseHeights); the GPX, KML, GeoJSON and Shapefile writers;GeoConverter.cs(FIT as a read-only source, the notices);FormatRegistry.cs;UserDataStore.cs/AppSettings.cs; the 15 language files;docs/ENGINES.mdand the README format lines.0x0000means "not computed"), file CRC, normal and compressed-timestamp record headers (5-bit rollover), definitions in both byte orders, developer fields skipped, invalid values, scale and offset.enhanced_altitude/enhanced_speedwin over the 16-bit fields. Timestamps below0x10000000are relative (clock not set yet) and never become an absolute time.recordfields,file_id.type,session.start_timeandsport, and thesportandfilenames, checked against the Profile in the FIT JavaScript SDK 21.218..fitto the geospatial donut of existing libraries unless the user already put it elsewhere. feat(pdf): add PDF editing tools to the donut #428 took 46 meanwhile; it may need another bump if something else lands first.FitReaderTests(12: decoder against bytes built by a testFitBuilder, the published header CRC vector, byte order, compressed timestamps, invalid values, developer fields, every refusal, limits, fixed-seed random input, relative timestamps);FitFormatTests(16: through the real converter into every format, XLSX and JSON compared row by row with the CSV);GeoPointTimesTests(5: writers with and without point times, existing inputs unchanged);FitFormatRegistrationTests(4); one CLI test. Four existingUserDataStoreTestslist every geospatial extension, sofitis added there.Details: test results
PackageRepackerTests,SpreadsheetFormatTests.LibreOffice_reads_our_ods_…) fail on main too, because of the LibreOffice on my Linux.EngineProbeTests.A_program_that_never_exits_is_given_up_on_and_killedfailed once while another run loaded the machine and passed three runs on its own. The FIT and geo tests passed five runs in a row.0x0000case, removing the timestamp rollover, and removing the FIT → GPX edge each make the matching tests fail.<when>asgx:coord(Coros:LineString+coordTimes). GeoJSON:coordTimesas long as the coordinates. Shapefile:PolyLineZ(Coros:PolyLinewith the notice). The Fenix 2's jump from 55 m to 142.4 m and back in the first seconds is in the file itself (checked with fitdecode)..fit. An activity without positions is refused for the map formats and converts to CSV.--convertas the Explorer menu does, XTest clicks. A donut conversion that succeeds shows no notice at all (the donut hands a job to the toast only when it fails or closes mid-conversion; that is how every notice works today), so the toast screenshot closes the donut mid-conversion.Details: sample output (Fenix 2 run)
GPX:
KML (2,809
<when>and 2,809<gx:coord>):KML when some heights are missing (Coros):
GeoJSON:
{"type":"Feature","properties":{"type":"running","time":"2015-08-15T14:45:08Z","coordTimes":["2015-08-15T14:45:08Z",…]}, "geometry":{"type":"LineString","coordinates":[[5.7288390398,58.9591828175,55],…]}}CSV:
Details: follow-ups and an existing behaviour
TrackPointExtension, recovering files with a bad CRC.timeattribute (from GeoJSON, KML…) is dropped on GPX tracks without a notice. This PR doesn't change it.Checklist
src/Filee.App/Assets/i18n/*.jsonfilesTHIRD-PARTY-NOTICES.md(none)