[Wifi] Send network info along with state change event: disconnected and connectingfailed

RESOLVED FIXED in 2.1 S5 (26sep)

Status

Firefox OS
Wifi
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: chucklee, Assigned: chucklee)

Tracking

unspecified
2.1 S5 (26sep)
All
Gonk (Firefox OS)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Some state change event like disconnected and connectingfailed doesn't contain network info, so app has to get network data from wifiManager.connectionInformation now.
But it doesn't contain network information, also current network info might be different from the network triggers the state change(especially disconnected and connectingfailed state).
We should revise and make sure we provided as much network info as possible with ever state change event.
Created attachment 8469876 [details] [diff] [review]
WIP - Provide network info for state change event.
Created attachment 8475030 [details] [diff] [review]
WIP - Provide network info for state change event.

Provide network information on each wifi state change, so Gaia can take these info to control UI more precisely.
WPS state is not included.
Attachment #8469876 - Attachment is obsolete: true
Attachment #8475030 - Flags: feedback?(vchang)
Attachment #8475030 - Flags: feedback?(hchang)
Comment on attachment 8475030 [details] [diff] [review]
WIP - Provide network info for state change event.

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

::: dom/wifi/DOMWifiManager.js
@@ +364,4 @@
>          this._connectionStatus = "disconnected";
>          this._lastConnectionInfo = null;
>          this._fireStatusChangeEvent();
> +        this._currentNetwork = null;

Can we use msg.network directly?
Attachment #8475030 - Flags: feedback?(vchang) → feedback+
Comment on attachment 8475030 [details] [diff] [review]
WIP - Provide network info for state change event.

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

The idea generally looks good to me! Thanks!

::: dom/wifi/WifiWorker.js
@@ +2004,5 @@
>        if (netId) {
>          WifiManager.disableNetwork(netId, function() {});
>        }
>      });
> +    self._fireEvent("onconnectingfailed", {network: netToDOM(self.currentNetwork)});

Does it mean the original version is not working?

@@ +2050,5 @@
>          self.currentNetwork.netId = this.id;
> +        WifiManager.getNetworkConfiguration(self.currentNetwork, function (){
> +          // Notify again because we get complete network information.
> +          self._fireEvent("onconnecting", { network: netToDOM(self.currentNetwork) });
> +        }.bind(self));

No need to bind |self| since the callback function doesn't use |this|.
Attachment #8475030 - Flags: feedback?(hchang) → feedback+
(In reply to Henry Chang [:henry] from comment #4)
> ::: dom/wifi/WifiWorker.js
> @@ +2004,5 @@
> >        if (netId) {
> >          WifiManager.disableNetwork(netId, function() {});
> >        }
> >      });
> > +    self._fireEvent("onconnectingfailed", {network: netToDOM(self.currentNetwork)});
> 
> Does it mean the original version is not working?
> 

Yes, but Gaia doesn't use event.network now, so it doesn't affect them now.
Created attachment 8476603 [details] [diff] [review]
WIP - Provide network info for state change event.

Address feedback comments.
Attachment #8475030 - Attachment is obsolete: true
Created attachment 8487748 [details] [diff] [review]
Provide network info for state change event.
Attachment #8476603 - Attachment is obsolete: true
Attachment #8487748 - Flags: review?(hchang)
Comment on attachment 8487748 [details] [diff] [review]
Provide network info for state change event.

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

Good patch and no concern to me. Thanks!
Attachment #8487748 - Flags: review?(hchang) → review+
Created attachment 8490587 [details] [diff] [review]
Provide network info for state change event. r=hchang

Add reviewer's name.
Attachment #8487748 - Attachment is obsolete: true
Try looks good, CPP Unit Test Fail is irrelevant because it has been failed several days and I only changes js file : https://tbpl.mozilla.org/?tree=Try&rev=826f124c7898
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c8b5a6377dad
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S5 (26sep)
You need to log in before you can comment on or make changes to this bug.