A quick check:

Device API => dump_power_status

CheckACPresent() => line_power_connected
GetACType() => N/A (expect things like USB_PD or USB or something else, which is the 'type' in sysfs
CheckBatteryPresent() => battery_present
GetCharge() => battery_charge? (supposed to be charge_now / 1000, mAH)
GetChargeFull() => battery_charge_full?
GetChargePct => battery_display_percent? (GetCharge() / GetChargeFull(), or energy_now / energy_full)
GetWearPct => charge_full / charge_full_design
GetChargeStatus() => ? ('status' of battery sysfs)
GetChargerCurrent() => ?
GetBatteryCurrent() => battery_current
GetBatteryDesignCapacity() => ?

I will add the following to dump_power_status:

- GetACType: line_power_type
- GetChargeStatus: battery_status (do you have a typo? GetChargeStatus doesn't exist)
- GetChargerCurrent: line_power_current

GetChargePct should be using battery_percent, not battery_display_percent. battery_display_percent is the value that we show in the UI; it's adjusted for powerd shutting down before we reach 0% and for batteries stopping charging before they reach 100%.

GetBatteryDesignCapacity is already printed as battery_charge_full_design.

regarding battery_display_percent, that's another interesting topic. We've been complained from partners that what they set for cut off level was wrong. For example:

"The spec said you have to cut off and ship between 60%~65%, and we cutoff at 60%, but when boot again, Chrome said the battery is 58%"

So we should never trust the battery_display_percent right?

GetChargeStatus was a typo get GetChargeState.

Re battery_display_percent, it depends on what you need. If you want to get the number that we'd display in the UI, then you can use battery_display_percent. If you just want a simple percentage equal to charge_now divided by charge_full, then use battery_percent.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/4eb21beaf3df890b6649667eb84934d44ef4cefc

commit 4eb21beaf3df890b6649667eb84934d44ef4cefc
Author: Daniel Erat <derat@chromium.org>
Date: Tue Jan 30 20:06:22 2018

power: Add more values to dump_power_status.

Add a few more values to the dump_power_status tool for
factory testing:

  line_power_type ("type" sysfs string)
  line_power_current (scaled "current_now" sysfs value)
  battery_status ("status" sysfs string)

BUG= chromium:805275 
TEST=ran it and verified that new fields are printed

Change-Id: I49d43a4d848cf118b4a9f333114caaaf71e9d3c8
Reviewed-on: https://chromium-review.googlesource.com/886834
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/4eb21beaf3df890b6649667eb84934d44ef4cefc/power_manager/tools/dump_power_status.cc

Let me know if you need anything more from me here.

Re-assign to chenghan.

Hi Chenghan, can you add a power implementation that collects data from dump_power_status? I think we'll have
 Power (abstract)
 ECToolPower (based on ectool)
 SysFSPower (based on /sys/fs, current Power)
 PowerDaemonPower (based on powerd - dump_power_status)

And on board=Linux, deafult to SysFSPower, and on board=ChromeOS, default to PowerDaemonPower.

I find it easier to have ectool as the default method to get power info, i.e.
  Power (top level, based on ectool)
  SysfsPower (inherit from Power, based on sysfs)
  PowerDaemonPower (inherit from Power, based on dump_power_status)

The reason is that there are things that can only be done by ectool, such as changing chargestate (`ectool chargecontrol discharge`). Also, only ectool can reliably get charger current. `dump_power_status` has a field "line_power_current", but the file "current_now" may not exist in sysfs, so its value will become 0.

@derat do you think what chenghan addressed in #11 can be fixed in powerd/sysfs?

Re#11
"changing state" can be done in a way like doing mixin, so it may become 
 PowerSource (read)
 PowerControl (write)

 ECToolPowerSource
 ECToolPowerControl

 ...
 Power(PowerSource, PowerControl)
 and linux=Power(SysFSPowerSource, DummyPowerControl)
 chromeos=Power(PowerDaemonPowerSource, ECToolPowerControl)

#12: Reporting current_now for chargers is tracked at issue 807753, I think.

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

commit d57879062db8ab7978fed08f37d509c735335271
Author: Cheng-Han Yang <chenghan@google.com>
Date: Thu May 31 19:26:19 2018

device: power: Get power info from `dump_power_status`.

Implement PowerDaemonPower to get power info from output of
command `dump_power_status`. This will be the default power
module used by chromeos board in the future.

BUG= chromium:805275 
TEST=make test

Change-Id: Ib278bae3e1b5699154a9ece094e4b336343b7409
Reviewed-on: https://chromium-review.googlesource.com/1075955
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Cheng-Han Yang <chenghan@chromium.org>
Reviewed-by: Yong Hong <yhong@google.com>

[modify] https://crrev.com/d57879062db8ab7978fed08f37d509c735335271/py/device/power.py
[modify] https://crrev.com/d57879062db8ab7978fed08f37d509c735335271/py/device/power_unittest.py

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

commit e27a41e8ebe569b7c3800fa4add997ab89ba313e
Author: Cheng-Han Yang <chenghan@google.com>
Date: Thu Jun 07 20:11:39 2018

device: power: Use mixin class to customize power.

To support flexible power configs, we use mixins to customize functions
of `PowerBase` class. For power control, we can only use ectool, and
for power info we can use sysfs, ectool or `dump_power_status` command.

The default power used by Linux board is `LinuxPower`, which does not
have power control and uses sysfs for power info. ChromeOS board uses
`ChromeOSPower` class which uses ectool to control power and command
`dump_power_status` to get power info. For compatibility there is also
`ChromeOSPowerLegacy` and `Power` which uses ectool power control and
sysfs power info.

BUG= chromium:805275 
TEST=make test

Change-Id: Ie70c40c93997c982bb0c7a6d424a090edf7ed882
Reviewed-on: https://chromium-review.googlesource.com/1088464
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Cheng-Han Yang <chenghan@chromium.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

[modify] https://crrev.com/e27a41e8ebe569b7c3800fa4add997ab89ba313e/py/device/power.py
[modify] https://crrev.com/e27a41e8ebe569b7c3800fa4add997ab89ba313e/py/device/power_unittest.py
[modify] https://crrev.com/e27a41e8ebe569b7c3800fa4add997ab89ba313e/py/device/boards/chromeos.py
[modify] https://crrev.com/e27a41e8ebe569b7c3800fa4add997ab89ba313e/py/device/boards/linux.py
[modify] https://crrev.com/e27a41e8ebe569b7c3800fa4add997ab89ba313e/py/test/utils/charge_manager.py

Hi derat@, I am working on gathering values from sysfs. Can you add another field in `dump_power_status` to read 'voltage_min_design' in sysfs? Thanks.

I am also curious about 'energy_now', 'energy_full' files in sysfs. They are used in factory code but I don't see them on my devices. Are they deprecated now or just driver dependent?

powerd uses the following logic to determine the nominal voltage:

  // Attempt to determine nominal voltage for time-remaining calculations. This
  // may or may not be the same as the instantaneous voltage |battery_voltage|,
  // as voltage levels vary over the time the battery is charged or discharged.
  // Some batteries don't have a voltage_min/max_design attribute, so just use
  // the current voltage in that case.
  double nominal_voltage = voltage;
  if (base::PathExists(path.Append("voltage_min_design")))
    nominal_voltage = ReadScaledDouble(path, "voltage_min_design");
  else if (base::PathExists(path.Append("voltage_max_design")))
    nominal_voltage = ReadScaledDouble(path, "voltage_max_design");

Will printing this as "battery_nominal_voltage work" for you, or do you specifically need voltage_min_design to be printed (with some value like 0 or -1 if the node is missing)?

I don't know anything about those other nodes. Someone on the kernel or EC team might.

re: energy_now, energy_full

AFAIK they're driver dependent.  I think for most cros devices you won't see either on intel based devices which use the acpi battery driver while on ARM they largely use the sbs driver which exposes 'energy_now' but not 'energy_full'.

Neither IMO are needed providing the equivalent 'charge_*' value is available which AFAIK has always been the case.

Thank you for the explanation. I will stop reporting 'energy_*' fields.

hungte, the current usage of reporting 'voltage_min_design' is only for logging (eventlog and testlog). Do you think we can just remove this field?

I'd say let's remove all reporting fields if it's hard to maintain.

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

commit 4bd906d7a41f573194f2acb52da2db9881d3ab07
Author: Cheng-Han Yang <chenghan@google.com>
Date: Tue Jun 12 18:48:31 2018

device: power: Move GetInfoDict to mixin base class

`GetInfoDict` uses member functions to get info instead of reading
directly from sysfs. Also remove 'voltage_min_design' and 'energy_*'
fields in the returned dict from this function.

BUG= chromium:805275 
TEST=make test

Change-Id: I0d6e9e4f6af9fdf8e0d1aadb18e2c3d57113d002
Reviewed-on: https://chromium-review.googlesource.com/1095123
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/4bd906d7a41f573194f2acb52da2db9881d3ab07/py/device/power.py
[modify] https://crrev.com/4bd906d7a41f573194f2acb52da2db9881d3ab07/py/device/power_unittest.py