qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH v4 4/4] hw/input/adb.c: implement QKeyCode support
Date: Sun, 13 Mar 2016 15:40:20 +0000

On 12 March 2016 at 05:40, Programmingkid <address@hidden> 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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]