wifi h/w capabilities test

wifi h/w capabilities test

[Sam Leffler, 2010-02-03 21:08:42]

This uses a little tool iwcap instead of parsing the output of iw.  If someone
can do the latter in python we can potentially junk iwcap.

[kmixter1, 2010-02-03 22:14:05]

LGTM

just nits.

http://codereview.chromium.org/563022/diff/1/8
File client/site_tests/network_WiFiCaps/network_WiFiCaps.py (right):

http://codereview.chromium.org/563022/diff/1/8#newcode14
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:14: version = 1
i think the convention is 4 spaces instead of 1 tab for indent.

http://codereview.chromium.org/563022/diff/1/8#newcode18
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:18: # create a empty
srcdir to prevent the error that checks .version file
80 chars

http://codereview.chromium.org/563022/diff/1/8#newcode20
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:20: utils.system('mkdir
%s' % self.srcdir)
os.mkdir(self.srcdir)

http://codereview.chromium.org/563022/diff/1/8#newcode36
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:36: def __RunIwcap(self,
phy, caps):
convention is usually lowercase and underscores on function names.

http://codereview.chromium.org/563022/diff/1/8#newcode37
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:37: dir = self.autodir +
'/deps/iwcap/'
Might instead want:
exec = os.path.join(self.autodir, 'deps', 'iwcap', 'iwcap')

http://codereview.chromium.org/563022/diff/1/8#newcode66
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:66: 'capability "%s" not
handled' %cap)
usually space after % operator

[petkov, 2010-02-03 22:14:51]

A few nits/style comments.
Is "iwcap" going to cross compile OK for ARM at some point -- do you foresee any
issues? It might be relatively easy to just grep through the "iw" output -- but
that's certainly not a priority at this point, as long as the test works.

http://codereview.chromium.org/563022/diff/1/2
File client/deps/iwcap/.gitignore (right):

http://codereview.chromium.org/563022/diff/1/2#newcode1
client/deps/iwcap/.gitignore:1: iwcap
Did you mean to check this control file? Theoretically, the build should happen
through build_autotest.sh, outside the source tree. If the file needs to be
here, shouldn't "iwcap" and "src/" be enough?

http://codereview.chromium.org/563022/diff/1/5
File client/deps/iwcap/iwcap.py (right):

http://codereview.chromium.org/563022/diff/1/5#newcode1
client/deps/iwcap/iwcap.py:1: #!/usr/bin/python
Add copyright.

http://codereview.chromium.org/563022/diff/1/7
File client/site_tests/network_WiFiCaps/control (right):

http://codereview.chromium.org/563022/diff/1/7#newcode1
client/site_tests/network_WiFiCaps/control:1: AUTHOR = "ChromeOS Team"
Add Chromium copyright.

http://codereview.chromium.org/563022/diff/1/8
File client/site_tests/network_WiFiCaps/network_WiFiCaps.py (right):

http://codereview.chromium.org/563022/diff/1/8#newcode5
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:5: import logging
Per local CODING_STYLE, import modules on one line.

http://codereview.chromium.org/563022/diff/1/8#newcode14
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:14: version = 1
Per local CODING_STYLE, 4 character indent (everywhere).

http://codereview.chromium.org/563022/diff/1/8#newcode18
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:18: # create a empty
srcdir to prevent the error that checks .version file
80 chars (would be fixed by fixing indent)

http://codereview.chromium.org/563022/diff/1/8#newcode22
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:22: def
__parse_iwcap(self, lines):
Per local CODING_STYLE, 2 empty lines between routines (here and below)

http://codereview.chromium.org/563022/diff/1/8#newcode26
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:26: parse_re =
re.compile(r'([a-z0-9]*):[ ]*(.*)')
I think "[ ]*" could be just " *"

http://codereview.chromium.org/563022/diff/1/8#newcode38
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:38: iwcap = utils.run(dir
+ 'iwcap ' + phy + ' ' + string.join(caps))
lots of spaces end )

[Sam Leffler, 2010-02-04 23:08:38]

iwcap should cross-compile fine; it's a much stripped down ripoff of iw.

As to parsing iw output; that can be added later if deemed useful.  This works
and simplifies checks that involve parsing the HT caps bitmask.

http://codereview.chromium.org/563022/diff/1/2
File client/deps/iwcap/.gitignore (right):

http://codereview.chromium.org/563022/diff/1/2#newcode1
client/deps/iwcap/.gitignore:1: iwcap
vestiges of having the code inline, removed

http://codereview.chromium.org/563022/diff/1/5
File client/deps/iwcap/iwcap.py (right):

