Re: [PATCH v5] hid-logitech-hidpp: read battery voltage from newer devices

[Filipe Laíns]

On Sat, 2019-08-31 at 13:56 -0400, Pedro Vanzella wrote:
> +static int hidpp20_battery_map_status_voltage(u8 data[3], int
> *voltage)
> +{
> +	int status;
> +
> +	switch (data[2]) {
> +	case 0x00: /* discharging */
> +		status = POWER_SUPPLY_STATUS_DISCHARGING;
> +		break;
> +	case 0x10: /* wireless charging */
> +	case 0x80: /* charging */
> +		status = POWER_SUPPLY_STATUS_CHARGING;
> +		break;
> +	case 0x81: /* fully charged */
> +		status = POWER_SUPPLY_STATUS_FULL;
> +		break;
> +	default:
> +		status = POWER_SUPPLY_STATUS_NOT_CHARGING;
> +	}
> +
> +	*voltage = get_unaligned_be16(data);
> +
> +	return status;
> +}

Here's the missing specification:

+--------+-------------------------------------------------------------------------+
|  byte  |                                    2                                    |
+--------+--------------+------------+------------+----------+----------+----------+
|   bit  |     0..2     |      3     |      4     |     5    |     6    |     7    |
+--------+--------------+------------+------------+----------+----------+----------+
| buffer | chargeStatus | fastCharge | slowCharge | critical | (unused) | extPower |
+--------+--------------+------------+------------+----------+----------+----------+
Table 1 - battery voltage (0x1001), getBatteryInfo() (ASE 0), 3rd byte

+-------+--------------------------------------+
| value |                meaning               |
+-------+--------------------------------------+
|   0   | Charging                             |
+-------+--------------------------------------+
|   1   | End of charge (100% charged)         |
+-------+--------------------------------------+
|   2   | Charge stopped (any "normal" reason) |
+-------+--------------------------------------+
|   7   | Hardware error                       |
+-------+--------------------------------------+
Table 2 - chargeStatus value

I know this is already on the 5th revision but could you please change
hidpp20_battery_map_status_voltage() to properly handle the 3rd byte?
Also, if you could make sure those values are properly exported via
sysfs it would be great! We will need them for proper upower support.

Sorry for not saying this in my first review. Since then I have done
some testing and I discovered that if we want to get accurate upower
support we will need the values exposed by the 3rd byte to be
exported properly.

I am not sure about the endianness of chargeStatus but you can find
that easily. 

Thank you!
Filipe LaÃns

Attachment: signature.asc
Description: This is a digitally signed message part