I've ordered one and will investigate further when I get back to Mountain
View.

powerd's system::PowerSupply class skips over batteries with a "scope" value of "Device" to try to avoid peripherals (which are reported to Chrome separately). Maybe that's not being set for this peripheral for some reason, though.

What does that mean if "scope" is available?
Do you mean we can skip all "Battery" in sysfs if it has a "scope" value, so the only batter left will be the "system battery" (usually BAT0)?

Only if the scope value is "Device". See https://chromium.googlesource.com/chromiumos/platform2/+/master/power_manager/powerd/system/power_supply.cc#615 and IsExternalPeripheral in the same file.

On my Pixelbook (eve), I'm seeing in /sys/class/power_supply,

  BAT0 (system battery) does not have 'scope'
  A HID device has scope and value is 'Device'

But I think the clue is good enough that I should skip all batteries with scope=Device in factory test code. Thanks!

Sorry, I thought you were reporting an issue with powerd's behavior. It sounds like it's instead an issue in factory test code.

I've tried to help us avoid duplicating powerd's logic in factory code in the past; see e.g.  issue 747501 . Is it possible to do the same thing here?

For example, can you use the output from the dump_power_status tool? That shares powerd's well-tested code for interpreting /sys/class/power_supply and prints data in a machine-parsable format. If there's additional information you need that it doesn't already print, it's probably easy for me to add it.

We discovered this "from factory test", but after checking with Benson, we filed this issue to make sure the other software stack on DUTs (including powerd) are behaving correctly because partner claimed there was no problem in all previous Chromebooks (except one, but that was known to be "fixed" by fw/ec update -- according to them) --especially we "think" the scanner should not have *any* batteries included.

So this is an issue to make sure we will be tracking what has been changed and how this may impact userland, especially if it's caused by changes in newer kernel.

And yes, using dump_power_status sounds like a good idea, but that's a huge change we'll do later. I can file another issue for that.

This task is for me to investigate this potentially sketchy USB device and potentially fix the upstream kernel (and backport the fix) so it doesn't spawn a battery device like its doing.

I a presuming the device is setting its descriptors in a funny way, or the kernel had some recent change that caused this device to be misconfigured. I seriously doubt it has a battery or this is intentional.

The factory test discussion is forked to  issue 805275 . Let's leave this for Benson to check if the barcode scanner is working as expected.

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

commit a77585c25eccf877cbaf9b179ec0b96eb9033037
Author: Hung-Te Lin <hungte@chromium.org>
Date: Wed Jan 24 09:30:50 2018

device: Change power to filter out peripheral batteries in power_supply.

We found that few HID peripherals, especially stylus, are recognized as
power_supply and would be selected as "default (system) battery" in
factory tests.

The right solution is to filter out by checking USB property scope!=Device.

BUG= chromium:804720 
TEST=make test

Change-Id: I9cc4770ce42cb4e0e35d007345db977eda863055
Reviewed-on: https://chromium-review.googlesource.com/882784
Commit-Ready: Hung-Te Lin <hungte@chromium.org>
Tested-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Shen-En Shih <petershih@chromium.org>

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

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chromeos/overlays/overlay-meowth-private/+/ef1509edbbc392103d0df7a091af8680ad4bfd53

commit ef1509edbbc392103d0df7a091af8680ad4bfd53
Author: Hung-Te Lin <hungte@chromium.org>
Date: Wed Jan 24 12:36:34 2018

+dtor.

FYI, dtor and I looked at the Symbol scanner, and it reports a battery hid descriptor incorrectly. Likely cloned its hid implementation from a wireless keyboard or something, so it gets this as a bonus.

Dtor mentioned the same thing Dan mentioned in comment #2, that this sort of device should have a "scope" property with "Device" set, so the factory test should ignore it.

Hung-te, can you modify the quick fix to ignore Battery type power supplies that have a "scope" property of "Device"?

Thanks,
Benson

Hung-te, oops, sorry for the noise. Looks like that's exactly what you did here: https://chromium-review.googlesource.com/#/c/chromiumos/platform/factory/+/882784/

I'll look into adding a quirk to ignore the battery descriptor on this device.

Dtor, do you still have this scanner? I've finally gotten around to this and want to add a quirk.

+yhong

The scanner was provided by partner in meowth or eve, so if you are in factory they should have it (ask the operators!)

Hi Hungte,

Yes, I remember. I purchased one after P1 build and Dmitry had been holding onto it for me. It's a pretty low priority to fix, but we should quirk that so it doesn't expose a power supply.

Finally got around to writing this quirk. Sent to the list here:
https://lkml.org/lkml/2018/11/9/688

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/070ee0acaa9e625bd06f979e4ac06593b90463d5

commit 070ee0acaa9e625bd06f979e4ac06593b90463d5
Author: Benson Leung <bleung@chromium.org>
Date: Tue Dec 04 11:11:11 2018

