VERY EXPERIMENTAL: refactoring storage of pause and resume by dennisguse · Pull Request #553 · OpenTracksApp/OpenTracks

dennisguse · 2020-12-22T20:52:24Z

For be able to recreate the TrackStatistics (export/import), we need to store some additional TrackPoints (aka track started, track ended) and these need to use the same base for timestamps (here device time; NOT GPS-provided time).

Cleanup:

gpsStatus.onLocationChanged(trackPoint); is called for each TrackPoint

Upgrade strategy:

Release new version of OSMDashboard (Dashboard API changes w/ versioning)
Release new version containing database upgrade/downgrade code only (first commit in this branch; version v3.14.2)
Release new version with all changes (version v3.15.0)
Add in release notes:
- that this upgrade may be harmful and tracks should be exported first
- that OSMDashboard needs to be upgraded to v2.9.0

dennisguse · 2021-01-01T19:13:21Z

@rgmf @pstorch
Happy new year ;)

If you have some time (and data to lose), here is a first version to change the internal storage of TrackPoints, so that we can restore the TrackStatistics completely (export/import).
So, if you tests this using a real database, please get a backup first.
Hope nothing burns....

PS: Downgrading from DB version 30 to version 29 does not restore deleted RESUME trackpoints.
So, upgrade->downgrade will result in changed data.
Don't know if this is really worth doing it.

Anyhow: 🤞, but keep a 🧯 handy 😎

pstorch · 2021-01-02T17:55:57Z

I think the update of the trackpoints table doesn't work. The latitude column is an integer not a double, so the where clause with 100.0 or 200.0 doesn't find anything.
When I look into the SQLite DB I see the type is still 0:

dennisguse · 2021-01-02T20:54:10Z

@pstorch Thx; one of those face palm moments ;)

pstorch · 2021-01-03T11:22:03Z

        db.execSQL("ALTER TABLE trackpoints ADD COLUMN type TEXT CHECK(type IN (-2, -1, 0, 1))");
-        db.execSQL("UPDATE trackpoints SET type = -2, latitude = NULL, longitude = NULL WHERE latitude = 200.0");
-        db.execSQL("UPDATE trackpoints SET type = 1, latitude = NULL, longitude = NULL WHERE latitude = 100.0");
+        db.execSQL("UPDATE trackpoints SET type = -2, latitude = NULL, longitude = NULL WHERE latitude = 200");


That's not gonna work either. It should be 200 * 1E6 😉

pstorch · 2021-01-03T11:48:36Z

PS: Downgrading from DB version 30 to version 29 does not restore deleted RESUME trackpoints.

I still don't get that "Downgrading". In which situation does this work? When I install the version from main branch (DB version 29), then the downgrade code from 30 to 29 is missing and the App is crashing. When I install the version from this branch it knows DB version 30 and doesn't downgrade either.

dennisguse · 2021-01-03T12:35:44Z

@pstorch Valid point, for a downgrade we would need to release a new version with version=29 that has the downgrade code for version 30. Then we could allow a real downgrade for a latter version.
So, technically we never allowed downgrades (with db changes) so far...
Thanks for pointing that out! Did not occur to me until know.

pstorch · 2021-01-04T10:17:36Z

The db migration to version 30 works now.

I did some tests with my 318 tracks in my OpenTracks (debug).

Before I started:

246 trackpoints with latitude 200
248 trackpoints with latitude 100
738161 "normal" trackpoints

Then I stumbled upon the migration bug, so I started over with the old version reimporting the 318 tracks in a clean db. Interestingly it was a bit different now:

219 trackpoints with latitude 200
219 trackpoints with latitude 100
738156 "normal" trackpoints

So I guess this export/reimport "cleaned" something up.

I installed the pauseResume branch and let the database migrate.

After that I got the same overall statistics:

and the trackpoints looked good:

219 of type 1
219 of type -2
738156 "normal" trackpoints.

I exported the 318 track after the migration.

Then I cleaned up the data and started over with importing the 318 files before the migration:

Now I got:

551 of type -1
737605 of type 0

🤔 what happened here?

The last test was reimporting the migrated files back to a clean database.

Looks like something bad happening here:

219 of type -2
219 of type 1
46127 of type 0

The pause/resume trackpoints match the initial values, but why are so many regular trackpoints lost?

pstorch · 2021-01-04T10:57:51Z

