Provide power profile value to gaia.

we take this sample file as a sample file. push this file to /system/etc/power_profile.xml

merge conflict

Comment on attachment 778393 [details] [diff] [review] readProfile Review of attachment 778393 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/power/PowerStatsManager.js @@ +75,5 @@ > + this.checkPrivileges(); > + let request = this.createRequest(); > + if (DEBUG) { > + debug("readProfile: " + item); > + } Nit: I think we don't need if(DEBUG) anymore. ::: dom/power/PowerStatsService.jsm @@ +301,5 @@ > + let item = msg.data; > + let profilePath = POWER_PROFILE_PATH; > + if (DEBUG) { > + debug("readProfile: " + item); > + } Nit: Remove if(DEBUG) @@ +315,5 @@ > + let found = 0; > + let result = []; > + let error = null; > + > + for (let i=0;i<target.length;i++) { Nit: space between "=", "<" and after ";" @@ +316,5 @@ > + let result = []; > + let error = null; > + > + for (let i=0;i<target.length;i++) { > + if(target[i].getAttribute('name') === item) { Nit: space after if @@ +323,5 @@ > + } > + } > + > + target = powerProfile.getElementsByTagName("array"); > + for (let i=0;i<target.length;i++) { Nit: space between "=", "<" and after ";" @@ +326,5 @@ > + target = powerProfile.getElementsByTagName("array"); > + for (let i=0;i<target.length;i++) { > + if(target[i].getAttribute('name') === item) { > + found = 1; > + for (let j = 0;j < Math.floor(target[i].childNodes.length/2);j++) { Nit: space after ";"

Thanks for your feedback. This is the improved patch.

We take this sample file as a sample file. Push this file to /system/etc/power_profile.xml

