Re: Make the hid-logitech-dj driver remove the HID++ nodes when the device disconnects

[Peter Hutterer <peter.hutterer@redhat.com>]

On 7/2/20 3:01 am, Benjamin Tissoires wrote:
> On Thu, Feb 6, 2020 at 4:42 PM Filipe Laíns <lains@archlinux.org> wrote:
>>
>> On Thu, 2020-02-06 at 13:13 +0100, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 2/6/20 12:51 PM, Filipe Laíns wrote:
>>>> On Thu, 2020-02-06 at 12:30 +0100, Hans de Goede wrote:
>>>>> HI,
>>>>>
>>>>> On 2/6/20 12:14 PM, Filipe Laíns wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Right now the hid-logitech-dj driver will export one node for each
>>>>>> connected device, even when the device is not connected. That causes
>>>>>> some trouble because in userspace we don't have have any way to know if
>>>>>> the device is connected or not, so when we try to communicate, if the
>>>>>> device is disconnected it will fail.
>>>>>
>>>>> I'm a bit reluctant to make significant changes to how the
>>>>> hid-logitech-dj driver works. We have seen a number of regressions
>>>>> when it was changed to handle the non unifying receivers and I would
>>>>> like to avoid more regressions.
>>>>>
>>>>> Some questions:
>>>>> 1. What is the specific use case where you are hitting this?
>>>>
>>>> For example, in libratbag we enumerate the devices and then probe them.
>>>> Currently if the device is not connected, the communication fails. To
>>>> get the device to show up we need to replug it, so it it triggers udev,
>>>> or restart the daemon.
>>>
>>> Thanks, that is exactly the sort of context to your suggested changes
>>> which I need.
>>>
>>>>> 2. Can't the userspace tools involved by modified to handle the errors
>>>>> they are getting gracefully?
>>>>
>>>> They can, but the approaches I see are not optimal:
>>>>     - Wait for HID events coming from the device, which could never
>>>> happen.
>>>>     - Poll the device until it wakes up.
>>>
>>> I guess we do get some (other or repeated?) event when the device does
>>> actually connect, otherwise your suggested changes would not be possible.
>>
>> No, I was thinking to just send the HID++ version identification
>> routine and see if the device replies.
> 
> Hmm, to continue on these questions:
> - yes, the current approach is to have the users of the HID++ device
> try to contact the device, get an error from the receiver, then keep
> the hidraw node open until we get something out of it, and then we can
> start talking to it
> - to your question Hans, when a device connects, it emits a HID++
> notification, which we should be relaying in the hidraw node. If not,
> well then starting to receive a key or abs event on the input node is
> a pretty good hint that the device connected.
> 
> So at any time, the kernel knows which devices are connected among
> those that are paired, so the kernel knows a lot more than user space.
> 
> The main problem Filipe is facing here is that we specifically
> designed libratbag to *not* keep the device nodes opened, and to not
> poll on the input events. The reason being... we do not want libratbag
> to be considered as a keylogger.

I'm wondering - can we really get around this long-term? Even if we have 
a separate HID++ node and/or udev change events and/or some other 
notification, in the end you still have some time T between that event 
and userspace opening the actual event node. Where the first key event 
wakes up the physical keyboard, you're now racing.

So the separate HID++ node works as long as libratbag *only* listens to 
that node, as soon as we need to start caring about a normal event it 
won't work any longer.

Cheers,
   Peter