Hi,
On 3/22/19 11:47 AM, Bastien Nocera wrote:
On Fri, 2019-03-22 at 11:24 +0100, Benjamin Tissoires wrote:
On Fri, Mar 22, 2019 at 11:14 AM Bastien Nocera <hadess@xxxxxxxxxx>
wrote:
On Fri, 2019-03-22 at 00:09 +0100, Hans de Goede wrote:
According to hidpp20_batterylevel_get_battery_info my Logitech
K270
keyboard reports only 2 battery levels. This matches with what
I've
seen
after testing with batteries at varying level of fullness, it
always
reports either 5% or 30%.
Windows reports "battery good" for the 30% level. I've captured
an
USB
trace of Windows reading the battery and it is getting the same
info
as the Linux hidpp code gets.
Now that Linux handles these devices as hidpp devices, it reports
the
battery as being low as it treats anything under 31% as low, this
leads
to the user constantly getting a "Keyboard battery is low"
warning
from
GNOME3, which is very annoying.
This commit fixes this by changing the low threshold to anything
under
30%, which I assume is what Windows does.
upower doesn't use the battery percentage if there's a "Capacity"
set.
If something else in the stack does, then there's a bug in that
component and the problem needs to be fixed there.
I think that is precisely what Hans is fixing here: on his device,
the
discrete battery percentage reported by the mouse for the normal
capacity level is 30.
With the current code, this is interpreted as Low and upower
interprets it as low as well.
Please remove the mentions of GNOME from the commit message. The commit
message makes it sound like the percentage is being used, and this
patch is a work-around for a GNOME problem.
The commit message does not suggest that this is a GNOME problem at all:
"Now that Linux handles these devices as hidpp devices, it reports the
battery as being low as it treats anything under 31% as low,"
Where it clearly points to "Linux" from the part of the sentence
before the comma. After that it continues:
"this leads to the user constantly getting a "Keyboard battery is low"
warning from GNOME3"
Where "this" clearly refers to *Linux* reporting the battery as being low.
I don't really feel like respinning this patch set again just to
remove an innocent reference to GNOME3, that just feels like busy
work to me and I'm busy enough as is.
Regards,
Hans
Cheers
I think the patch is safe to apply as it is. I'll let you the chance
to ack/nack it though.
Cheers,
Benjamin
See:
https://gitlab.freedesktop.org/upower/upower/blob/master/dbus/org.freedesktop.UPower.Device.xml#L514
https://gitlab.freedesktop.org/upower/upower/blob/master/dbus/org.freedesktop.UPower.Device.xml#L621
