811833 - [System] When the device is woken from sleep, wifi does not automatically connect to the network

Status: RESOLVED FIXED

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

## Environment :
Unagi phone, build 20121114
  
## Repro :
1. connect device to network through settings
2. let the device sleep
3. wake the device

## Expected :
1. network connection is established

## Actual :
1. you have to go to the settings, where wifi will then automatically turn back on and then select a network

## Note :
1. regression?

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo; no longer work for MoCo)]

This is a unagi specific issue, I don't see it on Otoro.

Comment 2

[Vincent Chang[:vchang][changyihsin]]

Finally I can see this bug from Tim's unagi phone. It's weird that others unagi phone don't have this bug. 

Can I have your device's wifi configuration file located in /data/misc/wifi/wpa_supplicant.conf ? In that file, you can find that parameter "disabled" is set to 1. It means the network is disabled and wpa_supplicant will not try to connect to the network automatically.  
network={
	ssid="Mozilla Guest"
	key_mgmt=NONE
	priority=9
	disabled=1
}
However, if I delete the wpa_supplicant.conf by hand, and try to reproduce the bug again. I can't reproduce the problem anymore. The "disabled" parameter is disappeared.   
network={
	ssid="Mozilla Guest"
	key_mgmt=NONE
}

Will try to check what's going on there.

Comment 3

[Dietrich Ayala (:dietrich)]

I see it on Otoro.

Comment 4

[Vincent Chang[:vchang][changyihsin]]

Hi mrbkap,

Comment 5

[Vincent Chang[:vchang][changyihsin]]

Hi mrbkap, 

I am wondering if there is a case that we disable certain network using DISABLE_NETWORK command and save that config to wpa_supplicant.conf using "SAVE_CONFIG" command. In that case, we could have "disabled=1" in wpa_supplicant.conf.

Comment 6

[Vincent Chang[:vchang][changyihsin]]

(In reply to Dietrich Ayala (:dietrich) from comment #3)
> I see it on Otoro.

Can you provide wpa_supplicant.conf in your otoro phone ?

Comment 7

[Vincent Chang[:vchang][changyihsin]]

I find this bug after doing the FOTA. I don't observe the same bug if I update firmware from nightly build manually. Not sure what's the difference between these two methods.

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo; no longer work for MoCo)]

Vincent, I need my dogfood phone back :)

Comment 9

[Marshall Culpepper [:marshall_law]]

