See patch
Created attachment 130402 [details] [review] linux: Add support for "capacity_level" attribute Some devices, such as a number of wireless Logitech unifying devices don't have a precise battery level reporting, and use the Linux POWER_SUPPLY_CAPACITY_LEVEL_* values. This minimal fix matches the levels against approximate percentage values. This is good enough to make the Logitech T650 report battery again when using the kernel HID++ battery support.
Ideally, we'd proxy the "capacity level" itself through D-Bus, but it's really quite a bit easier to convert this to an approximate percentage. Not great for devices with only 2 levels though: https://bugzilla.gnome.org/show_bug.cgi?id=780360 Ideas?
Created attachment 130414 [details] [review] linux: Add support for "capacity_level" attribute Some devices, such as a number of wireless Logitech unifying devices don't have a precise battery level reporting, and use the Linux POWER_SUPPLY_CAPACITY_LEVEL_* values. This minimal fix matches the levels against approximate percentage values. This is good enough to make the Logitech T650 report battery again when using the kernel HID++ battery support.
(In reply to Bastien Nocera from comment #3) > Created attachment 130414 [details] [review] [review] > linux: Add support for "capacity_level" attribute 2 changes: - remove the "capacity_level" support for laptop batteries (non-device batteries), it's not actually needed - Update the code and the tests, the "capacity_level" strings have linefeeds in them...
*** Bug 96857 has been marked as a duplicate of this bug. ***
After talking to Richard on IRC, I think I'll add a patch on top of this one to export the actual "capacity_level" value to API consumers. This will allow older apps to still work with the newer kernels, and not regress in supporting devices which use it. The API would allow front-ends to not show a full battery gauge when the reporting is too coarse to show any changes over time.
Created attachment 130694 [details] [review] daemon: Make warning levels for devices inclusive A 5% battery level should already be in "critical", this matches the configurable thresholds where the bounds of critical warnings are inclusive. This also makes it easier to create fake percentages for the Logitech devices with coarse battery level reporting.
Created attachment 130695 [details] [review] linux: Add support for "capacity_level" attribute Some devices, such as a number of wireless Logitech unifying devices don't have a precise battery level reporting, and use the Linux POWER_SUPPLY_CAPACITY_LEVEL_* values. This minimal fix matches the levels against approximate percentage values. This is good enough to make the Logitech T650 report battery again when using the kernel HID++ battery support.
Created attachment 130696 [details] [review] lib: Add more members to UpDeviceLevel struct Those levels will not be used to cover warning levels, but approximate battery levels that devices can use, exported by the kernel as POWER_SUPPLY_CAPACITY_LEVEL_* values. See linux/power_supply.h
Created attachment 130697 [details] [review] all: Add BatteryLevel property Export approximate battery levels that devices can use, exported by the kernel as POWER_SUPPLY_CAPACITY_LEVEL_* values. This avoids bizarrely accurate values showing up in UIs when we only have ok/warning levels of accuracy in some cases.
Created attachment 130698 [details] [review] daemon: Move a number of constants to a shared file
Comment on attachment 130694 [details] [review] daemon: Make warning levels for devices inclusive Review of attachment 130694 [details] [review]: ----------------------------------------------------------------- ::: src/up-daemon.c @@ +645,4 @@ > * into critical (or off) before any warnings */ > if (kind == UP_DEVICE_KIND_MOUSE || > kind == UP_DEVICE_KIND_KEYBOARD) { > + if (percentage <= 5.0f) This is from pre-existing code and but I am curious: percentage is a double, any reason the 'f' float suffix is used?
Created attachment 130731 [details] [review] all: Add BatteryLevel property Export approximate battery levels that devices can use, exported by the kernel as POWER_SUPPLY_CAPACITY_LEVEL_* values. This avoids bizarrely accurate values showing up in UIs when we only have ok/warning levels of accuracy in some cases.
Comment on attachment 130696 [details] [review] lib: Add more members to UpDeviceLevel struct Review of attachment 130696 [details] [review]: ----------------------------------------------------------------- ::: libupower-glib/up-types.c @@ +296,4 @@ > return UP_DEVICE_LEVEL_CRITICAL; > if (g_strcmp0 (level, "action") == 0) > return UP_DEVICE_LEVEL_ACTION; > + if (g_strcmp0 (level, "normal") == 0) Just for completeness: the first 'if'-case takes care of the case that level is NULL, so a non-null-safe comparison would suffice (like e.g. g_str_equal, strcmp).
Looks all good to me.
Attachment 130694 [details] pushed as 27a3eea - daemon: Make warning levels for devices inclusive Attachment 130695 [details] pushed as 6b14798 - linux: Add support for "capacity_level" attribute Attachment 130696 [details] pushed as 4f92309 - lib: Add more members to UpDeviceLevel struct Attachment 130698 [details] pushed as 660c8f3 - daemon: Move a number of constants to a shared file Attachment 130731 [details] pushed as 499d05b - all: Add BatteryLevel property
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.