OsmTravel / OsmGo

Osm Go !
https://osmgo.com/
MIT License
109 stars 15 forks source link

Duplicate POI nodes sent. #102

Open mpawelski opened 2 years ago

mpawelski commented 2 years ago

Hi, thanks for this great app!

I just had a changest created by OSM Go that sent a lot of duplicated POIs to OSM. https://www.openstreetmap.org/changeset/116081331 I fixed it manually later in iD editor.

I remember that I click "send" button in OSM Go and the changes were sending (They were disappearing from the list) by then I clicked back button and clicked "send" again because I though something went wrong. Later by accident I saw on iD editor that some nodes are duplicated. I didn't notice it at first. When I tried to move the POI to make it more accurate then I noticed that there were two nodes at the same position with exactly the same tags.

So maybe it's worth adding some check before sending POI to OSM to check if there is already some node in changeset with exactly the same position and tags.

Sorry for pretty vague description. But I thought that this is serious enough bug that it's worth to report event with such laconic description.

timothywashere commented 2 years ago

Same thing happens to me often. If the OSM Go app is interupted by being sent to the background or crashing or resizing the window (split screen) then you end up uploading the same changes twice. It also happens sometimes when nothing has gone wrong.

zhik commented 2 years ago

Bumping this issue. https://www.openstreetmap.org/changeset/119967097

Sometimes it adds two features or five. Not sure why.

Example - Different nodes but some POI 9680843018 9680846818

timothywashere commented 2 years ago

To avoid this issue happening I've developed a procedure before uploading. It only happens if you have a large change set. Always wait until your battery is over 60% charged before you start. Always delete data before you upload Also close and restart the app. Wait until your are on WiFi to upload

xcriptus commented 2 years ago

Bumping this "duplicate POI" issue. Version 1.5.0

I've been using OSM Go quite extensively in the last month. There is a reproductible bug. At least I can reproduce it quite easily on my phone and I believe that it would be the same yet depending on memory amount may be.

I observed this bug dozen of times.

I also apply a variant of tomothy techniques (1) if I just forgot to upload POIs (let's say if I have more than 50) I clear data (2) with no data no need to restart the app.

No difference for me between wifi or phone. I think that there is a timer or something like this, so depending on the phone, memory, networkd, etc, the app crashes at some point. Never tried to measure this but something like one minute, and if it get slower and slower it always crash. Sometime a few second on the last point before crash.

Having said that, OSM go rocks. It is really great and just perfect to complement StreetComplete on the street.

Thanks a lot to developer(s). I'm really having a good time with it. _

Binnette commented 2 years ago

Maybe fixed in 7e1e2174bbca778e3be9f742a48823f72da43944

DoFabien commented 2 years ago

Thank you for the very complete report! I was not able to reproduce this behavior.

I'm not sure if 7e1e2174bbca778e3be9f742a48823f72da43944 fixes the problem.

Currently, we send object by object, it works well for a small set of data, but it is not optimal for more than 50 changes...

I will try to send the changes in one time, it will be much faster. On the other hand, if an object is rejected (conflict, etc...) the other changes will also be rejected.

I try to do this before the end of the week

xcriptus commented 2 years ago

Bonsoir,

Here is a scenario that reproduced the bug as well as some faulty changeset. I've most screenshots if this helps. (BTW the name of the changeset is wrong, I forgot to change the name of the city :-( )

(1) Loaded 40362 data. (2) Created exactly 100 lamposts. (3) Upload. (4) Count down getting slower and slower. (5) Crash at 70 creations remaining, 4mn after start of count down. (6) So far so good. There are not duplicates on OSM. (6) Restart the application. (7) The queue indicates that 96 creations are to be upload. BUG: This is wrong, this should be 70. (8) Checked in OSM : changeset 121786059 has been opened 9mn ago. OSM indicates that 31 nodes were created. (9) Upload. It crashes at 66 more or less. (10) OSM indicates that the changeset has 62 nodes. (11) Restart the application. (12) Now there is 92 creations in the queue. (13) Crash about 60 (after 3 mn more or less) (14) Restart. Clear data. 88 uploads in less that 1 mn. (15) Everything ok. OSM Go readdy to map :-) (16) OSM indicates that 181 nodes are in the changese. 81 nodes are duplicates since there were 100 at the begining.


If the bug is complicated to fix an acceptable workaround would be to limitate the number of POIs in the queue and/or the size of data. I never seen the bug for queue < 50. I guess that the idea is to make it possible for offline mapping to create many objects. For me a gentle reminder "you have 50 POIs to upload ... ". Alternatively or at the same time it could make sense to limit the size of data. Quite often on forget to clear a zone when moving from one zone to another one, but it is quite acceptable IMHO to limit this.

Finally I would strongly recommend NOT sending groups of objects. It is really common for edits to fails so I would stop making edits if this kill a whole contributions. As a user I don't care to much waiting a bit. I just don't want to loose all my work just for a few edits.

I had quite some fun with OSM Go this afternoon. Thanks ! !-)

DoFabien commented 2 years ago

First of all, sorry for this bug which is very annoying.

In my opinion, it is caused by 3 reasons:

  1. The fact of sending the changes object by object

Solution: Use a diff to send the changes in one request

  1. There are too many objects to load. At each data loading, those are compared to each other and there are heavy space operations (merging new data with old data), especially for our phones...

Solution: Limit the amount of data that Osm Go keeps in memory. Optimize the computation

  1. Modifying an object while data is being downloaded/processed can cause a conflict

Solution: Disable the possibility to edit an object during the download phase, or queue them during this phase

For the first point, I just deployed this feature : b91a28c1184400b43f8aa40ac9113b97414299de

But don't worry, if the upload fails, your changes are safe. You will just have to undo the problematic change before trying again.

I've added a button to make it as easy as possible in the interface

image

I will try to work on points 2 and 3 during the next few days