1011358 - [wifi][wi-fi] Gecko doesn't notify Gaia when wpa_supplicant scans and finds new SSIDs/nodes

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Wifi, defect)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

Splitting this out from bug 987760, as the cause seems to have shifted from a dhcpd issue to the ateam node not always appearing after the wpa_supplicant's scanner has searched for SSIDs.

Device firmware (date) 	13 Apr 2014 23:02:50
Device firmware (incremental) 	76
Device firmware (release) 	4.3
Device identifier 	msm8610
Gaia date 	14 May 2014 09:23:38
Gaia revision 	3a1d67246a79
Gecko build 	20140515040207
Gecko revision 	e4843f4f08a7
Gecko version 	32.0a1

STR: (seems limited to our current setup)
1. Flash with the latest master Flame or Hamachi build
2. In the Mountain View QA lab, enable Wi-Fi through the UI, on-device
3. Look for the presence of the "ateam" SSID

Actual Results:

As both manual testing and automation shows, it sometimes isn't found at all, despite a signal strength of "good," which is around the 76 dbm mark.

If we click "Search again," it can often be found, but a few times, it took 4 or 5 searches to find it; this explains why test_settings_wifi fails.

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Test Report.html

Comment 2

[Stephen Donner [:stephend] Not actively reading bugmail]

And, sorry that I don't have a concise logcat from the manual case, but here's the corresponding logcat for the test report, from above:

https://app.box.com/s/mv5t65g70smlgk46odza

Comment 3

[Stephen Donner [:stephend] Not actively reading bugmail]

And, again, sorry, but this is another run (too many logs and was trying too many things), where you can see, around 10:15 - 11:05 (minutes in) that manually clicking on "Search again" 4-5 times finally finds, again, the "ateam" node, and the test passes.

https://drive.google.com/file/d/0B_XMhxrTPIrWMHVHNVpmN1l4Qnc/edit?usp=sharing

Comment 4

[Ken Chang[:kenkai]]

Vincent, have you ever get this kind of wifi problem before.

Comment 5

[Vincent Chang[:vchang][changyihsin]]

I saw this on Nexus 5 sometimes. But I think the cause is related to supplicant's scan and scan_results commands.

Comment 6

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: wifi-logcat.txt

Attached is a logfile (with Wi-Fi output in ADB enabled) that hopefully shows:

1) Failed testrun of test_settings_wifi.py, which tried to connect to the "ateam" network, but failed to find it, and timed out.
2) Manual press (3 or 4 times; sorry, I can't remember which) of the "Search again" button
3) I stopped the log as soon as the "ateam" node's SSID appeared

Environment is Hamachi, with 1.2 base image (sorry, that's all we have), with a master/nightly Engineering build:

sdonner-10779:B2G-flash-tool sdonner$ ./check_versions.sh
Gaia      c462d9183d294a2d8ecc472f593ea8cfa15bc9de
Gecko     https://hg.mozilla.org/mozilla-central/rev/a6f36c31aa8f
BuildID   20140521040203
Version   32.0a1
ro.build.version.incremental=324
ro.build.date=Thu Dec 19 14:04:55 CST 2013

Comment 7

[Jonathan Griffin (:jgriffin)]

Maybe related to the Android Eideticker problems we're having as well?

Comment 8

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to Jonathan Griffin (:jgriffin) from comment #7)
> Maybe related to the Android Eideticker problems we're having as well?

I'm sure hoping so :-)

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: test_logcat.txt

This logcat covers:

1) Failed run of test_settings_wifi.py (just to help trigger the state where "ateam" isn't found/displayed in the Wi-Fi UI, on-device
2) In interactive mode, I issued: scan, followed by:
3) scan_results

(I'll attach the scan_results output, but I'll also attach a screenshot.)

Note that in the upcoming screenshot, you can see:

"Mozilla Mobile"
"Mozilla Guest"
"openwireless.org"

But a whole bunch of SSIDs are missing from the UI, which are present in the scan_results

Comment 10

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: scan_results.txt

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: IMAG3463.jpg

Comment 12

