On Tue, Jan 14, 2020 at 10:55 AM Filipe Laíns <lains@xxxxxxxxxxxxx> wrote: > > On Tue, 2020-01-14 at 20:48 +1000, Benjamin Tissoires wrote: > > On Tue, Jan 14, 2020 at 1:31 AM Filipe Laíns <lains@xxxxxxxxxxxxx> > > wrote: > > > On Sun, 2020-01-12 at 20:50 +0000, Filipe Laíns wrote: > > > > I also marked all lightspeed devices as HID++ compatible. As the > > > > internal powerplay device does not have REPORT_TYPE_KEYBOARD or > > > > REPORT_TYPE_KEYBOARD it was not being marked as HID++ compatible > > > > in > > > > logi_hidpp_dev_conn_notif_equad. > > > > > > Actually I had another look at the code and I don't understand why > > > we > > > are manually setting |= HIDPP in > > > logi_hidpp_dev_conn_notif_equad/logi_hidpp_dev_conn_notif_27mhz. We > > > should set it in logi_dj_hidpp_event as it is triggered by > > > receiving a > > > HID++ packet. > > > > long story short: nope :) > > > > The whole purpose of setting the workitem->reports_supported is to be > > able to create the matching report descriptor in the new virtual > > device. So having this set in a callback will add an operation for > > nothing every time we get an event, and will also not ensure a proper > > separation of concerns. > > > > Cheers, > > Benjamin > > > > > What do you think Benjamin? > > > > > > -- > > > Filipe Laíns > > Okay, then is maybe better if I add HIDPP to reports_supported based on > the device ID (7). This is the only product to my knowledge that > exports a device with ID 7. It's a better solution than setting HIDPP > on all lightspeed devices. Yep, looks better. > > I will send a new patch if you agree with this approach. thanks. Cheers, Benjamin > > -- > Filipe Laíns
