* [Qemu-devel] [PATCH] ui/cocoa.m: Replace pc/xt keyboard keycode array with QKeyCode
@ 2016-03-01 22:12 Programmingkid
2016-03-01 23:16 ` Eric Blake
2016-03-01 23:18 ` Peter Maydell
0 siblings, 2 replies; 9+ messages in thread
From: Programmingkid @ 2016-03-01 22:12 UTC (permalink / raw)
To: Peter Maydell, qemu-devel qemu-devel
The old pc/xt keyboard keycode array is replaced with QEMU's own QKeyCode
layout.
Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
---
Maintainer note:
Please apply these patches before testing:
- qapi-schema.json: Add kp_equals and power keys
- adb.c: Replace pc_to_adb_keycode with more detailed array
- MacKeys.h: initial commit
ui/cocoa.m | 300 ++++++++++++++++++++++++++----------------------------------
1 files changed, 128 insertions(+), 172 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 3ee5549..8ea39ad 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -33,6 +33,7 @@
#include "sysemu/sysemu.h"
#include "qmp-commands.h"
#include "sysemu/blockdev.h"
+#include "include/hw/input/MacKeys.h"
#ifndef MAC_OS_X_VERSION_10_5
#define MAC_OS_X_VERSION_10_5 1050
@@ -72,178 +73,133 @@ bool stretch_video;
NSTextField *pauseLabel;
NSArray * supportedImageFileTypes;
-// keymap conversion
-int keymap[] =
-{
-// SdlI macI macH SdlH 104xtH 104xtC sdl
- 30, // 0 0x00 0x1e A QZ_a
- 31, // 1 0x01 0x1f S QZ_s
- 32, // 2 0x02 0x20 D QZ_d
- 33, // 3 0x03 0x21 F QZ_f
- 35, // 4 0x04 0x23 H QZ_h
- 34, // 5 0x05 0x22 G QZ_g
- 44, // 6 0x06 0x2c Z QZ_z
- 45, // 7 0x07 0x2d X QZ_x
- 46, // 8 0x08 0x2e C QZ_c
- 47, // 9 0x09 0x2f V QZ_v
- 0, // 10 0x0A Undefined
- 48, // 11 0x0B 0x30 B QZ_b
- 16, // 12 0x0C 0x10 Q QZ_q
- 17, // 13 0x0D 0x11 W QZ_w
- 18, // 14 0x0E 0x12 E QZ_e
- 19, // 15 0x0F 0x13 R QZ_r
- 21, // 16 0x10 0x15 Y QZ_y
- 20, // 17 0x11 0x14 T QZ_t
- 2, // 18 0x12 0x02 1 QZ_1
- 3, // 19 0x13 0x03 2 QZ_2
- 4, // 20 0x14 0x04 3 QZ_3
- 5, // 21 0x15 0x05 4 QZ_4
- 7, // 22 0x16 0x07 6 QZ_6
- 6, // 23 0x17 0x06 5 QZ_5
- 13, // 24 0x18 0x0d = QZ_EQUALS
- 10, // 25 0x19 0x0a 9 QZ_9
- 8, // 26 0x1A 0x08 7 QZ_7
- 12, // 27 0x1B 0x0c - QZ_MINUS
- 9, // 28 0x1C 0x09 8 QZ_8
- 11, // 29 0x1D 0x0b 0 QZ_0
- 27, // 30 0x1E 0x1b ] QZ_RIGHTBRACKET
- 24, // 31 0x1F 0x18 O QZ_o
- 22, // 32 0x20 0x16 U QZ_u
- 26, // 33 0x21 0x1a [ QZ_LEFTBRACKET
- 23, // 34 0x22 0x17 I QZ_i
- 25, // 35 0x23 0x19 P QZ_p
- 28, // 36 0x24 0x1c ENTER QZ_RETURN
- 38, // 37 0x25 0x26 L QZ_l
- 36, // 38 0x26 0x24 J QZ_j
- 40, // 39 0x27 0x28 ' QZ_QUOTE
- 37, // 40 0x28 0x25 K QZ_k
- 39, // 41 0x29 0x27 ; QZ_SEMICOLON
- 43, // 42 0x2A 0x2b \ QZ_BACKSLASH
- 51, // 43 0x2B 0x33 , QZ_COMMA
- 53, // 44 0x2C 0x35 / QZ_SLASH
- 49, // 45 0x2D 0x31 N QZ_n
- 50, // 46 0x2E 0x32 M QZ_m
- 52, // 47 0x2F 0x34 . QZ_PERIOD
- 15, // 48 0x30 0x0f TAB QZ_TAB
- 57, // 49 0x31 0x39 SPACE QZ_SPACE
- 41, // 50 0x32 0x29 ` QZ_BACKQUOTE
- 14, // 51 0x33 0x0e BKSP QZ_BACKSPACE
- 0, // 52 0x34 Undefined
- 1, // 53 0x35 0x01 ESC QZ_ESCAPE
- 220, // 54 0x36 0xdc E0,5C R GUI QZ_RMETA
- 219, // 55 0x37 0xdb E0,5B L GUI QZ_LMETA
- 42, // 56 0x38 0x2a L SHFT QZ_LSHIFT
- 58, // 57 0x39 0x3a CAPS QZ_CAPSLOCK
- 56, // 58 0x3A 0x38 L ALT QZ_LALT
- 29, // 59 0x3B 0x1d L CTRL QZ_LCTRL
- 54, // 60 0x3C 0x36 R SHFT QZ_RSHIFT
- 184,// 61 0x3D 0xb8 E0,38 R ALT QZ_RALT
- 157,// 62 0x3E 0x9d E0,1D R CTRL QZ_RCTRL
- 0, // 63 0x3F Undefined
- 0, // 64 0x40 Undefined
- 0, // 65 0x41 Undefined
- 0, // 66 0x42 Undefined
- 55, // 67 0x43 0x37 KP * QZ_KP_MULTIPLY
- 0, // 68 0x44 Undefined
- 78, // 69 0x45 0x4e KP + QZ_KP_PLUS
- 0, // 70 0x46 Undefined
- 69, // 71 0x47 0x45 NUM QZ_NUMLOCK
- 0, // 72 0x48 Undefined
- 0, // 73 0x49 Undefined
- 0, // 74 0x4A Undefined
- 181,// 75 0x4B 0xb5 E0,35 KP / QZ_KP_DIVIDE
- 152,// 76 0x4C 0x9c E0,1C KP EN QZ_KP_ENTER
- 0, // 77 0x4D undefined
- 74, // 78 0x4E 0x4a KP - QZ_KP_MINUS
- 0, // 79 0x4F Undefined
- 0, // 80 0x50 Undefined
- 0, // 81 0x51 QZ_KP_EQUALS
- 82, // 82 0x52 0x52 KP 0 QZ_KP0
- 79, // 83 0x53 0x4f KP 1 QZ_KP1
- 80, // 84 0x54 0x50 KP 2 QZ_KP2
- 81, // 85 0x55 0x51 KP 3 QZ_KP3
- 75, // 86 0x56 0x4b KP 4 QZ_KP4
- 76, // 87 0x57 0x4c KP 5 QZ_KP5
- 77, // 88 0x58 0x4d KP 6 QZ_KP6
- 71, // 89 0x59 0x47 KP 7 QZ_KP7
- 0, // 90 0x5A Undefined
- 72, // 91 0x5B 0x48 KP 8 QZ_KP8
- 73, // 92 0x5C 0x49 KP 9 QZ_KP9
- 0, // 93 0x5D Undefined
- 0, // 94 0x5E Undefined
- 0, // 95 0x5F Undefined
- 63, // 96 0x60 0x3f F5 QZ_F5
- 64, // 97 0x61 0x40 F6 QZ_F6
- 65, // 98 0x62 0x41 F7 QZ_F7
- 61, // 99 0x63 0x3d F3 QZ_F3
- 66, // 100 0x64 0x42 F8 QZ_F8
- 67, // 101 0x65 0x43 F9 QZ_F9
- 0, // 102 0x66 Undefined
- 87, // 103 0x67 0x57 F11 QZ_F11
- 0, // 104 0x68 Undefined
- 183,// 105 0x69 0xb7 QZ_PRINT
- 0, // 106 0x6A Undefined
- 70, // 107 0x6B 0x46 SCROLL QZ_SCROLLOCK
- 0, // 108 0x6C Undefined
- 68, // 109 0x6D 0x44 F10 QZ_F10
- 0, // 110 0x6E Undefined
- 88, // 111 0x6F 0x58 F12 QZ_F12
- 0, // 112 0x70 Undefined
- 110,// 113 0x71 0x0 QZ_PAUSE
- 210,// 114 0x72 0xd2 E0,52 INSERT QZ_INSERT
- 199,// 115 0x73 0xc7 E0,47 HOME QZ_HOME
- 201,// 116 0x74 0xc9 E0,49 PG UP QZ_PAGEUP
- 211,// 117 0x75 0xd3 E0,53 DELETE QZ_DELETE
- 62, // 118 0x76 0x3e F4 QZ_F4
- 207,// 119 0x77 0xcf E0,4f END QZ_END
- 60, // 120 0x78 0x3c F2 QZ_F2
- 209,// 121 0x79 0xd1 E0,51 PG DN QZ_PAGEDOWN
- 59, // 122 0x7A 0x3b F1 QZ_F1
- 203,// 123 0x7B 0xcb e0,4B L ARROW QZ_LEFT
- 205,// 124 0x7C 0xcd e0,4D R ARROW QZ_RIGHT
- 208,// 125 0x7D 0xd0 E0,50 D ARROW QZ_DOWN
- 200,// 126 0x7E 0xc8 E0,48 U ARROW QZ_UP
-/* completed according to http://www.libsdl.org/cgi/cvsweb.cgi/SDL12/src/video/quartz/SDL_QuartzKeys.h?rev=1.6&content-type=text/x-cvsweb-markup */
-
-/* Additional 104 Key XP-Keyboard Scancodes from http://www.computer-engineering.org/ps2keyboard/scancodes1.html */
-/*
- 221 // 0xdd e0,5d APPS
- // E0,2A,E0,37 PRNT SCRN
- // E1,1D,45,E1,9D,C5 PAUSE
- 83 // 0x53 0x53 KP .
-// ACPI Scan Codes
- 222 // 0xde E0, 5E Power
- 223 // 0xdf E0, 5F Sleep
- 227 // 0xe3 E0, 63 Wake
-// Windows Multimedia Scan Codes
- 153 // 0x99 E0, 19 Next Track
- 144 // 0x90 E0, 10 Previous Track
- 164 // 0xa4 E0, 24 Stop
- 162 // 0xa2 E0, 22 Play/Pause
- 160 // 0xa0 E0, 20 Mute
- 176 // 0xb0 E0, 30 Volume Up
- 174 // 0xae E0, 2E Volume Down
- 237 // 0xed E0, 6D Media Select
- 236 // 0xec E0, 6C E-Mail
- 161 // 0xa1 E0, 21 Calculator
- 235 // 0xeb E0, 6B My Computer
- 229 // 0xe5 E0, 65 WWW Search
- 178 // 0xb2 E0, 32 WWW Home
- 234 // 0xea E0, 6A WWW Back
- 233 // 0xe9 E0, 69 WWW Forward
- 232 // 0xe8 E0, 68 WWW Stop
- 231 // 0xe7 E0, 67 WWW Refresh
- 230 // 0xe6 E0, 66 WWW Favorites
-*/
+// Mac to QKeyCode conversion
+const int macToQKeyCodeMap[MAC_KEY_LARGEST_VALUE + 1] = {
+ [MAC_KEY_A] = Q_KEY_CODE_A,
+ [MAC_KEY_B] = Q_KEY_CODE_B,
+ [MAC_KEY_C] = Q_KEY_CODE_C,
+ [MAC_KEY_D] = Q_KEY_CODE_D,
+ [MAC_KEY_E] = Q_KEY_CODE_E,
+ [MAC_KEY_F] = Q_KEY_CODE_F,
+ [MAC_KEY_G] = Q_KEY_CODE_G,
+ [MAC_KEY_H] = Q_KEY_CODE_H,
+ [MAC_KEY_I] = Q_KEY_CODE_I,
+ [MAC_KEY_J] = Q_KEY_CODE_J,
+ [MAC_KEY_K] = Q_KEY_CODE_K,
+ [MAC_KEY_L] = Q_KEY_CODE_L,
+ [MAC_KEY_M] = Q_KEY_CODE_M,
+ [MAC_KEY_N] = Q_KEY_CODE_N,
+ [MAC_KEY_O] = Q_KEY_CODE_O,
+ [MAC_KEY_P] = Q_KEY_CODE_P,
+ [MAC_KEY_Q] = Q_KEY_CODE_Q,
+ [MAC_KEY_R] = Q_KEY_CODE_R,
+ [MAC_KEY_S] = Q_KEY_CODE_S,
+ [MAC_KEY_T] = Q_KEY_CODE_T,
+ [MAC_KEY_U] = Q_KEY_CODE_U,
+ [MAC_KEY_V] = Q_KEY_CODE_V,
+ [MAC_KEY_W] = Q_KEY_CODE_W,
+ [MAC_KEY_X] = Q_KEY_CODE_X,
+ [MAC_KEY_Y] = Q_KEY_CODE_Y,
+ [MAC_KEY_Z] = Q_KEY_CODE_Z,
+
+ [MAC_KEY_0] = Q_KEY_CODE_0,
+ [MAC_KEY_1] = Q_KEY_CODE_1,
+ [MAC_KEY_2] = Q_KEY_CODE_2,
+ [MAC_KEY_3] = Q_KEY_CODE_3,
+ [MAC_KEY_4] = Q_KEY_CODE_4,
+ [MAC_KEY_5] = Q_KEY_CODE_5,
+ [MAC_KEY_6] = Q_KEY_CODE_6,
+ [MAC_KEY_7] = Q_KEY_CODE_7,
+ [MAC_KEY_8] = Q_KEY_CODE_8,
+ [MAC_KEY_9] = Q_KEY_CODE_9,
+
+ [MAC_KEY_GRAVE_ACCENT] = Q_KEY_CODE_GRAVE_ACCENT,
+ [MAC_KEY_MINUS] = Q_KEY_CODE_MINUS,
+ [MAC_KEY_EQUAL] = Q_KEY_CODE_EQUAL,
+ [MAC_KEY_DELETE] = Q_KEY_CODE_BACKSPACE,
+ [MAC_KEY_CAPS_LOCK] = Q_KEY_CODE_CAPS_LOCK,
+ [MAC_KEY_TAB] = Q_KEY_CODE_TAB,
+ [MAC_KEY_RETURN] = Q_KEY_CODE_RET,
+ [MAC_KEY_LEFT_BRACKET] = Q_KEY_CODE_BRACKET_LEFT,
+ [MAC_KEY_RIGHT_BRACKET] = Q_KEY_CODE_BRACKET_RIGHT,
+ [MAC_KEY_BACKSLASH] = Q_KEY_CODE_BACKSLASH,
+ [MAC_KEY_SEMICOLON] = Q_KEY_CODE_SEMICOLON,
+ [MAC_KEY_APOSTROPHE] = Q_KEY_CODE_APOSTROPHE,
+ [MAC_KEY_COMMA] = Q_KEY_CODE_COMMA,
+ [MAC_KEY_PERIOD] = Q_KEY_CODE_DOT,
+ [MAC_KEY_FORWARD_SLASH] = Q_KEY_CODE_SLASH,
+ [MAC_KEY_LEFT_SHIFT] = Q_KEY_CODE_SHIFT,
+ [MAC_KEY_RIGHT_SHIFT] = Q_KEY_CODE_SHIFT_R,
+ [MAC_KEY_LEFT_CONTROL] = Q_KEY_CODE_CTRL,
+ [MAC_KEY_RIGHT_CONTROL] = Q_KEY_CODE_CTRL_R,
+ [MAC_KEY_LEFT_OPTION] = Q_KEY_CODE_ALT,
+ [MAC_KEY_RIGHT_OPTION] = Q_KEY_CODE_ALT_R,
+ [MAC_KEY_LEFT_COMMAND] = Q_KEY_CODE_META_L,
+ [MAC_KEY_RIGHT_COMMAND] = Q_KEY_CODE_META_R,
+ [MAC_KEY_SPACEBAR] = Q_KEY_CODE_SPC,
+
+ [MAC_KEY_KP_0] = Q_KEY_CODE_KP_0,
+ [MAC_KEY_KP_1] = Q_KEY_CODE_KP_1,
+ [MAC_KEY_KP_2] = Q_KEY_CODE_KP_2,
+ [MAC_KEY_KP_3] = Q_KEY_CODE_KP_3,
+ [MAC_KEY_KP_4] = Q_KEY_CODE_KP_4,
+ [MAC_KEY_KP_5] = Q_KEY_CODE_KP_5,
+ [MAC_KEY_KP_6] = Q_KEY_CODE_KP_6,
+ [MAC_KEY_KP_7] = Q_KEY_CODE_KP_7,
+ [MAC_KEY_KP_8] = Q_KEY_CODE_KP_8,
+ [MAC_KEY_KP_9] = Q_KEY_CODE_KP_9,
+ [MAC_KEY_KP_PERIOD] = Q_KEY_CODE_KP_DECIMAL,
+ [MAC_KEY_KP_ENTER] = Q_KEY_CODE_KP_ENTER,
+ [MAC_KEY_KP_PLUS] = Q_KEY_CODE_KP_ADD,
+ [MAC_KEY_KP_SUBTRACT] = Q_KEY_CODE_KP_SUBTRACT,
+ [MAC_KEY_KP_MULTIPLY] = Q_KEY_CODE_KP_MULTIPLY,
+ [MAC_KEY_KP_DIVIDE] = Q_KEY_CODE_KP_DIVIDE,
+ [MAC_KEY_KP_EQUAL] = Q_KEY_CODE_KP_EQUALS,
+ [MAC_KEY_KP_CLEAR] = Q_KEY_CODE_NUM_LOCK,
+
+ [MAC_KEY_UP] = Q_KEY_CODE_UP,
+ [MAC_KEY_DOWN] = Q_KEY_CODE_DOWN,
+ [MAC_KEY_LEFT] = Q_KEY_CODE_LEFT,
+ [MAC_KEY_RIGHT] = Q_KEY_CODE_RIGHT,
+
+ [MAC_KEY_HELP] = Q_KEY_CODE_INSERT,
+ [MAC_KEY_HOME] = Q_KEY_CODE_HOME,
+ [MAC_KEY_PAGE_UP] = Q_KEY_CODE_PGUP,
+ [MAC_KEY_PAGE_DOWN] = Q_KEY_CODE_PGDN,
+ [MAC_KEY_END] = Q_KEY_CODE_END,
+ [MAC_KEY_FORWARD_DELETE] = Q_KEY_CODE_DELETE,
+
+ [MAC_KEY_ESC] = Q_KEY_CODE_ESC,
+ //[MAC_KEY_F1] = Q_KEY_CODE_POWER, // Just in case you need the power key
+ [MAC_KEY_F1] = Q_KEY_CODE_F1,
+ [MAC_KEY_F2] = Q_KEY_CODE_F2,
+ [MAC_KEY_F3] = Q_KEY_CODE_F3,
+ [MAC_KEY_F4] = Q_KEY_CODE_F4,
+ [MAC_KEY_F5] = Q_KEY_CODE_F5,
+ [MAC_KEY_F6] = Q_KEY_CODE_F6,
+ [MAC_KEY_F7] = Q_KEY_CODE_F7,
+ [MAC_KEY_F8] = Q_KEY_CODE_F8,
+ [MAC_KEY_F9] = Q_KEY_CODE_F9,
+ [MAC_KEY_F10] = Q_KEY_CODE_F10,
+ [MAC_KEY_F11] = Q_KEY_CODE_F11,
+ [MAC_KEY_F12] = Q_KEY_CODE_F12,
+ [MAC_KEY_F13] = Q_KEY_CODE_PRINT,
+ [MAC_KEY_F14] = Q_KEY_CODE_SCROLL_LOCK,
+ [MAC_KEY_F15] = Q_KEY_CODE_PAUSE,
+
+ /*
+ * The eject and volume keys can't be used here because they are handled at
+ * a lower level than what an Application can see.
+ */
};
static int cocoa_keycode_to_qemu(int keycode)
{
- if (ARRAY_SIZE(keymap) <= keycode) {
+ if (ARRAY_SIZE(macToQKeyCodeMap) <= keycode) {
fprintf(stderr, "(cocoa) warning unknown keycode 0x%x\n", keycode);
return 0;
}
- return keymap[keycode];
+ return macToQKeyCodeMap[keycode];
}
/* Displays an alert dialog box with the specified message */
@@ -564,14 +520,14 @@ QemuCocoaView *cocoaView;
if (keycode) {
if (keycode == 58 || keycode == 69) { // emulate caps lock and num lock keydown and keyup
- qemu_input_event_send_key_number(dcl->con, keycode, true);
- qemu_input_event_send_key_number(dcl->con, keycode, false);
+ qemu_input_event_send_key_qcode(dcl->con, keycode, true);
+ qemu_input_event_send_key_qcode(dcl->con, keycode, false);
} else if (qemu_console_is_graphic(NULL)) {
if (modifiers_state[keycode] == 0) { // keydown
- qemu_input_event_send_key_number(dcl->con, keycode, true);
+ qemu_input_event_send_key_qcode(dcl->con, keycode, true);
modifiers_state[keycode] = 1;
} else { // keyup
- qemu_input_event_send_key_number(dcl->con, keycode, false);
+ qemu_input_event_send_key_qcode(dcl->con, keycode, false);
modifiers_state[keycode] = 0;
}
}
@@ -605,7 +561,7 @@ QemuCocoaView *cocoaView;
// handle keys for graphic console
} else if (qemu_console_is_graphic(NULL)) {
- qemu_input_event_send_key_number(dcl->con, keycode, true);
+ qemu_input_event_send_key_qcode(dcl->con, keycode, true);
// handlekeys for Monitor
} else {
@@ -653,7 +609,7 @@ QemuCocoaView *cocoaView;
}
if (qemu_console_is_graphic(NULL)) {
- qemu_input_event_send_key_number(dcl->con, keycode, false);
+ qemu_input_event_send_key_qcode(dcl->con, keycode, false);
}
break;
case NSMouseMoved:
@@ -823,7 +779,7 @@ QemuCocoaView *cocoaView;
for (index = 0; index < max_index; index++) {
if (modifiers_state[index]) {
modifiers_state[index] = 0;
- qemu_input_event_send_key_number(dcl->con, index, false);
+ qemu_input_event_send_key_qcode(dcl->con, index, false);
}
}
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Replace pc/xt keyboard keycode array with QKeyCode
2016-03-01 22:12 [Qemu-devel] [PATCH] ui/cocoa.m: Replace pc/xt keyboard keycode array with QKeyCode Programmingkid
@ 2016-03-01 23:16 ` Eric Blake
2016-03-02 0:12 ` Programmingkid
2016-03-01 23:18 ` Peter Maydell
1 sibling, 1 reply; 9+ messages in thread
From: Eric Blake @ 2016-03-01 23:16 UTC (permalink / raw)
To: Programmingkid, Peter Maydell, qemu-devel qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]
On 03/01/2016 03:12 PM, Programmingkid wrote:
> The old pc/xt keyboard keycode array is replaced with QEMU's own QKeyCode
> layout.
>
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>
> ---
> Maintainer note:
> Please apply these patches before testing:
> - qapi-schema.json: Add kp_equals and power keys
> - adb.c: Replace pc_to_adb_keycode with more detailed array
> - MacKeys.h: initial commit
Since this depends on those patches, it might have been nicer to submit
it as a series with a 0/4 cover letter, rather than making us chase
separate threads.
> +
> + [MAC_KEY_ESC] = Q_KEY_CODE_ESC,
> + //[MAC_KEY_F1] = Q_KEY_CODE_POWER, // Just in case you need the power key
> + [MAC_KEY_F1] = Q_KEY_CODE_F1,
The comment looks weird. Probably worth a mention in the commit message
why you need it.
>
> static int cocoa_keycode_to_qemu(int keycode)
> {
> - if (ARRAY_SIZE(keymap) <= keycode) {
> + if (ARRAY_SIZE(macToQKeyCodeMap) <= keycode) {
> fprintf(stderr, "(cocoa) warning unknown keycode 0x%x\n", keycode);
Pre-existing, but we should fix this to avoid fprintf.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Replace pc/xt keyboard keycode array with QKeyCode
2016-03-01 22:12 [Qemu-devel] [PATCH] ui/cocoa.m: Replace pc/xt keyboard keycode array with QKeyCode Programmingkid
2016-03-01 23:16 ` Eric Blake
@ 2016-03-01 23:18 ` Peter Maydell
2016-03-02 0:18 ` Programmingkid
1 sibling, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2016-03-01 23:18 UTC (permalink / raw)
To: Programmingkid; +Cc: qemu-devel qemu-devel
On 1 March 2016 at 22:12, Programmingkid <programmingkidx@gmail.com> wrote:
> The old pc/xt keyboard keycode array is replaced with QEMU's own QKeyCode
> layout.
>
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>
> ---
> Maintainer note:
> Please apply these patches before testing:
> - qapi-schema.json: Add kp_equals and power keys
> - adb.c: Replace pc_to_adb_keycode with more detailed array
> - MacKeys.h: initial commit
The correct way to do this is to send the patches as a single
patch series, which then will have them all in the right order,
together with 'cover letter' email which explains the series
and has all the patch mails as followups to it. Look at how
other people send out mails for multi-patch features.
(git format-patch and git send-email can do this for you.)
>
> ui/cocoa.m | 300 ++++++++++++++++++++++++++----------------------------------
> 1 files changed, 128 insertions(+), 172 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 3ee5549..8ea39ad 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -33,6 +33,7 @@
> #include "sysemu/sysemu.h"
> #include "qmp-commands.h"
> #include "sysemu/blockdev.h"
> +#include "include/hw/input/MacKeys.h"
This should use the OSX header for the keycodes.
> static int cocoa_keycode_to_qemu(int keycode)
> {
> - if (ARRAY_SIZE(keymap) <= keycode) {
> + if (ARRAY_SIZE(macToQKeyCodeMap) <= keycode) {
> fprintf(stderr, "(cocoa) warning unknown keycode 0x%x\n", keycode);
> return 0;
> }
> - return keymap[keycode];
> + return macToQKeyCodeMap[keycode];
> }
>
> /* Displays an alert dialog box with the specified message */
> @@ -564,14 +520,14 @@ QemuCocoaView *cocoaView;
>
> if (keycode) {
> if (keycode == 58 || keycode == 69) { // emulate caps lock and num lock keydown and keyup
Because cocoa_keycode_to_qemu() now returns a Q_KEY_* constant,
you need to change all the places like this that were checking
against hardcoded 58, 69, etc, to check against the correct
Q_KEY_* instead. (They're not the same numbers, so otherwise these
special cases will be broken.)
Otherwise this patch looks OK.
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Replace pc/xt keyboard keycode array with QKeyCode
2016-03-01 23:16 ` Eric Blake
@ 2016-03-02 0:12 ` Programmingkid
2016-03-02 0:25 ` Eric Blake
0 siblings, 1 reply; 9+ messages in thread
From: Programmingkid @ 2016-03-02 0:12 UTC (permalink / raw)
To: Eric Blake; +Cc: Peter Maydell, qemu-devel qemu-devel
On Mar 1, 2016, at 6:16 PM, Eric Blake wrote:
> On 03/01/2016 03:12 PM, Programmingkid wrote:
>> The old pc/xt keyboard keycode array is replaced with QEMU's own QKeyCode
>> layout.
>>
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>
>> ---
>> Maintainer note:
>> Please apply these patches before testing:
>> - qapi-schema.json: Add kp_equals and power keys
>> - adb.c: Replace pc_to_adb_keycode with more detailed array
>> - MacKeys.h: initial commit
>
> Since this depends on those patches, it might have been nicer to submit
> it as a series with a 0/4 cover letter, rather than making us chase
> separate threads.
Learning how to use git send-email is now on my to-do list.
>
>> +
>> + [MAC_KEY_ESC] = Q_KEY_CODE_ESC,
>> + //[MAC_KEY_F1] = Q_KEY_CODE_POWER, // Just in case you need the power key
>> + [MAC_KEY_F1] = Q_KEY_CODE_F1,
>
> The comment looks weird. Probably worth a mention in the commit message
> why you need it.
Maybe this:
[MAC_KEY_ESC] = Q_KEY_CODE_ESC,
//[MAC_KEY_F1] = Q_KEY_CODE_POWER, // You might need this key if you use Macsbug.
[MAC_KEY_F1] = Q_KEY_CODE_F1,
>
>>
>> static int cocoa_keycode_to_qemu(int keycode)
>> {
>> - if (ARRAY_SIZE(keymap) <= keycode) {
>> + if (ARRAY_SIZE(macToQKeyCodeMap) <= keycode) {
>> fprintf(stderr, "(cocoa) warning unknown keycode 0x%x\n", keycode);
>
> Pre-existing, but we should fix this to avoid fprintf.
What do you mean by pre-existing? I personally don't have anything against fprintf, but switching to printf is just fine with me.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Replace pc/xt keyboard keycode array with QKeyCode
2016-03-01 23:18 ` Peter Maydell
@ 2016-03-02 0:18 ` Programmingkid
0 siblings, 0 replies; 9+ messages in thread
From: Programmingkid @ 2016-03-02 0:18 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel qemu-devel
On Mar 1, 2016, at 6:18 PM, Peter Maydell wrote:
> On 1 March 2016 at 22:12, Programmingkid <programmingkidx@gmail.com> wrote:
>> The old pc/xt keyboard keycode array is replaced with QEMU's own QKeyCode
>> layout.
>>
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>>
>> ---
>> Maintainer note:
>> Please apply these patches before testing:
>> - qapi-schema.json: Add kp_equals and power keys
>> - adb.c: Replace pc_to_adb_keycode with more detailed array
>> - MacKeys.h: initial commit
>
> The correct way to do this is to send the patches as a single
> patch series, which then will have them all in the right order,
> together with 'cover letter' email which explains the series
> and has all the patch mails as followups to it. Look at how
> other people send out mails for multi-patch features.
> (git format-patch and git send-email can do this for you.)
Yes you are right. I will learn how to use git send-email.
>
>>
>> ui/cocoa.m | 300 ++++++++++++++++++++++++++----------------------------------
>> 1 files changed, 128 insertions(+), 172 deletions(-)
>>
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index 3ee5549..8ea39ad 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -33,6 +33,7 @@
>> #include "sysemu/sysemu.h"
>> #include "qmp-commands.h"
>> #include "sysemu/blockdev.h"
>> +#include "include/hw/input/MacKeys.h"
>
> This should use the OSX header for the keycodes.
If you really prefer I use Events.h and its keycodes, it shouldn't be a problem. A quick Find & Replace should make the changes that are needed.
>
>> static int cocoa_keycode_to_qemu(int keycode)
>> {
>> - if (ARRAY_SIZE(keymap) <= keycode) {
>> + if (ARRAY_SIZE(macToQKeyCodeMap) <= keycode) {
>> fprintf(stderr, "(cocoa) warning unknown keycode 0x%x\n", keycode);
>> return 0;
>> }
>> - return keymap[keycode];
>> + return macToQKeyCodeMap[keycode];
>> }
>>
>> /* Displays an alert dialog box with the specified message */
>> @@ -564,14 +520,14 @@ QemuCocoaView *cocoaView;
>>
>> if (keycode) {
>> if (keycode == 58 || keycode == 69) { // emulate caps lock and num lock keydown and keyup
>
> Because cocoa_keycode_to_qemu() now returns a Q_KEY_* constant,
> you need to change all the places like this that were checking
> against hardcoded 58, 69, etc, to check against the correct
> Q_KEY_* instead. (They're not the same numbers, so otherwise these
> special cases will be broken.)
Good idea.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Replace pc/xt keyboard keycode array with QKeyCode
2016-03-02 0:12 ` Programmingkid
@ 2016-03-02 0:25 ` Eric Blake
2016-03-02 1:20 ` Programmingkid
0 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2016-03-02 0:25 UTC (permalink / raw)
To: Programmingkid; +Cc: Peter Maydell, qemu-devel qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1911 bytes --]
On 03/01/2016 05:12 PM, Programmingkid wrote:
>>
>>> +
>>> + [MAC_KEY_ESC] = Q_KEY_CODE_ESC,
>>> + //[MAC_KEY_F1] = Q_KEY_CODE_POWER, // Just in case you need the power key
>>> + [MAC_KEY_F1] = Q_KEY_CODE_F1,
>>
>> The comment looks weird. Probably worth a mention in the commit message
>> why you need it.
>
> Maybe this:
>
> [MAC_KEY_ESC] = Q_KEY_CODE_ESC,
> //[MAC_KEY_F1] = Q_KEY_CODE_POWER, // You might need this key if you use Macsbug.
> [MAC_KEY_F1] = Q_KEY_CODE_F1,
Not a code comment, but a commit message comment stating why you aren't
providing a mapping for q_KEY_CODE_POWER. For that matter, I don't
think a code comment is useful, just nuke the entire line (the comment
doesn't add anything).
>
>
>>
>>>
>>> static int cocoa_keycode_to_qemu(int keycode)
>>> {
>>> - if (ARRAY_SIZE(keymap) <= keycode) {
>>> + if (ARRAY_SIZE(macToQKeyCodeMap) <= keycode) {
>>> fprintf(stderr, "(cocoa) warning unknown keycode 0x%x\n", keycode);
>>
>> Pre-existing, but we should fix this to avoid fprintf.
>
> What do you mean by pre-existing?
You weren't the original cause of the bug, so it is not necessarily this
patch's job to fix the bug. Therefore, "pre-existing". But since the
bug was observed during review of your patch, you may want to fix it
anyways, probably as a separate patch.
> I personally don't have anything against fprintf, but switching to printf is just fine with me.
No, don't switch to printf. The bug is that we DON'T want to use printf
or fprintf for error reporting. Rather, you should do proper error
reporting, such as populating an Error **errp parameter, if the error
needs reporting at all; or else delete the line if the code continues
just fine in spite of the unknown keycode.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Replace pc/xt keyboard keycode array with QKeyCode
2016-03-02 0:25 ` Eric Blake
@ 2016-03-02 1:20 ` Programmingkid
2016-03-02 2:14 ` Eric Blake
0 siblings, 1 reply; 9+ messages in thread
From: Programmingkid @ 2016-03-02 1:20 UTC (permalink / raw)
To: Eric Blake; +Cc: Peter Maydell, qemu-devel qemu-devel
On Mar 1, 2016, at 7:25 PM, Eric Blake wrote:
> On 03/01/2016 05:12 PM, Programmingkid wrote:
>
>>>
>>>> +
>>>> + [MAC_KEY_ESC] = Q_KEY_CODE_ESC,
>>>> + //[MAC_KEY_F1] = Q_KEY_CODE_POWER, // Just in case you need the power key
>>>> + [MAC_KEY_F1] = Q_KEY_CODE_F1,
>>>
>>> The comment looks weird. Probably worth a mention in the commit message
>>> why you need it.
>>
>> Maybe this:
>>
>> [MAC_KEY_ESC] = Q_KEY_CODE_ESC,
>> //[MAC_KEY_F1] = Q_KEY_CODE_POWER, // You might need this key if you use Macsbug.
>> [MAC_KEY_F1] = Q_KEY_CODE_F1,
>
> Not a code comment, but a commit message comment stating why you aren't
> providing a mapping for q_KEY_CODE_POWER. For that matter, I don't
> think a code comment is useful, just nuke the entire line (the comment
> doesn't add anything).
>
>>
>>
>>>
>>>>
>>>> static int cocoa_keycode_to_qemu(int keycode)
>>>> {
>>>> - if (ARRAY_SIZE(keymap) <= keycode) {
>>>> + if (ARRAY_SIZE(macToQKeyCodeMap) <= keycode) {
>>>> fprintf(stderr, "(cocoa) warning unknown keycode 0x%x\n", keycode);
>>>
>>> Pre-existing, but we should fix this to avoid fprintf.
>>
>> What do you mean by pre-existing?
>
> You weren't the original cause of the bug, so it is not necessarily this
> patch's job to fix the bug. Therefore, "pre-existing". But since the
> bug was observed during review of your patch, you may want to fix it
> anyways, probably as a separate patch.
So you want this:
if (ARRAY_SIZE(macToQKeyCodeMap) <= keycode) {
error_report("(cocoa) warning unknown keycode 0x%x\n", keycode);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Replace pc/xt keyboard keycode array with QKeyCode
2016-03-02 1:20 ` Programmingkid
@ 2016-03-02 2:14 ` Eric Blake
2016-03-02 9:29 ` Markus Armbruster
0 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2016-03-02 2:14 UTC (permalink / raw)
To: Programmingkid; +Cc: Peter Maydell, qemu-devel qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1039 bytes --]
On 03/01/2016 06:20 PM, Programmingkid wrote:
>> You weren't the original cause of the bug, so it is not necessarily this
>> patch's job to fix the bug. Therefore, "pre-existing". But since the
>> bug was observed during review of your patch, you may want to fix it
>> anyways, probably as a separate patch.
>
> So you want this:
>
> if (ARRAY_SIZE(macToQKeyCodeMap) <= keycode) {
> error_report("(cocoa) warning unknown keycode 0x%x\n", keycode);
Or something similar. Yes, error_report() is better than fprintf. But
error_report() is only good if you are directly interacting with the
user; if this code can be reached via a QMP monitor command, it would be
better to adjust signatures and propagate an Error **errp back to the
caller, so that the caller knows how best to report it. But that's more
plumbing effort, so it doesn't necessarily have to be you doing the
work, nor this series.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH] ui/cocoa.m: Replace pc/xt keyboard keycode array with QKeyCode
2016-03-02 2:14 ` Eric Blake
@ 2016-03-02 9:29 ` Markus Armbruster
0 siblings, 0 replies; 9+ messages in thread
From: Markus Armbruster @ 2016-03-02 9:29 UTC (permalink / raw)
To: Eric Blake; +Cc: Programmingkid, qemu-devel qemu-devel, Peter Maydell
Eric Blake <eblake@redhat.com> writes:
> On 03/01/2016 06:20 PM, Programmingkid wrote:
>
>>> You weren't the original cause of the bug, so it is not necessarily this
>>> patch's job to fix the bug. Therefore, "pre-existing". But since the
>>> bug was observed during review of your patch, you may want to fix it
>>> anyways, probably as a separate patch.
>>
>> So you want this:
>>
>> if (ARRAY_SIZE(macToQKeyCodeMap) <= keycode) {
>> error_report("(cocoa) warning unknown keycode 0x%x\n", keycode);
>
> Or something similar. Yes, error_report() is better than fprintf. But
> error_report() is only good if you are directly interacting with the
> user; if this code can be reached via a QMP monitor command, it would be
> better to adjust signatures and propagate an Error **errp back to the
> caller, so that the caller knows how best to report it.
An error that makes a QMP command fail must be propagated to the QMP
core with Error objects. To find out how, study the big comment in
error.h, and the copious examples in the code.
Reporting such errors with error_report(), fprintf() or similar instead
is a bug.
> But that's more
> plumbing effort, so it doesn't necessarily have to be you doing the
> work, nor this series.
Yes.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-03-02 9:29 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-01 22:12 [Qemu-devel] [PATCH] ui/cocoa.m: Replace pc/xt keyboard keycode array with QKeyCode Programmingkid
2016-03-01 23:16 ` Eric Blake
2016-03-02 0:12 ` Programmingkid
2016-03-02 0:25 ` Eric Blake
2016-03-02 1:20 ` Programmingkid
2016-03-02 2:14 ` Eric Blake
2016-03-02 9:29 ` Markus Armbruster
2016-03-01 23:18 ` Peter Maydell
2016-03-02 0:18 ` Programmingkid
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.