Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1060943002:
[Android] Add severable device provisioning. (Closed)
|
Created:
5 years, 8 months ago by jbudorick Modified:
5 years, 8 months ago CC:
chromium-reviews, klundberg+watch_chromium.org, yfriedman+watch_chromium.org, jbudorick+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Android] Add severable device provisioning.
BUG=
Committed: https://crrev.com/7f46f7bfba383e2082a947b5cb3e4e5bc736389f
Cr-Commit-Position: refs/heads/master@{#324298}
Patch Set 1 #Patch Set 2 : #Patch Set 3 : #Patch Set 4 : #Patch Set 5 : #Patch Set 6 : #
Total comments: 12
Messages
Total messages: 13 (3 generated)
jbudorick@chromium.org changed reviewers: + navabi@chromium.org
This lets us do the different parts of provisioning separately s.t. we could e.g. start provisioning, compile, then finish provisioning. ptal
navabi@google.com changed reviewers: + navabi@google.com
https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... File build/android/provision_devices.py (left): https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... build/android/provision_devices.py:204: device.Reboot(True, timeout=reboot_timeout, retries=0) we reboot after setting all the properties so they stick, and then we set the date. https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... File build/android/provision_devices.py (right): https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... build/android/provision_devices.py:55: devices = [options.device] This hasn't changed right? Only moved in the script? https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... build/android/provision_devices.py:203: device.RunShellCommand( Do we reboot before setting the date? I think I recall that if we set the date before the reboot (which we have to do when we wipe for the properties to stick), it would not be correctly set after the reboot. That's why we put it after the reboot. Did you test this locally? Please make sure the date is correctly set with this refactoring of provision devices.
https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... File build/android/provision_devices.py (left): https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... build/android/provision_devices.py:204: device.Reboot(True, timeout=reboot_timeout, retries=0) On 2015/04/07 20:52:37, navabi wrote: > we reboot after setting all the properties so they stick, and then we set the > date. Yeah, this ordering was maintained in the new version. https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... build/android/provision_devices.py:227: devices = [options.device] (1) https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... File build/android/provision_devices.py (right): https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... build/android/provision_devices.py:55: devices = [options.device] On 2015/04/07 20:52:37, navabi wrote: > This hasn't changed right? Only moved in the script? Yep, moved from (1) https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... build/android/provision_devices.py:203: device.RunShellCommand( On 2015/04/07 20:52:37, navabi wrote: > Do we reboot before setting the date? I think I recall that if we set the date > before the reboot (which we have to do when we wipe for the properties to > stick), it would not be correctly set after the reboot. That's why we put it > after the reboot. This does set the date after rebooting. > > Did you test this locally? Please make sure the date is correctly set with this > refactoring of provision devices. In local testing on an N7 on K, the date appeared to be set correctly even with this line commented out, so no real signal there.
https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... File build/android/provision_devices.py (left): https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... build/android/provision_devices.py:204: device.Reboot(True, timeout=reboot_timeout, retries=0) > Yeah, this ordering was maintained in the new version. Hmm. Maybe I'm just not seeing something obvious, but I don't see the reboot in the new code. FinishProvisioning does not reboot before setting date and SetProperties does not reboot after it is done (i.e. the last thing it does is the line 201 here). This is fine with me as long as it works. Just curious what reboot you are referring to.
https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... File build/android/provision_devices.py (left): https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... build/android/provision_devices.py:204: device.Reboot(True, timeout=reboot_timeout, retries=0) On 2015/04/08 19:39:27, navabi wrote: > > Yeah, this ordering was maintained in the new version. > > Hmm. Maybe I'm just not seeing something obvious, but I don't see the reboot in > the new code. FinishProvisioning does not reboot before setting date and > SetProperties does not reboot after it is done (i.e. the last thing it does is > the line 201 here). > > This is fine with me as long as it works. Just curious what reboot you are > referring to. I broke this into phases around the reboots, i.e., a reboot ends a phase. This allows us to exit if the client wants to run a phase and do something else while the device reboots. The reboot that used to precede the date setting is now at (2). https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... File build/android/provision_devices.py (right): https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... build/android/provision_devices.py:85: device.Reboot(False, retries=0) (3): both reboots are done here. https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... build/android/provision_devices.py:92: run_phase(SetProperties) (2): The reboot that previously preceded setting the date is now done in run_phase at (3)
lgtm https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... File build/android/provision_devices.py (left): https://codereview.chromium.org/1060943002/diff/100001/build/android/provisio... build/android/provision_devices.py:204: device.Reboot(True, timeout=reboot_timeout, retries=0) On 2015/04/08 19:44:35, jbudorick wrote: > On 2015/04/08 19:39:27, navabi wrote: > > > Yeah, this ordering was maintained in the new version. > > > > Hmm. Maybe I'm just not seeing something obvious, but I don't see the reboot > in > > the new code. FinishProvisioning does not reboot before setting date and > > SetProperties does not reboot after it is done (i.e. the last thing it does is > > the line 201 here). > > > > This is fine with me as long as it works. Just curious what reboot you are > > referring to. > > I broke this into phases around the reboots, i.e., a reboot ends a phase. This > allows us to exit if the client wants to run a phase and do something else while > the device reboots. The reboot that used to precede the date setting is now at > (2). Ah ha!! Reboot at the end of every run_phase. I like it.
lgtm
The CQ bit was checked by jbudorick@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1060943002/100001
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/7f46f7bfba383e2082a947b5cb3e4e5bc736389f Cr-Commit-Position: refs/heads/master@{#324298} |
