GPIO button test

GPIO button test

This checks recovery mode button, developer mode, firmware write protect states and logs them as perf variables, so that expected values can be specified on test invocation.

Also, added iotools to deps folder so that GPIO registers can be read.

[Nick Sanders, 2010-04-03 01:10:37]

iotools doesn't compile on arm, but is also only needed for x86 right now, how
do you disable a deps build for arm?

[petkov, 2010-04-05 18:26:40]

A few comments, mostly nits.

Also, what is the SAT tarball doing in this CL?

In regard to not building this for ARM: the right way might be to add the test
to all ARM board overlay autotest-blacklist files (e.g.,
src/overlays/overlay-arm-generic/autotest-blacklist).

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

http://codereview.chromium.org/1616002/diff/1/5#newcode1
client/deps/iotools/iotools.py:1: #!/usr/bin/python
Copyright?

http://codereview.chromium.org/1616002/diff/1/5#newcode3
client/deps/iotools/iotools.py:3: import os, common
Sort

http://codereview.chromium.org/1616002/diff/1/6
File client/site_tests/hardware_GpioSwitches/control (right):

http://codereview.chromium.org/1616002/diff/1/6#newcode16
client/site_tests/hardware_GpioSwitches/control:16:
job.run_test('hardware_GpioSwitches')
Please rename to hardware_GPIOSwitches. Naming convention doesn't lower case
acronyms -- e.g. Python's BaseHTTPServer class.

http://codereview.chromium.org/1616002/diff/1/7
File client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py (right):

http://codereview.chromium.org/1616002/diff/1/7#newcode7
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:7: import os,
logging
Sort alphabetically.

