[BUG?] hid-core.c: hid->name = dev->manufacturer + dev->product ... why not + dev->serial?

[BUG?] hid-core.c: hid->name = dev->manufacturer + dev->product ... why not + dev->serial?

[Melchior FRANZ]

Hi,

just recently I purchased two data acquisition HID devices that pretend to be
joysticks[1], because that way no special kernel drivers are required, and
because they are usually used for joystick-like purposes. The devices identify
themselves as

  idVendor           0x16c0
  idProduct          0x05ba 
  iManufacturer      1 Leo Bodnar
  iProduct           2 BU0836A Interface
  iSerial            3 A10123

The Linux kernel creates the HID/"joystick name" from only iManufacturer and
iProduct, ignoring iSerial. That way, JSIOCGNAME returns the same string
for all such devices, making an actual identification impossible.
joystick-api.txt says about JSIOCGNAME:

  #define JSIOCGNAME(len) /* get identifier string        char    */
  ...
  JSIOCGNAME(len) allows you to get the name string of the joystick

But this isn't actually true. "Leo Bodnar BU0836A Interface" is only
the name of the product, but not of the actual (joystick or whatever)
device. If you have more of these devices on the bus, then software has
no way to know which is which -- which is the one with the temperature
sensor on axis 0 etc. (Apart from doing all the USB querying itself,
of course.)

The patch below fixed the problem for me. But, certainly, the current
(mis)behaviour wasn't an accident, but intentional. But what was the
intention? What happens if I attach two actual identical joysticks?
What's the purpose of them having the same name on the system under
/dev/input/{js,event}? Not even two (human) identical twins have the
same first name, and for a reason!

I'm aware that at the moment, some software could rely on the fact
that a joystick name is just a generic product name, but that would
be easy to fix. I'm only aware of one such application, but exactly
in this case the serial number in the joystick name would actually
be an improvement!

m.




[1] http://www.leobodnar.com/products/BU0836A/





--- a/drivers/hid/usbhid/hid-core.c     2010-03-04 02:49:38.676850618 +0100
+++ b/drivers/hid/usbhid/hid-core.c     2010-03-04 03:03:43.269006052 +0100
@@ -1131,6 +1131,12 @@
                strlcat(hid->name, dev->product, sizeof(hid->name));
        }
 
+       if (dev->serial) {
+               if (hid->name[0])
+                       strlcat(hid->name, " ", sizeof(hid->name));
+               strlcat(hid->name, dev->serial, sizeof(hid->name));
+       }
+
        if (!strlen(hid->name))
                snprintf(hid->name, sizeof(hid->name), "HID %04x:%04x",
                         le16_to_cpu(dev->descriptor.idVendor),

Re: [BUG?] hid-core.c: hid->name = dev->manufacturer + dev->product ... why not + dev->serial?

[Dmitry Torokhov]

Hi Melchior,

I'm adding a few CCs and takinng discussion a bit away from your patch
;)

On Sun, Mar 07, 2010 at 04:55:06PM +0100, Melchior FRANZ wrote:
> Hi,
> 
> just recently I purchased two data acquisition HID devices that pretend to be
> joysticks[1], because that way no special kernel drivers are required, and
> because they are usually used for joystick-like purposes. The devices identify
> themselves as
> 
>   idVendor           0x16c0
>   idProduct          0x05ba 
>   iManufacturer      1 Leo Bodnar
>   iProduct           2 BU0836A Interface
>   iSerial            3 A10123
> 
> The Linux kernel creates the HID/"joystick name" from only iManufacturer and
> iProduct, ignoring iSerial. That way, JSIOCGNAME returns the same string
> for all such devices, making an actual identification impossible.
> joystick-api.txt says about JSIOCGNAME:
> 
>   #define JSIOCGNAME(len) /* get identifier string        char    */
>   ...
>   JSIOCGNAME(len) allows you to get the name string of the joystick
> 
> But this isn't actually true. "Leo Bodnar BU0836A Interface" is only
> the name of the product, but not of the actual (joystick or whatever)
> device. If you have more of these devices on the bus, then software has
> no way to know which is which -- which is the one with the temperature
> sensor on axis 0 etc. (Apart from doing all the USB querying itself,
> of course.)
> 

