Bug 32200

can you describe in more detail what's the actual problem here? I can get it from the InputClass comment but a bit more detail would be nice. Thanks.

More detail follows. Button numbers are reported here unmapped.

First click: button 13 down.
Second click: button 13 up, button 14 down.
Third click: button 14 up, button 15 down.
Fourth click: button 13 down, button 15 up.
Fifth click: as second.

It seems that without this patch and associated configuration, X is handling mouse movement after the first click as dragging.

For mice such as these, some suitable auto-release (or instant-release, if you like) configuration should probably be selected by default, but we're into quirks database territory there...

I take it the evtest output shows the same?

sounds like intentionally broken hardware to me...

It does.

Event: time 1291830645.966730, type 4 (Misc), code 4 (ScanCode), value 90009
Event: time 1291830645.966735, type 1 (Key), code 280 (?), value 1
Event: time 1291830645.966739, type 4 (Misc), code 4 (ScanCode), value 9000b
Event: time 1291830645.966742, type 1 (Key), code 282 (?), value 0
Event: time 1291830645.966749, -------------- Report Sync ------------
Event: time 1291830651.273195, type 4 (Misc), code 4 (ScanCode), value 90009
Event: time 1291830651.273201, type 1 (Key), code 280 (?), value 0
Event: time 1291830651.273204, type 4 (Misc), code 4 (ScanCode), value 9000a
Event: time 1291830651.273207, type 1 (Key), code 281 (?), value 1
Event: time 1291830651.273215, -------------- Report Sync ------------
Event: time 1291830653.473973, type 4 (Misc), code 4 (ScanCode), value 9000a
Event: time 1291830653.473979, type 1 (Key), code 281 (?), value 0
Event: time 1291830653.473982, type 4 (Misc), code 4 (ScanCode), value 9000b
Event: time 1291830653.473985, type 1 (Key), code 282 (?), value 1
Event: time 1291830653.473994, -------------- Report Sync ------------

Review of attachment 40885 [details] [review]:

not a big fan of this option because I expect a user-base of less than 10 over the lifetime of the driver. Either way, some comments on the patch.

- man page hunk missing
- can we share the option parsing code between draglock and autorelease somehow. it's not 100% identical, but close
- EvdevAutoReleaseButton can be replaced with calls to EvdevQueueButtonClicks
- CARD8 should be enough for the property
- please don't use CARD32 in the header file, just use uint32_t or similar
- typo in "fires button event messges */
- property should be list of button numbers instead of a bitmask. can still be a bitmask internally.
- style: no {} for single-line blocks please
- please add a comment at the top of the source file explaining what this feature does so that the next person can quickly get a grasp on the intent without having to browse git history.
- use TestBit for bit testing instead of manual & (<<)

Created attachment 44592 [details] [review]
AutoReleaseButtons configuration option (v2)

Updated version of the patch.

This adds a paragraph to the man page, gets rid of EvdevAutoReleaseButton (and the typo), uses uint32_t instead of CARD32, fixes some style issues (was copying style used elsewhere in the source...), and adds a descriptive comment.

Sharing of option parsing with the drag-lock code is not done; not sure that that's appropriate, really.

The property is still a bitmask and therefore can't sensibly be CARD8. TestBit seems to be for arrays...

(In reply to comment #6)
> The property is still a bitmask and therefore can't sensibly be CARD8. TestBit
> seems to be for arrays...

property as a list of CARD8 instead of a bitmask. if we ever change MAX_BUTTONS to higher than 32, the 32-bit bitmask would break.

btw, have you considered a kernel hack for this instead? it seems that other userspace processes would suffer as well from this broken hardware so we might as well fix it in one spot

Done, for the R.A.T.7. https://patchwork.kernel.org/patch/653481/

If that is accepted then the evdev patch still remains useful for working around this kind of thing in other mice until the kernel is patched for them. (I don't yet have the USB IDs for the other R.A.T. mice.)

Darren's evdev patch has probably more chances of being included somewhere than the kernel patch. Updated it.

Created attachment 91378 [details] [review]
xf86-input-evdev AutoReleaseButtons configuration option

Sorry, I won't merge this patch in evdev. The device in question breaks the evdev protocol by pressing a button but not releasing it when the button was released. It needs some quirk in the kernel rather than a generic option that needs to be manually configured, etc. Benjamin, can you have a look at the kernel patch please?

Regarding the kernel patch, I just noticed that somebody resubmitted it recently with the addition of others USB VID/PID. I will reply on the LKML, but also here.

Here are my comments:
- the quirk should be addressed in hid-saitek, not in hid-input (it is a very specific quirk)
- I don't think the set_bit call is needed. It should be done by input_event
- the previous thing may have been needed because you may need to send an input_sync event in between the press and release
- Why having 3 different buttons for the same button (I understand it is a mode, but as it is buggy currently)? I would say we should map only one input button to the physical button and release it once it is pressed. This way, you can map whatever to the physical button, and not depend on the current mode.

*** Bug 73443 has been marked as a duplicate of this bug. ***