Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[Peter Maydell]

On 15 March 2016 at 15:24, Programmingkid  wrote:
> I did several tests and found out that the USB keyboard actually
> works a lot better than the ADB keyboard. Would you accept a patch
> that changes the default keyboard and mouse to USB for the Mac99
> target?

Not my call, ask the PPC maintainer.

(Fixing the ADB keyboard is a useful cleanup anyway I think.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[Programmingkid]

On Mar 15, 2016, at 5:22 AM, Peter Maydell wrote:

> On 14 March 2016 at 23:06, Programmingkid  wrote:
>> On Mar 13, 2016, at 11:40 AM, Peter Maydell wrote:
>>> Also, you need to handle the power-key key-release
>>> scancode; as far as I can tell (by looking for info via
>>> google about the ADB protocol) power-key-down is
>>> 0x7f 0x7f, and power-key-up is 0xff 0xff. (This is
>>> kind of weird and suggests we should just take option
>>> (a) and special case the power key completely.)
>> 
>> The power-key-up (break) keycode is 0xff 0xff? Could you send me your
>> source for this information?
> 
> https://developer.apple.com/legacy/library/technotes/hw/hw_01.html
> (section "Apple Keyboard Protocol").
> 
> Power-key is a weird one both for key-up and key-down.

I did several tests and found out that the USB keyboard actually works a lot 
better than the ADB keyboard. Would you accept a patch that changes the default 
keyboard and mouse to USB for the Mac99 target? This target will always have 
USB available. USB peripherals are what the real counterpart uses. 

Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[Peter Maydell]

On 14 March 2016 at 23:06, Programmingkid  wrote:
> On Mar 13, 2016, at 11:40 AM, Peter Maydell wrote:
>> Also, you need to handle the power-key key-release
>> scancode; as far as I can tell (by looking for info via
>> google about the ADB protocol) power-key-down is
>> 0x7f 0x7f, and power-key-up is 0xff 0xff. (This is
>> kind of weird and suggests we should just take option
>> (a) and special case the power key completely.)
>
> The power-key-up (break) keycode is 0xff 0xff? Could you send me your
> source for this information?

https://developer.apple.com/legacy/library/technotes/hw/hw_01.html
(section "Apple Keyboard Protocol").

Power-key is a weird one both for key-up and key-down.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[M A]

On Mar 14, 2016, at 7:06 PM, Programmingkid wrote:

> 
> On Mar 13, 2016, at 11:40 AM, Peter Maydell wrote:
> 
>> On 12 March 2016 at 05:40, Programmingkid  wrote:
>>> 
>>> On Mar 11, 2016, at 10:30 PM, Peter Maydell wrote:
>>> 
 
> +}
> +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;
> }
>>> 
>>> Is this ok?
>>> 
>>>   /* The power key is the only two byte value key, so it is a special case. 
>>> */
>>>   if (keycode == (ADB_KEY_POWER & 0x00ff)) {
>>>   obuf[0] = ADB_KEY_POWER & 0x00ff;
>>>   obuf[1] = ADB_KEY_POWER & 0xff00 >> 8;
>>>   olen = 2;
>>>   } else {
>>>   obuf[0] = keycode;
>>>   /* NOTE: could put a second keycode if needed */
>>>   obuf[1] = 0xff;
>>>   olen = 2;
>>>   }
>>> 
>>> The keycode value comes from an 8 bit array so holding the
>>> full value of the power key is not possible.
>> 
>> Ah, I hadn't noticed that -- that is not a good approach.
>> You should either:
>> (a) deal with the fact that ADB_KEY values may be 16 bits
>> all the way through (including having that array be uint16_t
>> rather than uint8_t)
>> (b) have the power key be a completely special case which
>> is handled by
>> if (qcode == Q_KEY_CODE_POWER) {
>>   /* put power key into buffer */
>>   [...]
>> } else {
>>   keycode = qcode_to_adb_keycode[...];
>>   etc;
>> }
>> and not put it into the array at all.
>> 
>> Also, you need to handle the power-key key-release
>> scancode; as far as I can tell (by looking for info via
>> google about the ADB protocol) power-key-down is
>> 0x7f 0x7f, and power-key-up is 0xff 0xff. (This is
>> kind of weird and suggests we should just take option
>> (a) and special case the power key completely.)
> 
> The power-key-up (break) keycode is 0xff 0xff? Could you send me your source 
> for this information? I always thought the up (break) code was the keycode 
> AND with 0x80. Or this if code is easier to understand:
> break_keycode = keycode & 0x80.

* sorry I meant OR in the above code
break_keycode = keycode | 0x80.

Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[Programmingkid]

On Mar 13, 2016, at 11:40 AM, Peter Maydell wrote:

> On 12 March 2016 at 05:40, Programmingkid  wrote:
>> 
>> On Mar 11, 2016, at 10:30 PM, Peter Maydell wrote:
>> 
>>> 
 +}
 +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;
 }