This is nicely illustrates one of the reasons why input subsystem is
ill-suited for generic data acquisition. I call it "device anonymity" -
input subsystem only interested in the type of event (ABS_X, ABS_Y) but
not necessarily the device it came from. You may have 2 joysticks and
prefer to use one in one game and other in another game. With default
setup you could grab either one and start playing, ABS_X from either
mapping on horizontal movements, ABS_Y on vertical... Same with
keyboards - you plug an external keyboard into laptop and you just start
typing (sans multi-seat setups that we don't support well at the
moment). In other words for input we care very much about capabilities
of a device but care much less about the device itself.

Which data acquisition you care not only about hypothetical
ABS_TEMPERATIRE event but it is also _very_ important the exact device
or probe the data is coming from. Temperature from device A can't be
used in place of temperature form device B.

> The patch below fixed the problem for me. But, certainly, the current
> (mis)behaviour wasn't an accident, but intentional. But what was the
> intention? What happens if I attach two actual identical joysticks?
> What's the purpose of them having the same name on the system under
> /dev/input/{js,event}? 

They will have different event nodes.

> Not even two (human) identical twins have the
> same first name, and for a reason!
> 

But you as an observer have no idea what their names are, and they look the
same to you. And in case of joysticks ther serials are not easily
visible on them when you quickly grab one.

Nevertheless serials are exported in sysfs ('uniq' attribute). Also udev
can give persistent names device nodes based on physocal connection,
serial numbers, etc.

-- 
Dmitry

Re: [BUG?] hid-core.c: hid->name = dev->manufacturer + dev->product ... why not + dev->serial?

[Melchior FRANZ]

Hey,

* Dmitry Torokhov -- Tuesday 09 March 2010:
> I'm adding a few CCs and takinng discussion a bit away from your patch
> ;)

Hmm ... I wonder if they'll all be happy about that. But I can't cut them
off now, either. Sorry.  :-}



> On Sun, Mar 07, 2010 at 04:55:06PM +0100, Melchior FRANZ wrote:
> > The Linux kernel creates the HID/"joystick name" from only iManufacturer and
> > iProduct, ignoring iSerial. That way, JSIOCGNAME returns the same string
> > for all such devices, making an actual identification impossible.

> This is nicely illustrates one of the reasons why input subsystem is
> ill-suited for generic data acquisition.

No. It demonstrates that Linux shouldn't drop essential information on
the floor for no good reason. So, why is iSerial ignored? After all,
my simple patch solves my problems. Most input devices probably don't
have iSerial set anyway, so there wouldn't be much change. But those
devices that *do* set it may have a reason to do so.



> You may have 2 joysticks and prefer to use one in one game and other
> in another game.

Sure. And that's still possible, just like before. The difference is,
that *if* an application actually cares about the name, then it does
now get an actual identifier, not just a generic product name. And not
just "some" identifier, but the one that the manufacturer set, which
includes the serial number. If an application does only want to look
up the product, rather than the specific devices, then it can simply
search for the longest match in its db.



> With default setup you could grab either one and start playing, ABS_X
> from either mapping on horizontal movements, ABS_Y on vertical... Same
> with keyboards - you plug an external keyboard into laptop and you just
> start typing

Yes, and that works no matter if serial is in the name or not. Because
that's how usb/hid works. But *iff* an application wants to query the
device's identifier, then it better be specific, no? That's what identifiers
are about.



> > The patch below fixed the problem for me. But, certainly, the current
> > (mis)behaviour wasn't an accident, but intentional. But what was the
> > intention? What happens if I attach two actual identical joysticks?
> > What's the purpose of them having the same name on the system under
> > /dev/input/{js,event}? 
> 
> They will have different event nodes.

Sure. Unfortunatly that doesn't buy me anything. Of cousre, I can just
work around the strange omission and do the USB querying myself. But the
question still is, why do I have to? Because the kernel dropped information
that I now have to retrieve again the hard way?  :-)



> Nevertheless serials are exported in sysfs ('uniq' attribute). Also udev
> can give persistent names device nodes based on physocal connection,
> serial numbers, etc.

It's not much fun to use all sorts of additional Linux specific interfaces,
because the responsible one doesn't do its job.

Anyway, my kernel is "fixed", so it's not about me at all. I just feel a
bit uncomfortably if I have to hack my kernel. Before the fix I got this
information from js_demo/JSIOCGNAME:

  Joystick 0: "Leo Bodnar BU0836A Interface"
  Joystick 1: "Leo Bodnar BU0836A Interface"
  Joystick 2: "SAITEK CYBORG 3D USB"

