> And if we do batter level check again after step 5, it'll be an endless loop.

May I know what case that it will be an endless loop? From my understanding, there are at most two rounds only.

ex:
  (round 1)
  1. battery value is too low
  2. ask to charge the battery
  3. battery value is in the range
  4. ask to remove AC (--check-ac 'remove_ac') but operator didn't do it in time
  5. battery value is too large when operator pulls out charger
  (round 2)
  6. ask to discharge the battery
  7. battery is discharged to be in the range
  8. ask to remove AC and AC was already pulled out.
  9 cut off

some devices need to keep AC attached when doing cutoff, so you'll need to plug AC again in step 8, and if op is not there, it'll discharge too much and goes back to 1.

re#3

If --check-ac is connect_ac then cutoff will be performed at step 4 already so this is not the case? In case of --check-ac 'connect_ac' it will be 

ex1:
(round 1)
  1. battery value is too low
  2. ask to charge the battery
  3. battery value is in the range
  4. ask to connect AC (--check-ac 'connect_ac') and AC was already plugged.
  5. cutoff

ex2: 
(round 1)
  1. battery value is too high
  2. ask to discharge the battery
  3. battery value is in the range
  4. ask to plug AC in (--check-ac 'connect_ac') but operator didn't do it in time
  5. battery value is too low when operator plug in charger
  (round 2)
  6. ask to charge the battery
  7. battery is charged to be in the range
  8. ask to connect AC and AC was already plugged.
  9 cut off

Note that [1] is the CL which is planed to land on factory branch first to solve this issue.

[1] https://chromium-review.googlesource.com/#/c/551438/

many partners prefer to have a "final confirm" before performing cutoff (i.e, an input of ENTER). currently our frecon version doesn't really support that, but if was made in that way. So no matter which direction, staying in the final confirm screen will drive battery to exceed expected range. For ToT I'd expect a better change that (1) use stressapptest to drain power faster (2) real charge control so op need to only plug/unplug at most one round (3) ability to get back the "final confirm" screen.

 Issue 688952  has been merged into this issue.

> 1. Use 'ectool chargecontrol' if possible. This helps to allow discharging even if AC is connected.

Thanks for the info from [1], which indicated that 'ectool chargercontrol' is not working in cutoff.sh (after EC WP is enabled in finalize). I found the code snippet is at [2] for this limitation. Therefore to control charging in cutoff.sh is not possible now (except that we delay to enable EC WP).

We have two options as least now:

  1. Do battery check in finalize pytest in GRT before enabling EC WP and calling gooftool wipe.
  2. Based on CL in comment 5 and add appstresstest for enhancing discharging.

I can try to see 1. first. 

[1] https://chromium-review.googlesource.com/c/chromiumos/platform/factory/+/654469
[2] https://chromium.googlesource.com/chromiumos/platform/ec/+/master/common/charge_state_v2.c#1144

>  1. Do battery check in finalize pytest in GRT before enabling EC WP and calling gooftool wipe.

After investigating a bit, I found that

  1. Even if we can make sure battery level are kept in cutoff range then calling in_place_wiping without AC but inform_Shopfloor.sh [1] is still a place where can block the flow and drain the battery off the range.

> many partners prefer to have a "final confirm" before performing cutoff
  2. On the other hand, the requirement above called by HungTe in comment 7 is the another place to block the flow and make battery out of the range.

> 1. Use 'ectool chargecontrol' if possible. This helps to allow discharging even if AC is connected.

Hi Hung-Te,

I believe the assumption above is broken now so it seems there is no way to achieve the goal in this issue. Or any suggestion?

> 2. Based on CL in comment 5 and add appstresstest for enhancing discharging.
May the one above is the best effort we can do now?

[1] https://chromium.googlesource.com/chromiumos/platform/factory/+/master/sh/cutoff/inform_shopfloor.sh#43https://chromium.googlesource.com/chromiumos/platform/factory/+/master/sh/cutoff/inform_shopfloor.sh#43

