Closed Bug 816694 Opened 9 years ago Closed 8 years ago

add in temperature monitoring to devicemanager

Categories

(Testing :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla22

People

(Reporter: jmaher, Assigned: wlach)

References

Details

Attachments

(2 files)

I would like to call dm.getInfo('temp') and it would do a 'info temp' for me instead of:
temp = self.dm._runCmds([ { 'cmd': 'cat /sys/bus/platform/devices/temp_sensor_hwmon.0/temp1_input' }])

then I would print this in log files as such:
print "%s - TEMPERATURE: %s" % (time.strftime("%Y-%m-%d %H:%M:%S", time.gmtime()), temp)
this is very panda specific, if folks would rather I keep this with a _runCmds, I can do that as well.

-Joel
I don't see any problem with putting the interface in the agent. We can query different devices/files depending on platform to get the right information.
If you want to be a little forward thinking you can provide an optional argument about which temperature to query if there are multiple sensors present.
I have no idea what other sensors would look like.  Could we maybe define the files that we will look at in an array, and set a default to ['/sys/bus/platform/devices/temp_sensor_hwmon.0/temp1_input'] ?

The other problem is that we might have different types of data depending on the board.  in this case we are assuming the returned value of 44800 is 44.8 degrees Celcius, but it could mean something else.
That seems pretty reasonable. If you want to normalize values, normalizing to degrees C as a float seems sensible.
I have a script which monitors temperature on the pandas for a foopy.  This is what I have used successfully to monitor temperature while investigating panda failure rates.

http://people.mozilla.org/~jmaher/panda/temperature_monitor.py.txt
Add support for device temperature monitoring to agent.

I started working on this proof of concept when I was on the train yesterday and subsequently decided I may as well finish it. :) I haven't actually tested it on a panda, but I believe it should work. It definitely works on devices which don't have the interface.
Attachment #724073 - Flags: review?(jmaher)
Pretty straightforward stuff
Attachment #724075 - Flags: review?(jmaher)
Comment on attachment 724073 [details] [diff] [review]
Add device temperature monitoring to agent

Review of attachment 724073 [details] [diff] [review]:
-----------------------------------------------------------------

In general I don't like hardcoding stuff in the agent, but it makes logical sense to put temperature in the 'info' block.  If there were a different set of parameters to pull the temperature off a different device, how would we add that to DoCommand?

::: build/mobile/remoteautomation.py
@@ +1,1 @@
> +

please remove this blank line so we don't modify remoteautomation.py in this patch.

::: build/mobile/sutagent/android/DoCommand.java
@@ +2617,5 @@
> +
> +    public String GetTemperatureInfo()
> +        {
> +            String sTempVal = "unknown";
> +            String sDeviceFile = "/sys/bus/platform/devices/temp_sensor_hwmon.0/temp1_input";

this assumes a single proc CPU, but what we have now and for the short term future this solves our needs.

@@ +2628,5 @@
> +                // this only works on pandas (with the temperature sensors turned
> +                // on), other platforms we just get a file not found error... we'll
> +                // just return "unknown" for that case
> +                try {
> +                    sTempVal = String.valueOf(Integer.parseInt(output) / 1000.0);

I don't see how sTempVal gets assigned 'unknown'
Attachment #724073 - Flags: review?(jmaher) → review+
Attachment #724075 - Flags: review?(jmaher) → review+
(In reply to Joel Maher (:jmaher) from comment #9)
> Comment on attachment 724073 [details] [diff] [review]
> Add device temperature monitoring to agent
> 
> Review of attachment 724073 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> In general I don't like hardcoding stuff in the agent, but it makes logical
> sense to put temperature in the 'info' block.  If there were a different set
> of parameters to pull the temperature off a different device, how would we
> add that to DoCommand?

FWIW, I completely agree that hardcoding stuff in the agent is generally bad, just that this case is an exception. ;)

To answer your question, we'd put that logic in the GetTemperatureInfo() command. Probably check for the existence of difference files in /proc, then check the different files accordingly.

> > +                // this only works on pandas (with the temperature sensors turned
> > +                // on), other platforms we just get a file not found error... we'll
> > +                // just return "unknown" for that case
> > +                try {
> > +                    sTempVal = String.valueOf(Integer.parseInt(output) / 1000.0);
> 
> I don't see how sTempVal gets assigned 'unknown'
Pushed sutagent patch to inbound (with extra newline removed from remoteautomation.py): 

https://hg.mozilla.org/integration/mozilla-inbound/rev/b9c6db9049cd
https://hg.mozilla.org/mozilla-central/rev/b9c6db9049cd
Assignee: nobody → wlachance
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.