and now I get:

  Joystick 0: "Leo Bodnar BU0836A Interface A12107"
  Joystick 1: "Leo Bodnar BU0836A Interface A12100"
  Joystick 2: "SAITEK CYBORG 3D USB"

As you see, nothing changed for the Saitek. They don't bother setting
iSerial, as probably most vendors. (Reminder: These are circuit boards
and can have very different sensors attached to their inputs, so it's
essential to identify them safely).

m.

Re: [BUG?] hid-core.c: hid->name = dev->manufacturer + dev->product ... why not + dev->serial?

[Dmitry Torokhov]

On Tue, Mar 09, 2010 at 01:42:20PM +0100, Melchior FRANZ wrote:
> 
> > Nevertheless serials are exported in sysfs ('uniq' attribute). Also udev
> > can give persistent names device nodes based on physocal connection,
> > serial numbers, etc.
> 
> It's not much fun to use all sorts of additional Linux specific interfaces,
> because the responsible one doesn't do its job.
> 

Have you considered the fact that maybe you not using the
right interface? The legacy joystick interface (with all its warts like
"replaying" the button state, in-kernel scaling and defuzzing, etc.) is
not what I would think a good general-purpose data acquisition
interface.

FWIW evdev does provide access to serial numbers (EVIOCGUNIQ) but even
that in my opinion is not the proper interface, for the reasons I
mentioned in my previous email.

You need to work with guys at linux-iio@vger.kernel.org (which stands
for Industrial I/O which is exactly what you are working with).

-- 
Dmitry

Re: [BUG?] hid-core.c: hid->name = dev->manufacturer + dev->product ... why not + dev->serial?

[Melchior FRANZ]

* Dmitry Torokhov -- Tuesday 09 March 2010:
> Have you considered the fact that maybe you not using the right interface?

No. Because I'm not talking about *writing* an application, but about
*using* applications written by others. The producer of my two BU0836A
knows that the devices are different, I know that they are different,
the kernel knows that they are different, but the applications don't
know that, because the kernel doesn't tell them (via JSIOCGNAME).



> You need to work with guys at linux-iio@vger.kernel.org (which stands
> for Industrial I/O which is exactly what you are working with).

Hmm, then I wonder why the joystick names are different on Windows and
Linux already.  :-}
 
  unpatched Linux:  "Leo Bodnar BU0836A Interface"
  (Vista?) Windows: "BU0836A Interface" (not even the manufacturer ... pff)

Oh, well. Forget it. I'll just distribute the patch, then. I didn't have
much hope, anyway.

m.

Re: [BUG?] hid-core.c: hid->name = dev->manufacturer + dev->product ... why not + dev->serial?

[Dmitry Torokhov]

On Tue, Mar 09, 2010 at 07:52:40PM +0100, Melchior FRANZ wrote:
> * Dmitry Torokhov -- Tuesday 09 March 2010:
> > Have you considered the fact that maybe you not using the right interface?
> 
> No. Because I'm not talking about *writing* an application, but about
> *using* applications written by others.

Then you need to mention that to your ISV.

> The producer of my two BU0836A
> knows that the devices are different, I know that they are different,
> the kernel knows that they are different, but the applications don't
> know that, because the kernel doesn't tell them (via JSIOCGNAME).
> 
> 
> 
> > You need to work with guys at linux-iio@vger.kernel.org (which stands
> > for Industrial I/O which is exactly what you are working with).
> 
> Hmm, then I wonder why the joystick names are different on Windows and
> Linux already.  :-}
>  

Probably because these are 2 quite different OSes?

>   unpatched Linux:  "Leo Bodnar BU0836A Interface"
>   (Vista?) Windows: "BU0836A Interface" (not even the manufacturer ... pff)
> 
> Oh, well. Forget it. I'll just distribute the patch, then. I didn't have
> much hope, anyway.
>
> m.

-- 
Dmitry

Re: [BUG?] hid-core.c: hid->name = dev->manufacturer + dev->product ... why not + dev->serial?

[Melchior FRANZ]

* Melchior FRANZ -- Tuesday 09 March 2010:
> Hmm, then I wonder why the joystick names are different on Windows and
> Linux already.  :-}

