[Qemu-devel] [PATCH 2/2] Enhance sendkey with key hold time

[Qemu-devel] [PATCH 2/2] Enhance sendkey with key hold time

[Jan Kiszka]

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.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 hw/musicpal.c |    6 ------
 monitor.c     |   57 ++++++++++++++++++++++++++++++++++++++-------------------
 2 files changed, 38 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 <dirent.h>
-
-#ifdef CONFIG_PROFILER
-#include "qemu-timer.h" /* for ticks_per_sec */
-#endif
+#include "qemu-timer.h"
 
 //#define DEBUG
 //#define DEBUG_COMPLETION
@@ -925,14 +922,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) {
+        keycode = keycodes[nb_pending_keycodes-1];
+        if (keycode & 0x80)
+            kbd_put_keycode(0xe0);
+        kbd_put_keycode(keycode | 0x80);
+        nb_pending_keycodes--;
+    }
+}
+
+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);
@@ -943,7 +963,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;
             }
@@ -953,26 +973,22 @@ 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) + 1000000LL*hold_time);
 }
 
 static int mouse_button_state;
@@ -1359,8 +1375,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,
@@ -2644,6 +2660,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;
         }

Re: [Qemu-devel] [PATCH 2/2] Enhance sendkey with key hold time

[andrzej zaborowski]

On 22/05/2008, Jan Kiszka <jan.kiszka@web.de> wrote:
> 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.
>
>  Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
>  ---
>   hw/musicpal.c |    6 ------
>   monitor.c     |   57 ++++++++++++++++++++++++++++++++++++++-------------------
>   2 files changed, 38 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 <dirent.h>
>  -
>  -#ifdef CONFIG_PROFILER
>  -#include "qemu-timer.h" /* for ticks_per_sec */
>  -#endif
>  +#include "qemu-timer.h"
>
>   //#define DEBUG
>   //#define DEBUG_COMPLETION
>  @@ -925,14 +922,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) {
>  +        keycode = keycodes[nb_pending_keycodes-1];
>  +        if (keycode & 0x80)
>  +            kbd_put_keycode(0xe0);
>  +        kbd_put_keycode(keycode | 0x80);
>  +        nb_pending_keycodes--;
>  +    }
>  +}
>  +
>  +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);
>  @@ -943,7 +963,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;
>              }
>  @@ -953,26 +973,22 @@ 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) + 1000000LL*hold_time);

This should use ticks_per_sec.

Also, 100ms seems high for the default value, I think you rarely press
keys for that long.

Cheers
-- 
Please do not print this email unless absolutely necessary. Spread
environmental awareness.

Re: [Qemu-devel] [PATCH 2/2] Enhance sendkey with key hold time

[Bernhard Fischer]

On Wed, Jun 04, 2008 at 12:16:48PM +0200, andrzej zaborowski wrote:
>On 22/05/2008, Jan Kiszka <jan.kiszka@web.de> wrote:

>>  +static void release_keys(void *opaque)
>>  +{
>>  +    int keycode;
>>  +
>>  +    while (nb_pending_keycodes > 0) {

Perhaps just doing a post decrement on the nb_pending would be cleaner.

>>  +        keycode = keycodes[nb_pending_keycodes-1];
>>  +        if (keycode & 0x80)
>>  +            kbd_put_keycode(0xe0);
>>  +        kbd_put_keycode(keycode | 0x80);
>>  +        nb_pending_keycodes--;
>>  +    }

[Qemu-devel] Re: [PATCH 2/2] Enhance sendkey with key hold time

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 729 bytes --]

Bernhard Fischer wrote:
> On Wed, Jun 04, 2008 at 12:16:48PM +0200, andrzej zaborowski wrote:
>> On 22/05/2008, Jan Kiszka <jan.kiszka@web.de> wrote:
> 
>>>  +static void release_keys(void *opaque)
>>>  +{
>>>  +    int keycode;
>>>  +
>>>  +    while (nb_pending_keycodes > 0) {
> 
> Perhaps just doing a post decrement on the nb_pending would be cleaner.

I prefer a separate statement for this but...

> 
>>>  +        keycode = keycodes[nb_pending_keycodes-1];
>>>  +        if (keycode & 0x80)
>>>  +            kbd_put_keycode(0xe0);
>>>  +        kbd_put_keycode(keycode | 0x80);
>>>  +        nb_pending_keycodes--;
>>>  +    }

I agree that code reordering can save us the "-1".

Thanks,
Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

[Qemu-devel] Re: [PATCH 2/2] Enhance sendkey with key hold time

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 5317 bytes --]

andrzej zaborowski wrote:
> On 22/05/2008, Jan Kiszka <jan.kiszka@web.de> wrote:
>> 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.
>>
>>  Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
>>  ---
>>   hw/musicpal.c |    6 ------
>>   monitor.c     |   57 ++++++++++++++++++++++++++++++++++++++-------------------
>>   2 files changed, 38 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 <dirent.h>
>>  -
>>  -#ifdef CONFIG_PROFILER
>>  -#include "qemu-timer.h" /* for ticks_per_sec */
>>  -#endif
>>  +#include "qemu-timer.h"
>>
>>   //#define DEBUG
>>   //#define DEBUG_COMPLETION
>>  @@ -925,14 +922,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) {
>>  +        keycode = keycodes[nb_pending_keycodes-1];
>>  +        if (keycode & 0x80)
>>  +            kbd_put_keycode(0xe0);
>>  +        kbd_put_keycode(keycode | 0x80);
>>  +        nb_pending_keycodes--;
>>  +    }
>>  +}
>>  +
>>  +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);
>>  @@ -943,7 +963,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;
>>              }
>>  @@ -953,26 +973,22 @@ 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) + 1000000LL*hold_time);
> 
> This should use ticks_per_sec.

Will fix.

> 
> Also, 100ms seems high for the default value, I think you rarely press
> keys for that long.

That's what I measured for human-typed keys in the monitor, so I simply
applied that value to keep things consistent for kbd consumers.

Thanks,
Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

[Qemu-devel] [PATCH] Enhance sendkey with key hold time - v2

[Jan Kiszka]

[ 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.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 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 <dirent.h>
-
-#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;
         }

Re: [Qemu-devel] [PATCH] Enhance sendkey with key hold time - v2

[Anthony Liguori]

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 <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
> ---
>  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 <dirent.h>
> -
> -#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;
>          }
>
>
>