[Dimi Lee [ooo 16.04 - 21.04］]

Find a scenario that cause this problem:
1. wpa_supplicant scans and doesn't find 'ateam'
2. gecko notifies current scan result (without 'ateam')
3. wpa_supplicant re-scans and finds 'ateam'
4. gecko receives scan result update but doesn't notify GAIA
 - Original design is that gecko will not update scan result unless GAIA requests it.

Need to check if current design is correct.

Comment 13

[Jason Smith [:jsmith]]

Stephen - Is 1.4 affected here? Or just 2.0?

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to Jason Smith [:jsmith] from comment #13)
> Stephen - Is 1.4 affected here? Or just 2.0?

Hey Jason - we (or, at last I) now believe a lot of our Wi-Fi issues stem from the "ateam" node; to wit, http://selenium.qa.mtv2.mozilla.com:8080/job/b2g.hamachi.mozilla.b2g30_v1_4.ui.test_settings_wifi/, wherein I've only changed the Wi-Fi router the testrun connects to.  In this case, from "ateam" -> "qalab", which Clint set up.  We'll continue investigating, esp. as Dimi found a scenario -- at least on master -- in comment 12, that could also be affecting us.

Comment 15

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to Stephen Donner [:stephend] from comment #14)
> (In reply to Jason Smith [:jsmith] from comment #13)
> > Stephen - Is 1.4 affected here? Or just 2.0?
> 
> Hey Jason - we (or, at last I) now believe a lot of our Wi-Fi issues stem
> from the "ateam" node; to wit,
> http://selenium.qa.mtv2.mozilla.com:8080/job/b2g.hamachi.mozilla.b2g30_v1_4.
> ui.test_settings_wifi/, wherein I've only changed the Wi-Fi router the
> testrun connects to.  In this case, from "ateam" -> "qalab", which Clint set
> up.  We'll continue investigating, esp. as Dimi found a scenario -- at least
> on master -- in comment 12, that could also be affecting us.

(And, sorry, to directly answer the question: we see this, more often, under all branches: 1.3, 1.4, and master, when using the "ateam" SSID/Wi-Fi node; whereas the new "qalab" node Clint set up fares a lot better in every test which uses Wi-Fi, on each branch.)

Comment 16

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to Dimi Lee[:dimi][:dlee] from comment #12)
> Find a scenario that cause this problem:
> 1. wpa_supplicant scans and doesn't find 'ateam'
> 2. gecko notifies current scan result (without 'ateam')
> 3. wpa_supplicant re-scans and finds 'ateam'
> 4. gecko receives scan result update but doesn't notify GAIA
>  - Original design is that gecko will not update scan result unless GAIA
> requests it.
> 
> Need to check if current design is correct.

Dimi, do you have an update from the team, here, on this?  Thanks!

Comment 17

[Vincent Chang[:vchang][changyihsin]]

(In reply to Stephen Donner [:stephend] from comment #16)
> (In reply to Dimi Lee[:dimi][:dlee] from comment #12)
> > Find a scenario that cause this problem:
> > 1. wpa_supplicant scans and doesn't find 'ateam'
> > 2. gecko notifies current scan result (without 'ateam')
> > 3. wpa_supplicant re-scans and finds 'ateam'
> > 4. gecko receives scan result update but doesn't notify GAIA
> >  - Original design is that gecko will not update scan result unless GAIA
> > requests it.
> > 
> > Need to check if current design is correct.
> 
> Dimi, do you have an update from the team, here, on this?  Thanks!

Since wpa_supplicant may report scan result for a scan command twice, we would like to provide a new API to report scan result whenever it's available from wpa_supplicant.

The new API may look like this, 
  
  attribute EventHandler onScanResults;

Comment 19

[Jason Smith [:jsmith]]

Using the dupe's STR, we should check if this happens on 1.3.

Comment 21

[Stephen Donner [:stephend] Not actively reading bugmail]

Re-summarizing; Dimi/others - please correct as-needed - thanks!

Comment 22

[Stephen Donner [:stephend] Not actively reading bugmail]

Given that Taipei QA also sees this (see bug 1026435) from time-to-time, :dlee, :vchang, when might we see this addressed in master?   Thanks!

Comment 23

[Dimi Lee [ooo 16.04 - 21.04］]

Because this week is NFC work week, we may start to work on this next week.

Comment 24

[Henry Chang [:hchang]]

Dimi, Vincent,

I can help take care of this. So what we are going to address this issue is to

1) Add a event handler like onscanresult to report EVERY SINGLE scan result reported by wpa_supplicant.
2) Modify gaia side to fresh the SSID list on EVERY receipt of the scan result.

Please correct me if I am understanding wrong. Thanks!

Comment 25

[Vincent Chang[:vchang][changyihsin]]

That's what I understand.

Comment 26

[Vincent Chang[:vchang][changyihsin]]

Hi EJ, we would like to proposal a new api mentioned in comment 24 to prevent missing SSIDs issue. May we have your comments from gaia's perspective?

Comment 27

[EJ Chen [:eragonj][:小龍哥][Contributor mode on](inactive)]

Hi Vincent,

I think the API might be simple enough and there is nothing to be aware. Just want to remind you guys that we are refactoring wifi panels and got r+ already, it would be easier to patch this after refactoring. We will try to land the refactoring work in few days, so nothing to be worried, just a hint for you guys. (bug 973466)

Comment 28

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

I am unable to reproduce this issue on Flame 2.1, Flame 2.0, Flame 1.4, Buri 1.4, or Buri 1.3 using the steps in bug 1021920. 

Environmental Variables:
Device: Flame Master
BuildID: 20140801110356
Gaia: 9689218473b6fc4dd927ad6aa7b06c05f0843824
Gecko: ff1e09666c42
Version: 34.0a1 (Master) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Environmental Variables:
Device: Flame 2.0
BuildID: 20140801103111
Gaia: 3b61868a7f2817ef73b1c9303c072dcd6462a233
Gecko: e816c908a274
Version: 32.0 (2.0) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Environmental Variables:
Device: Flame 1.4
BuildID: 20140801103150
Gaia: 8b476ef4cca719a2e90eb30fdcef0c3617cf574b
Gecko: 641db5b6540c
Version: 30.0 (1.4) 
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Environmental Variables:
Device: Buri 1.4
BuildID: 20140801135259
Gaia: 8b476ef4cca719a2e90eb30fdcef0c3617cf574b
Gecko: d5625dcbd7f1
Version: 30.0 (1.4) 
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Environmental Variables:
Device: Buri 1.3
BuildID: 20140721151227
Gaia: 23f55be856cef53c6604a6fe4aeb09061afbc897
Gecko: 9727017eabb9
Version: 28.0 (1.3) 
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:28.0) Gecko/28.0 Firefox/28.0

Comment 29

[XiaoLe Sun]

Attachment: Bug1011358.patch

Comment 30

[Henry Chang [:hchang]]

Comment on attachment 8636480 [details] [diff] [review]
Bug1011358.patch

Review of attachment 8636480 [details] [diff] [review]:
-----------------------------------------------------------------

Overall looks good to me. Please fix the minor coding standard issue
and then we can ask DOM peer for reviewing webidls. Thanks!

::: dom/webidl/MozWifiManager.webidl
@@ +347,5 @@
> +
> +  /**
> +   * An event listener that is always called with information about the signal
> +   * strength and link speed.
> +   */

Please update the comments.

::: dom/webidl/MozWifiScanResultEvent.webidl
@@ +9,5 @@
> +{
> + /**
> +   * Network objects of scan result
> +   */
> +    readonly attribute any networks; 

Please remove the trailing white spaces and fix the indentation.

@@ +14,5 @@
> +};
> +
> +dictionary MozWifiScanResultEventInit : EventInit
> +{
> +    any networks = null;

The indentation should be 2 spaces.

::: dom/wifi/DOMWifiManager.js
@@ +415,5 @@
>    },
>  
> +  _fireScanResult: function onScanResult(nets){
> +    var netArray = new this._window.Array();
> +    for(var i in nets){ netArray.push( this._convertWifiNetwork(nets[i]) ); }

Please refer to the javascript coding style https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style

Besides, use |let| instead of |var| and don't use 'for in'. You may refer to http://stackoverflow.com/questions/500504/why-is-using-for-in-with-array-iteration-such-a-bad-idea

Comment 31

[XiaoLe Sun]

Attachment: Bug1011358.patch

Comment 32

[XiaoLe Sun]

Attachment: Bug1011358.patch

Comment 33

[Henry Chang [:hchang]]

Comment on attachment 8637016 [details] [diff] [review]
Bug1011358.patch

Review of attachment 8637016 [details] [diff] [review]:
-----------------------------------------------------------------

There are still a few format error. Besides, I don't understand the changes around
line 2372 in WifiWorker.js.

::: dom/webidl/MozWifiManager.webidl
@@ +345,5 @@
>     */
>    attribute EventHandler onstationinfoupdate;
> +
> +  /**
> +   * An event listener that is called with information about the scanned wifi 

Trailing white spaces

::: dom/wifi/DOMWifiManager.js
@@ +416,5 @@
>  
> +  _fireScanResult: function onScanResult(nets){
> +    var netArray = new this._window.Array();
> +    for(let i=0; i<nets.length ;i++){
> +      netArray.push( this._convertWifiNetwork(nets[i]) );

There are still a few format error around.

::: dom/wifi/WifiWorker.js
@@ +2369,4 @@
>      debug("Scan results are available! Asking for them.");
>      WifiManager.getScanResults(function(r) {
>        // Failure.
> +      if (!r && (self.wantScanResults.length !== 0) ) {

Why change this?

@@ +2432,5 @@
>            debug("Match didn't find anything for: " + lines[i]);
>          }
>        }
> +      if (self.wantScanResults.length === 0) {
> +        self._fireEvent("onscanresult", { networks : self.networksArray});

It should be 

self._fireEvent("onscanresult", { networks : self.networksArray });

Comment 34

[XiaoLe Sun]

Attachment: Bug1011358.patch

Comment 35

[XiaoLe Sun]

Attachment: Bug1011358.patch

Comment 36

[XiaoLe Sun]

Attachment: Bug1011358.patch

Comment 37

[Ethan Tseng [:ethan]]

Comment on attachment 8640366 [details] [diff] [review]
Bug1011358.patch

Review of attachment 8640366 [details] [diff] [review]:
-----------------------------------------------------------------

Cancel the review request. We are not working on this bug anymore.

Comment 38

[BMO Automation]

Firefox OS is not being worked on