Bug 70321

Example 2:
Some batteries do not even expose a status, such as those provided by Wacom wireless devices (through the wacom kernel module). For those batteries, /sys/class/power_supply/BATx/status is non-existent.

Upower will also always declare these batteries as currently charging, even when it is not correct.

The code in the power supply backend in UPower does this:
http://cgit.freedesktop.org/upower/tree/src/linux/up-device-supply.c#n721

Which I guess is something reasonable to do for laptop batteries.

As for the Wacom tablet, the problem was that this code above didn't check whether the device was a power supply. If it's not a power supply, we shouldn't change the state. I have a fix for this in my branch which I'll attach for you to test.

Created attachment 87505 [details] [review]
linux: Don't guess discharging state for devices

If the device state is unknown, don't guess based on the laptop's
power supplies (battery/power line) as it might not be charging
from there.

https://bugs.freedesktop.org/show_bug.cgi?id=70321#c1

(In reply to comment #3)
> Created attachment 87505 [details] [review] [review]
> linux: Don't guess discharging state for devices
> 
> If the device state is unknown, don't guess based on the laptop's
> power supplies (battery/power line) as it might not be charging
> from there.
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=70321#c1
The patch didn't solve the bug for me. Upower does seem to consider the wacom battery a "power supply", which makes your patch not have any effect on it.

$ ls /sys/class/power_supply/wacom_battery
capacity  device  power  subsystem  type  uevent

$ cat /sys/class/power_supply/wacom_battery/capacity
77

$ upower -i /org/freedesktop/UPower/devices/battery_wacom_battery
  native-path:          /sys/devices/pci0000:00/0000:00:1a.7/usb1/1-6/1-6.7/power_supply/wacom_battery
  power supply:         yes
  updated:              Sat Oct 12 21:39:49 2013 (21 seconds ago)
  has history:          yes
  has statistics:       yes
  battery
    present:             yes
    rechargeable:        yes
    state:               charging
    energy:              0 Wh
    energy-empty:        0 Wh
    energy-full:         0 Wh
    energy-full-design:  0 Wh
    energy-rate:         0 W
    percentage:          77%
    capacity:            100%

it is currently discharging.

>   power supply:         yes

This is the problem:
What's the output of:

cat /sys/class/power_supply/BAT0/scope

If it's something other than "device", then your kernel is buggy.

(In reply to comment #5)
> What's the output of:
> 
> cat /sys/class/power_supply/BAT0/scope
> 
> If it's something other than "device", then your kernel is buggy.
Sorry, I cannot find scope in sysfs, neither on a 3.5 nor 3.10 series kernel. Do I need any debug flags?

Sorry, I meant:
cat /sys/class/power_supply/wacom_battery/scope

Which it seems doesn't exist on your system.

This is the battery on a Bluetooth mouse:
$ ls  /sys/class/power_supply/hid-00\:14\:51\:d1\:69\:ed-battery/
capacity  device  model_name  online  power  powers  present  scope  status  subsystem	type  uevent

and

$ cat /sys/class/power_supply/hid-00\:14\:51\:d1\:69\:ed-battery/scope
Device

> $ ls /sys/class/power_supply/wacom_battery
> capacity  device  power  subsystem  type  uevent

Yours doesn't have status, present or scope... That looks like a kernel bug.

The main battery does not have a scope either. In fact, there are no files called scope in sysfs at all. I'm willing to consider this a kernel bug, but if it is, it'll be quite widespread.

Anyway, the wacom driver does not define a scope at all:
http://lxr.free-electrons.com/source/drivers/input/tablet/wacom_sys.c#L1095
http://lxr.free-electrons.com/source/drivers/input/tablet/wacom_wac.c#L1269


This also means that Upower by default assumes a "system" scope. Not unreasonable, as most batteries will indeed be. But it also means that that we get effects like with the wacom driver, lacking scope.


The second part is that Upower makes assumptions about system batteries with unknown statuses. That's independent from the wacom issue.
Wouldn't it be more reasonable to keep UP_DEVICE_STATE_UNKNOWN for such states?  Then again, an unknown state might be dangerous in situations where desktops are unaware that they're running on battery and it is discharging.

Or check current_now (I don't know how reliable that is however). Or check the SMAPI interface, if present. The latter sounds the most reasonable to me, but it would be a driver-specific implementation.

(In reply to comment #8)
> The main battery does not have a scope either. In fact, there are no files
> called scope in sysfs at all. I'm willing to consider this a kernel bug, but
> if it is, it'll be quite widespread.
> 
> Anyway, the wacom driver does not define a scope at all:
> http://lxr.free-electrons.com/source/drivers/input/tablet/wacom_sys.c#L1095
> http://lxr.free-electrons.com/source/drivers/input/tablet/wacom_wac.c#L1269

You're looking at the wrong driver :)

It's http://lxr.free-electrons.com/source/drivers/hid/hid-wacom.c

And feel free to look at the Copyright header too ;)

(though I'm guessing you have an Intuos4, I didn't write the code for that one).

> This also means that Upower by default assumes a "system" scope. Not
> unreasonable, as most batteries will indeed be. But it also means that that
> we get effects like with the wacom driver, lacking scope.

http://lxr.free-electrons.com/source/drivers/hid/hid-wacom.c#L65

> The second part is that Upower makes assumptions about system batteries with
> unknown statuses. That's independent from the wacom issue.
> Wouldn't it be more reasonable to keep UP_DEVICE_STATE_UNKNOWN for such
> states?  Then again, an unknown state might be dangerous in situations where
> desktops are unaware that they're running on battery and it is discharging.

I guess you mean laptops here. Yes, it's the reason why the code assumes "unknown" means discharging (some firmwares have batteries with that state when coming back from suspend).

> Or check current_now (I don't know how reliable that is however). Or check
> the SMAPI interface, if present. The latter sounds the most reasonable to
> me, but it would be a driver-specific implementation.

Or the kernel could return "not charging" as the status, which would fix your particular problem.

There's absolutely no way that we'll be adding driver specific code in UPower when there are well-defined interface to export battery information from there.

Nope, I'm pretty sure I have the right driver ;) I'm aware that there are different drivers, but I'm referring to just the regular "wacom" module here, not "hid_wacom".

I don't know which tablet the user has exactly, but it uses the Wacom Wireless Receiver Kit.



As for the laptop battery, I can agree that with that. But since the regular battery interface is ACPI only without drivers (afaik), is it even possible to feed information from specific drivers back to the interface? I always assumed there's a reason why tp_smapi does things the way it does.

I have opened an issue for tp_smapi to hear their opinion on this.
https://github.com/evgeni/tp_smapi/issues/17

(In reply to comment #10)
> Nope, I'm pretty sure I have the right driver ;) I'm aware that there are
> different drivers, but I'm referring to just the regular "wacom" module
> here, not "hid_wacom".

http://thread.gmane.org/gmane.linux.kernel.input/32276

> I don't know which tablet the user has exactly, but it uses the Wacom
> Wireless Receiver Kit.

> As for the laptop battery, I can agree that with that. But since the regular
> battery interface is ACPI only without drivers (afaik), is it even possible
> to feed information from specific drivers back to the interface? I always
> assumed there's a reason why tp_smapi does things the way it does.

I don't have a clue, but that's something that the tp_smapi developers need to figure out. I have little interest in working around problems created by out-of-tree drivers.

I've pushed this patch. Please file a new bug if there are still problems once the kernel/driver bugs have been fixed.

Now that's service :) Didn't even have to file a kernel bug for that!

Thanks!

(In reply to comment #14)
> Now that's service :) Didn't even have to file a kernel bug for that!

After staring at the code to implement those properties in hid-wacom.c (which does it properly) for 20-odd minutes, it was easy enough to implement :)