During the development of Wifi Direct, some of the common wifi native command and network facilities will be extracted to separate jsm. In order to remove the duplicated part and reduce the number of lines in WifiWorker.js, we can re-use those jsm in WifiWorker.js

What this patch does: 1. Move the fundamental control functions from WifiWorker to WifiCommand: - voidControlMessage - doCommand - doIntCommand - doBooleanCommand - doStringCommand 2. Move all fundamental wifi functions from WifiWorker to WifiCommand: //----------------------------------------- // Direct replacement //----------------------------------------- - startSupplicant - terminateSupplicant - stopSupplicant - killSupplicant - connectToSupplicant - closeSupplicantConnection - listNetworksCommand - addNetworkCommand - setNetworkVariableCommand - getNetworkVariableCommand - removeNetworkCommand - enableNetworkCommand - disableNetworkCommand - statusCommand - pingCommand - scanResultsCommand - disconnectCommand - reconnectCommand - reassociateCommand - doSetScanModeCommand - setLogLevel - getLogLevel - wpsPbcCommand - wpsPinCommand - wpsCancelCommand - startDriverCommand - stopDriverCommand - startPacketFiltering - stopPacketFiltering - doGetRssiCommand - getRssiCommand - getRssiApproxCommand - getLinkSpeedCommand - getConnectionInfoGB - getConnectionInfoICS - getMacAddressCommand - setPowerModeCommandICS - setPowerModeCommandJB - getPowerModeCommand - setNumAllowedChannelsCommand - getNumAllowedChannelsCommand - setBluetoothCoexistenceModeCommand - setBluetoothCoexistenceScanModeCommand - saveConfigCommand - reloadConfigCommand - setScanResultHandlingCommand - addToBlacklistCommand - clearBlacklistCommand - setSuspendOptimizationsCommand //------------------------------------------------------ // Some functions are fundamental but keep information // across function calls. So we don't direct replace // those functions. Instead we implement the actual // fundamental ones and make use of them in WifiWorker // These functions are regarded as internal APIs. //------------------------------------------------------ - loadDriver - unloadDriver - setBackgroundScan - scanCommand - setScanModeCommand - 3. Move all fundamental network functions from WifiWorker to WifiNetUtil - enableInterface - disableInterface - addHostRoute - removeHostRoutes - setDefaultRoute - getDefaultRoute - removeDefaultRoute - resetConnections - runDhcp - stopDhcp - releaseDhcpLease - getDhcpError - configureInterface - runDhcpRenew - runIpConfig p.s. In the current attached patch, dhcpInfo is a member variable of WifiNetUtil. I already move it out of WifiNetUtil in the new patch. I'll attach the latest patch after getting the initial feedback. Thanks.

Comment on attachment 809627 [details] [diff] [review] Bug 919426 - Part 1: Add new file WifiCommand.jsm Review of attachment 809627 [details] [diff] [review]: ----------------------------------------------------------------- The patch looks good. Thank you. Please help to address some nits below. ::: dom/wifi/WifiCommand.jsm @@ +9,5 @@ > +// JSHint directive to suppressing warnings > +/*global dump*/ > +/*global Components*/ > +/*global libcutils*/ > + Why do we need these comments ? @@ +210,5 @@ > + > + command.getLinkSpeed = function (callback) { > + doStringCommand("DRIVER LINKSPEED", function(reply) { > + if (reply) { > + reply = reply.split(" ")[1] | 0; // Format: LinkSpeed XX period at end of sentence. @@ +246,5 @@ > + > + command.getMacAddress = function (callback) { > + doStringCommand("DRIVER MACADDR", function(reply) { > + if (reply) { > + reply = reply.split(" ")[2]; // Format: Macaddr = XX.XX.XX.XX.XX.XX period at end of sentence. @@ +264,5 @@ > + command.getPowerMode = function (callback) { > + doStringCommand("DRIVER GETPOWER", function(reply) { > + if (reply) { > + reply = (reply.split()[2]|0); // Format: powermode = XX > + } period at end of sentence. @@ +276,5 @@ > + > + command.getNumAllowedChannels = function (callback) { > + doStringCommand("DRIVER SCAN-CHANNELS", function(reply) { > + if (reply) { > + reply = (reply.split()[2]|0); // Format: Scan-Channels = X period at end of sentence. @@ +292,5 @@ > + "OK", callback); > + }; > + > + command.saveConfig = function (callback) { > + // Make sure we never write out a value for AP_SCAN other than 1 period at end of sentence. @@ +330,5 @@ > + > + command.getMacAddress = function(callback) { > + doStringCommand("DRIVER MACADDR", function(reply) { > + if (reply) { > + reply = reply.split(" ")[2]; // Format: Macaddr = XX.XX.XX.XX.XX.XX period at end of sentence. @@ +355,5 @@ > + /* > + command.p2pDisconnect = function(callback) { > + doBooleanCommand("P2P_GROUP_REMOVE p2p0", "OK", callback); > + }; > + */ Do we still need this ? If not, please remove it. @@ +363,5 @@ > + }; > + > + command.p2pEnable = function(deviceName, callback) { > + var deviceType = "10-0050F204-5"; > + Is it constant value ? Not sure if it's a platform dependent variable ? @@ +364,5 @@ > + > + command.p2pEnable = function(deviceName, callback) { > + var deviceType = "10-0050F204-5"; > + > + var COMMAND_CHAIN = [ "SET device_name " + deviceName, s/COMMAND_CHAIN/commandChain/ @@ +427,5 @@ > + function doBooleanCommandChain(commandChain, callback, i) { > + if (undefined === i) { > + i = 0; > + } > + Maybe we can pass i=-1 when we call this function first time ? So that we can remove the condition check here. @@ +434,5 @@ > + return callback(false); > + } > + i++; > + if (i === commandChain.length || !commandChain[i]) { > + // reach the end or empty command period at end of sentence.