http://codereview.chromium.org/1616002/diff/1/7#newcode16
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:16:
self.sku_table = {
It bothers me a bit to have this table inside the test as opposed to a separate
file (see, for example, hardware_Components) but this is probably OK for now,
given that there's no clear strategy on how to differentiate configurations.

http://codereview.chromium.org/1616002/diff/1/7#newcode17
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:17: # SKU:
gpio_read, recovery GPIO, developer mode, firmware writeprotect
80 chars

http://codereview.chromium.org/1616002/diff/1/7#newcode22
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:22: def
setup(self):
Per CODING_STYLE, two empty lines between methods.

http://codereview.chromium.org/1616002/diff/1/7#newcode26
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:26:
utils.system('mkdir %s' % self.srcdir)
You could use os.mkdir.

http://codereview.chromium.org/1616002/diff/1/7#newcode38
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:38: systemsku =
"unknown"
Use " and ' consistently.

http://codereview.chromium.org/1616002/diff/1/7#newcode40
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:40:
utils.system('cat /proc/cpuinfo | grep "model name" | grep -qe "N4[0-9[0-9]"')
80 chars

http://codereview.chromium.org/1616002/diff/1/7#newcode43
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:43: systemsku =
'unknown'
You've already set systemsku to unknown by default, no need to set it again. Or,
just set it here.

http://codereview.chromium.org/1616002/diff/1/7#newcode53
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:53: raise
error.TestError('System settings not defined for board %s' % systemsku)
80 chars

http://codereview.chromium.org/1616002/diff/1/7#newcode58
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:58:
keyvals['recovery'] = recovery
Prefix the keys with "units" ... maybe "state_" or "level_" ?

http://codereview.chromium.org/1616002/diff/1/7#newcode77
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:77:
tp_gpio_iobase_str = utils.system_output('sudo ' + path +
autotest runs as root so no need for sudo.

http://codereview.chromium.org/1616002/diff/1/7#newcode83
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:83:
tp_gpio_mask_str = utils.system_output('sudo ' + path +
no need to sudo.

[Nick Sanders, 2010-04-05 20:19:54]

I believe I have a patch for iotools to build on arm.

http://codereview.chromium.org/1616002/diff/1/7
File client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py (right):

http://codereview.chromium.org/1616002/diff/1/7#newcode7
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:7: import os,
logging
On 2010/04/05 18:26:40, petkov wrote:
> Sort alphabetically.
> 

Done.

http://codereview.chromium.org/1616002/diff/1/7#newcode7
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:7: import os,
logging
On 2010/04/05 18:26:40, petkov wrote:
> Sort alphabetically.
> 

Done.

http://codereview.chromium.org/1616002/diff/1/7#newcode17
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:17: # SKU:
gpio_read, recovery GPIO, developer mode, firmware writeprotect
On 2010/04/05 18:26:40, petkov wrote:
> 80 chars
> 

Done.

http://codereview.chromium.org/1616002/diff/1/7#newcode22
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:22: def
setup(self):
On 2010/04/05 18:26:40, petkov wrote:
> Per CODING_STYLE, two empty lines between methods.
> 

Done.

http://codereview.chromium.org/1616002/diff/1/7#newcode26
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:26:
utils.system('mkdir %s' % self.srcdir)
On 2010/04/05 18:26:40, petkov wrote:
> You could use os.mkdir.
> 

Done.

http://codereview.chromium.org/1616002/diff/1/7#newcode38
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:38: systemsku =
"unknown"
On 2010/04/05 18:26:40, petkov wrote:
> Use " and ' consistently.
> 

Done.

http://codereview.chromium.org/1616002/diff/1/7#newcode40
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:40:
utils.system('cat /proc/cpuinfo | grep "model name" | grep -qe "N4[0-9[0-9]"')
On 2010/04/05 18:26:40, petkov wrote:
> 80 chars
> 

Done.

http://codereview.chromium.org/1616002/diff/1/7#newcode43
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:43: systemsku =
'unknown'
On 2010/04/05 18:26:40, petkov wrote:
> You've already set systemsku to unknown by default, no need to set it again.
Or,
> just set it here.
> 
> 

Done.

http://codereview.chromium.org/1616002/diff/1/7#newcode53
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:53: raise
error.TestError('System settings not defined for board %s' % systemsku)
On 2010/04/05 18:26:40, petkov wrote:
> 80 chars
> 

Done.

http://codereview.chromium.org/1616002/diff/1/7#newcode58
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:58:
keyvals['recovery'] = recovery
On 2010/04/05 18:26:40, petkov wrote:
> Prefix the keys with "units" ... maybe "state_" or "level_" ?
> 

Done.

http://codereview.chromium.org/1616002/diff/1/7#newcode77
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:77:
tp_gpio_iobase_str = utils.system_output('sudo ' + path +
On 2010/04/05 18:26:40, petkov wrote:
> autotest runs as root so no need for sudo.
> 

Done.

http://codereview.chromium.org/1616002/diff/1/7#newcode83
client/site_tests/hardware_GpioSwitches/hardware_GpioSwitches.py:83:
tp_gpio_mask_str = utils.system_output('sudo ' + path +
On 2010/04/05 18:26:40, petkov wrote:
> no need to sudo.
> 

Done.

[petkov, 2010-04-05 21:21:00]

One real issue, and a couple of nits.
LGTM otherwise.

http://codereview.chromium.org/1616002/diff/8001/9007
File client/site_tests/hardware_GPIOSwitches/hardware_GPIOSwitches.py (right):

http://codereview.chromium.org/1616002/diff/8001/9007#newcode21
client/site_tests/hardware_GPIOSwitches/hardware_GPIOSwitches.py:21: 'recovery':
6, 'developer': 7, 'fwwp': 10},
indent seems OFF

http://codereview.chromium.org/1616002/diff/8001/9007#newcode43
client/site_tests/hardware_GPIOSwitches/hardware_GPIOSwitches.py:43: 'grep -qe
"N4[0-9[0-9]"')
Does this work? It seems you're missing a ]

http://codereview.chromium.org/1616002/diff/8001/9007#newcode75
client/site_tests/hardware_GPIOSwitches/hardware_GPIOSwitches.py:75: 
Too many blank lines?

[Nick Sanders, 2010-04-05 23:03:31]

http://codereview.chromium.org/1616002/diff/8001/9007
File client/site_tests/hardware_GPIOSwitches/hardware_GPIOSwitches.py (right):

http://codereview.chromium.org/1616002/diff/8001/9007#newcode21
client/site_tests/hardware_GPIOSwitches/hardware_GPIOSwitches.py:21: 'recovery':
6, 'developer': 7, 'fwwp': 10},
On 2010/04/05 21:21:00, petkov wrote:
> indent seems OFF
> 

Done.

http://codereview.chromium.org/1616002/diff/8001/9007#newcode43
client/site_tests/hardware_GPIOSwitches/hardware_GPIOSwitches.py:43: 'grep -qe
"N4[0-9[0-9]"')
On 2010/04/05 21:21:00, petkov wrote:
> Does this work? It seems you're missing a ]
It worked at one point, seems to have lost a char somewhere. Thanks for catching
this!

http://codereview.chromium.org/1616002/diff/8001/9007#newcode75
client/site_tests/hardware_GPIOSwitches/hardware_GPIOSwitches.py:75: 
On 2010/04/05 21:21:00, petkov wrote:
> Too many blank lines?
> 

Done.