On 11 March 2016 at 09:32, Programmingkid <programmingk...@gmail.com> wrote: > Remove the old pc_to_adb_keycode array and replace it with QKeyCode support. > > Signed-off-by: John Arbuckle <programmingk...@gmail.com> > --- > Some of the keys do not translate as logically as we would think they would. > For > example the Q_KEY_CODE_CTRL_R does not work with ADB_KEY_RIGHT_CONTROL. The > wrong key would show up in the guest. These problem keys are commmented out > and > replaced with the number that does work correctly. This patch can be easily > tested with the Linux command xev or Mac OS's Key Caps.
I'm not sure what you mean here. If you press right-control on the host then shouldn't this correspond to right-control on the guest ? > /* debug ADB */ > //#define DEBUG_ADB > @@ -59,6 +62,9 @@ do { printf("ADB: " fmt , ## __VA_ARGS__); } while (0) > /* error codes */ > #define ADB_RET_NOTPRESENT (-2) > > +/* The adb keyboard doesn't have every key imaginable */ > +#define NO_KEY 0xff > + > static void adb_device_reset(ADBDevice *d) > { > qdev_reset_all(DEVICE(d)); > @@ -187,23 +193,138 @@ 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, > +int qcode_to_adb_keycode[] = { > + [Q_KEY_CODE_SHIFT] = ADB_KEY_LEFT_SHIFT, > + [Q_KEY_CODE_SHIFT_R] = 123, /* ADB_KEY_RIGHT_SHIFT, */ These should definitely be using some ADB_KEY_* constant on the RHS, not a decimal constant. > + [Q_KEY_CODE_ALT] = ADB_KEY_LEFT_OPTION, > + [Q_KEY_CODE_ALT_R] = 124, /* ADB_KEY_RIGHT_OPTION,*/ > + [Q_KEY_CODE_ALTGR] = ADB_KEY_RIGHT_OPTION, > + [Q_KEY_CODE_CTRL] = 54, /* ADB_KEY_LEFT_CONTROL, */ > + [Q_KEY_CODE_CTRL_R] = 125, /* ADB_KEY_RIGHT_CONTROL, */ > + [Q_KEY_CODE_META_L] = ADB_KEY_LEFT_COMMAND, > + > + /* 126 works as right super in Linux */ > + /* Use ADB_KEY_LEFT_COMMAND for Mac OS compatibility */ > + [Q_KEY_CODE_META_R] = ADB_KEY_LEFT_COMMAND, > + [Q_KEY_CODE_SPC] = ADB_KEY_SPACEBAR, > + > + [Q_KEY_CODE_ESC] = ADB_KEY_ESC, > + [Q_KEY_CODE_1] = ADB_KEY_1, > + [Q_KEY_CODE_2] = ADB_KEY_2, > + [Q_KEY_CODE_3] = ADB_KEY_3, > + [Q_KEY_CODE_4] = ADB_KEY_4, > + [Q_KEY_CODE_5] = ADB_KEY_5, > + [Q_KEY_CODE_6] = ADB_KEY_6, > + [Q_KEY_CODE_7] = ADB_KEY_7, > + [Q_KEY_CODE_8] = ADB_KEY_8, > + [Q_KEY_CODE_9] = ADB_KEY_9, > + [Q_KEY_CODE_0] = ADB_KEY_0, > + [Q_KEY_CODE_MINUS] = ADB_KEY_MINUS, > + [Q_KEY_CODE_EQUAL] = ADB_KEY_EQUAL, > + [Q_KEY_CODE_BACKSPACE] = ADB_KEY_DELETE, > + [Q_KEY_CODE_TAB] = ADB_KEY_TAB, > + [Q_KEY_CODE_Q] = ADB_KEY_Q, > + [Q_KEY_CODE_W] = ADB_KEY_W, > + [Q_KEY_CODE_E] = ADB_KEY_E, > + [Q_KEY_CODE_R] = ADB_KEY_R, > + [Q_KEY_CODE_T] = ADB_KEY_T, > + [Q_KEY_CODE_Y] = ADB_KEY_Y, > + [Q_KEY_CODE_U] = ADB_KEY_U, > + [Q_KEY_CODE_I] = ADB_KEY_I, > + [Q_KEY_CODE_O] = ADB_KEY_O, > + [Q_KEY_CODE_P] = ADB_KEY_P, > + [Q_KEY_CODE_BRACKET_LEFT] = ADB_KEY_LEFT_BRACKET, > + [Q_KEY_CODE_BRACKET_RIGHT] = ADB_KEY_RIGHT_BRACKET, > + [Q_KEY_CODE_RET] = ADB_KEY_RETURN, > + [Q_KEY_CODE_A] = ADB_KEY_A, > + [Q_KEY_CODE_S] = ADB_KEY_S, > + [Q_KEY_CODE_D] = ADB_KEY_D, > + [Q_KEY_CODE_F] = ADB_KEY_F, > + [Q_KEY_CODE_G] = ADB_KEY_G, > + [Q_KEY_CODE_H] = ADB_KEY_H, > + [Q_KEY_CODE_J] = ADB_KEY_J, > + [Q_KEY_CODE_K] = ADB_KEY_K, > + [Q_KEY_CODE_L] = ADB_KEY_L, > + [Q_KEY_CODE_SEMICOLON] = ADB_KEY_SEMICOLON, > + [Q_KEY_CODE_APOSTROPHE] = ADB_KEY_APOSTROPHE, > + [Q_KEY_CODE_GRAVE_ACCENT] = ADB_KEY_GRAVE_ACCENT, > + [Q_KEY_CODE_BACKSLASH] = ADB_KEY_BACKSLASH, > + [Q_KEY_CODE_Z] = ADB_KEY_Z, > + [Q_KEY_CODE_X] = ADB_KEY_X, > + [Q_KEY_CODE_C] = ADB_KEY_C, > + [Q_KEY_CODE_V] = ADB_KEY_V, > + [Q_KEY_CODE_B] = ADB_KEY_B, > + [Q_KEY_CODE_N] = ADB_KEY_N, > + [Q_KEY_CODE_M] = ADB_KEY_M, > + [Q_KEY_CODE_COMMA] = ADB_KEY_COMMA, > + [Q_KEY_CODE_DOT] = ADB_KEY_PERIOD, > + [Q_KEY_CODE_SLASH] = ADB_KEY_FORWARD_SLASH, > + [Q_KEY_CODE_ASTERISK] = ADB_KEY_KP_MULTIPLY, > + [Q_KEY_CODE_CAPS_LOCK] = ADB_KEY_CAPS_LOCK, > + > + [Q_KEY_CODE_F1] = ADB_KEY_F1, > + [Q_KEY_CODE_F2] = ADB_KEY_F2, > + [Q_KEY_CODE_F3] = ADB_KEY_F3, > + [Q_KEY_CODE_F4] = ADB_KEY_F4, > + [Q_KEY_CODE_F5] = ADB_KEY_F5, > + [Q_KEY_CODE_F6] = ADB_KEY_F6, > + [Q_KEY_CODE_F7] = ADB_KEY_F7, > + [Q_KEY_CODE_F8] = ADB_KEY_F8, > + [Q_KEY_CODE_F9] = ADB_KEY_F9, > + [Q_KEY_CODE_F10] = ADB_KEY_F10, > + [Q_KEY_CODE_F11] = ADB_KEY_F11, > + [Q_KEY_CODE_F12] = ADB_KEY_F12, > + [Q_KEY_CODE_PRINT] = ADB_KEY_F13, > + [Q_KEY_CODE_SYSRQ] = ADB_KEY_F13, > + [Q_KEY_CODE_SCROLL_LOCK] = ADB_KEY_F14, > + [Q_KEY_CODE_PAUSE] = ADB_KEY_F15, > + [Q_KEY_CODE_POWER] = ADB_KEY_POWER, > + > + [Q_KEY_CODE_NUM_LOCK] = ADB_KEY_KP_CLEAR, > + [Q_KEY_CODE_KP_EQUALS] = ADB_KEY_KP_EQUAL, > + [Q_KEY_CODE_KP_DIVIDE] = ADB_KEY_KP_DIVIDE, > + [Q_KEY_CODE_KP_MULTIPLY] = ADB_KEY_KP_MULTIPLY, > + [Q_KEY_CODE_KP_SUBTRACT] = ADB_KEY_KP_SUBTRACT, > + [Q_KEY_CODE_KP_ADD] = ADB_KEY_KP_PLUS, > + [Q_KEY_CODE_KP_ENTER] = ADB_KEY_KP_ENTER, > + [Q_KEY_CODE_KP_DECIMAL] = ADB_KEY_KP_PERIOD, > + [Q_KEY_CODE_KP_0] = ADB_KEY_KP_0, > + [Q_KEY_CODE_KP_1] = ADB_KEY_KP_1, > + [Q_KEY_CODE_KP_2] = ADB_KEY_KP_2, > + [Q_KEY_CODE_KP_3] = ADB_KEY_KP_3, > + [Q_KEY_CODE_KP_4] = ADB_KEY_KP_4, > + [Q_KEY_CODE_KP_5] = ADB_KEY_KP_5, > + [Q_KEY_CODE_KP_6] = ADB_KEY_KP_6, > + [Q_KEY_CODE_KP_7] = ADB_KEY_KP_7, > + [Q_KEY_CODE_KP_8] = ADB_KEY_KP_8, > + [Q_KEY_CODE_KP_9] = ADB_KEY_KP_9, > + > + [Q_KEY_CODE_UP] = 62, /* ADB_KEY_UP, */ > + [Q_KEY_CODE_DOWN] = 61, /* ADB_KEY_DOWN, */ > + [Q_KEY_CODE_LEFT] = 59, /* ADB_KEY_LEFT, */ > + [Q_KEY_CODE_RIGHT] = 60, /* ADB_KEY_RIGHT, */ > + > + [Q_KEY_CODE_HELP] = ADB_KEY_HELP, > + [Q_KEY_CODE_INSERT] = ADB_KEY_HELP, > + [Q_KEY_CODE_DELETE] = ADB_KEY_FORWARD_DELETE, > + [Q_KEY_CODE_HOME] = ADB_KEY_HOME, > + [Q_KEY_CODE_END] = ADB_KEY_END, > + [Q_KEY_CODE_PGUP] = ADB_KEY_PAGE_UP, > + [Q_KEY_CODE_PGDN] = ADB_KEY_PAGE_DOWN, > + > + [Q_KEY_CODE_LESS] = NO_KEY, > + [Q_KEY_CODE_STOP] = NO_KEY, > + [Q_KEY_CODE_AGAIN] = NO_KEY, > + [Q_KEY_CODE_PROPS] = NO_KEY, > + [Q_KEY_CODE_UNDO] = NO_KEY, > + [Q_KEY_CODE_FRONT] = NO_KEY, > + [Q_KEY_CODE_COPY] = NO_KEY, > + [Q_KEY_CODE_OPEN] = NO_KEY, > + [Q_KEY_CODE_PASTE] = NO_KEY, > + [Q_KEY_CODE_FIND] = NO_KEY, > + [Q_KEY_CODE_CUT] = NO_KEY, > + [Q_KEY_CODE_LF] = NO_KEY, > + [Q_KEY_CODE_COMPOSE] = NO_KEY, This is a little bit fragile, because if somebody adds a new Q_KEY_CODE later then its array entry won't be NO_KEY. Instead you can use a GCC extension: as the first thing in this array you put [ 0 ... Q_KEY_CODE__MAX ] = NO_KEY, which sets a default value for the whole array, and then after that you put all the [Q_KEY_CODE_WHATEVER] = ADB_KEY_THING, entries. (For instance we do this with the 'map' array in hw/arm/z2.c.) > }; > > static void adb_kbd_put_keycode(void *opaque, int keycode) > @@ -220,35 +341,25 @@ static void adb_kbd_put_keycode(void *opaque, int > keycode) > > static int adb_kbd_poll(ADBDevice *d, uint8_t *obuf) > { > - static int ext_keycode; > KBDState *s = ADB_KEYBOARD(d); > - int adb_keycode, keycode; > + int keycode; > int olen; > > olen = 0; > - for(;;) { > - if (s->count == 0) > - break; > - keycode = s->data[s->rptr]; > - if (++s->rptr == sizeof(s->data)) > - s->rptr = 0; > - s->count--; > - > - 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]; > - obuf[0] = adb_keycode | (keycode & 0x80); > - /* NOTE: could put a second keycode if needed */ > - obuf[1] = 0xff; > - olen = 2; > - ext_keycode = 0; > - break; > - } > + if (s->count <= 0) { > + return olen; olen is always 0 here so why not just return 0 ? > + } > + keycode = s->data[s->rptr]; > + if (++s->rptr == sizeof(s->data)) { > + s->rptr = 0; > } > + s->count--; > + > + obuf[0] = keycode; You are still trying to put a two byte keycode (ADB_KEY_POWER) into this one-byte array slot. I don't know what the right way to send a two-byte keycode is but this is obviously not it, as I said before. > + /* NOTE: could put a second keycode if needed */ > + obuf[1] = 0xff; > + olen = 2; > + > return olen; > } > > @@ -313,6 +424,24 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf, > return olen; > } > > +/* The is where keyboard events enter this file */ > +static void adb_keyboard_event(DeviceState *dev, QemuConsole *src, > + InputEvent *evt) > +{ > + KBDState *s = (KBDState *)dev; > + int qcode, keycode; > + > + qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER); I'm curious why you added this wakeup_request call -- does anything in the machines that use the ADB keyboard support suspending the machine? > + qcode = qemu_input_key_value_to_qcode(evt->u.key->key); > + keycode = qcode_to_adb_keycode[qcode]; You should really have a check here that qcode is < the array size before you dereference into it. > + > + if (evt->u.key->down == false) { /* if key up event */ > + keycode = keycode | 0x80; /* create keyboard break code */ > + } > + > + adb_kbd_put_keycode(s, keycode); > +} thanks -- PMM