http://codereview.chromium.org/563022/diff/1/5#newcode1
client/deps/iwcap/iwcap.py:1: #!/usr/bin/python
On 2010/02/03 22:14:51, petkov wrote:
> Add copyright.
> 

Done.

http://codereview.chromium.org/563022/diff/1/7
File client/site_tests/network_WiFiCaps/control (right):

http://codereview.chromium.org/563022/diff/1/7#newcode1
client/site_tests/network_WiFiCaps/control:1: AUTHOR = "ChromeOS Team"
On 2010/02/03 22:14:51, petkov wrote:
> Add Chromium copyright.
> 

Done.

http://codereview.chromium.org/563022/diff/1/8
File client/site_tests/network_WiFiCaps/network_WiFiCaps.py (right):

http://codereview.chromium.org/563022/diff/1/8#newcode5
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:5: import logging
On 2010/02/03 22:14:51, petkov wrote:
> Per local CODING_STYLE, import modules on one line.

Done.

http://codereview.chromium.org/563022/diff/1/8#newcode14
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:14: version = 1
On 2010/02/03 22:14:05, kmixter1 wrote:
> i think the convention is 4 spaces instead of 1 tab for indent.

Done.

http://codereview.chromium.org/563022/diff/1/8#newcode14
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:14: version = 1
On 2010/02/03 22:14:51, petkov wrote:
> Per local CODING_STYLE, 4 character indent (everywhere).
> 

Done.

http://codereview.chromium.org/563022/diff/1/8#newcode18
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:18: # create a empty
srcdir to prevent the error that checks .version file
On 2010/02/03 22:14:05, kmixter1 wrote:
> 80 chars

Done.

http://codereview.chromium.org/563022/diff/1/8#newcode18
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:18: # create a empty
srcdir to prevent the error that checks .version file
On 2010/02/03 22:14:51, petkov wrote:
> 80 chars (would be fixed by fixing indent)
> 

Done.

http://codereview.chromium.org/563022/diff/1/8#newcode20
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:20: utils.system('mkdir
%s' % self.srcdir)
On 2010/02/03 22:14:05, kmixter1 wrote:
> os.mkdir(self.srcdir)

Done.

http://codereview.chromium.org/563022/diff/1/8#newcode22
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:22: def
__parse_iwcap(self, lines):
On 2010/02/03 22:14:51, petkov wrote:
> Per local CODING_STYLE, 2 empty lines between routines (here and below)
> 

Done.

http://codereview.chromium.org/563022/diff/1/8#newcode26
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:26: parse_re =
re.compile(r'([a-z0-9]*):[ ]*(.*)')
Probably but left it alone as it works

http://codereview.chromium.org/563022/diff/1/8#newcode36
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:36: def __RunIwcap(self,
phy, caps):
On 2010/02/03 22:14:05, kmixter1 wrote:
> convention is usually lowercase and underscores on function names.

Done.

http://codereview.chromium.org/563022/diff/1/8#newcode37
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:37: dir = self.autodir +
'/deps/iwcap/'
On 2010/02/03 22:14:05, kmixter1 wrote:
> Might instead want:
> exec = os.path.join(self.autodir, 'deps', 'iwcap', 'iwcap')

Done.

http://codereview.chromium.org/563022/diff/1/8#newcode38
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:38: iwcap = utils.run(dir
+ 'iwcap ' + phy + ' ' + string.join(caps))
On 2010/02/03 22:14:51, petkov wrote:
> lots of spaces end )

Sorry, don't understand your point.  Do you want me to remove the spaces around
operators?

http://codereview.chromium.org/563022/diff/1/8#newcode66
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:66: 'capability "%s" not
handled' %cap)
On 2010/02/03 22:14:05, kmixter1 wrote:
> usually space after % operator

Done.

[petkov, 2010-02-04 23:14:48]

LGTM

http://codereview.chromium.org/563022/diff/1/8
File client/site_tests/network_WiFiCaps/network_WiFiCaps.py (right):

http://codereview.chromium.org/563022/diff/1/8#newcode38
client/site_tests/network_WiFiCaps/network_WiFiCaps.py:38: iwcap = utils.run(dir
+ 'iwcap ' + phy + ' ' + string.join(caps))
On 2010/02/04 23:08:38, Sam Leffler wrote:
> On 2010/02/03 22:14:51, petkov wrote:
> > lots of spaces end )
> 
> Sorry, don't understand your point.  Do you want me to remove the spaces
around
> operators?

Sorry, the comment should have been "80 chars" -- got confused by the review
tool. Seems fixed now.