Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Storing data without GPS #1033

Merged
merged 13 commits into from
Dec 28, 2021
Merged

Storing data without GPS #1033

merged 13 commits into from
Dec 28, 2021

Conversation

dennisguse
Copy link
Member

@dennisguse dennisguse commented Nov 21, 2021

OpenTracks now tries to create TrackPoints from BLE sensors (i.e., the ones that provide distance) every second.
It is a really naive implementation and works nicely if GPS is not used.

The current implementation has a preference of sensor-based TrackPoints as those will be triggered more often.
This will very likely result in ignoring useful GPS locations (which are expansive to get from an energy perspective).

For this, we need some more magic (aka don't try to store sensor data every second).

Fixed #500.

@dennisguse dennisguse added the enhancement New feature or request label Nov 21, 2021
@dennisguse
Copy link
Member Author

One simple option:

  • store sensor-based trackpoints if recording distance interval is fulfilled (relative to the previous stored [any] TrackPoint aka lastValidTrackPoint).
  • store GPS-based trackpoints if recording distance interval is fulfilled (relative to the previously stored GPS-based TrackPoint)

@dennisguse
Copy link
Member Author

Alternative with longer storage intervals was mentioned here: #500 (comment)

@dennisguse
Copy link
Member Author

@pstorch This PR affects OSMDashboard.
At the moment, we are creating TrackPoints of type TRACKPOINT that do not contain coordinates.
OSMDashboard handles this case using 0/0.

An alternative would be to introduce a new type like SENSOR (i.e., no coordinates), so that TRACKPOINT has always a coordinates.
Which option do you like more?

@pstorch
Copy link
Member

pstorch commented Nov 23, 2021

What should the OSMDashboard do with those trackpoints? Ignore? I assume that trackpoints with coordinates 0/0 will be displayed at the favourite NULL position in the ocean. I've not tested it.

@dennisguse
Copy link
Member Author

That's what happens and how I discovered that we need to talk about this :)
I guess ignore those TrackPoints as they contain no useful data (for now).
Data might be interesting if we would implement something like line color by speed.

Anyhow, it is a breaking change so we need to upgrade the protocol version.

@dennisguse
Copy link
Member Author

@ChristophWurst Did a test run today and it works :)
With some to be expected flaws 🌞

@pstorch
Copy link
Member

pstorch commented Nov 23, 2021

Is it an option to filter out those beforehand on OT side? I mean trackpoints without coordinates are not that useful on a map.
If not filter out 0/0 would be easy on the Dashboard side but a little bit a hack. Some apps do it like that, but 0/0 is still a valid coordinate. Not very useful though.
Better to have a flag in the data to know that kind of trackpoints. Make it explicit.

@dennisguse
Copy link
Member Author

dennisguse commented Nov 24, 2021

@pstorch I introduced a new TrackPoint type (SENSORPOINT, 2), but that requires a change in OSMDashboard:
Current implementation to determine pause would not be working anymore:

        this.pause = type != null ? type != 0 : latitude == PAUSE_LATITUDE;

I see two options:

  • modify the where to ignore Type=2; I propose allow list rather than disallow list
    ` try (final Cursor cursor = resolver.query(data, projection, TrackPoint._ID + "> ? AND TrackPoint.TYPE IN (-2, -1, 0, 1)", new String[]{Long.toString(lastTrackPointId)}, null)) {```
  • ignore Type=2 during actual processing

Anyhow, the new type is a breaking change (for the current OSMDashboard release), so we need to upgrade the API version.

@pstorch
Copy link
Member

pstorch commented Nov 24, 2021

As long as we cannot do anything useful on the map with those trackpoints I would prefer the first option to add it to the where clause, then they are not even transferred.

@dennisguse
Copy link
Member Author

This code is ready for review and testing.

One problem.
Until now the GPX export would be "broken": SENSORPOINT TrackPoints are not exported (as they do not contain GPS coordinates).
However, SENSORPOINTs contain cumulative data (i.e., distance and altitude gain/loss), which then will not be exported.
As a quick workaround, I implemented the TrackStatsExtension #652.
Two solutions:

  • remove cumulative data from GPX (i.e., opentracks:distance, opentracks:gain, and opentracks:loss) as it is only using TRACKPOINT data
  • compute SENSORPOINT data and add it to the next TRACKPOINT.
    (probably the better solution, but it complicates the GPXTrackExporter).

@pstorch Due to the introduction of SENSORPOINT, a new release of OSMDashboard is required to ignore those.
I would also increase the protocol version to be on the safe side. What do you think?

@rgmf Let's merge+release the profile changes first as it provides more value :)

@pstorch
Copy link
Member

pstorch commented Dec 4, 2021

@pstorch Due to the introduction of SENSORPOINT, a new release of OSMDashboard is required to ignore those.
I would also increase the protocol version to be on the safe side. What do you think?

I don't think an increase in protocol version is necessary. The type field is already available. There is "only" a new value. The query works with the older OpenTracks.

@pstorch
Copy link
Member

pstorch commented Dec 4, 2021

Anything else to change in OSMDashboard? I think I should release a new version soon, because of the sensor delay bug.

@dennisguse
Copy link
Member Author

@pstorch To the best of my knowledge OSMDashboard is prepared for this PR.

@dennisguse dennisguse requested a review from rgmf December 4, 2021 20:45
@dennisguse
Copy link
Member Author

@pstorch one thought about the statistics for the share pic functionality of OSMDashboard: SENSORPOINTs contain cumulative sensor data (i.e., distance); is this a problem for OSMDashboard if those are ignored? [don't know how the stats are computed...

@pstorch
Copy link
Member

pstorch commented Dec 5, 2021

@dennisguse OSMDashboard reads the statistic provided by OpenTracks and displays them as is, no computation. So the question is how OpenTracks calculates them 😉

@dennisguse
Copy link
Member Author

@pstorch Perfect :)
Just woke up and had this thought :D

Copy link
Member

@rgmf rgmf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works like a charm. Fantastic feature!!!

I've read the code and I've tried to understand. The first thing very well, the second one not as well 🙄 Maybe, I've should been read it with more attention 🤣

Seriously now, great job ;)

@dennisguse
Copy link
Member Author

@rgmf Sounds great!
I still would like to wait for #1012 as downgrading from this is not possible (we would need two releases to ship the db downgrade code etc). Releasing only the layout functionality, allows people to be able to downgrade (if there is some trouble with something). After some waiting time, we can release this then :)

@dennisguse
Copy link
Member Author

Found and fixed a regression: for creating a marker, the lastStoredTrackPointWithLocation should be used as markers require a valid location.

@dennisguse dennisguse merged commit 7193171 into main Dec 28, 2021
@dennisguse dennisguse deleted the indoor#500 branch December 28, 2021 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Indoor training
3 participants