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

[Programmingkid]

On Mar 1, 2016, at 6:34 PM, Peter Maydell wrote:

> 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.)

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.