>> 
>> Is this ok?
>> 
>>/* The power key is the only two byte value key, so it is a special case. 
>> */
>>if (keycode == (ADB_KEY_POWER & 0x00ff)) {
>>obuf[0] = ADB_KEY_POWER & 0x00ff;
>>obuf[1] = ADB_KEY_POWER & 0xff00 >> 8;
>>olen = 2;
>>} else {
>>obuf[0] = keycode;
>>/* NOTE: could put a second keycode if needed */
>>obuf[1] = 0xff;
>>olen = 2;
>>}
>> 
>> The keycode value comes from an 8 bit array so holding the
>> full value of the power key is not possible.
> 
> Ah, I hadn't noticed that -- that is not a good approach.
> You should either:
> (a) deal with the fact that ADB_KEY values may be 16 bits
> all the way through (including having that array be uint16_t
> rather than uint8_t)
> (b) have the power key be a completely special case which
> is handled by
> if (qcode == Q_KEY_CODE_POWER) {
>/* put power key into buffer */
>[...]
> } else {
>keycode = qcode_to_adb_keycode[...];
>etc;
> }
> and not put it into the array at all.
> 
> Also, you need to handle the power-key key-release
> scancode; as far as I can tell (by looking for info via
> google about the ADB protocol) power-key-down is
> 0x7f 0x7f, and power-key-up is 0xff 0xff. (This is
> kind of weird and suggests we should just take option
> (a) and special case the power key completely.)

The power-key-up (break) keycode is 0xff 0xff? Could you send me your source 
for this information? I always thought the up (break) code was the keycode AND 
with 0x80. Or this if code is easier to understand:
break_keycode = keycode & 0x80.

Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[Peter Maydell]

On 13 March 2016 at 16:39, Programmingkid  wrote:
> I did try this but a bunch of errors showed up.

> /include/migration/vmstate.h:248:48: error: invalid operands to binary - 
> (have 'uint8_t (*)[256]' and 'uint16_t (*)[128]')
>  #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0)
>
> /include/migration/vmstate.h:261:6: note: in expansion of macro 
> 'type_check_array'
>   type_check_array(_type, typeof_field(_state, _field), _num))
>
> So I'm not sure now changing the type to 16 bit is the best thing to do. It 
> would require a lot more changes to other files.

Right, you would need to also change the migration state
to say the array was 16 bit. (This is a migration compat
break, so awkward anyway.)

I made a typo in that email which unfortunately completely
reversed the meaning -- I meant to say "we should just
take option (b)"...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[Programmingkid]

On Mar 13, 2016, at 11:40 AM, Peter Maydell wrote:

> On 12 March 2016 at 05:40, Programmingkid  wrote:
>> 
>> On Mar 11, 2016, at 10:30 PM, Peter Maydell wrote:
>> 
>>> 
 +}
 +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;
 }
>> 
>> Is this ok?
>> 
>>/* The power key is the only two byte value key, so it is a special case. 
>> */
>>if (keycode == (ADB_KEY_POWER & 0x00ff)) {
>>obuf[0] = ADB_KEY_POWER & 0x00ff;
>>obuf[1] = ADB_KEY_POWER & 0xff00 >> 8;
>>olen = 2;
>>} else {
>>obuf[0] = keycode;
>>/* NOTE: could put a second keycode if needed */
>>obuf[1] = 0xff;
>>olen = 2;
>>}
>> 
>> The keycode value comes from an 8 bit array so holding the
>> full value of the power key is not possible.
> 
> Ah, I hadn't noticed that -- that is not a good approach.
> You should either:
> (a) deal with the fact that ADB_KEY values may be 16 bits
> all the way through (including having that array be uint16_t
> rather than uint8_t)

I did try this but a bunch of errors showed up. To see all the errors just make 
this change in adb.c:

typedef struct KBDState {
/*< private >*/
ADBDevice parent_obj;
/*< public >*/

uint16_t data[128];  <- was uint8_t
int rptr, wptr, count;
} KBDState;

The errors:

/include/migration/vmstate.h:248:48: error: invalid operands to binary - (have 
'uint8_t (*)[256]' and 'uint16_t (*)[128]')
 #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0)

/include/migration/vmstate.h:261:6: note: in expansion of macro 
'type_check_array'
  type_check_array(_type, typeof_field(_state, _field), _num))

So I'm not sure now changing the type to 16 bit is the best thing to do. It 
would require a lot more changes to other files. 

Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[Peter Maydell]

On 12 March 2016 at 04:27, Programmingkid  wrote:
>
> On Mar 11, 2016, at 10:30 PM, Peter Maydell wrote:
>
>> On 11 March 2016 at 09:32, Programmingkid  wrote:
>>> Remove the old pc_to_adb_keycode array and replace it with QKeyCode support.
>>>
>>> Signed-off-by: John Arbuckle 
>>> ---
>>> 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 ?
>
> It should. It makes logical sense. But when I tried it using a Mac OS X and
> Linux guest, the wrong key would be pressed. The theories I have are
> incorrect keyboard detection to CUDA translation problems.
>

