| Message ID | 801CD73C-2AB1-4421-9C09-1F7AE46AA272@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 1 March 2016 at 22:10, Programmingkid <programmingkidx@gmail.com> 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 <programmingkidx@gmail.com> > > --- > 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
On Mar 1, 2016, at 6:34 PM, Peter Maydell wrote: > On 1 March 2016 at 22:10, Programmingkid <programmingkidx@gmail.com> 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 <programmingkidx@gmail.com> >> >> --- >> 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.) Ok. I will rename number_to_adb_keycode to pc_to_adb_keycode. > > 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?) Yes it was. There was no keypad equal key and the QKeyCode looked like it provided this key. > >> + [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. If you have a Mac OS X guest, you could use Key Caps. It is kind of hidden in Mac OS 10.4. It is located here: System Preferences -> International pane -> Input Menu -> Check the Keyboard Viewer checkbox. There should be an icon of a flag on the menu bar. Click it and select Show Keyboard Viewer. Using it will allow you to test out all 100 keys in a few minutes. > >> + [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.) So you want another value in place of zero. What value did you want? > >> + [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... It works! > >> }; >> >> 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. Maybe the cuda code does something with the power button. I'm not sure.
On 2 March 2016 at 00:31, Programmingkid <programmingkidx@gmail.com> wrote: > > On Mar 1, 2016, at 6:34 PM, Peter Maydell wrote: > >> On 1 March 2016 at 22:10, Programmingkid <programmingkidx@gmail.com> 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. >>> >> 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?) > > Yes it was. There was no keypad equal key and the QKeyCode looked > like it provided this key. But your changes don't seem to be actually using QKeyCodes in the ADB keyboard model, so I'm confused about how you can get the keypad-equal to go through it. >> >>> + [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. > > If you have a Mac OS X guest, you could use Key Caps. I think it would be helpful if you make sure that you use separate patches for "just rearranging how this array is written" from "changing functionality". Then in reviewing the former I know I just need to check that the array contents are the same, and in reviewing the latter I only need to think about the consequences of the function changes. > >> >>> + [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.) > > So you want another value in place of zero. What value did you want? Any value which isn't a possible valid ADB scancode. >>> + [0x5e] = MAC_KEY_POWER, >> >> MAC_KEY_POWER is a two byte scancode, but... > > It works! How? You need to have a change which handles this deliberately and is clear about why it works. Just happening to work by accident isn't sufficient. > >> >>> }; >>> >>> 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. > > Maybe the cuda code does something with the power button. I'm not sure. We're writing bytes into the obuf[] array. It's not possible to put a 16 bit value into an 8 bit slot. Something odd is presumably happening by lucky accident, but you need to look at how to make it work correctly and do whatever the keyboard hardware does with 2 byte scancodes. (Which you need to go and find the hardware documentation for.) thanks -- PMM
On Mar 2, 2016, at 7:38 AM, Peter Maydell wrote: > On 2 March 2016 at 00:31, Programmingkid <programmingkidx@gmail.com> wrote: >> >> On Mar 1, 2016, at 6:34 PM, Peter Maydell wrote: >> >>> On 1 March 2016 at 22:10, Programmingkid <programmingkidx@gmail.com> 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. >>>> > >>> 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?) >> >> Yes it was. There was no keypad equal key and the QKeyCode looked >> like it provided this key. > > But your changes don't seem to be actually using QKeyCodes > in the ADB keyboard model, so I'm confused about how you > can get the keypad-equal to go through it. Now that I think about it, you are right. I'm actually using PS/2 to ADB here which isn't a bad thing. I see what is wrong. I completely forgot to send in my input-keymap.c patch. It is this file that I include the Q_KEY_CODE_KP_EQUALS key. Sorry for the confusion. > >>> >>>> + [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. >> >> If you have a Mac OS X guest, you could use Key Caps. > > I think it would be helpful if you make sure that you use > separate patches for "just rearranging how this array is > written" from "changing functionality". Then in reviewing > the former I know I just need to check that the array > contents are the same, and in reviewing the latter I > only need to think about the consequences of the function > changes. Would it be easier if I just made a new patch for adb.c that takes the existing pc_to_adb_keycode array and just expanded it directly by having the index on the left and the value as a named constant on the right. Original code: static const uint8_t pc_to_adb_keycode[256] = { 0, 53, 18, ... New code: [0x00] = 0, [0x01] = MAC_KEY_ESC, [0x02] = MAC_KEY_1, ... The order of the values in both arrays would be the same. Did you want the index to be in hexadecimal or just decimal? > >> >>> >>>> + [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.) >> >> So you want another value in place of zero. What value did you want? > > Any value which isn't a possible valid ADB scancode. This should do it: #define NO_KEY 0xFF > > >>>> + [0x5e] = MAC_KEY_POWER, >>> >>> MAC_KEY_POWER is a two byte scancode, but... >> >> It works! > > How? You need to have a change which handles this deliberately > and is clear about why it works. Just happening to work by > accident isn't sufficient. The adb.c file's function adb_kbd_request() handles keyboard requests. Its arguments include a buffer for storing keycodes and a len argument that keeps track of the number of bytes to read. Setting the len value to two is probably how a two byte value can be handled.
Hi, > 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?) /me suggests: Step #1: convert adb.c to the new input api, check "commit 66e6536 input: switch ps/2 kbd to new input api" how to do it. Step #2: remove pc scancodes from translation, by moving from qemu_input_key_value_to_scancode to qemu_input_key_value_to_qcode and replacing the pc_to_adb_keycode[] map by a qcode_to_adb_keycode map. Which will also simplify the lookup because you don't have to worry about extended keycodes (scancode == 0xe0) any more. You might also inspect commit "65e7545 input: switch sparc32 kbd to new input api" which does the same for sparc (with both steps in one commit). cheers, Gerd
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] = { + [0x2a] = MAC_KEY_LEFT_SHIFT, + [0x36] = MAC_KEY_LEFT, + [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 */ + [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, }; 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;
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 <programmingkidx@gmail.com> --- hw/input/adb.c | 142 +++++++++++++++++++++++++++++++++++++++++++++++-------- 1 files changed, 121 insertions(+), 21 deletions(-)