Re: Litra Glow on Linux

[Andreas Bergmeier <abergmeier@gmx.net>]

On Thu, 10 Nov 2022, Benjamin Tissoires wrote:

> On Thu, Nov 10, 2022 at 4:29 AM Andreas Bergmeier <abergmeier@gmx.net> wrote:
> >
> > On Wed, 9 Nov 2022 at 21:27, Andreas Bergmeier <abergmeier@gmx.net> wrote:
> > >
> > > Finally I have an environment where I can test my kernel code.
> > >
> > > On Mon, 31 Oct 2022 at 10:29, Benjamin Tissoires
> > > <benjamin.tissoires@redhat.com> wrote:
> > > > For identifying the GLOW device you should be adding an id in the
> > > > table of hid-logitech-hidpp, with a driver data that tells the driver
> > > > to look for 0x1990.
> > > >
> > > > >
> > > > > > - you need to add a hook in connect_event to register the led class
> > > > > > device that will hook on to the actual LED of the device
> > > Sadly my tests did not go very far. The code fails already when
> > > calling the `probe` callback (`hidpp_probe`).
> > > When it calls into `hidpp_root_get_protocol_version` it seems to
> > > receive `HIDPP_ERROR_RESOURCE_ERROR`.
> > > Which then leads to an error message: Device not connected
> > > Upon looking at `HIDPP_ERROR_RESOURCE_ERROR` (9) there is no
> > > documentation what it means in code.
> > > From a look into the docs it says that 9 is UNSUPPORTED error for 2.0
> > > devices. Thus I am wondering how the code knows
> > > that it is a problem with connectivity.
>
> From the top of my memory, this was told to us that this is the way we
> detect if the device was connected or not in the unifying case. Though
> in your case, it's a USB device, so there is no such thing as "not
> connected"...
So isn't the current error handling at a minimum misleading?


> > > Couldn't it also mean that the
> > > device is not supporting getting the protocol version?
>
> Probably. What happens if you comment out that protocol version
> request and force connected to be true?
Well I went the other way around. I had a look at the hidpp utility
sources:
https://github.com/cvuchener/hidpp/blob/057407fbb7248bbc6cefcfaa860758d0711c01b9/src/libhidpp/hidpp/Device.cpp#L82
Which seems to do a similar thing. From the top of my head the only
difference seems to be that they are sending `0x1` as a ping value instead
of `0x5a`. Might give that a shot next.
Anyway hidpp-list-features successfully reads the protocol version in
userspace (4, 2) as seen here:
https://github.com/abergmeier/litra_glow_linux/blob/main/hidpp-list-features

> In your case though, it would be interesting to know if we should
> bypass that verification.
Since reading the protocol version seems generally possible I think we
should understand what logitech-hidpp does wrong first.


> > Also, looking into `supported_reports` turned out to be 2 (very long).
>
> Oops, you mistook the bit definition with the value:
> #define HIDPP_REPORT_SHORT_SUPPORTED  BIT(0)  -> value of 1
> #define HIDPP_REPORT_LONG_SUPPORTED  BIT(1)  -> value of 2
> #define HIDPP_REPORT_VERY_LONG_SUPPORTED  BIT(2)  -> value of 4
Ah indeed, thx.

> And this is expected because you don't have VERY_LONG support on your device.
True. The question remains whether the upgrade from LONG to VERY_LONG
could be needed should a device only support VERY_LONG.