Skip to content

Feature/gpx export optimisations - #948

Merged
AgreeDK merged 3 commits into
OpenSAK-Org:betafrom
nagisml:feature/gpx-export-optimisations
Oct 1, 2026
Merged

AgreeDK merged 3 commits into
OpenSAK-Org:betafrom
nagisml:feature/gpx-export-optimisations

Conversation

@nagisml

@nagisml nagisml commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

nagisml and others added 3 commits September 30, 2026 21:40
…er tests

The quick-filter/filter-apply refresh runs on a background RefreshWorker
(OpenSAK-Org#740), so a 50ms qtbot.wait() raced the query on slow CI runners
(test_quick_filter_found_returns_zero saw 4 rows instead of 0).

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

AgreeDK commented Oct 1, 2026

Copy link
Copy Markdown
Member

@nagisml
Thanks, this looks really good! 👍

I've tested it locally, both on its own and merged on top of beta together with #947 and #949: the full unit suite passes (4410 tests) and mypy is clean.

What I like:

  • FileExportSettings.from_dict() falls back to defaults for missing or invalid keys, so saved settings keep loading when new options are added later. Nicely thought through.
  • The e2e fix uses the existing wait_for_refresh() helper from tests/data.py. That's the right approach, see below.

One observation, not a blocker: the new options (corrected coords on/off, max records) are only in the file export dialog, not in Send to GPS (gps_dialog.py). That's fine as scope for this PR, but users will probably ask for the same options there, so it might be worth a follow-up issue.

Merge order: all three of your open PRs (#947, #948, #949) change the same flaky test in tests/e2e-tests/test_e2e_filter.py, each in a slightly different way, so only the first one merges cleanly. Since this PR's version is the best one (it reuses wait_for_refresh()), I plan to merge this one first. I'll comment on the other two.

@AgreeDK
AgreeDK merged commit a2b9749 into OpenSAK-Org:beta Oct 1, 2026
7 checks passed
@nagisml

nagisml commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor Author

@AgreeDK Regarding Send2GPS. Why should we duplicate the GPX/... export settings? I would propose to link the Send2GPS towards a Saves GPX Export setting and allow no own configuration an all.

This was the reason why I started with the GPX Export

@AgreeDK

AgreeDK commented Oct 1, 2026

Copy link
Copy Markdown
Member

@nagisml

@AgreeDK

AgreeDK commented Oct 1, 2026

Copy link
Copy Markdown
Member

@nagisml
Good point, I agree: one place to configure what goes into an export is better than two near-identical sets of options.

One nuance: Send to GPS already has some options of its own in gps_dialog.py, and they're of two kinds:

  • Content (what goes into the file): corrected coordinates, max caches, and later things like child waypoints, number of logs, name format. These should come from the saved export profile, as you suggest, and the existing max-caches spinbox in Send to GPS should go, so we don't end up with two of them.
  • Delivery (how it gets to the device): device vs. file, use database name as file name, delete old GPX files, and GPX vs. GGZ (LOC doesn't make sense for a Garmin). These are specific to Send to GPS and I'd keep them there. The profile's format setting would simply be ignored in this case.

One more detail: I think Send to GPS should remember its own selected profile, rather than following the file export's "last used". Otherwise a file export with max 100 caches would silently limit the next send to the GPS as well.

Does that split make sense to you? If so, feel free to open an issue for it and go ahead.

@nagisml

nagisml commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

@AgreeDK my idea related to your last comment

  • GPX Export should contain all content features. So when it's complete we have to migrate the Send2GPS features over
  • Delivery are most likely needed on both sides. but ...
  • I would always see that a caller (here Send2GPS) as master and decide if it overwrites a values which is stored in the slave (here the GPX Export)

Meaning:

  • Send2GPX may overwrite the folder always to the Temp folder
  • Send2GPX may keep the dynamic filename of the export

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