collect icon indicating copy to clipboard operation
collect copied to clipboard

GeoPoint takes the current location, even though marker was dropped elsewhere

Open zestyping opened this issue 6 years ago • 8 comments

Software and hardware versions

Collect v1.18.0

Problem description

When entering a GeoPoint, if the marker is placed by the initial GPS fix or placed by the user tapping the "add marker" button, the marker on the map does not reflect what is saved; the user's current location at the time they tap the "save" button is recorded as the point, instead.

Steps to reproduce the problem (case 1, initial GPS fix)

  1. Open a form that has a GeoPoint question with "placement-map" appearance. screenshot_2019-01-17-10-59-55

  2. Launch the map and wait for a GPS fix. The location icon (green flechette) and point marker (black balloon) appear in the same place. screenshot_2019-01-17-11-00-19

  3. Walk some distance away from the marker. screenshot_2019-01-17-11-00-57

  4. Tap the "save" button (floppy disk icon). The activity closes, taking us back to the question. screenshot_2019-01-17-11-01-08

  5. Launch the map again. Notice that the marker has jumped—the recorded coordinates are those of the last location where you were standing, not of the marker that was placed on the map. (Google Maps will also confirm that the numeric coordinates shown in step 4 also correspond to the last location.) screenshot_2019-01-17-11-01-27

Steps to reproduce the problem (case 2, dropped marker)

  1. While the map is open, tap the "trash" button to clear the marker. The marker disappears, leaving the green flechette. screenshot_2019-01-17-11-01-42

  2. Walk to a new location. screenshot_2019-01-17-11-02-17

  3. Tap the "add marker" button (balloon with plus) to drop a marker at your current location. screenshot_2019-01-17-11-02-23

  4. Walk some distance away from the marker. screenshot_2019-01-17-11-03-15

  5. Tap the "save" button (floppy disk icon). The activity closes, taking us back to the question. screenshot_2019-01-17-11-03-22

  6. Launch the map again and observe that the marker has jumped from the place where you dropped it to the location where you were standing in step 10. screenshot_2019-01-17-11-03-28

Expected behavior

The marker should always represent the point that will be recorded when you tap "save".

Other information

To me, this seems like it has to be a bug—it makes no sense that your data should look any different when you save it vs. when you then view what you saved, for any kind of data in any activity.

Oddly, however, this behaviour is quite deliberately coded into the activity; there are flags that keep track of how the marker was placed, and the method that returns the result uses the flags to decide whether to return the marker position or the current GPS location.

So, on the one hand, this seems like clearly a problem to be fixed, and quite a serious one as it invisibly introduces errors in the collected data; and on the other hand, something motivated the original author to code it this way, and perhaps there is a remote possibility that a few users expect it to work this way. It also seems plausible that no users have reported this problem because they usually tap "save" immediately after dropping the marker.

zestyping avatar Jan 17 '19 10:01 zestyping

This does feel like a bug to me. Geopoint without placement-map grabs the current location and so maybe the idea was to keep that behavior consistent.

This code was pulled in from GeoODK and so that behavior might have been useful there? Does git blame provide any insight? Either way, I think we should fix it.

yanokwa avatar Jan 17 '19 22:01 yanokwa

@yanokwa Okay. Yeah, I'd like to fix this. I'm assuming you mean after 1.19, right?

If you meant to squeeze it into 1.19, I'm happy to do it and think it will be pretty easy from a code perspective; probably of greater concern is the time to do a code review and another round of QA.

zestyping avatar Jan 18 '19 09:01 zestyping

Sorry about the delay, @zestyping. Let's wait to get the standing geo PRs merged, then we can take this on for the next release.

yanokwa avatar Jan 21 '19 22:01 yanokwa

@zestyping I think this issue was fixed in https://github.com/opendatakit/collect/pull/2865. Can you confirm?

yanokwa avatar Mar 12 '19 05:03 yanokwa

This was not fixed as part of #2865. There's a bunch of messy logic in there that isn't needed anymore if we no longer want this behaviour, so I'd like to clean that up. PR coming soon.

zestyping avatar Mar 27 '19 21:03 zestyping

Hello @zestyping, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 15 days.

You can reclaim this issue or claim any other issue by commenting @opendatakit-bot claim on that issue.

Thanks for your contributions, and hope to see you again soon!

getodk-bot avatar Apr 07 '19 14:04 getodk-bot

I talked to @zestyping who gave a great reason for why this should be bumped to later. I can't exactly remember what it is, though! I think it's that if we're considering changing the way points are entered to dragging the map rather than tapping it, this issue will go away.

lognaturel avatar Jun 04 '19 20:06 lognaturel

Yes, that's exactly right. I think we have a consensus that the method described in this comment for placing point would be easier to use and more precise, and implementing that method would obsolete this issue. (We should prioritize when we want to go ahead with that work.)

zestyping avatar Jun 04 '19 23:06 zestyping