On Mar 2, 2016, at 7:38 AM, Peter Maydell wrote:
> On 2 March 2016 at 00:31, Programmingkid <programmingk...@gmail.com> wrote:
>>
>> On Mar 1, 2016, at 6:34 PM, Peter Maydell wrote:
>>
>>> On 1 March 2016 at 22:10, Programmingkid <programmingk...@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.
