On Mon, Aug 24, 2009 at 10:24:06AM +0100, Colin Watson wrote:
> On Mon, Aug 24, 2009 at 12:46:29AM +0200, Vladimir 'phcoder' Serbinenko wrote:
> 
> [Could you please preserve attributions in your replies? I've reinserted
> one here so that it's clear who wrote what.]
> 
> > Colin Watson wrote:
> > > The values I picked for the constants were convenient for i386-pc, but
> > > that's because it's the only architecture that currently needs an
> > > assembly implementation and it seemed to make sense to me to put the
> > > harder work of transforming values around into C code.
> > >
> > > I'm not sure what you mean by "distinguish" here though. I included
> > > constants for control and alt that are currently unused, which may
> > > actually have been a mistake since the original Red Hat patch to GRUB 1
> > > honoured those too in its hiddenmenu handling. Is that what you mean?
> > 
> > I mean that upper layer code knows whether it was left or right shift
> > but it doesn't know whether it was left or right alt.
> 
> Oh, I see. There's no real need to distinguish here, so the attached
> patch removes the distinction. Note that this should NOT be applied
> as-is, as I have not updated the assembly implementation for this;
> Robert is going to clean-room this in C (which should take all of a
> minute or so :-) ), and I hope he can deal with the bit-banging at the
> same time.

Hi,

I don't have time to test it, but what we want is roughly something like
this.  If you would please test and confirm it's working the way you expected,
it can be committed.

Also, please remember to include a ChangeLog entry.

I ommitted the USB part of your patch, because I had some comments.  Once
that's fixed it can be added as well:

> +static int
> +grub_usb_keyboard_keystatus (void)
> +{
> +  unsigned char data[8];

Please use grub_uint8_t.

> +  for (i = 0; i < 50; i++)
> +    {
> +      /* Get_Report.  */
> +      err = grub_usb_keyboard_getreport (usbdev, data);
> +
> +      if (! err)
> +     break;
> +    }

If the 50 is a "timeout", it should be using grub_get_time_ms() instead;
if it's a number given by spec (e.g. number of times an I/O operation must
be performed), then please macroify it to indicate what it is.

-- 
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."
Index: kern/term.c
===================================================================
--- kern/term.c	(revision 2520)
+++ kern/term.c	(working copy)
@@ -140,6 +140,15 @@ grub_checkkey (void)
   return (grub_cur_term_input->checkkey) ();
 }
 
+int
+grub_getkeystatus (void)
+{
+  if (grub_cur_term_input->getkeystatus)
+    return (grub_cur_term_input->getkeystatus) ();
+  else
+    return 0;
+}
+
 grub_uint16_t
 grub_getxy (void)
 {
Index: include/grub/term.h
===================================================================
--- include/grub/term.h	(revision 2520)
+++ include/grub/term.h	(working copy)
@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2003,2005,2007,2008  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2003,2005,2007,2008,2009  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -72,6 +72,12 @@ grub_term_color_state;
 #define GRUB_TERM_NEED_INIT	(1 << 16)
 
 
+/* Bitmasks for modifier keys returned by grub_getkeystatus.  */
+#define GRUB_TERM_STATUS_SHIFT	(1 << 0)
+#define GRUB_TERM_STATUS_CTRL	(1 << 1)
+#define GRUB_TERM_STATUS_ALT	(1 << 2)
+
+
 /* Unicode characters for fancy graphics.  */
 #define GRUB_TERM_DISP_LEFT	0x2190
 #define GRUB_TERM_DISP_UP	0x2191
@@ -157,6 +163,9 @@ struct grub_term_input
 
   /* Get a character.  */
   int (*getkey) (void);
+
+  /* Get keyboard modifier status.  */
+  int (*getkeystatus) (void);
 };
 typedef struct grub_term_input *grub_term_input_t;
 
@@ -275,6 +284,7 @@ void EXPORT_FUNC(grub_putcode) (grub_uin
 grub_ssize_t EXPORT_FUNC(grub_getcharwidth) (grub_uint32_t code);
 int EXPORT_FUNC(grub_getkey) (void);
 int EXPORT_FUNC(grub_checkkey) (void);
+int EXPORT_FUNC(grub_getkeystatus) (void);
 grub_uint16_t EXPORT_FUNC(grub_getwh) (void);
 grub_uint16_t EXPORT_FUNC(grub_getxy) (void);
 void EXPORT_FUNC(grub_gotoxy) (grub_uint8_t x, grub_uint8_t y);
Index: include/grub/i386/pc/console.h
===================================================================
--- include/grub/i386/pc/console.h	(revision 2520)
+++ include/grub/i386/pc/console.h	(working copy)
@@ -42,6 +42,7 @@
 /* These are global to share code between C and asm.  */
 int grub_console_checkkey (void);
 int grub_console_getkey (void);
+int grub_console_getkeystatus (void);
 grub_uint16_t grub_console_getxy (void);
 void grub_console_gotoxy (grub_uint8_t x, grub_uint8_t y);
 void grub_console_cls (void);
Index: include/grub/i386/pc/memory.h
===================================================================
--- include/grub/i386/pc/memory.h	(revision 2520)
+++ include/grub/i386/pc/memory.h	(working copy)
@@ -78,6 +78,15 @@
 /* The data segment of the pseudo real mode.  */
 #define GRUB_MEMORY_MACHINE_PSEUDO_REAL_DSEG	0x20
 
+#define GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR	0x400
+
+static struct grub_machine_bios_data_area
+{
+  grub_uint8_t unused1[0x17];
+  grub_uint16_t keyboard_status; /* 0x17 */ 
+  grub_uint8_t unused2[0xf0 - 0x19];
+}
+
 #ifndef ASM_FILE
 
 struct grub_machine_mmap_entry
Index: commands/sleep.c
===================================================================
--- commands/sleep.c	(revision 2520)
+++ commands/sleep.c	(working copy)
@@ -1,7 +1,7 @@
 /* sleep.c - Command to wait a specified number of seconds.  */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2006,2007,2008  Free Software Foundation, Inc.
+ *  Copyright (C) 2006,2007,2008,2009  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -43,6 +43,20 @@ do_print (int n)
   grub_printf ("%d    ", n);
 }
 
+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 @@ grub_interruptible_millisleep (grub_uint
   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;
 
   return 0;
@@ -74,7 +87,7 @@ grub_cmd_sleep (grub_extcmd_t cmd, int a
   if (n == 0)
     {
       /* Either `0' or broken input.  */
-      return 0;
+      return grub_check_keyboard ();
     }
 
   xy = grub_getxy ();
Index: term/i386/pc/console.c
===================================================================
--- term/i386/pc/console.c	(revision 2520)
+++ term/i386/pc/console.c	(working copy)
@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2003,2005,2007,2008  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2003,2005,2007,2008,2009  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -16,15 +16,25 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <grub/machine/memory.h>
 #include <grub/machine/console.h>
 #include <grub/term.h>
 #include <grub/types.h>
 
+static const struct grub_machine_bios_data_area *bios_data_area = GRUB_MEMORY_MACHINE_BIOS_DATA_AREA_ADDR;
+
+static grub_uint16_t
+grub_console_getkeystatus (void)
+{
+  return bios_data_area->keyboard_status;
+}
+
 static struct grub_term_input grub_console_term_input =
   {
     .name = "console",
     .checkkey = grub_console_checkkey,
     .getkey = grub_console_getkey,
+    .getkeystatus = grub_console_getkeystatus,
   };
 
 static struct grub_term_output grub_console_term_output =
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to