Print OMAP temperature data

Print OMAP temperature data

Print an info message giving the temperature at which the OMAP decided to
thermally throttle.
Print an info message giving the current OMAP temperature every time there is
a check for thermal throttling.
Include the device serial number in all thermal throttling messages.

BUG=164989
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=172357

[Anthony Berent, 2012-12-10 16:26:51]

I think this should cover your needs for OMAP based devices - I will, when I get
time, investigate other devices.

[bulach, 2012-12-10 17:45:04]

lgtm, thanks anthony! just nits and small suggestions..

https://codereview.chromium.org/11499010/diff/1/build/android/pylib/thermal_t...
File build/android/pylib/thermal_throttle.py (right):

https://codereview.chromium.org/11499010/diff/1/build/android/pylib/thermal_t...
build/android/pylib/thermal_throttle.py:41: logging.warning('>>> Device ' +
serial_number +
nit: logging is like printf, takes a format and a vararg.
so this (and all the followings) could be:
logging.warning('>>> Device %s Thermally Throttled', serial_number)

https://codereview.chromium.org/11499010/diff/1/build/android/pylib/thermal_t...
build/android/pylib/thermal_throttle.py:52: temp = [s for s in line.split() if
s.isdigit()][0]
if this is guaranteed to be on hundredths of degrees, maybe easiest to:
temp = int([s for s in line.split() if s.isdigit()][0]) / 100

then the format below would be '%3.2f'

https://codereview.chromium.org/11499010/diff/1/build/android/pylib/thermal_t...
build/android/pylib/thermal_throttle.py:56:
'/sys/devices/platform/omap/omap_temp_sensor.0/temperature'):
nit: probably best to make this a constant in 54, avoids duplication and maybe
wrapping :)

https://codereview.chromium.org/11499010/diff/1/build/android/pylib/thermal_t...
build/android/pylib/thermal_throttle.py:59: temp = tempdata[0]
as above, maybe temp = int(tempdata[0][:-5]) / 100

[Isaac (away), 2012-12-11 03:15:49]

lgtm with Marcus's comments.  Ideally we would also print the battery
temperature though as it gives an indication of generate phone temp vs. cpu
temp.  Would you be willing to add it?

[aberent, 2012-12-11 13:10:49]

On 2012/12/11 03:15:49, Isaac wrote:
> lgtm with Marcus's comments.  Ideally we would also print the battery
> temperature though as it gives an indication of generate phone temp vs. cpu
> temp.  Would you be willing to add it?

Done.

[aberent, 2012-12-11 13:11:20]

Updated patch will follow

https://chromiumcodereview.appspot.com/11499010/diff/1/build/android/pylib/th...
File build/android/pylib/thermal_throttle.py (right):

https://chromiumcodereview.appspot.com/11499010/diff/1/build/android/pylib/th...
build/android/pylib/thermal_throttle.py:41: logging.warning('>>> Device ' +
serial_number +
On 2012/12/10 17:45:05, bulach wrote:
> nit: logging is like printf, takes a format and a vararg.
> so this (and all the followings) could be:
> logging.warning('>>> Device %s Thermally Throttled', serial_number)

Done.

https://chromiumcodereview.appspot.com/11499010/diff/1/build/android/pylib/th...
build/android/pylib/thermal_throttle.py:52: temp = [s for s in line.split() if
s.isdigit()][0]
Actually it is in thousandths of a degree, and there is a bug in my code, but
the last two digits are always 0, so am changing this as you suggest, but with
only one decimal place to avoid false precision. 
On 2012/12/10 17:45:05, bulach wrote:
> if this is guaranteed to be on hundredths of degrees, maybe easiest to:
> temp = int([s for s in line.split() if s.isdigit()][0]) / 100
> 
> then the format below would be '%3.2f'

https://chromiumcodereview.appspot.com/11499010/diff/1/build/android/pylib/th...
build/android/pylib/thermal_throttle.py:56:
'/sys/devices/platform/omap/omap_temp_sensor.0/temperature'):
On 2012/12/10 17:45:05, bulach wrote:
> nit: probably best to make this a constant in 54, avoids duplication and maybe
> wrapping :)

Done.