I've started a similar branch on the OSMDashboard https://github.com/OpenTracksApp/OSMDashboard/tree/pauseResume to read this new column.

Unfortunately the ContentProvider PROJECTION isn't backward compatible with older OT versions. As soon as I add the new type column I don't get any trackpoints from the old OT anymore. So I had to set the PROJECTION to null for the time being.

We could add a version attribute into the API (as an Intent extra), so that the OSMDashboard can decide which PROJECTION to use.

dennisguse · 2021-01-04T16:41:30Z

@pstorch Thanks for the test.
The migration does not update the trackstatistics (nor deletes tracks), so the migration in itself is only visible in the number of trackpoints and thus the aggregated stats will remain the same.
But there is an issue - I guess in the exporter; let me check.

About OSMDashboard integration: do you need to send a projection (i.e., non null)?
If yes, we can add a version attribute to the URI - no problem.

pstorch · 2021-01-04T18:11:25Z

About OSMDashboard integration: do you need to send a projection (i.e., non null)?

No, it's not needed. It works with null. The only drawback is that then all columns are transferred, more than needed. Don't know if this has a performance/memory impact.

dennisguse · 2021-01-05T16:29:17Z

@pstorch Adding a version as extra sounds good!

About the data import->export->import:
I checked the exporter and it looks fine.
And I am not really sure, how to proceed as in my simple test cases it is working as expected and with the original data, the imports look fine.
So, it must be some weird "feature" of export+import together and I am not sure, how to figure that one out.
Can you provide some test data?

pstorch · 2021-01-05T20:49:26Z

Very strange: I tried to reproduce the last testcase (reimporting the migrated and exported tracks). Now it worked and the aggregated stats and the db trackpoints had the correct values.

Sorry for the confusion.

rgmf · 2021-01-05T21:24:32Z

I get this error in 40 of my 161 tracks. I've not checked all the 40 files but the error is triggered in the files where there is a pause/restart:

Parser error: org.xml.sax.SAXException: Parsing error at line: 4380 column: 0.
Invalid location detected: time=1583051351000 (type=SEGMENT_START_AUTOMATIC(-1)): lat=100.0 lng=0.0

Here is the line when the error is triggered (with the context):

<trkpt  lat="100" lon="0">
<time>2020-03-01T08:29:11Z</time>
</trkpt>
<trkpt  lat="200" lon="0">
<time>2020-03-01T08:33:49.166Z</time>
</trkpt>

To reproduce:

Install this branch.
Export all tracks to GPX format.
Import all tracks.
Get this error.

Also, when you are recording then the intervals doesn't work. Maybe it needs to be refactored because intervals uses TrackPoints directly.

dennisguse · 2021-01-06T18:07:50Z

@pstorch If you can reproduce it again (or some other weird behavior), let me know.

@rgmf Those trackpoints should not actually not have been exported - they should have become segment open/close. We can now either ignore these - or try to restore the segment. But those trackpoints have device time while the rest has GPS time.

rgmf · 2021-01-07T08:36:16Z

@rgmf Those trackpoints should not actually not have been exported - they should have become segment open/close. We can now either ignore these - or try to restore the segment. But those trackpoints have device time while the rest has GPS time.

I see.

I guess it's not that important for now but I'd try to restore segments altough stats are not the same: better that than end up with errors.

I'll test import/export from this branch then ;)

So, far specific latitude for segment start/end (i.e., PAUSE/RESUME) was used. #316.

#316.

Existing data is not migrated. #316.

KML exports sensor data if actually present. #316.

…trackpoints were exported.

#316.

Fixes #561.

Location values are stored directly.

#316.

ExportImportTest were not checking TrackStatistics correctly. #316.

#316.

…restored. #316.

- read protocol version and choose right projection OpenTracksApp/OpenTracks#553

dennisguse · 2021-01-24T20:54:54Z

I missed to include the elevation_loss column in the db upgrade/downgrade for version 29/30.

dennisguse · 2021-01-24T21:30:38Z

I fixed this and:

released v3.14.3
removed the tag for v3.14.2 (so F-Droid does not build it); see Create tag for v3.14.2 #576
merged this branch locally

dennisguse added the maintenance Required maintenance work label Dec 22, 2020

dennisguse force-pushed the pauseResume#316 branch 2 times, most recently from 0b59ea9 to dea0185 Compare December 25, 2020 18:32

dennisguse changed the title ~~DO NOT MERGE: refactoring storage of pause and resume~~ VERY EXPERIMENTAL: refactoring storage of pause and resume Dec 25, 2020

