On Wed, Nov 13, 2002 at 04:15:58PM +0100, Sven Neumann wrote:
> Hi,

Hi Sven,

> what is the rationale behind this?

Uhm, so that a single "physical" press of a key does not generate a
boatload of key_press_event/key_release_events.  Wreaks havoc when you
are trying to use a remote to navigate a list of items.

> You can easily distinguish between
> repeat events and real press events since there's no key-release event
> generated for the former.

Oh, if only it were so .  But it's not.  I hooked two functions (that
simply print to stderr) to the key_press_event and key_release_events
of a GTK+ application and this is what I get when I hold down one of
the keys on my remote:

key_press_event
key_release_event
key_press_event
key_release_event
key_press_event
key_release_event
key_press_event
key_release_event
key_press_event
key_release_event
key_press_event
key_release_event
key_press_event
key_release_event
key_press_event
key_release_event

> I don't think it's a good idea to suppress
> events from the driver.

If the events are delivered properly, I agree.  But in this case
(again, as observed in GTK+-DirectFB) the events are not being
delivered in a fashion that reflects what is really happening.

> This should be handled on the application
> level instead.

Well, I had the following hack installed in the key_press_event of my
top-most widget(s):


static guint32 last_time;

if (event->time - last_time < 200) 
  {
    // discard it
    return TRUE;
  }

last_time = event->time;
return FALSE;


But it sucks rocks.  Knowing there was actually a repeat from the
device (like lirc does) is the best way to deal with it.

Determining key repeats and not delivering key_release events is just
not handled with remotes in DirectFB.

Here is the lirc handling loop:

     while ((readlen = read( data->fd, buf, 128 )) > 0 || errno == EINTR) {
          dfb_thread_testcancel( thread );

          if (readlen < 1)
               continue;

          evt.key_symbol = lirc_parse_line( buf );

          if (evt.key_symbol != DIKS_NULL) {
               evt.type = DIET_KEYPRESS;
               evt.flags = DIEF_KEYSYMBOL;
               dfb_input_dispatch( data->device, &evt );

               evt.type = DIET_KEYRELEASE;
               evt.flags = DIEF_KEYSYMBOL;
               dfb_input_dispatch( data->device, &evt );
          }
     }

It simply reads from the lirc fd and then calls lirc_parse_line() to
extract the key that was pressed and then queues up a DIET_KEYPRESS
immediately followed by a DIET_KEYRELEASE, with no check in there for
a key repeat (i.e. a held key).

lirc_parse_line() is able to determine a key repeat (as my patch
demonstrates), but without modifying/creating a new datatype, I cannot
see a way to convey that information back to lircEventThread() easily.

Something that had this effect should work:

Index: inputdrivers/lirc/lirc.c
===================================================================
RCS file: /cvs/directfb/DirectFB/inputdrivers/lirc/lirc.c,v
retrieving revision 1.15
diff -u -r1.15 lirc.c
--- inputdrivers/lirc/lirc.c    25 Sep 2002 11:18:43 -0000      1.15
+++ inputdrivers/lirc/lirc.c    14 Nov 2002 09:25:03 -0000
@@ -58,6 +58,7 @@
      int          fd;
 } LircData;
 
+bool lirc_key_repeated;
 
 static int keynames_compare (const void *key,
                              const void *base)
@@ -90,7 +91,11 @@
      if (!s || !s[1])
           return DIKS_NULL;
 
-     s = strchr( ++s, ' ' );
+        lirc_key_repeated = false;
+        if (atoi(++s) > 0)
+                lirc_key_repeated = true;
+
+     s = strchr( s, ' ' );
      if (!s|| !s[1])
           return DIKS_NULL;
 
@@ -140,9 +145,11 @@
                evt.flags = DIEF_KEYSYMBOL;
                dfb_input_dispatch( data->device, &evt );
 
-               evt.type = DIET_KEYRELEASE;
-               evt.flags = DIEF_KEYSYMBOL;
-               dfb_input_dispatch( data->device, &evt );
+                          if (!lirc_key_repeated) {
+                    evt.type = DIET_KEYRELEASE;
+                    evt.flags = DIEF_KEYSYMBOL;
+                    dfb_input_dispatch( data->device, &evt );
+                          }
           }
      }
 
Although I don't think the use of global used in this manner is good
programming practice.

Thots?
b.

-- 
Brian J. Murrell

Attachment: msg01063/pgp00000.pgp
Description: PGP signature

Reply via email to