>>> +[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.
>
> Ok. It would look something like this:
> [Q_KEY_CODE_SHIFT_R]   = ADB_KEY_LEFT,

I think we definitely need to figure out what is going on here.
Sending the key-left code for right-shift is definitely wrong.
(Presumably this also implies that the actual left-arrow key
is broken...)

Possibly relatedly, the Apple Extended Keyboard apparently won't send
the separate keycodes for right-shift, right-option, right-control
until the guest OS sends the keyboard a command to enable them.
(see
http://www.archive.org/stream/apple-guide-macintosh-family-hardware/Apple_Guide_to_the_Macintosh_Family_Hardware_2e#page/n347/mode/2up
page 309).

I suggest that for this patchset you leave the code so that
it continues to send the same ADB keycodes for these keys
that it has done before (whatever those are). Then once
we've got the conversion to using qcodes in we can look
at fixing this bug as a separate patch.

(Similarly, you might want to split out the code to support
the power key as a separate patch.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[Peter Maydell]

On 12 March 2016 at 05:40, Programmingkid  wrote:
>
> On Mar 11, 2016, at 10:30 PM, Peter Maydell wrote:
>
>>
>>> +}
>>> +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;
>>> }
>
> Is this ok?
>
> /* The power key is the only two byte value key, so it is a special case. 
> */
> if (keycode == (ADB_KEY_POWER & 0x00ff)) {
> obuf[0] = ADB_KEY_POWER & 0x00ff;
> obuf[1] = ADB_KEY_POWER & 0xff00 >> 8;
> olen = 2;
> } else {
> obuf[0] = keycode;
> /* NOTE: could put a second keycode if needed */
> obuf[1] = 0xff;
> olen = 2;
> }
>
> The keycode value comes from an 8 bit array so holding the
> full value of the power key is not possible.

Ah, I hadn't noticed that -- that is not a good approach.
You should either:
 (a) deal with the fact that ADB_KEY values may be 16 bits
all the way through (including having that array be uint16_t
rather than uint8_t)
 (b) have the power key be a completely special case which
is handled by
 if (qcode == Q_KEY_CODE_POWER) {
/* put power key into buffer */
[...]
 } else {
keycode = qcode_to_adb_keycode[...];
etc;
 }
and not put it into the array at all.

Also, you need to handle the power-key key-release
scancode; as far as I can tell (by looking for info via
google about the ADB protocol) power-key-down is
0x7f 0x7f, and power-key-up is 0xff 0xff. (This is
kind of weird and suggests we should just take option
(a) and special case the power key completely.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[Programmingkid]

On Mar 11, 2016, at 10:30 PM, Peter Maydell wrote:

> 
>> +}
>> +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;
>> }

Is this ok?

/* The power key is the only two byte value key, so it is a special case. */
if (keycode == (ADB_KEY_POWER & 0x00ff)) {
obuf[0] = ADB_KEY_POWER & 0x00ff;
obuf[1] = ADB_KEY_POWER & 0xff00 >> 8;
olen = 2;
} else {
obuf[0] = keycode;
/* NOTE: could put a second keycode if needed */
obuf[1] = 0xff;
olen = 2;
}

The keycode value comes from an 8 bit array so holding the full value of the 
power key is not possible. That is the reason for the "if (keycode == 
(ADB_KEY_POWER & 0x00ff))". 

The code might be a little more efficient if we did this:

/* The power key is the only two byte value key, so it is a special case. */
if (keycode == 0x7f) {
obuf[0] = 0x7f;
obuf[1] = 0x7f;
olen = 2;
} else {
obuf[0] = keycode;
/* NOTE: could put a second keycode if needed */
obuf[1] = 0xff;
olen = 2;
}

The speed difference isn't noticeable so either way works well.

Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[Programmingkid]

On Mar 11, 2016, at 10:30 PM, Peter Maydell wrote:

> On 11 March 2016 at 09:32, Programmingkid  wrote:
>> Remove the old pc_to_adb_keycode array and replace it with QKeyCode support.
>> 
>> Signed-off-by: John Arbuckle 
>> ---
>> 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 ?

It should. It makes logical sense. But when I tried it using a Mac OS X and 
Linux guest, the wrong key would be pressed. The theories I have are incorrect 
keyboard detection to CUDA translation problems. 


>> /* 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.

Ok. It would look something like this:
[Q_KEY_CODE_SHIFT_R]   = ADB_KEY_LEFT,

It looks wrong, but it works.

> 
>> +[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,

Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[Peter Maydell]

On 11 March 2016 at 09:32, Programmingkid  wrote:
> Remove the old pc_to_adb_keycode array and replace it with QKeyCode support.
>
> Signed-off-by: John Arbuckle 
> ---
> 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] = 

[Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support

[Programmingkid]

Remove the old pc_to_adb_keycode array and replace it with QKeyCode support.

Signed-off-by: John Arbuckle 
---
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.

 hw/input/adb.c | 223 +
 1 file changed, 177 insertions(+), 46 deletions(-)

diff --git a/hw/input/adb.c b/hw/input/adb.c
index f0ad0d4..d176d39 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -25,6 +25,9 @@
 #include "hw/hw.h"
 #include "hw/input/adb.h"
 #include "ui/console.h"
+#include "include/hw/input/adb-keys.h"
+#include "ui/input.h"
+#include "sysemu/sysemu.h"
 
 /* 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, */
+[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,
+