dennisguse force-pushed the pauseResume#316 branch 7 times, most recently from aeb5bd1 to bdf9c3d Compare January 1, 2021 19:04

dennisguse marked this pull request as ready for review January 1, 2021 19:06

pstorch reviewed Jan 3, 2021

View reviewed changes

dennisguse force-pushed the pauseResume#316 branch from 9425684 to 9402de1 Compare January 3, 2021 12:07

dennisguse force-pushed the pauseResume#316 branch from 9402de1 to 6e71934 Compare January 3, 2021 12:37

dennisguse force-pushed the pauseResume#316 branch from 6e71934 to 6d705f4 Compare January 8, 2021 06:42

dennisguse added 18 commits January 24, 2021 21:36

Segments: use TrackPoint.type.

3cb2a33

So, far specific latitude for segment start/end (i.e., PAUSE/RESUME) was used. #316.

Use system time instead of GPS-provided time.

3b02291

#316.

Segments: only insert one trackpoint and set the type.

0fcbd4c

Existing data is not migrated. #316.

Segments: import/export is working incl. restoring TrackStatistics.

24657b1

KML exports sensor data if actually present. #316.

Cleanup.

9b26f4f

Segments: on import restore manual segment start/end if resume/pause …

871b0a2

…trackpoints were exported.

Segments: document new TrackPoint segment structure.

ff2789e

#316.

Dashboard API: send protocol version v2.

9923059

Replace long time_ms with Instant and Duration, respectively.

7fa955c

Fixes #561.

Cleanup.

1f6f192

TrackStatistics startTime/stopTime is null on creation.

7de402f

TrackDataHub always sends via handler.

da21d06

Marker refactoring.

63faeaf

Location values are stored directly.

TrackStatisticsUpdater updates only by adding TrackPoints.

3b721f8

#316.

Import: compute TrackStatistics onTrackEnd.

91c4349

ExportImportTest were not checking TrackStatistics correctly. #316.

Import: segments might contain no TrackPoints.

4dab1eb

#316.

Import: empty tracks cannot be imported as TrackStatistics cannot be …

641df07

…restored. #316.

TrackStatistics: merge previous segment whenever a new starts/ends.

c45f867

dennisguse force-pushed the pauseResume#316 branch from c8b1027 to c45f867 Compare January 24, 2021 20:36

dennisguse merged commit 376887c into main Jan 24, 2021

dennisguse deleted the pauseResume#316 branch January 24, 2021 20:37

pstorch added a commit to OpenTracksApp/OSMDashboard that referenced this pull request Jan 24, 2021

add trackpoint type to recognize pause/resume (#89)

dfc4626

- read protocol version and choose right projection OpenTracksApp/OpenTracks#553

dennisguse restored the pauseResume#316 branch January 24, 2021 20:54

dennisguse deleted the pauseResume#316 branch January 24, 2021 21:50

This was referenced Feb 13, 2021

Pause/resume refactoring regarding GPX #587

Closed

Request for comments: internal data storage issues #486

Closed

Concept: how to store pause/resume #316

Closed

Export and import: restored stats are not identical #192

Closed

Uh oh!

Conversation

dennisguse commented Dec 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dennisguse commented Jan 1, 2021

Uh oh!

pstorch commented Jan 2, 2021

Uh oh!

dennisguse commented Jan 2, 2021

Uh oh!

pstorch Jan 3, 2021

Choose a reason for hiding this comment

Uh oh!

dennisguse Jan 3, 2021

Choose a reason for hiding this comment

Uh oh!

pstorch commented Jan 3, 2021

Uh oh!

dennisguse commented Jan 3, 2021

Uh oh!

pstorch commented Jan 4, 2021

Uh oh!

pstorch commented Jan 4, 2021

Uh oh!

dennisguse commented Jan 4, 2021

Uh oh!

pstorch commented Jan 4, 2021

Uh oh!

dennisguse commented Jan 5, 2021

Uh oh!

pstorch commented Jan 5, 2021

Uh oh!

rgmf commented Jan 5, 2021

Uh oh!

dennisguse commented Jan 6, 2021

Uh oh!

rgmf commented Jan 7, 2021

Uh oh!

dennisguse commented Jan 24, 2021

Uh oh!

dennisguse commented Jan 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dennisguse commented Dec 22, 2020 •

edited

Loading