1035321 - Handle gelocation accuracy

Status: RESOLVED FIXED

(Firefox OS Graveyard :: FindMyDevice, defect)

Description

[JR Conlin [:jrconlin,:jconlin]]

On device GPS returns an "accuracy" value. Device should include this in the tracking information. The server should either display area of accuracy and normalize position to avoid "jumping", or discard results that do not have high grade accuracy.

Comment 1

[Guilherme Gonçalves [:ggp]]

Attachment: gaia pull request

This is actually a one line fix, but testing it was a bit cumbersome, so the diff ended up huge.

I refactored MockGeolocation a little bit to make testing easier, and moved the unit test file findmydevice_test.js into findmydevice_commands_test.js, because it only contained tests for commands. I then added a test for this bug (and all the necessary scaffolding) in the file findmydevice_test.js.

Comment 2

[:gerard-majax]

Comment on attachment 8451800 [details] [review]
gaia pull request

Thanks for splitting this in a more readable way! I already made some comments on the real payload of the patch. There are some nits and some things that would need to be addressed, otherwise it looks good.

I'll have a look at the moving parts.

Comment 3

[:gerard-majax]

And now that I was about to test the patch on my device, I'm getting 401 again. This seems to be related to the switching to the production server from the stage one.

Comment 4

[:gerard-majax]

My device is properly registered but the production website cannot connect to it, that will make testing this patch very hard ...

Comment 5

[:gerard-majax]

After switching back to stage server (which are supposed to have the code for the geolocation accuracy), stage website can connect to my device but the geolocation does not even start on the device.

This may be because of bug 1036423.

Comment 6

[:gerard-majax]

As agreed on IRC, I'm adding needinfo? for you so that you can make sure the accuracy sent by the device to the server is in the proper format, after bug 1036423 is fixed and before accuracy is used on stage/prod :)

Comment 7

[:gerard-majax]

Thanks for fixing the previous feedback. I did some other comments, we are heading towards the proper direction. Meanwhile, do you mind addressing my latests comments and squash you patch but keep the separation that ease reviewing for now, since it helps a lot ?

Comment 8

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #6)
> As agreed on IRC, I'm adding needinfo? for you so that you can make sure the
> accuracy sent by the device to the server is in the proper format, after bug
> 1036423 is fixed and before accuracy is used on stage/prod :)

Okay this bug has been fixed, my Flame got localized on FMD Stage. Device ID is 2033d21e40b2d79380a858449b9fbbbe, can you check that the accuracy is properly sent ? LOcation has been acquired via MLS as far as I can say.

Comment 9

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #8)
> (In reply to Alexandre LISSY :gerard-majax from comment #6)
> > As agreed on IRC, I'm adding needinfo? for you so that you can make sure the
> > accuracy sent by the device to the server is in the proper format, after bug
> > 1036423 is fixed and before accuracy is used on stage/prod :)
> 
> Okay this bug has been fixed, my Flame got localized on FMD Stage. Device ID
> is 2033d21e40b2d79380a858449b9fbbbe, can you check that the accuracy is
> properly sent ? LOcation has been acquired via MLS as far as I can say.

> E/GeckoConsole(  326): *** WIFI GEO: gls returned status: 200 --> {"location":{"lat":48.8717855,"lng":2.3413425},"accuracy":100}
> I/Gecko   (  326): *** WIFI GEO: gls returned status: 200 --> {"location":{"lat":48.8717855,"lng":2.3413425},"accuracy":100}
> I/GeckoDump( 1035): [findmydevice] updating location to (48.8717855, 2.3413425)

If the server side properly received this accuracy, then we are good :)

Comment 10

[:gerard-majax]

Comment on attachment 8451800 [details] [review]
gaia pull request

Thanks for addressing all the comments. That all seems to be good now. Let's wait for green try and for making sure the server side was okay, then it can be merged :)

Comment 11

[JR Conlin [:jrconlin,:jconlin]]

(In reply to Alexandre LISSY :gerard-majax from comment #9)
> (In reply to Alexandre LISSY :gerard-majax from comment #8)
> > (In reply to Alexandre LISSY :gerard-majax from comment #6)
> > > As agreed on IRC, I'm adding needinfo? for you so that you can make sure the
> > > accuracy sent by the device to the server is in the proper format, after bug
> > > 1036423 is fixed and before accuracy is used on stage/prod :)
> > 
> > Okay this bug has been fixed, my Flame got localized on FMD Stage. Device ID
> > is 2033d21e40b2d79380a858449b9fbbbe, can you check that the accuracy is
> > properly sent ? LOcation has been acquired via MLS as far as I can say.
> 
> > E/GeckoConsole(  326): *** WIFI GEO: gls returned status: 200 --> {"location":{"lat":48.8717855,"lng":2.3413425},"accuracy":100}
> > I/Gecko   (  326): *** WIFI GEO: gls returned status: 200 --> {"location":{"lat":48.8717855,"lng":2.3413425},"accuracy":100}
> > I/GeckoDump( 1035): [findmydevice] updating location to (48.8717855, 2.3413425)
> 
> If the server side properly received this accuracy, then we are good :)

Yep, client is sending accuracy to the stage box:
### Sending update to 1 pages: {"Latitude":48.87187313,"Longitude":2.34102594,"Altitude":0,"Accuracy":49,"Time":1404993510000,"Cmd":{"t":{"acc":49,"la":48.87187313,"lo":2.34102594,"ok":true,"ti":1.40499351e+12}}}

Comment 12

[Guilherme Gonçalves [:ggp]]

Travis had an infrastructure error, plus two other errors in completely unrelated tests which I had already seen before, when landing bug 1030448. I think it's safe to check this in, but sheriffs, let me know if I should re-run this when Travis is green again.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/f8aa626eecc1690ca5d7991d08160ba3a909623b

Comment 14

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/d46332fbf16a683a7a29ad3c78d865ca1767d354

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Reverted from 2.0 for Gaia unit test failures.
v2.0: https://github.com/mozilla-b2g/gaia/commit/c24d115bb51e40d5cb28c49324102b893966944b

https://tbpl.mozilla.org/php/getParsedLog.php?id=43621158&tree=Mozilla-Aurora

Comment 16

[Guilherme Gonçalves [:ggp]]

Attachment: v2.0 pull request

We required a file that was introduced in bug 1004973. After consulting with the author, I found out that the patch is not being uplifted, so I'm just copying the file from master.

https://bugzilla.mozilla.org/show_bug.cgi?id=1004973#c46

Comment 17

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/73272278cecbeedbf21c4c149c21ac006d5021bb