Closed
Bug 1035321
Opened 10 years ago
Closed 10 years ago
Handle gelocation accuracy
Categories
(Firefox OS Graveyard :: FindMyDevice, defect)
Tracking
(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: jrconlin, Assigned: ggp)
References
Details
Attachments
(2 files)
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.
Updated•10 years ago
|
Assignee: nobody → ggoncalves
blocking-b2g: --- → 2.0?
OS: Windows 7 → Gonk (Firefox OS)
Hardware: x86_64 → All
Assignee | ||
Comment 1•10 years ago
|
||
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.
Attachment #8451800 -
Flags: review?(lissyx+mozillians)
Updated•10 years ago
|
Target Milestone: --- → 2.0 S6 (18july)
Updated•10 years ago
|
Summary: Handle gelocation accuracy → Handle geolocation accuracy
Updated•10 years ago
|
blocking-b2g: 2.0? → 2.0+
Summary: Handle geolocation accuracy → Handle gelocation accuracy
Comment 2•10 years ago
|
||
Comment on attachment 8451800 [details]
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•10 years ago
|
||
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•10 years ago
|
||
My device is properly registered but the production website cannot connect to it, that will make testing this patch very hard ...
Comment 5•10 years ago
|
||
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•10 years ago
|
||
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 :)
Flags: needinfo?(jrconlin)
Comment 7•10 years ago
|
||
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 ?
Flags: needinfo?(ggoncalves)
Comment 8•10 years ago
|
||
(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•10 years ago
|
||
(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•10 years ago
|
||
Comment on attachment 8451800 [details]
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 :)
Attachment #8451800 -
Flags: review?(lissyx+mozillians) → review+
Reporter | ||
Comment 11•10 years ago
|
||
(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}}}
Flags: needinfo?(jrconlin)
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(ggoncalves)
Assignee | ||
Comment 12•10 years ago
|
||
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.
Keywords: checkin-needed
Comment 13•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-b2g-v2.0:
--- → affected
status-b2g-v2.1:
--- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Comment 14•10 years ago
|
||
Comment 15•10 years ago
|
||
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
Assignee | ||
Comment 16•10 years ago
|
||
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
Flags: needinfo?(ggoncalves)
Assignee | ||
Updated•10 years ago
|
Keywords: branch-patch-needed
Comment 17•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•