From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1K3zvG-0002nR-2o for qemu-devel@nongnu.org; Wed, 04 Jun 2008 16:49:30 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1K3zvF-0002nF-8p for qemu-devel@nongnu.org; Wed, 04 Jun 2008 16:49:29 -0400 Received: from [199.232.76.173] (port=44887 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1K3zvF-0002nC-2m for qemu-devel@nongnu.org; Wed, 04 Jun 2008 16:49:29 -0400 Received: from an-out-0708.google.com ([209.85.132.249]:11220) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1K3zvE-0004G5-IO for qemu-devel@nongnu.org; Wed, 04 Jun 2008 16:49:28 -0400 Received: by an-out-0708.google.com with SMTP id d18so72435and.130 for ; Wed, 04 Jun 2008 13:49:28 -0700 (PDT) Message-ID: <4846FFBE.8020106@codemonkey.ws> Date: Wed, 04 Jun 2008 15:49:02 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] Enhance sendkey with key hold time - v2 References: <48357281.1090501@web.de> <4846DCF5.5030506@web.de> In-Reply-To: <4846DCF5.5030506@web.de> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Jan Kiszka wrote: > [ Addressing reviewer comments ] > > Current key injection via the monitor basically generates no key hold > time. This is fine for keyboard emulations that have their own queues, > but it causes troubles for those how don't (like the MusicPal - it > simply does not work with injected keys). Moreover, I would like to use > this mechanism to simulate pressed buttons during power-up. > > Therefore, this patch enhances the key injection with a configurable > release delay (by default 100 virtual milliseconds). > > This feature allows to get rid of the initial sleep() in musicpal_init > because one can now simply start qemu with -S and issue "sendkey m 1000" > and "continue" in the monitor to achieve the desired effect of a pressed > menu button during power-up. So there is no need for a per-musicpal or > even qemu-wide "-hold-button" switch. > Looks good to me. Reviewed-by: Anthony Liguori Regards, Anthony Liguori > Signed-off-by: Jan Kiszka > --- > hw/musicpal.c | 6 ------ > monitor.c | 58 +++++++++++++++++++++++++++++++++++++++------------------- > 2 files changed, 39 insertions(+), 25 deletions(-) > > Index: b/hw/musicpal.c > =================================================================== > --- a/hw/musicpal.c > +++ b/hw/musicpal.c > @@ -1504,12 +1504,6 @@ static void musicpal_init(ram_addr_t ram > > qemu_add_kbd_event_handler(musicpal_key_event, pic[MP_GPIO_IRQ]); > > - /* > - * Wait a bit to catch menu button during U-Boot start-up > - * (to trigger emergency update). > - */ > - sleep(1); > - > mv88w8618_eth_init(&nd_table[0], MP_ETH_BASE, pic[MP_ETH_IRQ]); > > mixer_i2c = musicpal_audio_init(MP_AUDIO_BASE, pic[MP_AUDIO_IRQ]); > Index: b/monitor.c > =================================================================== > --- a/monitor.c > +++ b/monitor.c > @@ -35,10 +35,7 @@ > #include "audio/audio.h" > #include "disas.h" > #include > - > -#ifdef CONFIG_PROFILER > -#include "qemu-timer.h" /* for ticks_per_sec */ > -#endif > +#include "qemu-timer.h" > > //#define DEBUG > //#define DEBUG_COMPLETION > @@ -920,14 +917,37 @@ static int get_keycode(const char *key) > return -1; > } > > -static void do_sendkey(const char *string) > +#define MAX_KEYCODES 16 > +static uint8_t keycodes[MAX_KEYCODES]; > +static int nb_pending_keycodes; > +static QEMUTimer *key_timer; > + > +static void release_keys(void *opaque) > +{ > + int keycode; > + > + while (nb_pending_keycodes > 0) { > + nb_pending_keycodes--; > + keycode = keycodes[nb_pending_keycodes]; > + if (keycode & 0x80) > + kbd_put_keycode(0xe0); > + kbd_put_keycode(keycode | 0x80); > + } > +} > + > +static void do_sendkey(const char *string, int has_hold_time, int hold_time) > { > - uint8_t keycodes[16]; > - int nb_keycodes = 0; > char keyname_buf[16]; > char *separator; > int keyname_len, keycode, i; > > + if (nb_pending_keycodes > 0) { > + qemu_del_timer(key_timer); > + release_keys(NULL); > + } > + if (!has_hold_time) > + hold_time = 100; > + i = 0; > while (1) { > separator = strchr(string, '-'); > keyname_len = separator ? separator - string : strlen(string); > @@ -937,7 +957,7 @@ static void do_sendkey(const char *strin > term_printf("invalid key: '%s...'\n", keyname_buf); > return; > } > - if (nb_keycodes == sizeof(keycodes)) { > + if (i == MAX_KEYCODES) { > term_printf("too many keys\n"); > return; > } > @@ -947,26 +967,23 @@ static void do_sendkey(const char *strin > term_printf("unknown key: '%s'\n", keyname_buf); > return; > } > - keycodes[nb_keycodes++] = keycode; > + keycodes[i++] = keycode; > } > if (!separator) > break; > string = separator + 1; > } > + nb_pending_keycodes = i; > /* key down events */ > - for(i = 0; i < nb_keycodes; i++) { > + for (i = 0; i < nb_pending_keycodes; i++) { > keycode = keycodes[i]; > if (keycode & 0x80) > kbd_put_keycode(0xe0); > kbd_put_keycode(keycode & 0x7f); > } > - /* key up events */ > - for(i = nb_keycodes - 1; i >= 0; i--) { > - keycode = keycodes[i]; > - if (keycode & 0x80) > - kbd_put_keycode(0xe0); > - kbd_put_keycode(keycode | 0x80); > - } > + /* delayed key up events */ > + qemu_mod_timer(key_timer, > + qemu_get_clock(vm_clock) + ticks_per_sec * hold_time); > } > > static int mouse_button_state; > @@ -1353,8 +1370,8 @@ static term_cmd_t term_cmds[] = { > { "i", "/ii.", do_ioport_read, > "/fmt addr", "I/O port read" }, > > - { "sendkey", "s", do_sendkey, > - "keys", "send keys to the VM (e.g. 'sendkey ctrl-alt-f1')" }, > + { "sendkey", "si?", do_sendkey, > + "keys [hold_ms]", "send keys to the VM (e.g. 'sendkey ctrl-alt-f1', default hold time=100 ms)" }, > { "system_reset", "", do_system_reset, > "", "reset the system" }, > { "system_powerdown", "", do_system_powerdown, > @@ -2638,6 +2655,9 @@ void monitor_init(CharDriverState *hd, i > int i; > > if (is_first_init) { > + key_timer = qemu_new_timer(vm_clock, release_keys, NULL); > + if (!key_timer) > + return; > for (i = 0; i < MAX_MON; i++) { > monitor_hd[i] = NULL; > } > > >