Re: Input Device Key Mapping (driver authoring advice?)

[Vojtech P. <vo...@su...>]

On Tue, Jul 05, 2005 at 06:59:36PM -0400, Micah F. Galizia wrote:
> Hello,
> 
> I've attached the patch that makes all keys on the Logitech UltraX Media
> Remote work nicely.  I will verify that the previous patch correctly
> assigns the channel keys in the consumer page, however, I cannot give
> props to the mapping of props (sorry).  0x209 on this remote says
> "Info/EPG", but it has been assigned to KEY_PROPS.  Maybe I'm just not
> understanding the meaning of "props."  This patch does reassign KEY_INFO
> to 0x209 in the consumer page; it that was the wrong thing to do, I'll
> resubmit.
> 
> I used my best judgement when assigning keys, since the text on the
> device does not always match up with a key defined in input.h.  Here is
> a list of the potentially ambiguous or incorrect mappings:
> 
> Start->KEY_RED
> Pictures->KEY_MEDIA
> DVD Menu->KEY_MENU
> SAP->KEY_AUDIO
> Repeat->KEY_AGAIN
> CC/Teletext->KEY_SUBTITLE
> Music->KEY_MP3

The problem is that the markings on the device aren't important in case
of the Consumer HID usage page. We need to follow the spec
(http://www.usb.org/developers/devclass_docs/Hut1_11.pdf).

> Allow me to justify the silly ones.  The start key is big and red,
> Google told me SAP stands for "Secondary Audio Program", and there is no
> key close to pictures/graphics/image.

We can add KEY_* definitions, if they are likely to be used on other
devices, too.


> The patch failed on account of HID_UP_LOGIVENDOR not being defined.  I
> fixed two hunks manually, so it should work fine with your tree,
> Vojtech.

Sorry for that, it's defined in an earlier patch I didn't send to you.

> Thanks again for all your help.  Please keep me posted if something is
> wrong.

> --- gitandhidpatches/drivers/usb/input/hid-input.c	2005-07-05 18:29:32.726219000 -0400
> +++ micah/drivers/usb/input/hid-input.c	2005-07-05 18:42:15.489888750 -0400
> @@ -237,6 +237,7 @@
>  				case 0x000: goto ignore;
>  				case 0x034: map_key_clear(KEY_SLEEP);		break;
>  				case 0x036: map_key_clear(BTN_MISC);		break;
> +				case 0x045: map_key_clear(KEY_RADIO);		break;

This is rather suspicious, considering what Consumer.0045 is supposed to
mean. I can add it, but I expect it'll be incorrect for other devices.

>  				case 0x08a: map_key_clear(KEY_WWW);		break;
>  				case 0x08d: map_key_clear(KEY_PROGRAM);		break;
>  				case 0x095: map_key_clear(KEY_HELP);		break;
> @@ -265,7 +266,7 @@
>  				case 0x201: map_key_clear(KEY_NEW);		break;
>  				case 0x207: map_key_clear(KEY_SAVE);		break;
>  				case 0x208: map_key_clear(KEY_PRINT);		break;
> -				case 0x209: map_key_clear(KEY_PROPS);		break;
> +				case 0x209: map_key_clear(KEY_INFO);		break;

See the spec on this one.

>  				case 0x21a: map_key_clear(KEY_UNDO);		break;
>  				case 0x21b: map_key_clear(KEY_COPY);		break;
>  				case 0x21c: map_key_clear(KEY_CUT);		break;
> @@ -309,9 +310,27 @@
>  		case HID_UP_MSVENDOR:
>    		case HID_UP_LOGIVENDOR:
>   		case HID_UP_LOGIVENDOR2:
> -
> -			goto ignore;
> -
> +			  switch(usage->hid & HID_USAGE) {
> +			  	case 0x004: map_key_clear(KEY_AGAIN);		break;
> +			  	case 0x00d: map_key_clear(KEY_HOME);		break;
> +			  	case 0x024: map_key_clear(KEY_SHUFFLE);		break;
> +			  	case 0x025: map_key_clear(KEY_TV);		break;
> +			  	case 0x026: map_key_clear(KEY_MENU);		break;
> +			  	case 0x031: map_key_clear(KEY_AUDIO);		break;
> +			  	case 0x032: map_key_clear(KEY_SUBTITLE);	break;
> +			  	case 0x033: map_key_clear(KEY_LAST);		break;
> +			  	case 0x047: map_key_clear(KEY_MP3);		break;
> +			  	case 0x048: map_key_clear(KEY_DVD);		break;
> +			  	case 0x049: map_key_clear(KEY_MEDIA);		break;
> +			  	case 0x04a: map_key_clear(KEY_VIDEO);		break;
> +			  	case 0x04b: map_key_clear(KEY_ANGLE);		break;
> +			  	case 0x04c: map_key_clear(KEY_LANGUAGE);	break;
> +			  	case 0x04d: map_key_clear(KEY_SUBTITLE);	break;
> +			  	case 0x051: map_key_clear(KEY_RED);		break;
> +			  	case 0x052: map_key_clear(KEY_CLOSE);		break;
> +			  	default:    goto ignore;
> +			  }
> +			  break;

You need to separate the LOGIVENDOR2 case, the switch() isn't valid for
LOGIVENDOR and MSVENDOR.

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR