Re: [Qemu-devel] [PATCH] hw/input/adb.c: Replace pc_to_adb_keycode with more detailed array

[Peter Maydell]

On 1 March 2016 at 22:10, Programmingkid <address@hidden> wrote:
> The pc_to_adb_keycode array was not very easy to work with. The replacement
> array number_to_adb_keycode list all the element indexes on the left and its
> value on the right. This makes finding a particular index or the meaning for
> that index very easy.
>
> Signed-off-by: John Arbuckle <address@hidden>
>
> ---
>  hw/input/adb.c |  142 +++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 files changed, 121 insertions(+), 21 deletions(-)
>
> diff --git a/hw/input/adb.c b/hw/input/adb.c
> index f0ad0d4..58ab1a0 100644
> --- a/hw/input/adb.c
> +++ b/hw/input/adb.c
> @@ -25,6 +25,7 @@
>  #include "hw/hw.h"
>  #include "hw/input/adb.h"
>  #include "ui/console.h"
> +#include "include/hw/input/MacKeys.h"
>
>  /* debug ADB */
>  //#define DEBUG_ADB
> @@ -187,23 +188,121 @@ typedef struct ADBKeyboardClass {
>      DeviceRealize parent_realize;
>  } ADBKeyboardClass;
>
> -static const uint8_t pc_to_adb_keycode[256] = {
> -  0, 53, 18, 19, 20, 21, 23, 22, 26, 28, 25, 29, 27, 24, 51, 48,
> - 12, 13, 14, 15, 17, 16, 32, 34, 31, 35, 33, 30, 36, 54,  0,  1,
> -  2,  3,  5,  4, 38, 40, 37, 41, 39, 50, 56, 42,  6,  7,  8,  9,
> - 11, 45, 46, 43, 47, 44,123, 67, 58, 49, 57,122,120, 99,118, 96,
> - 97, 98,100,101,109, 71,107, 89, 91, 92, 78, 86, 87, 88, 69, 83,
> - 84, 85, 82, 65,  0,  0, 10,103,111,  0,  0,110, 81,  0,  0,  0,
> -  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
> -  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
> -  0,  0,  0, 94,  0, 93,  0,  0,  0,  0,  0,  0,104,102,  0,  0,
> -  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0, 76,125,  0,  0,
> -  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,105,  0,  0,  0,  0,  0,
> -  0,  0,  0,  0,  0, 75,  0,  0,124,  0,  0,  0,  0,  0,  0,  0,
> -  0,  0,  0,  0,  0,  0,  0,115, 62,116,  0, 59,  0, 60,  0,119,
> - 61,121,114,117,  0,  0,  0,  0,  0,  0,  0, 55,126,  0,127,  0,
> -  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
> -  0,  0,  0,  0,  0, 95,  0,  0,  0,  0,  0,  0,  0,  0,  0,  0,
> +#define LARGEST_KEY_INDEX 0xe4
> +/* number refers to the qcode_to_number array in input-keymap.c */
> +static const unsigned int number_to_adb_keycode[LARGEST_KEY_INDEX + 1] = {

This renaming is I think not needed. The numbers in the index
are PC scancodes, which is why the 'pc'. (The input-keymap.c
array is the qcode-to-pc-scancode conversion.)

Scancodes are guaranteed to be single byte so you can just use
[256] like the old array.

In any case this whole array ought at some point to be
replaced with a Q_KEY code to ADB code lookup -- at the
moment we will convert Q_KEY to pc scancode to ADB code,
which is unfortunate if the pc scancodes don't include
some keys that ADB and the host keyboard do. (In fact,
wasn't this the reason why you wanted to do these patches?)

> +    [0x2a] = MAC_KEY_LEFT_SHIFT,
> +    [0x36] = MAC_KEY_LEFT,

This part of the patch is going to be very painful to review,
because I have to go through and manually correlate
the new array against the old table (including cross
referencing it against your new MAC_KEY_* codes) to
see if there are any changes in here beyond the format.

> +    [0x38] = MAC_KEY_LEFT_OPTION,
> +    [0xb8] = MAC_KEY_RIGHT,
> +    [0x64] = MAC_KEY_LEFT_OPTION,
> +    [0xe4] = MAC_KEY_RIGHT_OPTION,
> +    [0x1d] = MAC_KEY_RIGHT_COMMAND,
> +    [0x9d] = MAC_KEY_DOWN,
> +    [0xdb] = MAC_KEY_LEFT_COMMAND,
> +    [0xdc] = MAC_KEY_LEFT_COMMAND,
> +    [0xdd] = 0, /* no Macintosh equivalent to the menu key */

Not a new problem, but you'll note that MAC_KEY_A is 0, so
all these zero entries don't mean "no key, ignore", but "send
A instead"... (A fix for that bug deserves a patch of its own.)

> +    [0x01] = MAC_KEY_ESC,
> +    [0x02] = MAC_KEY_1,
> +    [0x03] = MAC_KEY_2,
> +    [0x04] = MAC_KEY_3,
> +    [0x05] = MAC_KEY_4,
> +    [0x06] = MAC_KEY_5,
> +    [0x07] = MAC_KEY_6,
> +    [0x08] = MAC_KEY_7,
> +    [0x09] = MAC_KEY_8,
> +    [0x0a] = MAC_KEY_9,
> +    [0x0b] = MAC_KEY_0,
> +    [0x0c] = MAC_KEY_MINUS,
> +    [0x0d] = MAC_KEY_EQUAL,
> +    [0x0e] = MAC_KEY_DELETE,
> +    [0x0f] = MAC_KEY_TAB,
> +    [0x10] = MAC_KEY_Q,
> +    [0x11] = MAC_KEY_W,
> +    [0x12] = MAC_KEY_E,
> +    [0x13] = MAC_KEY_R,
> +    [0x14] = MAC_KEY_T,
> +    [0x15] = MAC_KEY_Y,
> +    [0x16] = MAC_KEY_U,
> +    [0x17] = MAC_KEY_I,
> +    [0x18] = MAC_KEY_O,
> +    [0x19] = MAC_KEY_P,
> +    [0x1a] = MAC_KEY_LEFT_BRACKET,
> +    [0x1b] = MAC_KEY_RIGHT_BRACKET,
> +    [0x1c] = MAC_KEY_RETURN,
> +    [0x1e] = MAC_KEY_A,
> +    [0x1f] = MAC_KEY_S,
> +    [0x20] = MAC_KEY_D,
> +    [0x21] = MAC_KEY_F,
> +    [0x22] = MAC_KEY_G,
> +    [0x23] = MAC_KEY_H,
> +    [0x24] = MAC_KEY_J,
> +    [0x25] = MAC_KEY_K,
> +    [0x26] = MAC_KEY_L,
> +    [0x27] = MAC_KEY_SEMICOLON,
> +    [0x28] = MAC_KEY_APOSTROPHE,
> +    [0x29] = MAC_KEY_GRAVE_ACCENT,
> +    [0x2b] = MAC_KEY_BACKSLASH,
> +    [0x2c] = MAC_KEY_Z,
> +    [0x2d] = MAC_KEY_X,
> +    [0x2e] = MAC_KEY_C,
> +    [0x2f] = MAC_KEY_V,
> +    [0x30] = MAC_KEY_B,
> +    [0x31] = MAC_KEY_N,
> +    [0x32] = MAC_KEY_M,
> +    [0x33] = MAC_KEY_COMMA,
> +    [0x34] = MAC_KEY_PERIOD,
> +    [0x35] = MAC_KEY_FORWARD_SLASH,
> +    [0x37] = MAC_KEY_KP_MULTIPLY,
> +    [0x39] = MAC_KEY_SPACEBAR,
> +    [0x3a] = MAC_KEY_CAPS_LOCK,
> +    [0x3b] = MAC_KEY_F1,
> +    [0x3c] = MAC_KEY_F2,
> +    [0x3d] = MAC_KEY_F3,
> +    [0x3e] = MAC_KEY_F4,
> +    [0x3f] = MAC_KEY_F5,
> +    [0x40] = MAC_KEY_F6,
> +    [0x41] = MAC_KEY_F7,
> +    [0x42] = MAC_KEY_F8,
> +    [0x43] = MAC_KEY_F9,
> +    [0x44] = MAC_KEY_F10,
> +    [0x45] = MAC_KEY_KP_CLEAR,
> +    [0x46] = MAC_KEY_F14,
> +    [0xb5] = MAC_KEY_KP_DIVIDE,
> +    [0x37] = MAC_KEY_KP_MULTIPLY,
> +    [0x4a] = MAC_KEY_KP_SUBTRACT,
> +    [0x4e] = MAC_KEY_KP_PLUS,
> +    [0x9c] = MAC_KEY_KP_ENTER,
> +    [0x53] = MAC_KEY_KP_PERIOD,
> +    [0x54] = MAC_KEY_F13,
> +    [0x55] = MAC_KEY_KP_EQUAL,
> +    [0x52] = MAC_KEY_KP_0,
> +    [0x4f] = MAC_KEY_KP_1,
> +    [0x50] = MAC_KEY_KP_2,
> +    [0x51] = MAC_KEY_KP_3,
> +    [0x4b] = MAC_KEY_KP_4,
> +    [0x4c] = MAC_KEY_KP_5,
> +    [0x4d] = MAC_KEY_KP_6,
> +    [0x47] = MAC_KEY_KP_7,
> +    [0x48] = MAC_KEY_KP_8,
> +    [0x49] = MAC_KEY_KP_9,
> +    [0x56] = 0, /* There is no LESS key on the Macintosh keyboard */
> +    [0x57] = MAC_KEY_F11,
> +    [0x58] = MAC_KEY_F12,
> +    [0xb7] = MAC_KEY_F13,
> +    [0xc7] = MAC_KEY_HOME,
> +    [0xc9] = MAC_KEY_PAGE_UP,
> +    [0xd1] = MAC_KEY_PAGE_DOWN,
> +    [0xcf] = MAC_KEY_END,
> +    [0xcb] = MAC_KEY_LEFT_CONTROL,
> +    [0xc8] = MAC_KEY_RIGHT_CONTROL,
> +    [0xd0] = MAC_KEY_RIGHT_OPTION,
> +    [0xcd] = MAC_KEY_RIGHT_SHIFT,
> +    [0xd2] = MAC_KEY_HELP,
> +    [0xd3] = MAC_KEY_FORWARD_DELETE,
> +    [0x73] = 0, /* There is no Ro key on the Macintosh keyboard */
> +    [0x7e] = 0, /* There is no keypad comma key on the Macintosh keyboard */
> +    [0x5e] = MAC_KEY_POWER,

MAC_KEY_POWER is a two byte scancode, but...

>  };
>
>  static void adb_kbd_put_keycode(void *opaque, int keycode)
> @@ -237,10 +336,11 @@ static int adb_kbd_poll(ADBDevice *d, uint8_t *obuf)
>          if (keycode == 0xe0) {
>              ext_keycode = 1;
>          } else {
> -            if (ext_keycode)
> -                adb_keycode =  pc_to_adb_keycode[keycode | 0x80];
> -            else
> -                adb_keycode =  pc_to_adb_keycode[keycode & 0x7f];
> +            if (ext_keycode) {
> +                adb_keycode = number_to_adb_keycode[keycode | 0x80];
> +            } else {
> +                adb_keycode = number_to_adb_keycode[keycode & 0x7f];
> +            }
>              obuf[0] = adb_keycode | (keycode & 0x80);
>              /* NOTE: could put a second keycode if needed */
>              obuf[1] = 0xff;

...this code which writes keycodes into the output buffer assumes
all ADB scancodes are single byte.

thanks
-- PMM