Comment on attachment 780196 [details] [diff] [review] readProfile Review of attachment 780196 [details] [diff] [review]: ----------------------------------------------------------------- This patch looks good. Thank you. ::: dom/power/nsIDOMPowerStatsManager.idl @@ +34,5 @@ > */ > nsIDOMDOMRequest clearAllData(); > + > + /** > + * Read the average power consumption of item in power profile. What is the item ? Should it be hardware components ?

We found that the power profile is quite sensitive information to vendor. So, we do not create a DOM API. The values in power profile is average current consumption of each I/O component. These values help PowerStatsService estimate the power consumption.

Comment on attachment 784847 [details] [diff] [review] readPowerProfile Hi Blake, I'm not quite familiar with the xml parser. Not sure if you can help to review this patch. This patch tries to parse the xml format file and return the value of specific item. You can find the example file from the attached "PowerProfile sample file". Gavin is our intern who is helping us to implement per app power consumption feature.

Comment on attachment 784847 [details] [diff] [review] readPowerProfile Review of attachment 784847 [details] [diff] [review]: ----------------------------------------------------------------- Hi Gavin, Instead of rolling your own "read file and parser XML out of it," I think you should be able to use XMLHttpRequest directly, passing it a file: URI and getting the responseXML after it loads. That should be much simpler.

Hi Blake, Thanks for your advice. I will refine this soon.

Hi Blake, I'm curious that can XMLHttpRequest open a file in /system/etc/? I got some problems to use XMLHttpRequest to open the power profile in /system/etc/. The status of XMLHttpRequest is always 0. I use the same code to open http://yahoo.com. The status of XMLHttpRequest is 200. I can't figure out anything wrong in this code.... Any advices will be thankful.

Comment on attachment 790736 [details] [diff] [review] (wip)readPowerProfile Review of attachment 790736 [details] [diff] [review]: ----------------------------------------------------------------- Hi Gavin, I think I see what's going on. See below. ::: dom/power/PowerStatsService.jsm @@ +325,5 @@ > + const fileuri = "file:///system/etc/power_profile.xml"; > + let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"] > + .createInstance(Ci.nsIXMLHttpRequest); > + xhr.open("GET", fileuri, true); > + xhr.responseType = "text"; I think you want responseType to be "document" instead of "text". @@ +333,5 @@ > + xhr.onreadystatechange = function() { > + if (xhr.readyState != Ci.nsIXMLHttpRequest.DONE) { > + return; > + } > + if (xhr.status === 200) { XMLHttpRequest.status doesn't change for file:// URIs. It only makes sense for HTTP requests. You should simply use xhr.responseXML when readyState is DONE.

Hi Blake, Thanks for your big help. It works well right now. I have one more question to ask...I'm trying to modify the readFile funcfion using XMLHttpRequest to read files. Read local files using XMLHttpRequest seems asynchronous. However, lots of codes(bug: 854204) need the results of read file immediately. If I insert callback to solve this problem, I think it is a little bit hard than origin to read these codes... Is it fine to use XMLHttpRequest in synchronous? Or, using current readFile function? Or, other better way to solve this situation?

(In reply to Gavin Lai[:glai] from comment #15) > I have one more question to ask...I'm trying to modify the readFile funcfion > using XMLHttpRequest to read files. Read local files using XMLHttpRequest > seems asynchronous. However, lots of codes(bug: 854204) need the results of > read file immediately. If I insert callback to solve this problem, I think > it is a little bit hard than origin to read these codes... Are you trying to get these synchronous responses on the main thread? If so, then you should definitely *not* be doing any synchronous I/O. It's worth a couple of extra callbacks in order to avoid locking up the main thread and preventing the user from interacting with the page or browser UI. If you're off the main thread, then synchronous I/O would be OK.

Modify read file related functions to do asynchronous I/O.

Fit nit and bugs.

Fix nit.

Comment on attachment 796484 [details] [diff] [review] readPowerProfile Review of attachment 796484 [details] [diff] [review]: ----------------------------------------------------------------- I still have some questions, but this looks pretty good. ::: dom/power/PowerStatsService.jsm @@ +46,5 @@ > const HTTPS_URL = "https://"; > const APP_TYPE = "app"; > const SYSTEM_TYPE = "system"; > const BROWSER = "app://browser.gaiamobile.org/manifest.webapp"; > +const POWER_PROFILE_PATH = "/system/etc/power_profile.xml"; This seems to be unused. @@ +289,5 @@ > }); > }, > + > + // cpu.speeds.count read the value of counter in different CPU speed step. > + readProfile: function readProfile(item, callback) { This seems to be unused. @@ +303,5 @@ > + let result = []; > + this.readFile(CPU_COUNT_PATH, function read(text) { > + let data = text.split(/\W+/); > + for (let i = 0; i < Math.floor(data.length/2); i++) { > + result.push(data[2*i+1]); Please add a comment explaining what the input looks like and why we're only picking the odd elements out of it. @@ +312,5 @@ > + }, > + > + // readPowerProfile reads the average current consumption in power profile > + // file. The power profile is in xml format. > + readPowerProfile: function readPowerProfile() { Do we have to worry about this being called twice (and therefore that powerProfile might not be an empty object)? @@ +331,5 @@ > + > + // parse the xml item > + for (let i = 0; i < target.length; i++) { > + let item = target[i].getAttribute('name'); > + powerProfile[item] = target[i].childNodes[0].nodeValue; I wonder if it wouldn't be more conservative to use target[i].textContent instead of assuming a single child node. @@ +341,5 @@ > + let item = target[i].getAttribute('name'); > + let arr = []; > + > + for (let j = 0; j < Math.floor(target[i].childNodes.length/2); j++) { > + arr.push(target[i].childNodes[2*j+1].childNodes[0].nodeValue); Can you explain why you're only picking every other child node? In general, some comments explaining what the input looks like would help greatly in understand what the code is trying to do. @@ +357,5 @@ > + const DONE = 4; > + let aSync = true; > + let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"] > + .createInstance(Ci.nsIXMLHttpRequest); > + xhr.open("GET", uri, aSync); I hate to do this to you, but for plain text, I think it's much preferable to use OS.File (see <https://developer.mozilla.org/en-US/docs/JavaScript_OS.File/OS.File_for_the_main_thread#Example.3A_Read_the_contents_of_a_file_as_text>) for plain text files. XMLHttpRequest is only better if you're actually reading XML data.

(In reply to Blake Kaplan (:mrbkap) [PTO Sep 5-Sep 19] from comment #20) > I hate to do this to you, but for plain text, I think it's much preferable > to use OS.File (see It's fine. I learn a lot! I'll refine soon.

The power_profile.json describes the coefficients for calculating power consumption in JSON format. This is an example power_profile.json for Unagi. To run this patch, please: 1. download the power_profile.json and put it under B2G/device/qcom/unagi/ 2. Modify B2G/device/qcom/unagi/full_unagi.mk and append the following line to PRODUCT_COPY_FILES device/qcom/unagi/power_profile.json:system/etc/power_profile.json

Hi Blake, cloud you help review this new patch? Thx. Update note: 1. Use OS.File instead of XMLHttpRequest. 2. Remove codes of reading CPU counters for clean code. (The readCPUCounter() will be moved to Bug 854204 so that we can unblock that bug.)

Comment on attachment 8339800 [details] [diff] [review] Bug-854202-readPowerProfile.patch Review of attachment 8339800 [details] [diff] [review]: ----------------------------------------------------------------- This looks much better. I only have one comment. r=me with it addressed. ::: dom/power/PowerStatsService.jsm @@ +86,5 @@ > + result = JSON.parse(decoder.decode(data)); > + } catch (ex) { > + Cu.reportError("Cannot parse power_profile.json to a json object"); > + } > + debug("readPowerProfile():" + JSON.stringify(result)); I would take these debug statements out of the code before landing this patch. Even though they won't be printed, JSON.stringify will be called which isn't ideal if we're going to throw the result out.

Hi Blake: Thanks for your comments. Here is the patch. Update Notes: 1. remove debug messages in _readPowerProfile()

Comment on attachment 8342801 [details] [diff] [review] Bug-854202-readPowerProfile.patch Looks great! Thanks for sticking with this.

Thanks for you help, Blake!

Firefox OS is not being worked on