(In reply to Vincent Chang[:vchang] from comment #7)
> I find this bug after doing the FOTA. I don't observe the same bug if I
> update firmware from nightly build manually. Not sure what's the difference
> between these two methods.

To clarify, we've only supplied OTA (i.e gecko only) updates to dogfooders so far. The first FOTA update will be coming shortly.

The dogfood / stable URL is only updated once a week right now -- so this could also be a difference in bugs fixed on nightly vs stable.

Comment 10

[Dietrich Ayala (:dietrich)]

Created attachment 682531 [details]
otoro wpa_supplicant.conf

Comment 11

[Vincent Chang[:vchang][changyihsin]]

(In reply to Dietrich Ayala (:dietrich) from comment #10)
> Created attachment 682531 [details]
> otoro wpa_supplicant.conf
The file itself looks no problem. After associating with an AP(Mozilla Guest), the wpa_supplicant.conf will be updated as below. 
============================================================
ctrl_interface=DIR=/data/system/wpa_supplicant GROUP=wifi
update_config=1
wfd_session_mgmt_ctrl_port=554
wfd_device_max_throughput=10

network={
	ssid="Mozilla Guest"
	key_mgmt=NONE
}
============================================================

Comment 12

[Vincent Chang[:vchang][changyihsin]]

(In reply to Marshall Culpepper [:marshall_law] from comment #9)
> (In reply to Vincent Chang[:vchang] from comment #7)
> > I find this bug after doing the FOTA. I don't observe the same bug if I
> > update firmware from nightly build manually. Not sure what's the difference
> > between these two methods.
> 
> To clarify, we've only supplied OTA (i.e gecko only) updates to dogfooders
> so far. The first FOTA update will be coming shortly.
> 
> The dogfood / stable URL is only updated once a week right now -- so this
> could also be a difference in bugs fixed on nightly vs stable.

I also tried to reproduce the bug with my unagi, but it's weird, I can't reproduce the bug. May need to borrow Tim's dogfooding phone and try again. 

Does it only update /system/b2g ? The update process seems to get two files, one is b2g_stable_update_2012-11-15_102959.mar the other one is update.zip. Not sure the difference between these two files.

Comment 13

[Marshall Culpepper [:marshall_law]]

(In reply to Vincent Chang[:vchang] from comment #12)
> Does it only update /system/b2g ? The update process seems to get two files,
> one is b2g_stable_update_2012-11-15_102959.mar the other one is update.zip.
> Not sure the difference between these two files.

Sounds like you do have the FOTA update :) MAR is a custom archive format used for gecko updates, and update.zip is the archive that google's FOTA client uses. In summary:

- FOTA (full / system) updates wrap an update.zip inside the MAR so we can reuse the existing Gecko delivery code. These updates can write to any partition on the device

- OTA (gecko + gaia) updates are MARs with files that map to /system/b2g. An OTA update can't touch anything outside of /system/b2g,

Comment 14

[Blake Kaplan (:mrbkap) (inactive, use needinfo)]

(In reply to Vincent Chang[:vchang] from comment #5)
> I am wondering if there is a case that we disable certain network using
> DISABLE_NETWORK command and save that config to wpa_supplicant.conf using
> "SAVE_CONFIG" command. In that case, we could have "disabled=1" in
> wpa_supplicant.conf.

I've been worrying about that. It might make sense to make sure that we only ever write out disabled=0 (enabled) networks and maybe also ensuring that all networks that we read in are enabled. Mind writing that patch?

Comment 15

[Chris Lee [:clee]]

I was able to easily reproduce this on the 11/16 nightly build, but then I updated my phone today and now it crashes randomly.  I'll see if I can get my phone in a state to debug.

Comment 16

[Vincent Chang[:vchang][changyihsin]]

Created attachment 683912 [details] [diff] [review]
Patch v1.0

This patch forces to enable the network when we connect to wpa_supplicant. Also, enable the network when _needToEnableNetworks is true.

Comment 17

[Blake Kaplan (:mrbkap) (inactive, use needinfo)]

Comment on attachment 683912 [details] [diff] [review]
Patch v1.0

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

::: dom/wifi/WifiWorker.js
@@ +1779,2 @@
>      var currentNetwork = self.currentNetwork;
> +    if (currentNetwork && currentNetwork.netId) {

The second check should check that currentNetwork.netId is a number. 0 is a valid netId.

@@ +1779,4 @@
>      var currentNetwork = self.currentNetwork;
> +    if (currentNetwork && currentNetwork.netId) {
> +      // Remove the network when the user entered an incorrect password.
> +      WifiManager.removeNetwork(currentNetwork.netId, function(ok) {

I don't understand why we're removing the network now instead of disabling it. Can you explain?

@@ +1788,5 @@
> +        });
> +      });
> +    } else {
> +      // TODO Bug-813880, we can't get netId when connecting to wep network. 
> +      WifiManager.disconnect(function(ok) {dump("disconnected current network")});

Nits: "Bug 813880" and trailing whitespace.

Non-nits: Please use debug() instead of dump(). Also, if you call disconnect, you should call reassociate afterwards, since disconnect tells wpa_supplicant to wait until we do so before it tries to connect to any more networks.

Comment 18

[Vincent Chang[:vchang][changyihsin]]

Created attachment 684301 [details] [diff] [review]
Patch v1.1

Updated the patch to address the review comments

Comment 19

[Blake Kaplan (:mrbkap) (inactive, use needinfo)]

Comment on attachment 684301 [details] [diff] [review]
Patch v1.1

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

Looks good. One small comment, but let's get this in.

::: dom/wifi/WifiWorker.js
@@ +1779,2 @@
>      var currentNetwork = self.currentNetwork;
> +    if (currentNetwork && !isNaN(currentNetwork.netId)) {

Instead of using isNaN, why not use |typeof currentNetwork.netId === "number"| since we don't actually use NaN anywhere else in the file?

Comment 20

[Vincent Chang[:vchang][changyihsin]]

(In reply to Blake Kaplan (:mrbkap) from comment #19)
> Comment on attachment 684301 [details] [diff] [review]
> Patch v1.1
> 
> Review of attachment 684301 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good. One small comment, but let's get this in.
> 
> ::: dom/wifi/WifiWorker.js
> @@ +1779,2 @@
> >      var currentNetwork = self.currentNetwork;
> > +    if (currentNetwork && !isNaN(currentNetwork.netId)) {
> 
> Instead of using isNaN, why not use |typeof currentNetwork.netId ===
> "number"| since we don't actually use NaN anywhere else in the file?

Thanks for your suggestion, let me do it in Bug 813880.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/85c1a0de5374

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/85c1a0de5374

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/06329b918184
https://hg.mozilla.org/releases/mozilla-beta/rev/dde5165cc0df

Comment 24

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I'm still seeing this on unagi with the 11-27 stable build.  I sent a wifi log to mrbkap.

Comment 25

[Blake Kaplan (:mrbkap) (inactive, use needinfo)]

I think that we determined with cjones that this was somehow related to his wpa_supplicant.conf and once he blew the existing one away and re-configured his network, it started to work.

Comment 27

[Faramarz Rashed]

I'm reopening this bug due to a behavior I see on a Peak device (1.3 pre-release build).  Wifi does not reconnect to known networks previously connected to.

STR:
1. connect to a wifi.
2. move out of range (outside the building etc).
3. return to wifi location.

## Expected:
device should reconnect to the previously known wifi it was connected to.

## Actual:
device does not connect.

Comment 28

[Jason Smith [:jsmith]]

This is an old bug - I'm going to open a new bug for this issue you are seeing.

Comment 29

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #28)
> This is an old bug - I'm going to open a new bug for this issue you are
> seeing.

I opened bug 979655 for the issue seen in comment 27.