Last Comment Bug 740079 - B2G Wifi: Add a synchronous API to get the current wifi state
: B2G Wifi: Add a synchronous API to get the current wifi state
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: unspecified
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla14
Assigned To: Blake Kaplan (:mrbkap) (please use needinfo!)
:
Mentors:
Depends on:
Blocks: b2g-wifi
  Show dependency treegraph
 
Reported: 2012-03-28 11:41 PDT by Blake Kaplan (:mrbkap) (please use needinfo!)
Modified: 2012-04-17 22:27 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed fix v1 (4.45 KB, patch)
2012-04-08 20:35 PDT, Blake Kaplan (:mrbkap) (please use needinfo!)
cjones.bugs: review+
Details | Diff | Review
Painted (4.57 KB, patch)
2012-04-13 04:40 PDT, Blake Kaplan (:mrbkap) (please use needinfo!)
no flags Details | Diff | Review
Painted the right color (4.58 KB, patch)
2012-04-16 08:26 PDT, Blake Kaplan (:mrbkap) (please use needinfo!)
fabien: feedback+
Details | Diff | Review

Description Blake Kaplan (:mrbkap) (please use needinfo!) 2012-03-28 11:41:25 PDT
Right now, the synchronous wireless APIs allow a page to get the "connected" network which is a boolean state: connected or not. We need to expose state like "connecting", "obtaining IP address", etc.
Comment 1 Fabien Cazenave [:kaze] 2012-03-28 15:46:14 PDT
Either that, or more boolean properties. I’m not sure which would be better.
Comment 2 Blake Kaplan (:mrbkap) (please use needinfo!) 2012-04-08 20:35:57 PDT
Created attachment 613221 [details] [diff] [review]
Proposed fix v1

This patch renames connectedNetwork -> connectionStatus. My eventual hope is to also add things like the BSS/security used and IP address. Right now it contains the connected network (under "network") and the current status (under "status"). The status names are strings that map to the events that we currently fire.

This requires a couple of small changes to wifi.js.
Comment 3 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-04-08 21:59:30 PDT
Comment on attachment 613221 [details] [diff] [review]
Proposed fix v1

