Skip to content

Fix SaveCurrentTrackTask GPX write and error reporting#25198

Open
dmvkosm wants to merge 2 commits into
osmandapp:masterfrom
dmvkosm:fix/save-current-track-write-errors
Open

Fix SaveCurrentTrackTask GPX write and error reporting#25198
dmvkosm wants to merge 2 commits into
osmandapp:masterfrom
dmvkosm:fix/save-current-track-write-errors

Conversation

@dmvkosm

@dmvkosm dmvkosm commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Write each GpxFile from collectRecordedData() instead of always writing the task constructor instance
  • Propagate write failures to SaveGpxListener.onSaveGpxFinished(Exception)
  • Use mkdirs() for the target directory

Related issues

#25076 (silent save failure when finishing track recording)

Test plan

  • Finish recording a track with storage available — file appears
  • Simulate write failure (read-only dir) — listener receives error, path not cleared

@dmvkosm
dmvkosm force-pushed the fix/save-current-track-write-errors branch from 35736f4 to f6f9e99 Compare June 3, 2026 20:50
@Chumva

Chumva commented Jun 5, 2026

Copy link
Copy Markdown
Member

This fixes the core bug of writing the wrong GpxFile, but it breaks existing callback users. Several callers use the original gpxFile.getPath() after onSaveGpxFinished() to share/export the saved current track. With this change, only the collected per-file gpxFile gets its path set by writeGpxFile(), while the constructor gpx can remain empty, so callers may end up with new File("").

The task should return/set the actual saved output file path for the callback flow, or the callers need to be updated to use the saved file(s). Also if (error == null && gpx.getPath().isEmpty()) gpx.setPath("") is a no-op now

dmytromakariev-gl and others added 2 commits June 5, 2026 16:27
collectRecordedData() returns per-file GpxFile instances but the task always wrote the constructor gpx. Write failures were ignored and onSaveGpxFinished(null) was always called. Write each map entry, use mkdirs(), and propagate the first IOException to SaveGpxListener. Related to osmandapp#25076.
Co-authored-by: Cursor <cursoragent@cursor.com>
@dmvkosm
dmvkosm force-pushed the fix/save-current-track-write-errors branch from f6f9e99 to a4d415f Compare June 5, 2026 13:28
@dmvkosm

dmvkosm commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Updated: SaveResult now sets gpx.setPath(savedPath) before onSaveGpxFinished(), then clears path only for cache-dir saves after the callback (share/export flow).

@Chumva

Chumva commented Jun 19, 2026

Copy link
Copy Markdown
Member

The error propagation is a good fix, but this version introduces a couple of regressions.

collectRecordedData() can return multiple date-split GPX files, while the task stores only the last savedPath. Export/share callers read only gpx.getPath(), so multi-day recordings may export only the last saved part.

Also, when the input gpx already has a path, this can replace it with tracksDir/<date>.gpx, changing the caller’s original path unexpectedly.

I think this task should either write the passed GpxFile once for export/share, or delegate full recorded-track persistence to saveDataToGpx() instead of partially duplicating it.

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.

3 participants