New issue
Advanced search Search tips

Issue 831236 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Feature



Sign in to add a comment

Make powerd report more details about multiple power sources

Project Member Reported by derat@chromium.org, Apr 10 2018

Issue description

The PowerSupplyProperties protobuf that powerd sends over D-Bus should contain more information to help clients determine connected sources when both USB Type C and barrel-jack sources are available.

Some background:

https://docs.google.com/document/d/1tEY4u31a9j4ZPJam8Yy76pgidv6dIHKMWk_QuGDPHQ4/edit?usp=sharing
https://docs.google.com/document/d/1365nwq1luBwBD5QOmNbFyeqnJYJWK0xgu19bh8U1XlY/edit?usp=sharing

I think that adding a "type" field to the PowerSource submessage and ensuring that powerd preferentially reports the USB_PD source as active when both it and a Mains source are listed due to the ACPI driver being enabled would be a good start.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/7aaece4a0870323c3a1e5d3f7afecd7b15e188ae

commit 7aaece4a0870323c3a1e5d3f7afecd7b15e188ae
Author: Daniel Erat <derat@chromium.org>
Date: Thu Apr 12 01:08:45 2018

system_api: Add type field to PowerSource message.

Update the PowerSupplyProperties protocol buffer's
PowerSource submessage to include a type field that includes
more information about the source.

BUG= chromium:831236 
TEST=none

Change-Id: I0519d9806b638c5a639981b040ec6dba927efa2d
Reviewed-on: https://chromium-review.googlesource.com/1005624
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Benson Leung <bleung@chromium.org>
Reviewed-by: Todd Broch <tbroch@chromium.org>

[modify] https://crrev.com/7aaece4a0870323c3a1e5d3f7afecd7b15e188ae/dbus/power_manager/power_supply_properties.proto

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 13 2018

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

commit d0443da4e07022f892870534a2d2c46e30ab1e8f
Author: Daniel Erat <derat@chromium.org>
Date: Fri Apr 13 03:35:03 2018

power: Remove PowerStatus c'tors and d'tors.

Remove explicitly-defined constructors and destructors for
PowerStatus and PowerStatus::Port. Needing to update the
Port c'tor every time a new field is added is a waste of
time.

BUG= chromium:831236 
TEST=it builds and unit tests pass

Change-Id: Ifde9e90db80f5dfc2db933aa05721f4b89619c7d
Reviewed-on: https://chromium-review.googlesource.com/1010834
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/d0443da4e07022f892870534a2d2c46e30ab1e8f/power_manager/powerd/system/power_supply.cc
[modify] https://crrev.com/d0443da4e07022f892870534a2d2c46e30ab1e8f/power_manager/powerd/system/power_supply_unittest.cc
[modify] https://crrev.com/d0443da4e07022f892870534a2d2c46e30ab1e8f/power_manager/powerd/system/power_supply.h

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 13 2018

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

commit 426c3ad923f91cfa77d7fb43b90678a5052cac0d
Author: Daniel Erat <derat@chromium.org>
Date: Fri Apr 13 06:10:52 2018

power: Favor non-Mains line power supplies.

Make powerd favor reporting Type-C power supplies as the
active source when "Mains" sources (reported by the ACPI
driver) are also present. This sometimes happens when people
forget to turn off the ACPI driver on Type-C-only devices,
but can also happen if a device can use both Type-C and
barrel jack ports for charging.

BUG= chromium:831236 
TEST=added unit test

Change-Id: I6681c450b15a0bfe39107857446faef38c5023f3
Reviewed-on: https://chromium-review.googlesource.com/1006493
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/426c3ad923f91cfa77d7fb43b90678a5052cac0d/power_manager/powerd/system/power_supply.cc
[modify] https://crrev.com/426c3ad923f91cfa77d7fb43b90678a5052cac0d/power_manager/powerd/system/power_supply_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 13 2018

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

commit b653cac212945b668baf4f94db76bca5d97da871
Author: Daniel Erat <derat@chromium.org>
Date: Fri Apr 13 06:10:50 2018

power: Rename PowerStatus::Port::Connection enum to Role.

Rename the blandly-named PowerStatus::Port::Connection enum
to "Role" to more accurately describe what it represents
(and to make room for Port storing more-specific information
about different types of devices in the future).

BUG= chromium:831236 
TEST=builds and tests pass

Change-Id: Id00ae67995ae64af31b4929e5bb2abd964f49cf4
Reviewed-on: https://chromium-review.googlesource.com/1010907
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/b653cac212945b668baf4f94db76bca5d97da871/power_manager/tools/power_supply_info.cc
[modify] https://crrev.com/b653cac212945b668baf4f94db76bca5d97da871/power_manager/powerd/metrics_collector.cc
[modify] https://crrev.com/b653cac212945b668baf4f94db76bca5d97da871/power_manager/powerd/system/power_supply.h
[modify] https://crrev.com/b653cac212945b668baf4f94db76bca5d97da871/power_manager/powerd/system/power_supply_unittest.cc
[modify] https://crrev.com/b653cac212945b668baf4f94db76bca5d97da871/power_manager/powerd/system/power_supply.cc
[modify] https://crrev.com/b653cac212945b668baf4f94db76bca5d97da871/power_manager/powerd/metrics_collector_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 13 2018

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

commit 7e62c269a831fc2123340a3b447111bdb91945d4
Author: Daniel Erat <derat@chromium.org>
Date: Fri Apr 13 06:10:51 2018

power: Fill PowerSupplyProperties::PowerSource::type.

Make powerd fill each available power source's "type" field
in the PowerSupplyProperties protobufs that it sends over
D-Bus. This field contains a coarse classification of the
source's type: MAINS, USB_C, USB_BC_1_2, or OTHER.

BUG= chromium:831236 
TEST=updated unit tests

Change-Id: Ia463906e75ba95ab33b03a2ff1867401ec73a50f
Reviewed-on: https://chromium-review.googlesource.com/1011421
Commit-Ready: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/7e62c269a831fc2123340a3b447111bdb91945d4/power_manager/powerd/system/power_supply.cc
[modify] https://crrev.com/7e62c269a831fc2123340a3b447111bdb91945d4/power_manager/powerd/system/power_supply_unittest.cc
[modify] https://crrev.com/7e62c269a831fc2123340a3b447111bdb91945d4/power_manager/powerd/system/power_supply.h

Comment 6 by derat@chromium.org, Apr 13 2018

Status: Fixed (was: Started)
Feel free to reopen this if it turns out that there's more that you need here.

Sign in to add a comment