Re: [PATCH] Detect key modifier status in 'sleep --interruptible'

[Robert Millan]

On Wed, Aug 26, 2009 at 04:33:56PM +0100, Colin Watson wrote:
> Index: commands/sleep.c
> ===================================================================
> --- commands/sleep.c  (revision 2535)
> +++ commands/sleep.c  (working copy)
> [...]
>  
> +static int
> +grub_check_keyboard (void)
> +{
> +  int mods = grub_getkeystatus ();
> +  if (mods >= 0 && (mods & GRUB_TERM_STATUS_SHIFT) != 0)
> +    return 1;
> +
> +  if (grub_checkkey () >= 0 &&
> +      GRUB_TERM_ASCII_CHAR (grub_getkey ()) == GRUB_TERM_ESC)
> +    return 1;
> +
> +  return 0;
> +}
> +
>  /* Based on grub_millisleep() from kern/generic/millisleep.c.  */
>  static int
>  grub_interruptible_millisleep (grub_uint32_t ms)
> @@ -52,8 +66,7 @@
>    start = grub_get_time_ms ();
>  
>    while (grub_get_time_ms () - start < ms)
> -    if (grub_checkkey () >= 0 &&
> -     GRUB_TERM_ASCII_CHAR (grub_getkey ()) == GRUB_TERM_ESC)
> +    if (grub_check_keyboard ())
>        return 1;

I'm still not convinced that we want the sleep bits.  They seem too ad-hoc,
and I'd like to hear what others think first.

But if I understood correctly, you plan on using keystatus command only?

> +static int
> +grub_console_getkeystatus (void)
> +{
> +  grub_uint8_t status = bios_data_area->keyboard_flag_lower;
> +  int mods = 0;
> +
> +  if (status & 0x03)
> +    mods |= GRUB_TERM_STATUS_SHIFT;
> +  if (status & 0x04)
> +    mods |= GRUB_TERM_STATUS_CTRL;
> +  if (status & 0x08)
> +    mods |= GRUB_TERM_STATUS_ALT;
> +
> +  return mods;
> +}

This should be macroified (but for the time being, I have no problem with
our internal representation matching the one in BIOS Data Area, hence no
translation would be needed on i386-pc).

> +  /* Set idle time to the minimum offered by the spec (4 milliseconds) so
> +     that we can find out the current state.  */
> +  grub_usb_control_msg (usbdev, 0x21, 0x0A, 1<<8, 0, 0, 0);
> +
> +  grub_usb_control_msg (usbdev, (1 << 7) | (1 << 5) | 1, 0x02, 0, 0, 1, 
> (char *) data);
> +
> [...]
> +  /* Go back to reporting every time an event occurs and not more often than
> +     that.  */
> +  grub_usb_control_msg (usbdev, 0x21, 0x0A, 0<<8, 0, 0, 0);

If they represent fixed numbers like registers/ports, these hex values should
be macros too.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."