Assigned to chenghan to see if he has some thoughts.

Update:
  There are already two partners reported this issue on ToT.

We may need to increase the priority.

> I think we should maintain battery level in GRT or any place before EC WP. After EC WP discharge and idle is not work.

We need some EC engineer to confirm this behavior. And since we didn't reboot system between enabling EC WP and wiping, is there some way we can still control EC before cutoff.

+rong who's expert of EC battery code.

Rong, can you shed some lights here, or add some owner who may know the details?

re#17,

Please refer to CL [1] which relieve this limitation now.

[1] https://chromium.googlesource.com/chromiumos/platform/ec/+/e4deceba8d6a8afff50ca2223be59b755ee3eca2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/5006d855f961855ececd059805b8214a35342022

commit 5006d855f961855ececd059805b8214a35342022
Author: Cheng-Han Yang <chenghan@google.com>
Date: Thu Nov 30 09:13:07 2017

factory: Improve cutoff process.

Use 'ectool chargecontrol' to control battery charging/discharging.
Operators do not need to remove AC to discharge, hence avoiding an
endless loop of charging and discharging.

BUG= chromium:738286 
TEST=manually test on DUT

Change-Id: Ie7ac23893a66179b82c90fbc582e8fd775955a79
Reviewed-on: https://chromium-review.googlesource.com/798954
Commit-Ready: Cheng-Han Yang <chenghan@chromium.org>
Tested-by: Cheng-Han Yang <chenghan@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/5006d855f961855ececd059805b8214a35342022/sh/cutoff/cutoff.sh

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/4000c155d3d7a683249f868c3584a60df905f5a2

commit 4000c155d3d7a683249f868c3584a60df905f5a2
Author: Cheng-Han Yang <chenghan@google.com>
Date: Tue Dec 05 05:49:01 2017

factory: Improve cutoff process.

Use 'ectool chargecontrol' to control battery charging/discharging.
Operators do not need to remove AC to discharge, hence avoiding an
endless loop of charging and discharging.

BUG= chromium:738286 
TEST=manually test on DUT

Change-Id: Ie7ac23893a66179b82c90fbc582e8fd775955a79
Reviewed-on: https://chromium-review.googlesource.com/798954
Commit-Ready: Cheng-Han Yang <chenghan@chromium.org>
Tested-by: Cheng-Han Yang <chenghan@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
(cherry picked from commit 5006d855f961855ececd059805b8214a35342022)
Reviewed-on: https://chromium-review.googlesource.com/805577
Tested-by: Anfernee Chen <anfernee_chen@wistron.corp-partner.google.com>
Commit-Queue: Anfernee Chen <anfernee_chen@wistron.corp-partner.google.com>

[modify] https://crrev.com/4000c155d3d7a683249f868c3584a60df905f5a2/sh/cutoff/cutoff.sh

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/factory/+/6f12dc4c31e3e77698ee9649c8940027fbbb2743

commit 6f12dc4c31e3e77698ee9649c8940027fbbb2743
Author: Cheng-Han Yang <chenghan@google.com>
Date: Thu Dec 07 16:04:58 2017

factory: Speed up battery discharge in cutoff.

Use 'stressapptest' to drain battery faster in cutoff script.

BUG= chromium:738286 
TEST=manually test on DUT

Change-Id: I5d7c1cdc38a14b8d1eeb7b2e98e7fd5d00de3db1
Reviewed-on: https://chromium-review.googlesource.com/799613
Commit-Ready: Cheng-Han Yang <chenghan@chromium.org>
Tested-by: Cheng-Han Yang <chenghan@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/6f12dc4c31e3e77698ee9649c8940027fbbb2743/py/gooftool/wipe.py
[modify] https://crrev.com/6f12dc4c31e3e77698ee9649c8940027fbbb2743/sh/cutoff/cutoff.sh