Re: hidmt: clickpad detection
On Tue, Sep 13, 2022 at 11:06:33PM +0200, Ulf Brosziewski wrote: Hello Ulf, > The diff below improves the way hidmt identifies clickpads, and > addresses the problems described in > https://marc.info/?l=openbsd-tech&m=165296530618617&w=2 > and > https://marc.info/?l=openbsd-tech&m=165980534415586&w=2 > > If devices don't report a HUD_BUTTON_TYPE property, the driver checks > whether they claim to have an external left button, and if they do, > hidmt doesn't treat them as clickpads. > > I think this part of the logic should be turned around: hidmt should > treat everything as clickpad except if there is no "clickpad button" > (HUP_BUTTON 1) *and* both an external left and an external right button > (HUP BUTTON 2 and 3) are being reported. Touchpads without the internal > button are still usable with the clickpad configuration, it does less > harm to err on this side. > > Tests and OKs would be welcome. Thanks for doing this! This fixes the Framework touchpad and also seems like a better heuristic than we currently have (certainly it seems better to me than the Framework-specific quirk I added!). I think it would be great to try this in snapshots for a little while to check whether it causes any issues, though I'm optimistic that it will Just Work! Laurie
Re: hidmt: clickpad detection
On Tue, Sep 13, 2022 at 11:06:33PM +0200, Ulf Brosziewski wrote: > The diff below improves the way hidmt identifies clickpads, and > addresses the problems described in > https://marc.info/?l=openbsd-tech&m=165296530618617&w=2 > and > https://marc.info/?l=openbsd-tech&m=165980534415586&w=2 > > If devices don't report a HUD_BUTTON_TYPE property, the driver checks > whether they claim to have an external left button, and if they do, > hidmt doesn't treat them as clickpads. > > I think this part of the logic should be turned around: hidmt should > treat everything as clickpad except if there is no "clickpad button" > (HUP_BUTTON 1) *and* both an external left and an external right button > (HUP BUTTON 2 and 3) are being reported. Touchpads without the internal > button are still usable with the clickpad configuration, it does less > harm to err on this side. > > Tests and OKs would be welcome. Not qualified to give an OK, but I've been running this for the past few days on a second gen Framework and double/triple click works as expected. Lucas
hidmt: clickpad detection
The diff below improves the way hidmt identifies clickpads, and addresses the problems described in https://marc.info/?l=openbsd-tech&m=165296530618617&w=2 and https://marc.info/?l=openbsd-tech&m=165980534415586&w=2 If devices don't report a HUD_BUTTON_TYPE property, the driver checks whether they claim to have an external left button, and if they do, hidmt doesn't treat them as clickpads. I think this part of the logic should be turned around: hidmt should treat everything as clickpad except if there is no "clickpad button" (HUP_BUTTON 1) *and* both an external left and an external right button (HUP BUTTON 2 and 3) are being reported. Touchpads without the internal button are still usable with the clickpad configuration, it does less harm to err on this side. Tests and OKs would be welcome. Index: dev/hid/hidmt.c === RCS file: /cvs/src/sys/dev/hid/hidmt.c,v retrieving revision 1.12 diff -u -p -r1.12 hidmt.c --- dev/hid/hidmt.c 9 Jul 2020 21:01:08 - 1.12 +++ dev/hid/hidmt.c 13 Sep 2022 19:31:38 - @@ -154,11 +154,13 @@ hidmt_setup(struct device *self, struct mt->sc_rep_cap, hid_feature, &cap, NULL)) { d = hid_get_udata(rep, capsize, &cap); mt->sc_clickpad = (d == 0); - } else { - /* if there's not a 2nd button, this is probably a clickpad */ - if (!hid_locate(desc, dlen, HID_USAGE2(HUP_BUTTON, 2), - mt->sc_rep_input, hid_input, &cap, NULL)) - mt->sc_clickpad = 1; + } else if (hid_locate(desc, dlen, HID_USAGE2(HUP_BUTTON, 1), + mt->sc_rep_input, hid_input, &cap, NULL) + || !hid_locate(desc, dlen, HID_USAGE2(HUP_BUTTON, 2), + mt->sc_rep_input, hid_input, &cap, NULL) + || !hid_locate(desc, dlen, HID_USAGE2(HUP_BUTTON, 3), + mt->sc_rep_input, hid_input, &cap, NULL)) { + mt->sc_clickpad = 1; } /*