Oh, wait. That was an inaccurate statement. I have no idea how that
application retrieved the joystick name. Maybe it has a choice and
just didn't bother to ask for the manufacturer.

m

Re: [BUG?] hid-core.c: hid->name = dev->manufacturer + dev->product ... why not + dev->serial?

[Melchior FRANZ]

* Melchior FRANZ -- Tuesday 09 March 2010:
> I'll just distribute the patch, then.

For the archive: http://members.aon.at/mfranz/bu0836a.html

m.

Re: [BUG?] hid-core.c: hid->name = dev->manufacturer + dev->product ... why not + dev->serial?

[Melchior FRANZ]

* Melchior FRANZ -- Tuesday 09 March 2010:
> Anyway, my kernel is "fixed", so it's not about me at all. I just feel a
> bit uncomfortably if I have to hack my kernel. Before the fix I got this
> information from js_demo/JSIOCGNAME:
> 
>   Joystick 0: "Leo Bodnar BU0836A Interface"
>   Joystick 1: "Leo Bodnar BU0836A Interface"
>   Joystick 2: "SAITEK CYBORG 3D USB"
> 
> and now I get:
> 
>   Joystick 0: "Leo Bodnar BU0836A Interface A12107"
>   Joystick 1: "Leo Bodnar BU0836A Interface A12100"
>   Joystick 2: "SAITEK CYBORG 3D USB"

As my fix[1] for this kernel flaw was brushed away without arguments (other than
a disingenious "the joystick interface sucks, anyway, just use something else"),
but people come across this thread and then ask me privatly about solutions,
(which doesn't bother me at all, btw), here again, strictly for the archive:

There's now a preload library[1] available, which overrides ioctl(), watches
for JSIOCGNAME and replaces the retarded generic product name by the actual
device identifier, which is taken from the /dev/input/by-id/ name. For some
reason the latter interface actually thinks the serial number belongs to the
joystick name. I totally agree with it!  :-}

Howto build:

  $ git clone git://gitorious.org/bu0836/bu0836.git
  $ cd bu0836/js_serial_preload/
  $ make
  $ sudo make install

Howto use:

  $ LD_PRELOAD=/usr/local/lib/js_serial_preload.so fgfs --aircraft=bo105

Viola. Everything works as intended without having to patch the broken kernel.

m.


[1] http://members.aon.at/mfranz/bu0836a.html
[2] http://gitorious.org/bu0836/

Re: [BUG?] hid-core.c: hid->name = dev->manufacturer + dev->product ... why not + dev->serial?

[Melchior FRANZ]

* Melchior FRANZ -- Tuesday 09 March 2010:
> Anyway, my kernel is "fixed", so it's not about me at all. I just feel a
> bit uncomfortably if I have to hack my kernel. Before the fix I got this
> information from js_demo/JSIOCGNAME:
> 
>   Joystick 0: "Leo Bodnar BU0836A Interface"
>   Joystick 1: "Leo Bodnar BU0836A Interface"
>   Joystick 2: "SAITEK CYBORG 3D USB"
> 
> and now I get:
> 
>   Joystick 0: "Leo Bodnar BU0836A Interface A12107"
>   Joystick 1: "Leo Bodnar BU0836A Interface A12100"
>   Joystick 2: "SAITEK CYBORG 3D USB"

As my fix[1] for this kernel flaw was brushed away without arguments (other than
a disingenious "the joystick interface sucks, anyway, just use something else"),
but people come across this thread and then ask me privatly about solutions,
(which doesn't bother me at all, btw), here again, strictly for the archive:

There's now a preload library[1] available, which overrides ioctl(), watches
for JSIOCGNAME and replaces the retarded generic product name by the actual
device identifier, which is taken from the /dev/input/by-id/ name. For some
reason the latter interface actually thinks the serial number belongs to the
joystick name. I totally agree with it!  :-}

Howto build:

  $ git clone git://gitorious.org/bu0836/bu0836.git
  $ cd bu0836/js_serial_preload/
  $ make
  $ sudo make install

Howto use:

  $ LD_PRELOAD=/usr/local/lib/js_serial_preload.so fgfs --aircraft=bo105

Viola. Everything works as intended without having to patch the broken kernel.

m.


[1] http://members.aon.at/mfranz/bu0836a.html
[2] http://gitorious.org/bu0836/