FROMGIT: HID: input: Ignore battery reported by Symbol DS4308

The Motorola/Zebra Symbol DS4308-HD is a handheld USB barcode scanner
which does not have a battery, but reports one anyway that always has
capacity 2.

Let's apply the IGNORE quirk to prevent it from being treated like a
power supply so that userspaces don't get confused that this
accessory is almost out of power and warn the user that they need to charge
their wired barcode scanner.

Reported here: https://bugs.chromium.org/p/chromium/issues/detail?id=804720

Signed-off-by: Benson Leung <bleung@chromium.org>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
(cherry picked from commit 0fd791841a6d67af1155a9c3de54dea51220721e
 from git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git)

BUG= chromium:804720 
TEST=Plug in DS4308 scanner. check in /sys/class/power_supply that
no hid power supply appears for the DS4308.

Change-Id: I6c3bf7e53cf9a4141d5003cf92708cdcd524a527
Reviewed-on: https://chromium-review.googlesource.com/1331018
Commit-Ready: Benson Leung <bleung@google.com>
Tested-by: Benson Leung <bleung@google.com>
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>

[modify] https://crrev.com/070ee0acaa9e625bd06f979e4ac06593b90463d5/drivers/hid/hid-input.c
[modify] https://crrev.com/070ee0acaa9e625bd06f979e4ac06593b90463d5/drivers/hid/hid-ids.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/59da277d77757cc905cbbe752babd93df0de5d67

commit 59da277d77757cc905cbbe752babd93df0de5d67
Author: Benson Leung <bleung@chromium.org>
Date: Tue Dec 04 11:11:09 2018

FROMGIT: HID: input: Ignore battery reported by Symbol DS4308

The Motorola/Zebra Symbol DS4308-HD is a handheld USB barcode scanner
which does not have a battery, but reports one anyway that always has
capacity 2.

Let's apply the IGNORE quirk to prevent it from being treated like a
power supply so that userspaces don't get confused that this
accessory is almost out of power and warn the user that they need to charge
their wired barcode scanner.

Reported here: https://bugs.chromium.org/p/chromium/issues/detail?id=804720

Signed-off-by: Benson Leung <bleung@chromium.org>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
(cherry picked from commit 0fd791841a6d67af1155a9c3de54dea51220721e
 from git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git)

BUG= chromium:804720 
TEST=Plug in DS4308 scanner. check in /sys/class/power_supply that
no hid power supply appears for the DS4308.

Change-Id: I6c3bf7e53cf9a4141d5003cf92708cdcd524a527
Reviewed-on: https://chromium-review.googlesource.com/1331016
Commit-Ready: Benson Leung <bleung@google.com>
Tested-by: Benson Leung <bleung@google.com>
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>

[modify] https://crrev.com/59da277d77757cc905cbbe752babd93df0de5d67/drivers/hid/hid-input.c
[modify] https://crrev.com/59da277d77757cc905cbbe752babd93df0de5d67/drivers/hid/hid-ids.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/4e9fc1703a1efebb97e30ccec5d9c62d431bb2b1

commit 4e9fc1703a1efebb97e30ccec5d9c62d431bb2b1
Author: Benson Leung <bleung@chromium.org>
Date: Tue Dec 04 11:11:12 2018

FROMGIT: HID: input: Ignore battery reported by Symbol DS4308

The Motorola/Zebra Symbol DS4308-HD is a handheld USB barcode scanner
which does not have a battery, but reports one anyway that always has
capacity 2.

Let's apply the IGNORE quirk to prevent it from being treated like a
power supply so that userspaces don't get confused that this
accessory is almost out of power and warn the user that they need to charge
their wired barcode scanner.

Reported here: https://bugs.chromium.org/p/chromium/issues/detail?id=804720

Signed-off-by: Benson Leung <bleung@chromium.org>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
(cherry picked from commit 0fd791841a6d67af1155a9c3de54dea51220721e
 from git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git)

BUG= chromium:804720 
TEST=Plug in DS4308 scanner. check in /sys/class/power_supply that
no hid power supply appears for the DS4308.

Change-Id: I6c3bf7e53cf9a4141d5003cf92708cdcd524a527
Reviewed-on: https://chromium-review.googlesource.com/1331017
Commit-Ready: Benson Leung <bleung@google.com>
Tested-by: Benson Leung <bleung@google.com>
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>

[modify] https://crrev.com/4e9fc1703a1efebb97e30ccec5d9c62d431bb2b1/drivers/hid/hid-input.c
[modify] https://crrev.com/4e9fc1703a1efebb97e30ccec5d9c62d431bb2b1/drivers/hid/hid-ids.h

Fixed for the 3 most recent kernels where we might run into this issue at the factory.