Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(71)

Issue 3018533002: Implementing a Monsoon power monitor trace agent, utilizing the UI infrastructure that the BattOr a…

Created:
2 years, 1 month ago by ddeptford
Modified:
2 years ago
CC:
catapult-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
catapult
Visibility:
Public.

Description

Implementing a Monsoon power monitor trace agent, utilizing the UI infrastructure that the BattOr agent provides.

Patch Set 1 #

Patch Set 2 : Fixing test failures. #

Total comments: 5

Patch Set 3 : Added "IsConnectionOwner" method to determine connection controllers instead of explicitly looking … #

Patch Set 4 : Updating static methods and fixing test fakes. #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+435 lines, -10 lines) Patch
A + common/monsoon/monsoon/__init__.py View 1 chunk +4 lines, -8 lines 0 comments Download
A common/monsoon/monsoon/monsoon_utils.py View 1 1 chunk +62 lines, -0 lines 0 comments Download
A common/monsoon/monsoon/monsoon_wrapper.py View 1 chunk +124 lines, -0 lines 0 comments Download
M systrace/profile_chrome/fake_agent_1.py View 1 2 3 1 chunk +4 lines, -0 lines 2 comments Download
M systrace/profile_chrome/fake_agent_2.py View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M systrace/systrace/__init__.py View 1 chunk +1 line, -0 lines 0 comments Download
M systrace/systrace/run_systrace.py View 1 chunk +2 lines, -1 line 0 comments Download
M systrace/systrace/systrace_runner.py View 1 chunk +3 lines, -1 line 0 comments Download
M systrace/systrace/tracing_agents/__init__.py View 1 2 3 1 chunk +15 lines, -0 lines 3 comments Download
A systrace/systrace/tracing_agents/monsoon_agent.py View 1 2 3 1 chunk +185 lines, -0 lines 0 comments Download
M systrace/systrace/tracing_controller.py View 1 2 3 chunks +31 lines, -0 lines 1 comment Download

Messages

Total messages: 34 (21 generated)
ddeptford
This change utilizes the BattOr visualization with data from the Monsoon power monitor. The Monsoon ...
2 years, 1 month ago (2017-09-18 17:47:51 UTC) #10
Chris Craik
Mostly looks good, but would like to hear from Zhen about usb stop/restart stuff https://codereview.chromium.org/3018533002/diff/20001/systrace/systrace/tracing_agents/monsoon_agent.py ...
2 years, 1 month ago (2017-09-19 18:47:02 UTC) #11
Zhen Wang
Notice that Catapult is switching to Gerrit Code Review instead of Rietveld on 9/27. So ...
2 years, 1 month ago (2017-09-19 23:53:50 UTC) #12
Chris Craik
On 2017/09/19 at 23:53:50, zhenw wrote: > Notice that Catapult is switching to Gerrit Code ...
2 years, 1 month ago (2017-09-19 23:58:07 UTC) #13
Zhen Wang
> The other lifecycle methods I was imagining would be: > > 1 start tracing ...
2 years, 1 month ago (2017-09-20 00:04:42 UTC) #14
Chris Craik
Sounds good, let's just go with the 'isConnectionOwnerAgent' bool method that monsoon agent can override, ...
2 years, 1 month ago (2017-09-20 22:00:10 UTC) #15
ddeptford
https://codereview.chromium.org/3018533002/diff/20001/systrace/systrace/tracing_agents/monsoon_agent.py File systrace/systrace/tracing_agents/monsoon_agent.py (right): https://codereview.chromium.org/3018533002/diff/20001/systrace/systrace/tracing_agents/monsoon_agent.py#newcode134 systrace/systrace/tracing_agents/monsoon_agent.py:134: return True On 2017/09/19 18:47:02, Chris Craik wrote: > ...
2 years, 1 month ago (2017-09-20 23:45:58 UTC) #16
Chris Craik
LGTM, assuming there's a reason for the agent change, but please wait for LGTM from ...
2 years ago (2017-09-26 17:57:35 UTC) #25
ddeptford
https://codereview.chromium.org/3018533002/diff/60001/systrace/profile_chrome/fake_agent_1.py File systrace/profile_chrome/fake_agent_1.py (right): https://codereview.chromium.org/3018533002/diff/60001/systrace/profile_chrome/fake_agent_1.py#newcode41 systrace/profile_chrome/fake_agent_1.py:41: # pylint: disable=no-self-use On 2017/09/26 17:57:35, Chris Craik wrote: ...
2 years ago (2017-09-26 18:05:49 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/3018533002/60001
2 years ago (2017-09-26 20:01:46 UTC) #28
commit-bot: I haz the power
Try jobs failed on following builders: Catapult Presubmit on master.tryserver.client.catapult (JOB_FAILED, https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Presubmit/builds/9535)
2 years ago (2017-09-26 20:06:18 UTC) #30
Zhen Wang
+simonhatch for common/ +charliea FYI Daniel, feel free to commit after Simon has taken a ...
2 years ago (2017-09-26 21:32:09 UTC) #33
Zhen Wang
2 years ago (2017-09-28 01:58:49 UTC) #34
https://codereview.chromium.org/3018533002/diff/60001/systrace/systrace/traci...
File systrace/systrace/tracing_agents/__init__.py (right):

https://codereview.chromium.org/3018533002/diff/60001/systrace/systrace/traci...
systrace/systrace/tracing_agents/__init__.py:93: # pylint: disable=no-self-use
Why do we need this pylint disable?

https://codereview.chromium.org/3018533002/diff/60001/systrace/systrace/traci...
systrace/systrace/tracing_agents/__init__.py:99: For example, a power monitoring
tracing agent may disable the USB port to
Please name Monsoon explicitly here. There is another power monitoring agent for
BattOr.

https://codereview.chromium.org/3018533002/diff/60001/systrace/systrace/traci...
systrace/systrace/tracing_agents/__init__.py:101: 
Can you also add that the connection owner will start last and stop earliest,
and USB disable/enable is done in StartAgentTracing/StopAgentTracing.

Also see comments in tracing_controller.py.

https://codereview.chromium.org/3018533002/diff/60001/systrace/systrace/traci...
File systrace/systrace/tracing_controller.py (right):

https://codereview.chromium.org/3018533002/diff/60001/systrace/systrace/traci...
systrace/systrace/tracing_controller.py:209: if not
self._connection_owner.StopCollectionWithClockSync(sync_id,
Do we still need this |StopCollectionWithClockSync| function. Can we just call
StopAgentTracing here for connection owner?

It seems to me that StartAgentTracing takes care of USB disable. Similarly,
StopAgentTracing should take care of USB enable. You can make a comment in
__init__.py.

Powered by Google App Engine
This is Rietveld 408576698