Re: [PATCH 0/2] HID: logitech-hidpp: fix Bolt hi-res wheel handling

["Erik Håkansson" <erikhakan@gmail.com>]

Hi again!

I have some semi-good news regarding both the quirks, the conflict with user-
space setting in Solaar, and regarding being able to separate reports by 
device, so e.g. different capability mice can be used simultaneously.

Long text with background and description first, so scroll down for the actual 
solution. :) 

In an effort to figure out if we could have separation of events by device, I 
tried digging into whether I could force the Bolt receiver into DJ mode, 
similar to what is done for Unifying devices, and I investigated how it's done 
for my Lightspeed device, given that it's treated differently from regular 
Unifying devices in code.
Some looking at the HID++ protocols that are available online, as well as some 
probing of the USB interfaces revealed that the Lightspeed receiver does 
indeed support DJ, but unfortunately the Bolt receiver does not.
At least it does not support the same command as Unifying and Lightspeed 
receivers and no digging online or by probing hardware revealed anything.

However, when digging around I realized that the Lightspeed receiver is NOT 
put into DJ mode, but still all three interfaces are claimed by the DJ driver, 
yet the mouse works as it should. With tshark capture of wire data I saw that 
for Lightspeed, mouse events come as regular native HID, not as HID++ 
reports., i.e. the DJ mode is not used. And frankly, since the Lightspeed 
supports at most 1 mouse and 1 keyboard, separating devices are not necessary, 
except for the HID++ report on device status, such as battery and connection 
status, and those are sent on HID++ regardless of mode.

But previously, the theory has been that generic HID reports can't go through 
the DJ driver, but obviously it works for Lightspeed but not Bolt mice. 
Although keyboard does work. So I investigated some more and it turns out that 
when the DJ driver creates a virtual HID device it sends a descriptor on to 
the HID core, and this descriptor describes a bunch of stuff, including the 
size of the mouse movement reports. This descriptor is different for different 
receivers, and the default one has a movement report size of 12 bits. However, 
when inspecting the real Bolt descriptor (the same one read by the native HID 
when we do NOT let DJ claim the mouse and keyboard interfaces), I saw that the 
real report is 16 bits. Same as most relatively modern receivers.

So, I now *strongly* suspect that the reports of overly sensitive or phantom 
moves, and possibly scrolls, are the result of the HID layer trying to 
interpret a 16-bit report as 12 bits.
Without a Bolt mouse I can't verify of course, so I'll need some help with 
that.

During my digging, I *also* realized that HID++ reports already carry a device 
index, thus making every report that comes through HID++ separable by device, 
as well as the fact that for the existing 0x2121 (High resolution scroll over 
HID++) functionality, there was a bug, and scrolling seems to not have worked 
for any mouse over HID++, regardless of receiver type.

So, to the solution then:

I'm currently working on a new version of the patch which will align the Bolt 
receiver much closer to how other receivers work, most close to the Lightspeed 
receiver, by letting the DJ driver claim all three interfaces again, but fixing 
the device descriptor, which should let the mouse movement work as intended.
This also means that scrolling, whether high or low resolution, should work as 
intended even when use_hidpp=0.
I'm also improving the 0x2121 handling so that the existing bug that would 
break scrolling for any existing mouse using `use_hidpp=1`, as well as 
retaining the improvement to respect user settings in Solaar. Also, the 0x2121 
protocol notes that the scroll invert setting only affects events sent through 
native HID, not diverted, so I will also fix that so inversion will work 
regardless of diversion.
I.e. now scrolling should work as intended regardless of `use_hidpp` and 
`high_resolution`, and there will be no more conflict between kernel and user-
space settings in Solaar or elsewhere!

However, as noted, only HID++ reports carry a device index. Thus, regular 
mouse and keyboard reports that will still go through interface 0 and 1 will 
be indistinguishable between devices, but for special keyboard keys (volume, 
brightness, screenshot, etc.) as well as scroll events when `use_hidpp=1`. 
I.e. for the reports that actually matter, distinction will be possible!

tl;dr:
I'm writing a new patch that I'm pretty confident will work without quirky 
workarounds, it aligns better with existing receivers and thus less complex 
code, while also improving 0x2121 scrolling for all mice and getting rid of 
the conflict with Solaar.

Patch should be coming within the next few days.

Oleksandr Natalenko wrote:
> 
> You are right, the "Scroll Wheel Diversion" diversion switch was off for me.
> I've enabled it along with "Scroll Wheel Resolution", and the scrolling
> stays normal and survives both resume and device switching.

Great, then I'm even more confident in my new patch. With it, it should work 
with Scroll Wheel Diversion on or off!

> 
> Somewhat unrelated note: if I enable "Thumb Wheel Diversion", the thumb
> wheel stops working altogether. It's probably not supported by the driver
> at all in HID++ mode?
> 

Correct, the driver does not implement 0x2150 (Thumbwheel) in HID++ at all. 
However, since we're interested in diverting through HID++ for device 
separation purposes, I'm looking into implementing 0x2150 too to enable that 
diversion for that too.
And while reading the protocol there, I noticed that it also supports touch 
events on the thumbwheel. There's three different statuses, single_tap, proxy, 
and touch. Touch is continuous touch, single_tap is tap, and proxy is whether 
a finger is in close proximity but not quite touching. 
Does your mouse by any chance support that? If so, can you tell if it sends a 
button event on tap or touch when in non-diverted mode? Or possibly if it does 
so in Windows. I don't know what I should map those events to really :) 

Benjamin Tissoires wrote:
> Bolt in kernel was adding in 7.3, so I'm going to revert it now, and
> we'll have to have a fully working plan for an eventual inclusion in a
> future kernel version.