Bug 86510

Summary: Bluetooth HID keyboard battery shows up as laptop battery
Product: upower Reporter: Sean Cross <xobs>
Component: generalAssignee: Richard Hughes <richard>
Status: RESOLVED DUPLICATE QA Contact:
Severity: normal    
Priority: medium CC: bugzilla, xobs
Version: unspecified   
Hardware: ARM   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: A patch to add support for detecting hid devices, in addition to bluetooth devices

Description Sean Cross 2014-11-21 05:43:55 UTC
Created attachment 109792 [details] [review]
A patch to add support for detecting hid devices, in addition to bluetooth devices

Bluetooth HID devices such as the Lenovo Compact Bluetooth Keyboard have power_supply devices underneath them.  UPower supports reading this battery, but misidentifies the battery type as UP_DEVICE_KIND_BATTERY.  This gives the user the impression of two main system batteries.

The reason for this is that the keyboard is a "hid" class device, and not a "bluetooth" class device.  When up_device_supply_coldplug() scans, it detects that the device is "bluetooth" and attempts to locate the "input" directory.  When it fails to find the "input" directory (because it is under the "hid" class), it gives up and sets the type to UP_DEVICE_KIND_BATTERY.

A workaround is proposed in the attached patch.  This patch causes upower to look through a list of classes, starting with "hid".  Applying this patch fixes keyboard battery detection on my system.
Comment 1 Bastien Nocera 2015-04-03 15:21:57 UTC
Comment on attachment 109792 [details] [review]
A patch to add support for detecting hid devices, in addition to bluetooth devices

Review of attachment 109792 [details] [review]:
-----------------------------------------------------------------

I'm guessing that the patch is in whatever format is used for debian changelogs.

Please submit one that's been git-formatted ("git format-patch -1 ...").

::: upower-0.99.1.orig/src/linux/up-device-supply.c
@@ +990,5 @@
>  			type = UP_DEVICE_KIND_LINE_POWER;
>  		} else if (g_ascii_strcasecmp (device_type, "battery") == 0) {
>  
> +			int i;
> +			const char *classes[] = {"hid", "bluetooth"};

Space, after the opening bracket and before the closing one.

@@ +991,5 @@
>  		} else if (g_ascii_strcasecmp (device_type, "battery") == 0) {
>  
> +			int i;
> +			const char *classes[] = {"hid", "bluetooth"};
> +			int num_classes = sizeof(classes) / sizeof(*classes);

Use G_N_ELEMENTS() directly.

@@ +994,5 @@
> +			const char *classes[] = {"hid", "bluetooth"};
> +			int num_classes = sizeof(classes) / sizeof(*classes);
> +
> +			for (i = 0; i < num_classes && type == UP_DEVICE_KIND_UNKNOWN; i++) {
> +

No need for that stray linefeed.

@@ +997,5 @@
> +			for (i = 0; i < num_classes && type == UP_DEVICE_KIND_UNKNOWN; i++) {
> +
> +				input_path = up_device_get_input_path (native, classes[i]);
> +
> +				if (input_path != NULL) {

It would be more readable with:
if (input_path == NULL)
   continue;
Comment 2 Martin Pitt 2015-05-29 08:10:30 UTC
This was handled and fixed in bug 90222.

*** This bug has been marked as a duplicate of bug 90222 ***

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.