Recording and Uploading of GPX Traces

[goldbattle]

Hi, thanks again for the project.
This is trying to address the following issues:

• Add GPS trail for paths walked #2209
• fixes Add an ability to record a track #2327
• fixes "Add missing way" tool #2871

Here is an example user experience idea of how to start recording a track, and then composing a note. Right now I am able to record the history of gps points, and have it stored in the NoteEdits database object for later uploading to the server. I also changed to visualization to highlight these recorded segments in red.

bandicam.2021-12-08.00-15-26-335.mp4

The idea is that the GPX track will be linked to the note (same as how images are currently) so this can provide additional information for editors and can be a part of the online "Public GPS Traces" overlay. The next step is for me to implement uploading to the gps trace server (I was looking at using some of this code), so any feedback is much appreciated.

[goldbattle]

I have completed the initial uploader, and am open to feedback / review.
Here is an example upload I have created:
https://www.openstreetmap.org/note/2994710
https://www.openstreetmap.org/user/Goldbattle/traces/4028133

I am not sure why the filename is not what is specified, but I think what it is now is pretty good.
The editor can download the GPX trace, drag and drop into the editor, and then see the path:

---

It needs to have additional permissions requested from the OSM api so that it can upload and get info about the GPX track.

Here is an application I created to test with (de/westnordost/streetcomplete/data/user/UserModule.kt):

    private const val CONSUMER_KEY = "tbHqDSVojwRZT4hQ2jMsOzdN4i1YsTQqTvAVeFcD"
    private const val CONSUMER_SECRET = "X3tc6G6aD4DPWVOhmvQowFdvPzwXvebSYnJStHWL"

[westnordost]

First off, thank you for the effort so far, the PR and the nice demo video!

There are some points to clarify first, as if I remember correctly, some problems where talked about in prior issues. If I recall correctly, the main pain points were:

• Google Play compliance: As the track should probably continue to be recorded while the app is (shortly) in the background, location permission for the background would be required. Additionally, a foreground notification must be shown that allows to stop recording the track. Google Play has very strict rules regarding use of background location and precise location, so strict in fact that their compliance bot marked the app as non-compliant and removed it. It was a false-positive, but there was no getting through (to a human) at Google. I do not want to go through this again. See App currently not available on Google Play #2909 and then Google Play Compliance: Precise location supposedly sent off device #3466
• Privacy: Due to privacy concerns, this may never be on by default. The user should also not be able to accidentally turn it on because he does not know what it implies
• Feature for advanced users: This feature is likely only interesting for advanced users, maybe those who would use that GPS trace later at the desktop to add some ways or whatever with JOSM. Hence, as the app is primarily for non-technical people, the addition of such a feature should not be in the way in the regular UI, not take up any space in the normal UI.

Further, the question is what (additional) use cases the recording of tracks should cover. Two come to my mind:

• Generally contributing (public, anonymized) GPS traces for better alignment of imagery
• Directly referencing a (short) trace in a note so that it can be added and/or corrected via JOSM, maybe by the note creator himself

What use case should this feature cover?

[smichel17]

This feature is likely only interesting for advanced users, maybe those who would use that GPS trace later at the desktop to add some ways or whatever with JOSM

I am not sure I agree. Maybe I qualify as "advanced", but I have no interest in using iD or JOSM to add new ways. However, I would be happy to walk down a path that is not mapped and upload a trace so that some armchair mapper can add it later. That said,

1. I'm not sure what the advantage is of having this function in SC instead of a separate app.
2. I agree that this use case is rare enough that it should not take (much) space in the GUI. Nothing which is always visible, at least. I think a menu option would be ok.

[goldbattle]

@westnordost I think the purpose is very specific to SC and is what you mentioned:

Directly referencing a (short) trace in a note so that it can be added and/or corrected via JOSM, maybe by the note creator himself

SC also allows for uploading of a photo along to provide more context also, which is a big advantage compared to other thirdparty apps. I personally have not used another app, and would rather just use SC instead of having to switch between apps. Some example use cases I can think of if you need motivation for this feature:

• A sidewalk connection between buildings is missing. I can record a small track to attach to the note
• Trail under a tree is missing since it is not visible from satellite imagery. I can record this to provide a trail record for them.
• A trashcan has moved. I can record a stationary trace of my location to provide information on where the trashcan should be.
• A road / walkway should be shifted. I can record a small trace to provide as a reference.

Re: Only for advance users. I am not sure I agree with this. I don't see why adding more information to the note would not be appreciated by someone online. Might me just being naïve here since I have not really used the online editor at all.

@smichel17 I agree that adding more to the UI would be a bad idea, thus I hid this away in the long-press menu where a user can create a note already. Also the user can only record one trace at a time (and can still answer quests while recording), so this simplifies and only adds a single button to the screen to stop the recording.

---

I understand the google play issues, and will read up on this, but right now if the user leaves the app, no location information is recorded in the track. This causes a "jump" in the recording, but I think it is a bit of work to require background location access and since the traces are so short I am not sure if it is worth it.

I think maybe adding a popup when the user first clicks could help inform the user about where their location information is being sent? Should I add this?

[HolgerJeromin]

I was very sceptic about this feature.
But as it is only active on request of the mapper I am very happy with the presented video. I would explicit vote for no location updates in the background.
The track is be an additional way to provide context for one specific missing OSM data. (continuous gps tracks with mapping stops on a side walk are not so useful)