>diff --git a/dom/wifi/nsIWifi.idl b/dom/wifi/nsIWifi.idl
>index 91b7022..b2ae6c9 100644
>--- a/dom/wifi/nsIWifi.idl
>+++ b/dom/wifi/nsIWifi.idl
>@@ -99,9 +99,11 @@ interface nsIDOMWifiManager : nsISupports {
>     readonly attribute boolean enabled;
> 
>     /**
>-     * A network object describing the currently connected network.
>+     * An object containing the following information:
>+     *  - status ("disconnected", "connecting", "associated", "connected")
>+     *  - network
>      */
>-    readonly attribute jsval connectedNetwork;
>+    readonly attribute jsval connectionStatus;
> 

I shall bikeshed this, because I can!

If we're going to change the interface here, how about calling this
|connection|, so that clients can say |connection.status| instead of
|connectionStatus.status|.

Also, the object returned here can never be null, right?  Should doc that.

It's a little confusing to return an object |o| in which |o.status =
"disconnected"| (e.g.) is a no-op, but I defer to DOM folks on API
conventions.

Implementation looks good, r=me with bikeshed painted.
Comment 4 Fabien Cazenave [:kaze] 2012-04-11 08:44:03 PDT
(In reply to Blake Kaplan (:mrbkap) from comment #2)
> This patch renames connectedNetwork -> connectionStatus.

If this means that we’ll have:
  
 • `mozWifiManager.connection.network' returning a real network object (i.e. comparable with an element of the `getNetworks' output)

 • `mozWifiManager.connection.status' returning the state of the current/requested network

then I’m OK. As you mention, it will be easy to extend if we need more information (e.g. IP address, bandwidth…).
Comment 5 Fabien Cazenave [:kaze] 2012-04-12 06:35:45 PDT
Another thing: I’d love to have an `onstatuschange' event to get along with the synchronous `status' property. There are a few use cases where this would be easier than a set of onconnect / onconnecting / ondisconnect / etc. event listeners and it would be more XHR-like.
Comment 6 Blake Kaplan (:mrbkap) (please use needinfo!) 2012-04-13 04:40:25 PDT
Created attachment 614735 [details] [diff] [review]
Painted
Comment 7 Fabien Cazenave [:kaze] 2012-04-14 11:37:45 PDT
>-  get connectedNetwork() {
>+  get status() {
>     if (!this._hasPrivileges)
>       throw new Components.Exception("Denied", Cr.NS_ERROR_FAILURE);
>-    return this._currentNetwork;
>+    return { status: this._connectionStatus, network: this._currentNetwork };
>   },

Does that mean we get this?
 • mozWifiManager.status.status
 • mozWifiManager.status.network

If 'status' and 'network' are properties of the same JS object, I’d prefer this object to carry a different name than 'status' (Chris suggested 'connection')… but that’s up to you, I’ll adapt the Gaia front-end accordingly when this lands.
Comment 8 Mounir Lamouri (:mounir) 2012-04-16 01:40:31 PDT
Why not making this leave in navigator.mozConnection?
Like:
navigator.mozConnection.wifi.status
navigator.mozConnection.wifi.network // or .ssid, don't know what .network is for exactly
Comment 9 Blake Kaplan (:mrbkap) (please use needinfo!) 2012-04-16 08:25:14 PDT
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #8)
> Why not making this leave in navigator.mozConnection?

The entirety of mozWifi(Manager) should live under mozConnection, IMO. gal and I actually talked about that in the beginning. I'll file a followup on doing that.

> navigator.mozConnection.wifi.network // or .ssid, don't know what .network
> is for exactly

I'd prefer keeping ...network.ssid. That'll make it easier to add any additional information (like what security is available) later.
Comment 10 Blake Kaplan (:mrbkap) (please use needinfo!) 2012-04-16 08:26:06 PDT
Created attachment 615340 [details] [diff] [review]
Painted the right color

I grabbed the wrong brush the last time.
Comment 11 Blake Kaplan (:mrbkap) (please use needinfo!) 2012-04-16 09:43:35 PDT
https://hg.mozilla.org/mozilla-central/rev/14084207e9d1
Comment 12 Philipp von Weitershausen [:philikon] 2012-04-17 22:07:22 PDT
(In reply to Fabien Cazenave (:kaze) from comment #5)
> Another thing: I’d love to have an `onstatuschange' event to get along with
> the synchronous `status' property. There are a few use cases where this
> would be easier than a set of onconnect / onconnecting / ondisconnect / etc.
> event listeners and it would be more XHR-like.

+1. File it?

(In reply to Blake Kaplan (:mrbkap) from comment #9)
> (In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #8)
> > Why not making this leave in navigator.mozConnection?
> 
> The entirety of mozWifi(Manager) should live under mozConnection, IMO. gal
> and I actually talked about that in the beginning. I'll file a followup on
> doing that.

In that case, we should perhaps move navigator.mozMobileConnection to .mozConnection.mobile as well. But I want to do that in a follow-up, too :)
Comment 13 Philipp von Weitershausen [:philikon] 2012-04-17 22:27:53 PDT
(In reply to Philipp von Weitershausen [:philikon] from comment #12)
> (In reply to Fabien Cazenave (:kaze) from comment #5)
> > Another thing: I’d love to have an `onstatuschange' event to get along with
> > the synchronous `status' property. There are a few use cases where this
> > would be easier than a set of onconnect / onconnecting / ondisconnect / etc.
> > event listeners and it would be more XHR-like.
> 
> +1. File it?

Never mind... bug 745114 :)

Note You need to log in before you can comment on or make changes to this bug.