Re: [PATCH 3/3] HID: logitech-hidpp: change low battery level threshold from 31 to 30 percent

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 22-03-19 00:27, Filipe Laíns 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.

Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
  drivers/hid/hid-logitech-hidpp.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
logitech-hidpp.c
index 421a190583eb..d811c3bab495 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1054,7 +1054,7 @@ static int hidpp_map_battery_level(int
capacity)
  {
  	if (capacity < 11)
  		return POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
-	else if (capacity < 31)
+	else if (capacity < 30)
  		return POWER_SUPPLY_CAPACITY_LEVEL_LOW;
  	else if (capacity < 81)
  		return POWER_SUPPLY_CAPACITY_LEVEL_NORMAL;

Shouldn't the other values be changed as well to keep somewhat of a
standard? People reading this code without prior knowledge of the file
history will just scratch their head when seeing this.

Well, the values come from the HIDPP 2.0 specification, but in this
case some devices seem to not stick to the spec (and Windows accepts
the "30" value from these devices as indicating "Good" batteries)

But you are right that this deviation from the spec warrants some
explanation for future readers of the code, so I've added the
following comment for v2:

        /*
         * The spec says this should be < 31 but some devices report 30
         * with brand new batteries and Windows reports 30 as "Good".
         */
        else if (capacity < 30)
                return POWER_SUPPLY_CAPACITY_LEVEL_LOW;

Regards,

Hans



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux