| Message ID | 2C310CE8-A8DE-4383-BC90-C4589FCE9D79@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 6 May 2016 at 03:37, Programmingkid <programmingkidx@gmail.com> wrote: > The original pc_to_adb_keycode mapping did have several keys that were > incorrectly mapped. This patch fixes these mappings. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > --- > hw/input/adb.c | 33 ++++++++++++--------------------- > 1 file changed, 12 insertions(+), 21 deletions(-) > > diff --git a/hw/input/adb.c b/hw/input/adb.c > index 715642f..6d4f4dc 100644 > --- a/hw/input/adb.c > +++ b/hw/input/adb.c > @@ -62,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 > + This... > static void adb_device_reset(ADBDevice *d) > { > qdev_reset_all(DEVICE(d)); > @@ -191,19 +194,21 @@ typedef struct ADBKeyboardClass { > } ADBKeyboardClass; > > int qcode_to_adb_keycode[] = { > + /* Make sure future additions are automatically set to NO_KEY */ > + [0 ... 0xff] = NO_KEY, ...and this (and the removal of the "= 0" entries) should be in the same patch which adds the "don't send key if NO_KEY" check. > [Q_KEY_CODE_SHIFT] = ADB_KEY_LEFT_SHIFT, > [Q_KEY_CODE_SHIFT_R] = ADB_KEY_RIGHT_SHIFT, > [Q_KEY_CODE_ALT] = ADB_KEY_LEFT_OPTION, > [Q_KEY_CODE_ALT_R] = ADB_KEY_RIGHT_OPTION, > - [Q_KEY_CODE_ALTGR] = 0, > + [Q_KEY_CODE_ALTGR] = ADB_KEY_RIGHT_OPTION, > [Q_KEY_CODE_CTRL] = ADB_KEY_LEFT_CONTROL, > [Q_KEY_CODE_CTRL_R] = ADB_KEY_RIGHT_CONTROL, > [Q_KEY_CODE_META_L] = ADB_KEY_LEFT_COMMAND, > > /* ADB_KEY_RIGHT_COMMAND works as right super in Linux */ > /* Use ADB_KEY_LEFT_COMMAND for Mac OS compatibility */ > - [Q_KEY_CODE_META_R] = 0x7e, > + [Q_KEY_CODE_META_R] = ADB_KEY_LEFT_COMMAND, This looks weird. Given that the Apple Extended Keyboard hardware documentation just says that both left and right Command keys return 0x37, we should probably just call the #define ADB_KEY_COMMAND. (That in turn means that the comments are unnecessary and you can just delete them from the patch you put them in.) > [Q_KEY_CODE_SPC] = ADB_KEY_SPACEBAR, > > [Q_KEY_CODE_ESC] = ADB_KEY_ESC, > @@ -272,13 +277,13 @@ int qcode_to_adb_keycode[] = { > [Q_KEY_CODE_F10] = ADB_KEY_F10, > [Q_KEY_CODE_F11] = ADB_KEY_F11, > [Q_KEY_CODE_F12] = ADB_KEY_F12, > - [Q_KEY_CODE_PRINT] = 0, > - [Q_KEY_CODE_SYSRQ] = 0, > + [Q_KEY_CODE_PRINT] = ADB_KEY_F13, > + [Q_KEY_CODE_SYSRQ] = ADB_KEY_F13, > [Q_KEY_CODE_SCROLL_LOCK] = ADB_KEY_F14, > - [Q_KEY_CODE_PAUSE] = 0, > + [Q_KEY_CODE_PAUSE] = ADB_KEY_F15, > > [Q_KEY_CODE_NUM_LOCK] = ADB_KEY_KP_CLEAR, > - [Q_KEY_CODE_KP_EQUALS] = 0, > + [Q_KEY_CODE_KP_EQUALS] = ADB_KEY_KP_EQUAL, > [Q_KEY_CODE_KP_DIVIDE] = ADB_KEY_KP_DIVIDE, > [Q_KEY_CODE_KP_MULTIPLY] = ADB_KEY_KP_MULTIPLY, > [Q_KEY_CODE_KP_SUBTRACT] = ADB_KEY_KP_SUBTRACT, > @@ -301,27 +306,13 @@ int qcode_to_adb_keycode[] = { > [Q_KEY_CODE_LEFT] = ADB_KEY_LEFT, > [Q_KEY_CODE_RIGHT] = ADB_KEY_RIGHT, > > - [Q_KEY_CODE_HELP] = 0, > + [Q_KEY_CODE_HELP] = ADB_KEY_HELP, > [Q_KEY_CODE_INSERT] = ADB_KEY_HELP, > [Q_KEY_CODE_DELETE] = ADB_KEY_FORWARD_DELETE, > [Q_KEY_CODE_HOME] = ADB_KEY_HOME, > [Q_KEY_CODE_END] = ADB_KEY_END, > [Q_KEY_CODE_PGUP] = ADB_KEY_PAGE_UP, > [Q_KEY_CODE_PGDN] = ADB_KEY_PAGE_DOWN, > - > - [Q_KEY_CODE_LESS] = 0xa, > - [Q_KEY_CODE_STOP] = 0, > - [Q_KEY_CODE_AGAIN] = 0, > - [Q_KEY_CODE_PROPS] = 0, > - [Q_KEY_CODE_UNDO] = 0, > - [Q_KEY_CODE_FRONT] = 0, > - [Q_KEY_CODE_COPY] = 0, > - [Q_KEY_CODE_OPEN] = 0, > - [Q_KEY_CODE_PASTE] = 0, > - [Q_KEY_CODE_FIND] = 0, > - [Q_KEY_CODE_CUT] = 0, > - [Q_KEY_CODE_LF] = 0, > - [Q_KEY_CODE_COMPOSE] = 0, The rest of these changes look OK. thanks -- PMM
On May 16, 2016, at 2:04 PM, Peter Maydell wrote: > On 6 May 2016 at 03:37, Programmingkid <programmingkidx@gmail.com> wrote: >> The original pc_to_adb_keycode mapping did have several keys that were >> incorrectly mapped. This patch fixes these mappings. >> >> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >> --- >> hw/input/adb.c | 33 ++++++++++++--------------------- >> 1 file changed, 12 insertions(+), 21 deletions(-) >> >> diff --git a/hw/input/adb.c b/hw/input/adb.c >> index 715642f..6d4f4dc 100644 >> --- a/hw/input/adb.c >> +++ b/hw/input/adb.c >> @@ -62,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 >> + > > This... > >> static void adb_device_reset(ADBDevice *d) >> { >> qdev_reset_all(DEVICE(d)); >> @@ -191,19 +194,21 @@ typedef struct ADBKeyboardClass { >> } ADBKeyboardClass; >> >> int qcode_to_adb_keycode[] = { >> + /* Make sure future additions are automatically set to NO_KEY */ >> + [0 ... 0xff] = NO_KEY, > > ...and this (and the removal of the "= 0" entries) should > be in the same patch which adds the "don't send key if > NO_KEY" check. > >> [Q_KEY_CODE_SHIFT] = ADB_KEY_LEFT_SHIFT, >> [Q_KEY_CODE_SHIFT_R] = ADB_KEY_RIGHT_SHIFT, >> [Q_KEY_CODE_ALT] = ADB_KEY_LEFT_OPTION, >> [Q_KEY_CODE_ALT_R] = ADB_KEY_RIGHT_OPTION, >> - [Q_KEY_CODE_ALTGR] = 0, >> + [Q_KEY_CODE_ALTGR] = ADB_KEY_RIGHT_OPTION, >> [Q_KEY_CODE_CTRL] = ADB_KEY_LEFT_CONTROL, >> [Q_KEY_CODE_CTRL_R] = ADB_KEY_RIGHT_CONTROL, >> [Q_KEY_CODE_META_L] = ADB_KEY_LEFT_COMMAND, >> >> /* ADB_KEY_RIGHT_COMMAND works as right super in Linux */ >> /* Use ADB_KEY_LEFT_COMMAND for Mac OS compatibility */ >> - [Q_KEY_CODE_META_R] = 0x7e, >> + [Q_KEY_CODE_META_R] = ADB_KEY_LEFT_COMMAND, > > This looks weird. Given that the Apple Extended Keyboard > hardware documentation just says that both left and right > Command keys return 0x37, we should probably just call > the #define ADB_KEY_COMMAND. (That in turn means that the > comments are unnecessary and you can just delete them > from the patch you put them in.) I liked the idea of giving someone who might need to tell the difference between left and right command keys a way to accomplish their goal. If I did replace ADB_KEY_LEFT_COMMAND and ADB_KEY_RIGHT_COMMAND with ADB_KEY_COMMAND, then patches 1 and 2 would have to be changed. > >> [Q_KEY_CODE_SPC] = ADB_KEY_SPACEBAR, >> >> [Q_KEY_CODE_ESC] = ADB_KEY_ESC, >> @@ -272,13 +277,13 @@ int qcode_to_adb_keycode[] = { >> [Q_KEY_CODE_F10] = ADB_KEY_F10, >> [Q_KEY_CODE_F11] = ADB_KEY_F11, >> [Q_KEY_CODE_F12] = ADB_KEY_F12, >> - [Q_KEY_CODE_PRINT] = 0, >> - [Q_KEY_CODE_SYSRQ] = 0, >> + [Q_KEY_CODE_PRINT] = ADB_KEY_F13, >> + [Q_KEY_CODE_SYSRQ] = ADB_KEY_F13, >> [Q_KEY_CODE_SCROLL_LOCK] = ADB_KEY_F14, >> - [Q_KEY_CODE_PAUSE] = 0, >> + [Q_KEY_CODE_PAUSE] = ADB_KEY_F15, >> >> [Q_KEY_CODE_NUM_LOCK] = ADB_KEY_KP_CLEAR, >> - [Q_KEY_CODE_KP_EQUALS] = 0, >> + [Q_KEY_CODE_KP_EQUALS] = ADB_KEY_KP_EQUAL, >> [Q_KEY_CODE_KP_DIVIDE] = ADB_KEY_KP_DIVIDE, >> [Q_KEY_CODE_KP_MULTIPLY] = ADB_KEY_KP_MULTIPLY, >> [Q_KEY_CODE_KP_SUBTRACT] = ADB_KEY_KP_SUBTRACT, >> @@ -301,27 +306,13 @@ int qcode_to_adb_keycode[] = { >> [Q_KEY_CODE_LEFT] = ADB_KEY_LEFT, >> [Q_KEY_CODE_RIGHT] = ADB_KEY_RIGHT, >> >> - [Q_KEY_CODE_HELP] = 0, >> + [Q_KEY_CODE_HELP] = ADB_KEY_HELP, >> [Q_KEY_CODE_INSERT] = ADB_KEY_HELP, >> [Q_KEY_CODE_DELETE] = ADB_KEY_FORWARD_DELETE, >> [Q_KEY_CODE_HOME] = ADB_KEY_HOME, >> [Q_KEY_CODE_END] = ADB_KEY_END, >> [Q_KEY_CODE_PGUP] = ADB_KEY_PAGE_UP, >> [Q_KEY_CODE_PGDN] = ADB_KEY_PAGE_DOWN, >> - >> - [Q_KEY_CODE_LESS] = 0xa, >> - [Q_KEY_CODE_STOP] = 0, >> - [Q_KEY_CODE_AGAIN] = 0, >> - [Q_KEY_CODE_PROPS] = 0, >> - [Q_KEY_CODE_UNDO] = 0, >> - [Q_KEY_CODE_FRONT] = 0, >> - [Q_KEY_CODE_COPY] = 0, >> - [Q_KEY_CODE_OPEN] = 0, >> - [Q_KEY_CODE_PASTE] = 0, >> - [Q_KEY_CODE_FIND] = 0, >> - [Q_KEY_CODE_CUT] = 0, >> - [Q_KEY_CODE_LF] = 0, >> - [Q_KEY_CODE_COMPOSE] = 0, > > The rest of these changes look OK. > > thanks > -- PMM
On 16 May 2016 at 21:42, Programmingkid <programmingkidx@gmail.com> wrote: > > On May 16, 2016, at 2:04 PM, Peter Maydell wrote: > >> On 6 May 2016 at 03:37, Programmingkid <programmingkidx@gmail.com> wrote: >>> /* ADB_KEY_RIGHT_COMMAND works as right super in Linux */ >>> /* Use ADB_KEY_LEFT_COMMAND for Mac OS compatibility */ >>> - [Q_KEY_CODE_META_R] = 0x7e, >>> + [Q_KEY_CODE_META_R] = ADB_KEY_LEFT_COMMAND, >> >> This looks weird. Given that the Apple Extended Keyboard >> hardware documentation just says that both left and right >> Command keys return 0x37, we should probably just call >> the #define ADB_KEY_COMMAND. (That in turn means that the >> comments are unnecessary and you can just delete them >> from the patch you put them in.) > > I liked the idea of giving someone who might need to tell the > difference between left and right command keys a way to > accomplish their goal. We're emulating a real bit of hardware here (ie the Apple Extended Keyboard). If that hardware does not have distinct left and right command keys then that's what we have to emulate. thanks -- PMM
On May 16, 2016, at 4:48 PM, Peter Maydell wrote: > On 16 May 2016 at 21:42, Programmingkid <programmingkidx@gmail.com> wrote: >> >> On May 16, 2016, at 2:04 PM, Peter Maydell wrote: >> >>> On 6 May 2016 at 03:37, Programmingkid <programmingkidx@gmail.com> wrote: >>>> /* ADB_KEY_RIGHT_COMMAND works as right super in Linux */ >>>> /* Use ADB_KEY_LEFT_COMMAND for Mac OS compatibility */ >>>> - [Q_KEY_CODE_META_R] = 0x7e, >>>> + [Q_KEY_CODE_META_R] = ADB_KEY_LEFT_COMMAND, >>> >>> This looks weird. Given that the Apple Extended Keyboard >>> hardware documentation just says that both left and right >>> Command keys return 0x37, we should probably just call >>> the #define ADB_KEY_COMMAND. (That in turn means that the >>> comments are unnecessary and you can just delete them >>> from the patch you put them in.) >> >> I liked the idea of giving someone who might need to tell the >> difference between left and right command keys a way to >> accomplish their goal. > > We're emulating a real bit of hardware here (ie the Apple > Extended Keyboard). If that hardware does not have distinct > left and right command keys then that's what we have to emulate. I think the Apple Extended keyboard could tell the difference between left and right command keys, but Apple just decided not to document this feature. I know the USB keyboards can see the difference and Apple did not document that feature. In the end I suppose it is just easier to have just an ADB_KEY_COMMAND constant.
diff --git a/hw/input/adb.c b/hw/input/adb.c index 715642f..6d4f4dc 100644 --- a/hw/input/adb.c +++ b/hw/input/adb.c @@ -62,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)); @@ -191,19 +194,21 @@ typedef struct ADBKeyboardClass { } ADBKeyboardClass; int qcode_to_adb_keycode[] = { + /* Make sure future additions are automatically set to NO_KEY */ + [0 ... 0xff] = NO_KEY, [Q_KEY_CODE_SHIFT] = ADB_KEY_LEFT_SHIFT, [Q_KEY_CODE_SHIFT_R] = ADB_KEY_RIGHT_SHIFT, [Q_KEY_CODE_ALT] = ADB_KEY_LEFT_OPTION, [Q_KEY_CODE_ALT_R] = ADB_KEY_RIGHT_OPTION, - [Q_KEY_CODE_ALTGR] = 0, + [Q_KEY_CODE_ALTGR] = ADB_KEY_RIGHT_OPTION, [Q_KEY_CODE_CTRL] = ADB_KEY_LEFT_CONTROL, [Q_KEY_CODE_CTRL_R] = ADB_KEY_RIGHT_CONTROL, [Q_KEY_CODE_META_L] = ADB_KEY_LEFT_COMMAND, /* ADB_KEY_RIGHT_COMMAND works as right super in Linux */ /* Use ADB_KEY_LEFT_COMMAND for Mac OS compatibility */ - [Q_KEY_CODE_META_R] = 0x7e, + [Q_KEY_CODE_META_R] = ADB_KEY_LEFT_COMMAND, [Q_KEY_CODE_SPC] = ADB_KEY_SPACEBAR, [Q_KEY_CODE_ESC] = ADB_KEY_ESC, @@ -272,13 +277,13 @@ int qcode_to_adb_keycode[] = { [Q_KEY_CODE_F10] = ADB_KEY_F10, [Q_KEY_CODE_F11] = ADB_KEY_F11, [Q_KEY_CODE_F12] = ADB_KEY_F12, - [Q_KEY_CODE_PRINT] = 0, - [Q_KEY_CODE_SYSRQ] = 0, + [Q_KEY_CODE_PRINT] = ADB_KEY_F13, + [Q_KEY_CODE_SYSRQ] = ADB_KEY_F13, [Q_KEY_CODE_SCROLL_LOCK] = ADB_KEY_F14, - [Q_KEY_CODE_PAUSE] = 0, + [Q_KEY_CODE_PAUSE] = ADB_KEY_F15, [Q_KEY_CODE_NUM_LOCK] = ADB_KEY_KP_CLEAR, - [Q_KEY_CODE_KP_EQUALS] = 0, + [Q_KEY_CODE_KP_EQUALS] = ADB_KEY_KP_EQUAL, [Q_KEY_CODE_KP_DIVIDE] = ADB_KEY_KP_DIVIDE, [Q_KEY_CODE_KP_MULTIPLY] = ADB_KEY_KP_MULTIPLY, [Q_KEY_CODE_KP_SUBTRACT] = ADB_KEY_KP_SUBTRACT, @@ -301,27 +306,13 @@ int qcode_to_adb_keycode[] = { [Q_KEY_CODE_LEFT] = ADB_KEY_LEFT, [Q_KEY_CODE_RIGHT] = ADB_KEY_RIGHT, - [Q_KEY_CODE_HELP] = 0, + [Q_KEY_CODE_HELP] = ADB_KEY_HELP, [Q_KEY_CODE_INSERT] = ADB_KEY_HELP, [Q_KEY_CODE_DELETE] = ADB_KEY_FORWARD_DELETE, [Q_KEY_CODE_HOME] = ADB_KEY_HOME, [Q_KEY_CODE_END] = ADB_KEY_END, [Q_KEY_CODE_PGUP] = ADB_KEY_PAGE_UP, [Q_KEY_CODE_PGDN] = ADB_KEY_PAGE_DOWN, - - [Q_KEY_CODE_LESS] = 0xa, - [Q_KEY_CODE_STOP] = 0, - [Q_KEY_CODE_AGAIN] = 0, - [Q_KEY_CODE_PROPS] = 0, - [Q_KEY_CODE_UNDO] = 0, - [Q_KEY_CODE_FRONT] = 0, - [Q_KEY_CODE_COPY] = 0, - [Q_KEY_CODE_OPEN] = 0, - [Q_KEY_CODE_PASTE] = 0, - [Q_KEY_CODE_FIND] = 0, - [Q_KEY_CODE_CUT] = 0, - [Q_KEY_CODE_LF] = 0, - [Q_KEY_CODE_COMPOSE] = 0, }; static void adb_kbd_put_keycode(void *opaque, int keycode)
The original pc_to_adb_keycode mapping did have several keys that were incorrectly mapped. This patch fixes these mappings. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- hw/input/adb.c | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-)