1067677 - Make wifi scanning in FTU more reliable and quicker

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::First Time Experience, defect)

Description

[Giovanni Charles]

The FTU scans for wifi networks every time the user navigates to the wifi page. Ongoing connection attempts can interfere with the scanning process causing erratic behaviour.

Wifi networks should be stored and only refreshed when the user requests. When the FTU needs to scan it should wait for connection attempts to finish before attempting to scan.

Comment 1

[Giovanni Charles]

Attachment: patch to defer wifi network scans and remember known networks through navigation

Comment 2

[Fernando Campo (:fcampo)]

Comment on attachment 8489722 [details] [review]
patch to defer wifi network scans and remember known networks through navigation

stealing the review of FTU stuff (assuming that Etienne doesn't mind) as I'm more familiar with the code and he's probably focused on system stuff. Feel free to reassign if I'm mistaken.

Comment 3

[Fernando Campo (:fcampo)]

Comment on attachment 8489722 [details] [review]
patch to defer wifi network scans and remember known networks through navigation

First wave of review done, few nits and a rebase needed, patch can't apply so I couldn't test it properly on a device.

Also, if this will is gonna be a serie of small patches to tackle all the issues independently, could you number them to follow an order?
Thanks.

Comment 4

[Giovanni Charles]

Cool, those changes and the rebase have been applied to the PR.

Ah, I did not realise you could make dependent patches and order them. If this is possible, then I should do this for the big patch. At the moment these patches are independent so that they can land in any order. Independent in the sense that each patch on top of master will make a functioning/test-passing FTU by itself.

Comment 5

[Fernando Campo (:fcampo)]

(In reply to Giovanni Charles from comment #4)
> Cool, those changes and the rebase have been applied to the PR.
> 
> Ah, I did not realise you could make dependent patches and order them. If
> this is possible, then I should do this for the big patch.
Well it's not exactly dependent patches, it's more like a manual thing :D
just adding a number to the description of the patch when you upload it, and if possible, the total of patches to expect. e.g.
[1/5] Remember wifi through navigation
[patch2] Wait for connection attempt before re-scan

just a little help to keep an order in case of backouts and help the reviewer to see the big picture.

> At the moment these patches are independent so that they can land in any order.
> Independent in the sense that each patch on top of master will make a
> functioning/test-passing FTU by itself.
Great, that's the perfect way of doing it

Thanks again for taking care of this.

P.S. The PR is closed in github, so I'll wait till you update with a new one with the r? flags.

Comment 6

[Giovanni Charles]

Attachment: Same patch re-requested after Try's recovery

Comment 7

[Fernando Campo (:fcampo)]

I've been testing the patch and I was wondering if the lag we are creating by deleting this [https://github.com/mozilla-b2g/gaia/pull/24709/files#diff-870058003588bd48d96ceec41624b685L351] is acceptable for UX (you can see it the first time we enter the wifi screen, so the scan is performed, we're able to briefly see the empty wifi screen, with empty list and 'hidden wifi' button).

Also, I'd like to test it when no wifi is available to check the changes on timeouts, but I'm gonna probably need a faraday cage for that (here you get wifi signal even in the tube), so it's gonna take me another half day to make the full review.

Apart from that, code looks good :)
Thanks

Comment 8

[Giovanni Charles]

Ah yes, the gap in the UX needs to be looked at.

Just to let you know, my time is up and I will be unreachable until the 20th October. I'll catch up when I get back and continue contributing to the FTU :) In the meantime I will have to relinquish my bugs, thanks for all the feedback!

Comment 9

[Fernando Campo (:fcampo)]

Comment on attachment 8498938 [details] [review]
Same patch re-requested after Try's recovery

We were waiting for Giovanni's comeback, but after some time we should go forward with this.

Patch needs just a small nit and a rebase, tested on device and working nice (only errors in wifi scans repro also on master, so non-related).

I'll send a new PR with Giovanni's code rebased with latest from master and merge it.

Comment 10

[Fernando Campo (:fcampo)]

Attachment: Rebased PR - https://github.com/mozilla-b2g/gaia/pull/26025

Same patch but rebased, so I'm adding the r+ flag already as it's still valid.

Comment 11

[Fernando Campo (:fcampo)]

merged on master - https://github.com/mozilla-b2g/gaia/commit/4961cb7c9d0b4c2e5d0bc8a2a9e34965b9ad3bdf