Create an interface for getting wifi connection speed currently.

Comment on attachment 787932 [details] [diff] [review] get WiFi link speed Review of attachment 787932 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/wifi/WifiWorker.js @@ +2816,5 @@ > }).bind(this)); > }, > > + getLinkSpeed: function() { > + return this._lastConnectionInfo.linkSpeed; this._lastConnectionInfo could be null right, please add some sanity check here. ::: dom/wifi/nsIWifi.idl @@ +59,5 @@ > void getWifiScanResults(in nsIWifiScanResultsReady callback); > + > + /** > + * Returns WiFi link speed from this._lastConnectionInfo.linkSpeed. > + */ Return Wifi connected speed in Mbit/s

Hi Vincent, Gavin: I wonder whether we could send an observer notification (containing linkSpeed) to PowerStatsService.jsm whenever WifiWorker fires a connectionInfoUpdate event instead of creating a new method on the WifiWorker for querying linkSpeed. I check WifiWorker.js and it shows that the connectionInfoUpdate event is fired every 5 seconds. But, if the linkSpeed is not changed (as well as the change of RSSI is less than 10%), the event would not be fired. That means the linkSpeed may not be changed in some circumstance such like users leave his/her phone on the table. So, I think that it could be more efficient if we use notification to update a linkSpeed variable record inside PowerStatsService instead of querying the linkSpeed everytime when we want to calculate WiFi power consumption. Any suggestion about this?

Hi Borting: I think observer notification would be more efficient. The only concern I have is PowerStatsService does not have the link speed value and PowerStatsService does not receive the notification. Is there any possibilities that PowerStatsService does not have link speed value? For example, after calling the clear data function or PowerStatsService is just initialized. If it would have the chance PowerStatsService does not have the link speed value, I suggest the querying method could be remained as a secondary method. The observer notification is the primary method.

Hi Gavin: Your concern is right. I double checked the WiFiWorker.js and I found when we complete the connection with AP, the WiFiWorker will call getConnectionInformation() [1] which fires the first connectionInfoUpdate event. So, if we add the notification code befire firing event (around here [2]), I think PowerStatsService would obtain the linkSpeed before fxos receives traffic from AP. Is there any situation we cannot receive a linkSpeed update in advance? [1] http://mxr.mozilla.org/mozilla-central/source/dom/wifi/WifiWorker.js#2169 [2] http://mxr.mozilla.org/mozilla-central/source/dom/wifi/WifiWorker.js#2162

Hi Vincent: Could you help review this patch? Thx a lot. Update Notes: 1. Send out a notification containing linkSpeed before firing a connectionInfoUpdate event.

Comment on attachment 8343644 [details] [diff] [review] Obtain WiFi Link Speed Review of attachment 8343644 [details] [diff] [review]: ----------------------------------------------------------------- Using the wifi API seems more straightforward to me. I would prefer to use the original patch to get link speed directly.

Hi Vincent: I've changed the getLinkSpeed method to the original one. Please help review this patch. Thanks!

Comment on attachment 8349821 [details] [diff] [review] Bug-899596-getWiFiLinkSpeed.patch Review of attachment 8349821 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/wifi/WifiWorker.js @@ +2484,5 @@ > this._sendMessage(message, true, networks, msg); > }).bind(this)); > }, > > + getLinkSpeed: function() { Nit: We like to early return. if (!this._lastConnectionInfo.linkSpeed) return 0; return this._lastConnectionInfo.linkSpeed;

fix nit. Hi Vincent, can you help review this patch? Thanks a lot.

Comment on attachment 8353900 [details] [diff] [review] Bug-899596-getWiFiLinkSpeed.patch Review of attachment 8353900 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Thank you.

Firefox OS is not being worked on