[smichel17]

I would explicit vote for no location updates in the background.

We might need some text to explain this. And/or, maybe putting the app into the background (for more than a few seconds) should automatically end the trace, since at this point it is no longer accurate. There are probably other options, too.

[matkoniecz]

My fundamental problem with this is:

• this is not a critical feature

• in my opinion (which is a guess) there is decent chance that Googlebot will go berserk and ban StreetComplete again once this is added. Lets say 10% chance for immediate ban. It can also cause ban in the future.

Given that I am scared and would prefer to avoid adding this.

[westnordost]

IIRC Google bot removed the app because it claimed that the app used background location and hence, stricter policies applied to the app that the app did not comply with. The solution, in the end, was to comply with the stricter policies even though the app does not use background location.

The other Google bot issue I mentioned was related to "sharing user's location", which the app also doesn't do, but as we all agreed, does so indirectly by 1. downloading map tiles at the position of the user and 2. inherently by requiring that solving quests is done on-site and the user's OSM history is public. We solved this by simply stating that "yes it does share location" in the questionaire. It was just a questionaire, apparently this information is shown to the user somewhere on google play or whatever.

So, as long as a GPX track feature works the way as @goldbattle described, I think nothing should change from the viewpoint of Google.

Anyway, I can agree with the others here that it sounds like a really useful feature. Especially outside cities where footpaths are not well recorded yet, this can be a really handy method to let other users know of an unmapped footpath and conveniently already supply the (exact) path with it. I'll review the code now, maybe I have some more questions then

[westnordost]

Overall, looks quite good! I didn't do a complete review yet because I found a few things on the way that should be changed.

In general, stuff that is exclusively related to GPS tracks (API wrapper, data classes) should go into an own package. Also, the naming is off (as explained in one comment).

I did not read the code about the upload yet in detail but the "upload data map" data structure strikes me as odd. But as I wrote, I did not look into this in detail yet.
Also, it may be preferrable to have the GPS traces upload in a separate class, like it is done for the image upload.

[goldbattle]

Great, thanks for the specific feedback, was a bit unfamiliar so wasn't sure where best put things in some case (just created a proof of concept). I will find some time later this week or weekend to make the requested changes, thanks!

[westnordost]

Here's another small review of the things you moved to the new package

[westnordost]

You made some changes. Let me know when it is ready for another review.

[goldbattle]

I need to make a few more based on the first set of comments. Will request a review once I am able to make the changes, thanks!

[smichel17]

I've reviewed this for coding style and for bugs (didn't find any 🎉), but only at the surface level (each change locally within that file).

In other words, I have not considered how these changes interact with each other or the rest of the app. I think westnordost already did that, anyway. Also, I did this review entirely here on GitHub, so my bug-hunting was limited :).

There's a bunch of comments but they're all both pretty minor & easy to address— overall, it looks good!

[goldbattle]

@goldbattle when you enter the developer options on your phone, there should be an option "Don't keep activities". If you turn this on, does the feature work as expected when you e.g. switch to another app and then back some time later?

It still works as expected when I have this enabled.

@FloEdelmann I have made the requested changes, let me know if anything else.

[FloEdelmann]

Looks good to me now (still only code review, no extensive testing) :)

[westnordost]

Changed just a few tiny things

[westnordost]

Going to merge this now. The PR is really of excellent quality, great work!

After merge, I am going to add/change a few things:

• adapt the tests for NoteEditsDao to also include testing for persisting the trackpoints
• try if the different color for recording-tracks can be achieved with less YAML tangram-styling code
• remove the usage of "with" in LocationAwareMapFragment because (personally) I think "with" is not a good idea and it is not used anywhere else in the code

[goldbattle]

Great, thanks so much, sorry it took so long! Make sure the OSM API permissions get updated for the application before you release otherwise it will get into an infinite loop asking for permissions.

[westnordost]

Right, thanks for the hint!

[goldbattle]

See the bottom of this comment:
#3573 (comment)

---

It needs to have additional permissions requested from the OSM api so that it can upload and get info about the GPX track.

[westnordost]

Fuck...

I accidentally deleted the OAuth1 profile because I first created a new profile under the same name with the new permissions but only then noticed I could also just edit the old profile to include the new permissions. So then I deleted the "old" profile (the second in the list). But it looks like the newest profile is displayed first in the list :-|

So, for damage control, gotta create a update for v43 now that uses the new profile because the old profile is irrevocably gone, looks like.

[westnordost]

Though before I hastily do this, better test what exactly happens code-/api-wise in the app if it uses an unknown oauth key...

[westnordost]

Okay, the hotfix v43.2 is deployed. It logs out the user automatically on first start so that the user needs to login again with the new OAuth profile. 😥

On the plus side, I could remove the permission upgrade request dialog, check and strings, see 31ebb42 becaue this upgrade plan obviously failed (my fault).

[goldbattle]

Yeah that seems the cleanest now since all users will have the new oauth (with track perms) so no need for the added complexity. Sorry for the all the trouble :(

[westnordost]

Thanks, but I caused the trouble onto myself, should have been more cautious when editing/deleting the profile.

[matkoniecz]

I committed matkoniecz@06704ea to unbreak tests, hopefully without breaking anything new