ReviewOS

stacks/ts-watches

downloadActivityFit only writes to a directory, so a server has no way to get the bytes

#12
Closed glennmichael123 opened this 8 days ago ·
8 days ago

GarminConnectClient.downloadActivityFit(activityId, outputDir) is the only route to an activity's GPS through this package, and it can only put the file on a disk:

async downloadActivityFit(activityId: number, outputDir: string): Promise<void> {
  this.ensureLoggedIn()
  await this.client.downloadOriginalActivityData({ activityId }, outputDir, 'zip')
}

A server that wants to parse the recording — rather than keep it — has to make a temporary directory, let the library name the file, read it back and clean up:

const dir = await mkdtemp(join(tmpdir(), 'hq-garmin-'))
try {
  await client.downloadActivityFit(activityId, dir)
  const [name] = await readdir(dir)          // the name is Garmin's to choose
  const bytes = new Uint8Array(await Bun.file(join(dir, name)).arrayBuffer())
  ...
}
finally {
  await rm(dir, { recursive: true, force: true })
}

Three things that costs:

  • Nothing is learned from the filesystem. The bytes are parsed in memory and thrown away; the disk round-trip exists only because the signature requires it.
  • The caller cannot know the filename, so it has to read the directory and take whatever is there. That is fine for a directory it just made and wrong for any directory it did not.
  • It leaks on the error path unless every caller remembers the finally. A sync loop that forgets it fills /tmp an activity at a time.

Why the other methods are not an alternative

  • getActivityDetails maps to { seconds, heartRate, power, speed } — the GPS in Garmin's activityDetails response is dropped before the caller sees it.
  • getActivity returns records: [] (parseActivityResponse hardcodes it), so the Activity.records with position.lat/lng that toGeoJSON reads are never populated by the Connect client.

So the FIT download is the only way to a route, and it is the one method that cannot hand back what it fetched.

Suggestion

A sibling that returns the bytes, with the existing method kept as a thin wrapper over it:

downloadOriginalActivity(activityId: number, format?: 'zip' | 'fit'): Promise<{ bytes: Uint8Array, filename: string }>

That also makes the filename explicit rather than something the caller rediscovers with readdir.

Happy to send a PR if the shape looks right.

Found while adding GPS tracks to synced activities in hq.training: Garmin's activity list carries every summary number and no coordinates, so the original recording is the only source for a route.

8 days ago

Fixed in aedb79e, released in v0.1.3.

GarminConnectClient now has the sibling suggested here:

downloadOriginalActivity(activityId: number, format?: 'zip' | 'tcx' | 'gpx' | 'kml'): Promise<{ bytes: Uint8Array, filename: string }>
  • zip (the default) is Connect's "Export Original": the device's own recording, usually a .fit inside a zip. tcx/gpx/kml are Garmin's conversions. filename is {activityId}.{format}, so the caller no longer has to readdir for it.
  • Nothing touches the disk, so there is no temporary directory to create or leak on the error path.
  • downloadActivityFit(activityId, outputDir) is now a thin wrapper over it. It writes {outputDir}/{activityId}.zip and returns that path (it used to return void). The directory is created only after the download has succeeded, so a failed download leaves nothing behind.
  • In ts-garmin, getOriginalActivityData returns the bytes and downloadOriginalActivityData builds on it and returns the path.
  • HttpClient: an arraybuffer request now always gets bytes back. Before, a response labelled application/json was parsed instead.

Tests are in packages/ts-watches/test/garmin-original-activity.test.ts, run against a stubbed fetch. The API is documented in docs/api.md and docs/usage.md.

getActivityDetails still leaves out GPS and getActivity still returns records: []. This change doesn't touch either one, because the original recording remains the way to get a route.

Sign in to comment on this issue.