https://chromiumcodereview.appspot.com/11499010/diff/1/build/android/pylib/th...
build/android/pylib/thermal_throttle.py:59: temp = tempdata[0]
On 2012/12/10 17:45:05, bulach wrote:
> as above, maybe temp = int(tempdata[0][:-5]) / 100
Done, with changes as above. I don't think the [:-5] is needed since the whole
string is the temperature.

[bulach, 2012-12-11 13:30:14]

lgtm, thanks! just small nits:

https://chromiumcodereview.appspot.com/11499010/diff/8001/build/android/pylib...
File build/android/pylib/thermal_throttle.py (right):

https://chromiumcodereview.appspot.com/11499010/diff/8001/build/android/pylib...
build/android/pylib/thermal_throttle.py:49: temp = float([s for s in
line.split() if s.isdigit()][0])/1000.0
nit: spaces around /

It's not quite clear the [0] part: wouldn't that return just the first digit of
the string created by the list comprehension?

https://chromiumcodereview.appspot.com/11499010/diff/8001/build/android/pylib...
build/android/pylib/thermal_throttle.py:50: # \u00B0 is the degree symbol
nit: it'd be clearer and avoid the comment to have:
degree_symbol = u'\u00B0C'

then use a %s passing degree_symbol here, 59 and 67

https://chromiumcodereview.appspot.com/11499010/diff/8001/build/android/pylib...
build/android/pylib/thermal_throttle.py:58: temp = float(tempdata[0])/1000.0
nit: spaces around /

https://chromiumcodereview.appspot.com/11499010/diff/8001/build/android/pylib...
build/android/pylib/thermal_throttle.py:66: btemp = float([s for s in
line.split() if s.isdigit()][0])/10.0
nit: same for [0] here..

https://chromiumcodereview.appspot.com/11499010/diff/8001/build/android/pylib...
build/android/pylib/thermal_throttle.py:68: serial_number, btemp)
nit: serial should be aligned with parens..

[aberent, 2012-12-11 14:03:05]

https://chromiumcodereview.appspot.com/11499010/diff/8001/build/android/pylib...
File build/android/pylib/thermal_throttle.py (right):

https://chromiumcodereview.appspot.com/11499010/diff/8001/build/android/pylib...
build/android/pylib/thermal_throttle.py:49: temp = float([s for s in
line.split() if s.isdigit()][0])/1000.0
Spacing done. The list expression splits the line into a list of space delimited
sub-strings and then keeps only those that contain only digits, so the [0] gives
the first of these, which is the complete temperature value.

On 2012/12/11 13:30:14, bulach wrote:
> nit: spaces around /
> 
> It's not quite clear the [0] part: wouldn't that return just the first digit
of
> the string created by the list comprehension?

https://chromiumcodereview.appspot.com/11499010/diff/8001/build/android/pylib...
build/android/pylib/thermal_throttle.py:50: # \u00B0 is the degree symbol
On 2012/12/11 13:30:14, bulach wrote:
> nit: it'd be clearer and avoid the comment to have:
> degree_symbol = u'\u00B0C'
> 
> then use a %s passing degree_symbol here, 59 and 67

Done.

https://chromiumcodereview.appspot.com/11499010/diff/8001/build/android/pylib...
build/android/pylib/thermal_throttle.py:58: temp = float(tempdata[0])/1000.0
On 2012/12/11 13:30:14, bulach wrote:
> nit: spaces around /

Done.

https://chromiumcodereview.appspot.com/11499010/diff/8001/build/android/pylib...
build/android/pylib/thermal_throttle.py:66: btemp = float([s for s in
line.split() if s.isdigit()][0])/10.0
As above

On 2012/12/11 13:30:14, bulach wrote:
> nit: same for [0] here..

https://chromiumcodereview.appspot.com/11499010/diff/8001/build/android/pylib...
build/android/pylib/thermal_throttle.py:68: serial_number, btemp)
On 2012/12/11 13:30:14, bulach wrote:
> nit: serial should be aligned with parens..

Done.

[commit-bot: I haz the power, 2012-12-11 14:07:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aberent@chromium.org/11499010/8002

[commit-bot: I haz the power, 2012-12-11 18:41:14]

Change committed as 172357