(In reply to Vincent Chang[:vchang] from comment #5) > Comment on attachment 809627 [details] [diff] [review] > Bug 919426 - Part 1: Add new file WifiCommand.jsm > > Review of attachment 809627 [details] [diff] [review]: > ----------------------------------------------------------------- > > The patch looks good. Thank you. > Please help to address some nits below. > > ::: dom/wifi/WifiCommand.jsm > @@ +9,5 @@ > > +// JSHint directive to suppressing warnings > > +/*global dump*/ > > +/*global Components*/ > > +/*global libcutils*/ > > + > > Why do we need these comments ? They are jshint directives to declare the external symbols. > @@ +355,5 @@ > > + /* > > + command.p2pDisconnect = function(callback) { > > + doBooleanCommand("P2P_GROUP_REMOVE p2p0", "OK", callback); > > + }; > > + */ > > Do we still need this ? If not, please remove it. Well, I think I can just remove all p2p functions since they won't be used at this time. > @@ +427,5 @@ > > + function doBooleanCommandChain(commandChain, callback, i) { > > + if (undefined === i) { > > + i = 0; > > + } > > + > > Maybe we can pass i=-1 when we call this function first time ? So that we > can remove the condition check here. It's the common default argument alternative in javascript. If the internal coding guide considers this a bad practice or harmful, I can change all the calling point to doBooleanCommandChain(xxxChain, callback, 0) I'll attach a new patch to add all the missing trailing period, remove all p2p functions and add my dhcpInfo changes I mentioned in the previous post. Thanks!

Comment on attachment 809628 [details] [diff] [review] Bug 919426 - Part 2: Added new file WifiNetUtil.jsm Review of attachment 809628 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Thank you. ::: dom/wifi/WifiNetUtil.jsm @@ +11,5 @@ > +/*global XPCOMUtils*/ > +/*global gNetworkManager*/ > +/*global libcutils*/ > +/*global dump*/ > + Why do we need these comments ? @@ +125,5 @@ > + > + util.releaseDhcpLease = function (ifname, callback) { > + controlMessage({ cmd: "dhcp_release_lease", ifname: ifname }, function(data) { > + util.dhcpInfo = null; > + //notify("dhcplost"); Please remove it.

Comment on attachment 809629 [details] [diff] [review] Bug 919426 - Part 3: Make use of WifiCommand/WifiNetUtil Review of attachment 809629 [details] [diff] [review]: ----------------------------------------------------------------- Looks good.

Added new 3 patches (marked v2). The changes are: 1) Added trailing periods. 2) Removed jshint directives. 3) Removed currently unused p2p commands. 4) Renamed COMMAND_CHAIN to commandChain. 5) Moved out WifiNetUtil::dhcpInfo to WifiWorker.js (See line 367/381/393/560/1128 @ new WifiWorker.js).

Comment on attachment 811795 [details] [diff] [review] Bug 919426 - Part1: Add WifiCommand.jsm v2 Review of attachment 811795 [details] [diff] [review]: ----------------------------------------------------------------- Looks good.

Comment on attachment 811796 [details] [diff] [review] Bug 919426 - Part 2: Added new file WifiNetUtil.jsm v2 Review of attachment 811796 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Thank you.

Comment on attachment 811798 [details] [diff] [review] Bug 919426 - Part 3: Make use of WifiCommand/WifiNetUtil v2 Review of attachment 811798 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Thank you.

Thanks Vincent! I also obsoleted old patches. The current 3 patches are the ones for checkin-needed flag. Thanks.

https://hg.mozilla.org/integration/b2g-inbound/rev/ac83fa32e867 https://hg.mozilla.org/integration/b2g-inbound/rev/21754eba4931 https://hg.mozilla.org/integration/b2g-inbound/rev/f75c25e53e53

https://hg.mozilla.org/mozilla-central/rev/ac83fa32e867 https://hg.mozilla.org/mozilla-central/rev/21754eba4931 https://hg.mozilla.org/mozilla-central/rev/f75c25e53e53