Re: [RFC 1/6] Input: Rename extra mouse buttons defines to match their actual usage

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 03-04-19 16:12, Benjamin Tissoires wrote:
On Mon, Apr 1, 2019 at 1:11 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:

Hi,

On 01-04-19 12:34, Peter Hutterer wrote:
On Mon, Apr 01, 2019 at 09:16:22AM +0200, Hans de Goede wrote:
Hi,

On 01-04-19 05:05, Peter Hutterer wrote:
On Sat, Mar 30, 2019 at 02:37:30PM +0100, Hans de Goede wrote:
Almost all modern mice use HID. For buttons 1 - 3 the order of left /
right / middle specified in the HUT matches with the BTN_LEFT, BTN_RIGHT
and BTN_MIDDLE order of the evdev code defines since the HUT does not
specify any meaning for button usages >= 4 the HID input driver simply
maps buttons to BTN_LEFT + (number - 1).

In practice on most mice which have more then 3 buttons, usage 4 is
used for the back(ward) button and usage 5 for the forward button.

This means that these buttons generate an input_event code of 0x113 resp.
0x114, which looking at the old defines maps to BTN_SIDE and BTN_EXTRA.

Under X these are mapped to buttons 8 resp. 9; and under wayland the
input_event code is passed through unmodified. Apps, e.g. Firefox both
when running as Wayland client under GNOME3 and when running under Xorg,
correctly interpret these as back and forward.

So we've back and forward buttons generating BTN_SIDE and BTN_EXTRA,
which the apps then interpret as BTN_BACK and BTN_FORWARD, rather then as
BTN_SIDE and BTN_EXTRA (which have no clear meaning).

I find this all very confusing, this commit tries to remove the confusion
by deprecating the old defines and adding new defines which assign labels
to the 0x113 - 0x116 input_event codes which match how they are actually
used today.

Note there are no functional changes here, after this userspace will
see the exact same input_event codes as before, this purely about
assigning a human-readable label to these codes which matches with how
they are actually being used.
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
    Documentation/input/event-codes.rst    |  2 +-
    drivers/hid/hid-debug.c                |  6 +++---
    drivers/input/mousedev.c               |  8 ++++----
    include/uapi/linux/input-event-codes.h | 13 ++++++++++++-
    4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
index b24b5343f5eb..db52b96e7a83 100644
--- a/Documentation/input/event-codes.rst
+++ b/Documentation/input/event-codes.rst
@@ -422,7 +422,7 @@ should be used to report when the tool is in contact with the tablet.
    BTN_{STYLUS,STYLUS2} should be used to report buttons on the tool itself. Any
    button may be used for buttons on the tablet except BTN_{MOUSE,LEFT}.
    BTN_{0,1,2,etc} are good generic codes for unlabeled buttons. Do not use
-meaningful buttons, like BTN_FORWARD, unless the button is labeled for that
+meaningful buttons, like BTN_FORWRD, unless the button is labeled for that

I haven't thought of a better name yet, but using BTN_FORWRD is bound to
introduce bugs, especially when BTN_FORWARD still resolves. I don't think
that name is a good idea but I don't have a better suggestion yet.

Good point about the typos my plan for the kernel is to add #ifndef __KERNEL__
around the compat defines once all users are moved over.

good, but not enough. the kernel is easiest here because you could literally
just add a script to a test suite to make sure it doesn't use BTN_FORWARD.
It's all the userspace that'll introduce typos.

Thought of the day: if HID doesn't have any meaning, is it really worth
changing these names based on what you hope a client interprets it as?
would a more generic naming scheme that admits this is just button 4 be
better? BTN_HID_4 or something?

The HUT spec may not say anything about this (note I did not check the
gazillion errata I really wish they would just fold those into an
updated version), but Microsoft writes the following about this and it
is reasonable to expect mice vendors to follow this (and in practice they do):
https://docs.microsoft.com/en-us/windows-hardware/drivers/hid/keyboard-and-mouse-hid-client-drivers

"Buttons 4 & 5 on are mapped to WM_APPCOMMAND messages and correspond to App_Back and App_Forward."

BTN_APP_FORWARD and BTN_APP_BACK then? seems obvious enough and is a lot
easier to visually identify than a missing A in FORWRD.

Yes I was thinking the same about 5 minutes after sending the mail you
replied to, so I think we agree on this, will fix for the the first non
RFC version.

For the record, I also agree with BTN_APP_FORWARD and BTN_APP_BACK.

We made some archeology this morning with Peter.
So now is story time, gather around children:
BTN_SIDE and BTN_EXTRA were added in 2.3.36 with patch-2.3.36.bz2 that
can be found at
https://mirrors.edge.kernel.org/pub/linux/kernel/v2.3/.

It's actually the big switch from Vojtech from a custom USB mouse boot
protocol to an actual HID bus. It's also the creation of the evdev
protocol.

And those BTN_* were added to suport mice with extra buttons. I guess
it was pretty uncommon for such mice in Jan 2000 and the buttons were
not commonly used like today as back and forward.

Note that it's actually easy to go back to 2.4.0 by using:
https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/log/?ofs=63400

So, my fears that the buttons would have a meaning is relieved, and I
would Ack such a series (I haven't fully read it so it's an ACK on the
principle).

Ok, so before I send a v1 of this patch-set, Dmitry do you have any remarks?

Regards,

Hans




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux