caps-lock / control key swap weirdness (adb keyboard)

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

caps-lock / control key swap weirdness (adb keyboard)

patrick keshishian
Hello,

Originally I posted [1] about a key-repeat issue which Otto pointed me
to documentation that helped me resolve the issue.  In the original
post I also mentioned a strange problem I'm seeing with the Control
key after I swap the CapsLock and Control keys.   Digging a bit more
I think this may be have to do with how key events are generated.
Google showed me a reference [2] that claimed ADB keyboard design
flaw. Quoting the article:

<quote>
ADB Keyboard Mis-design

    * When the key to the left of the 'A' (CapsLock) is
      pressed, the ADB keyboard sends both a keyPress event
      and a keyRelease event.
    * When the CapsLock key is then released, the ADB keyboard
      sends NO events.
    * When the CapsLock key is next pressed, the ADB keyboard
      sends NO events.
    * When the CapsLock key is then released, the ADB keyboard
      sends both a keyPress event and a keyRelease
      event.
    * The above cycle repeats over and over.
</quote>

I, however, don't see this behavior using xev.  Unless I'm mis-
reading the output of xev.  I see upon pressing the key labeled
"caps lock" a KeyPress event is received.  On actual key release
of "caps lock" no event is received.  On a subsequent press of
"caps lock" a KeyRelease event is received.  On release of the
"caps lock" key, two events are received: KeyPress followed by
a KeyRelease [3,4] (note the proximity of each time-stamp).

I suspect this behavior might be causing some state confusion.
I can't confirm this right now, though.


I suppose a work-around to this problem might be to use the
swapped control key like a caps-lock key.  Pressing control toggles
it on, and to continue working after using the control key, press
and release it again to toggle it now off.  Eek :-\

If this in fact is a hardware (ADB keyboard) issue, which it
seems to be, there doesn't seem to be a clean software solution
for it.  In which case my post is only to serve as a reference
for the next person with similar question.



[1] http://thread.gmane.org/gmane.os.openbsd.ppc/1143

[2] http://www.linuxjournal.com/article/5610

[3] Before swap:
KeyPress event, serial 27, synthetic NO, window 0x1600001,
    root 0x44, subw 0x1600002, time 4065151542, (43,52), root:(76,103),
    state 0x0, keycode 66 (keysym 0xffe5, Caps_Lock), same_screen YES,
    XLookupString gives 0 bytes:
    XmbLookupString gives 0 bytes:
    XFilterEvent returns: False

KeyRelease event, serial 27, synthetic NO, window 0x1600001,
    root 0x44, subw 0x1600002, time 4065159862, (43,52), root:(76,103),
    state 0x2, keycode 66 (keysym 0xffe5, Caps_Lock), same_screen YES,
    XLookupString gives 0 bytes:
    XFilterEvent returns: False

KeyPress event, serial 27, synthetic NO, window 0x1600001,
    root 0x44, subw 0x1600002, time 4065164672, (43,52), root:(76,103),
    state 0x2, keycode 66 (keysym 0xffe5, Caps_Lock), same_screen YES,
    XLookupString gives 0 bytes:
    XmbLookupString gives 0 bytes:
    XFilterEvent returns: False

KeyRelease event, serial 27, synthetic NO, window 0x1600001,
    root 0x44, subw 0x1600002, time 4065164672, (43,52), root:(76,103),
    state 0x2, keycode 66 (keysym 0xffe5, Caps_Lock), same_screen YES,
    XLookupString gives 0 bytes:
    XFilterEvent returns: False

[4] After key swap:
KeyPress event, serial 24, synthetic NO, window 0x1800001,
    root 0x44, subw 0x0, time 4065934482, (150,176), root:(221,265),
    state 0x0, keycode 66 (keysym 0xffe3, Control_L), same_screen YES,
    XLookupString gives 0 bytes:
    XmbLookupString gives 0 bytes:
    XFilterEvent returns: False

KeyRelease event, serial 27, synthetic NO, window 0x1800001,
    root 0x44, subw 0x0, time 4065936632, (150,176), root:(221,265),
    state 0x4, keycode 66 (keysym 0xffe3, Control_L), same_screen YES,
    XLookupString gives 0 bytes:
    XFilterEvent returns: False

KeyPress event, serial 27, synthetic NO, window 0x1800001,
    root 0x44, subw 0x0, time 4065936702, (150,176), root:(221,265),
    state 0x0, keycode 66 (keysym 0xffe3, Control_L), same_screen YES,
    XLookupString gives 0 bytes:
    XmbLookupString gives 0 bytes:
    XFilterEvent returns: False

KeyRelease event, serial 27, synthetic NO, window 0x1800001,
    root 0x44, subw 0x0, time 4065936702, (150,176), root:(221,265),
    state 0x4, keycode 66 (keysym 0xffe3, Control_L), same_screen YES,
    XLookupString gives 0 bytes:
    XFilterEvent returns: False

Reply | Threaded
Open this post in threaded view
|

Re: caps-lock / control key swap weirdness (adb keyboard)

Miod Vallat
> <quote>
> ADB Keyboard Mis-design
>
>    * When the key to the left of the 'A' (CapsLock) is
>      pressed, the ADB keyboard sends both a keyPress event
>      and a keyRelease event.
>    * When the CapsLock key is then released, the ADB keyboard
>      sends NO events.
>    * When the CapsLock key is next pressed, the ADB keyboard
>      sends NO events.
>    * When the CapsLock key is then released, the ADB keyboard
>      sends both a keyPress event and a keyRelease
>      event.
>    * The above cycle repeats over and over.
> </quote>

That's not quite right. The first keypress only produces a ``key down''
event, and no ``key up'' event. And the last key release only produces a
``key up'' event and no ``key down'' event.

> I, however, don't see this behavior using xev.  Unless I'm mis-
> reading the output of xev.  I see upon pressing the key labeled
> "caps lock" a KeyPress event is received.  On actual key release
> of "caps lock" no event is received.  On a subsequent press of
> "caps lock" a KeyRelease event is received.  On release of the
> "caps lock" key, two events are received: KeyPress followed by
> a KeyRelease [3,4] (note the proximity of each time-stamp).

The OpenBSD adb code synthesizes the missing key release and key press
event, because the upper input layers would be confused without.

So when you first press capslock, a key press and a key release are fed
to the wscons layer. When you release capslock, nothing happens. When
you press it again, nothing happens. When you release it a second time,
a key press and a key release are fed to wscons.

There's an extra problem caused by the fact that the power key sends the
same scan code as the caps lock key. So if you press the power key while
caps is still down, the kernel can not be sure of what happened.

Miod

Reply | Threaded
Open this post in threaded view
|

Re: caps-lock / control key swap weirdness (adb keyboard)

patrick keshishian
On 4/14/07, Miod Vallat (on the road) <[hidden email]> wrote:

> > <quote>
> > ADB Keyboard Mis-design
> >
> >    * When the key to the left of the 'A' (CapsLock) is
> >      pressed, the ADB keyboard sends both a keyPress event
> >      and a keyRelease event.
> >    * When the CapsLock key is then released, the ADB keyboard
> >      sends NO events.
> >    * When the CapsLock key is next pressed, the ADB keyboard
> >      sends NO events.
> >    * When the CapsLock key is then released, the ADB keyboard
> >      sends both a keyPress event and a keyRelease
> >      event.
> >    * The above cycle repeats over and over.
> > </quote>
>
> That's not quite right. The first keypress only produces a ``key down''
> event, and no ``key up'' event. And the last key release only produces a
> ``key up'' event and no ``key down'' event.


This is interesting, because that is not what I see, err... saw
when I captured the xev output.  The sequence I witnessed was
as follows:

    1. Physically hold down the "caps-lock" key/button by depressing
       and holding down the key/button. xev notes KeyPress event.
    2. Release "caps-lock" key/button by removing finger depressing
       key/button. xev shows no event!
    3. Hold down the "caps-lock" key/button by depressing and holding
       down the key/button. xev notes KeyRelease event.
    4. Release "caps-lock" key/button by removing finger depressing
       key/button. xev notes KeyPress _and_ KeyRelease events with
       same exact time-stamp!


I think something is in fact getting screwed up somewhere that gets
me in this state, if in fact what you describe below is how things
are expected to work.


> > I, however, don't see this behavior using xev.  Unless I'm mis-
> > reading the output of xev.  I see upon pressing the key labeled
> > "caps lock" a KeyPress event is received.  On actual key release
> > of "caps lock" no event is received.  On a subsequent press of
> > "caps lock" a KeyRelease event is received.  On release of the
> > "caps lock" key, two events are received: KeyPress followed by
> > a KeyRelease [3,4] (note the proximity of each time-stamp).
>
> The OpenBSD adb code synthesizes the missing key release and key press
> event, because the upper input layers would be confused without.
>
> So when you first press capslock, a key press and a key release are fed
> to the wscons layer. When you release capslock, nothing happens. When
> you press it again, nothing happens. When you release it a second time,
> a key press and a key release are fed to wscons.


I don't have any understanding (exposure) to the wscons layer, so
I am not sure how this works.  But this sequence of events isn't
what I saw when I sent out my email.

Now, I just booted into OpenBSD, I have not yet messed with key
swaps and I ran xev and noted the sequence of events reported [1].
This sequence is what I expected, and it is completely different
from what I have seen when I wrote my original message.

Pressing "caps-lock" and holding it down produces KeyPress event
(according to xev).  Releasing the depressed key produces a
KeyRelease event.

Now, I feed xmodmap sequence [2] to swap the Caps_Lock and Control_L
and things seem to work as expected for a short period of time, but
soon something goes screwy.  The now Control_L key (physical keyboard
key labeled "caps lock") acts as a toggled control key.  Press on,
press off.

Now, xev reports the strange event delivery for the key presses
as I had noted in my original email and described above [3].

Swapping the keys once again using same sequence [2] doesn't remedy
this issue.  The event delivery is once again screwy.


[1] Freshly booted OpenBSD and X session.
KeyPress event, serial 27, synthetic NO, window 0x1600001,
    root 0x44, subw 0x0, time 4101403022, (161,133), root:(194,184),
    state 0x2, keycode 66 (keysym 0xffe5, Caps_Lock), same_screen YES,
    XLookupString gives 0 bytes:
    XmbLookupString gives 0 bytes:
    XFilterEvent returns: False

KeyRelease event, serial 27, synthetic NO, window 0x1600001,
    root 0x44, subw 0x0, time 4101408542, (161,133), root:(194,184),
    state 0x2, keycode 66 (keysym 0xffe5, Caps_Lock), same_screen YES,
    XLookupString gives 0 bytes:
    XFilterEvent returns: False

KeyPress event, serial 27, synthetic NO, window 0x1600001,
    root 0x44, subw 0x0, time 4101415032, (161,133), root:(194,184),
    state 0x0, keycode 66 (keysym 0xffe5, Caps_Lock), same_screen YES,
    XLookupString gives 0 bytes:
    XmbLookupString gives 0 bytes:
    XFilterEvent returns: False

KeyRelease event, serial 27, synthetic NO, window 0x1600001,
    root 0x44, subw 0x0, time 4101416562, (161,133), root:(194,184),
    state 0x2, keycode 66 (keysym 0xffe5, Caps_Lock), same_screen YES,
    XLookupString gives 0 bytes:
    XFilterEvent returns: False


[2] From EXAMPLES section of xmodmap(1)
! Swap Caps_Lock and Control_L
remove Lock = Caps_Lock
remove Control = Control_L
keysym Control_L = Caps_Lock
keysym Caps_Lock = Control_L
add Lock = Caps_Lock
add Control = Control_L


[3] After swapping "caps lock" and "control" things go screwy:

 - Here I physically press down _and_ hold down the button labeled
   "caps lock" and xev shows the following output:

KeyPress event, serial 24, synthetic NO, window 0x1600001,
    root 0x44, subw 0x0, time 4102532472, (60,170), root:(628,291),
    state 0x0, keycode 66 (keysym 0xffe3, Control_L), same_screen YES,
    XLookupString gives 0 bytes:
    XmbLookupString gives 0 bytes:
    XFilterEvent returns: False

 - I release the "caps lock" button by removing my finger off of
   it, but xev shows no events!

 - Next I press the "caps lock" button again and hold it down with
   my finger. xev shows the following event:

KeyRelease event, serial 27, synthetic NO, window 0x1600001,
    root 0x44, subw 0x0, time 4102534792, (60,170), root:(628,291),
    state 0x4, keycode 66 (keysym 0xffe3, Control_L), same_screen YES,
    XLookupString gives 0 bytes:
    XFilterEvent returns: False

 - I now remove my finger from the "caps lock" key so it is no longer
   depressed and xev displays the following two events one right after
   the other!  Take a note of their time-stamps:

KeyPress event, serial 27, synthetic NO, window 0x1600001,
    root 0x44, subw 0x0, time 4102536032, (60,170), root:(628,291),
    state 0x0, keycode 66 (keysym 0xffe3, Control_L), same_screen YES,
    XLookupString gives 0 bytes:
    XmbLookupString gives 0 bytes:
    XFilterEvent returns: False

KeyRelease event, serial 27, synthetic NO, window 0x1600001,
    root 0x44, subw 0x0, time 4102536032, (60,170), root:(628,291),
    state 0x4, keycode 66 (keysym 0xffe3, Control_L), same_screen YES,
    XLookupString gives 0 bytes:
    XFilterEvent returns: False

Reply | Threaded
Open this post in threaded view
|

Re: caps-lock / control key swap weirdness (adb keyboard)

Jun-ichiro itojun Hagino 2.0
In reply to this post by patrick keshishian
> Originally I posted [1] about a key-repeat issue which Otto pointed me
> to documentation that helped me resolve the issue.  In the original
> post I also mentioned a strange problem I'm seeing with the Control
> key after I swap the CapsLock and Control keys.   Digging a bit more
> I think this may be have to do with how key events are generated.
> Google showed me a reference [2] that claimed ADB keyboard design
> flaw. Quoting the article:

        iirc macppc ADB capslock code does some trick to avoid weirdness of
        capslock (to present proper keypress signals to upper layer).
        it was taken from netbsd (sys/arch/macppc/dev/akbd.c, CAPS_IS_CONTROL).
        if there's any mistake in the trick, it should be corrected.

itojun

Reply | Threaded
Open this post in threaded view
|

Re: caps-lock / control key swap weirdness (adb keyboard)

patrick keshishian
Sorry for the cross-post to tech@, I'm attaching a diff for akbd.c
that I think solves my problem.

After looking at the source (akbd.c) and playing with it, with
some of the debug printf()s turned on and a few of mine added
(see ADB_DEBUG_VERBOSE) I saw that the events reported aren't
very consistent. The code works as-is fine with the caps-lock
functionality. First press and release turns caps-lock on and
a subsequent press and release turns caps-lock off.

However, if one swaps the caps-lock and control keys, (typical
Unix keyboard layout, well... sorta), there are much opportunity
for confusing the current code.

I read the comment where it mentions that 0xffff serves as a
key-up for the caps-lock key.  This seems to be true in most
cases.  However, if one happens to chord the caps-lock key
with any other key, the 0xffff doesn't always get delivered!

It seems, from my tests, that sometimes the up-event of the
chorded key suppresses the 0xffff delivery.  As you can see,
this would confuse the current code into keeping the caps-lock
key "locked", which isn't bad if it were not re-assigned to
be the control key.

There was one additional case I saw where even if the caps-lock
key was not chorded, but the sequence of key presses of caps-lock
and another key were close enough, the up-event for the other
key would arrive first followed by 0xffff.

Initially I came up with a pretty ugly and somewhat complicated
patch, which seemed to solve the problem.  It introduced a new
CL_DOWN* flag. Fortunately, I had to toss that patch as I noticed
a few cases it didn't cover while composing the email to submit
the patch.

My current solution uses two additional flags, and it seems to
be much cleaner than my first one.  Still it is ugly, but not
much could I do given the inconsistent way events are delivered.
I didn't dig too much down to the lower levels, where there may
(or may not) have been a cleaner solution.

Here is a brief description of what I did.  The two new flags
added are CL_WANT_KEYUP and CL_CHORDED.

CL_WANT_KEYUP is set whenever the caps-lock key-down event is
observed.  It sets the state for the code to know that an up-
event is expected.  If the expected 0xffff is received this
flag is cleared.  However, if we don't see the 0xffff event,
but see another key-up event (possibly the chorded key) we
use CL_WANT_KEYUP state to set the CL_DOWN_LOGICAL state.
If the 0xffff event were to arrive after the chorded key's
up-event, we note that the CL_DOWN_LOGICAL _and_ CL_WANT_UPKEY
state flags are set and we only unset the CL_WANT_UPKEY flag.
In the case where the caps-lock key is kept depressed and
the user continues to type (e.g., user has remapped caps-lock
and ctrl keys, and is working in vi) the first key-down event
will clear the CL_WANT_KEYUP event.

The second flag, CL_CHORDED is used for when we get the caps-
lock down and a chorded key-down event.  e.g., 0x39 0x38.  This
case will invoke akbd_capslockwrapper() two times. Once for
0x39 (caps-lock down), which will set CL_WANT_KEYUP state and
a second time for 0x38, which is a key-down event which normally
would clear the CL_WANT_KEYUP state.  The CL_CHORDED state is
set whenever we notice a chorded key sequence and this flag is
used in akbd_capslockwrapper() to not unset the CL_WANT_KEYUP
state, and so prevent future confusion of the code.

I've tested this patch for a bit now, and I think it is working
pretty well.  I don't have any other machines with adb keyboards
to test it against, so I can't be 100% sure that it will not
break anything else.

Comments are of course welcome, just don't thrash (too hard)
on me :-)




On 4/15/07, Jun-ichiro itojun Hagino <[hidden email]> wrote:

> > Originally I posted [1] about a key-repeat issue which Otto pointed me
> > to documentation that helped me resolve the issue.  In the original
> > post I also mentioned a strange problem I'm seeing with the Control
> > key after I swap the CapsLock and Control keys.   Digging a bit more
> > I think this may be have to do with how key events are generated.
> > Google showed me a reference [2] that claimed ADB keyboard design
> > flaw. Quoting the article:
>
>         iirc macppc ADB capslock code does some trick to avoid weirdness of
>         capslock (to present proper keypress signals to upper layer).
>         it was taken from netbsd (sys/arch/macppc/dev/akbd.c, CAPS_IS_CONTROL).
>         if there's any mistake in the trick, it should be corrected.

I couldn't find any reference to CAPS_IS_CONTROL in -current
source tree.

--patrick

[demime 1.01d removed an attachment of type application/octet-stream which had a name of akbd.c.diff]

Reply | Threaded
Open this post in threaded view
|

Re: caps-lock / control key swap weirdness (adb keyboard)

Jun-ichiro itojun Hagino 2.0
> >         iirc macppc ADB capslock code does some trick to avoid weirdness of
> >         capslock (to present proper keypress signals to upper layer).
> >         it was taken from netbsd (sys/arch/macppc/dev/akbd.c, CAPS_IS_CONTROL).
> >         if there's any mistake in the trick, it should be corrected.
>
> I couldn't find any reference to CAPS_IS_CONTROL in -current
> source tree.

        #if CAPS_IS_CONTROL is in netbsd.  openbsd code at this moment has
        tricky dances equivalent to when CAPS_IS_CONTROL enabled on netbsd.

itojun

Reply | Threaded
Open this post in threaded view
|

Re: caps-lock / control key swap weirdness (adb keyboard)

patrick keshishian
In reply to this post by patrick keshishian
I was reminded that I need to include the patch in-line.
Hopefully this works this time.

My apologies.

--patrick


Index: akbd.c
===================================================================
RCS file: /cvs/src/sys/dev/adb/akbd.c,v
retrieving revision 1.6
diff -u -p -r1.6 akbd.c
--- akbd.c      13 Mar 2007 20:56:56 -0000      1.6
+++ akbd.c      16 Apr 2007 09:07:29 -0000
@@ -59,6 +59,11 @@
  */
 int    akbdmatch(struct device *, void *, void *);
 void   akbdattach(struct device *, struct device *, void *);
+static void akbd_synth_cl_updn(struct akbd_softc *sc);
+#ifdef ADB_DEBUG_VERBOSE
+static void akbd_show_state(char const *p, struct akbd_softc *sc,
+       adb_event_t *event, int key);
+#endif

 /* Driver definition. */
 struct cfattach akbd_ca = {
@@ -476,6 +481,8 @@ akbd_rawrepeat(void *v)
 #define        CL_DOWN_ADB     0x01
 #define        CL_DOWN_LOGICAL 0x02
 #define        CL_DOWN_RESET   0x04
+#define        CL_WANT_KEYUP   0x08
+#define        CL_CHORDED      0x10

 /*
  * Given a keyboard ADB event, decode the keycodes and pass them to wskbd.
@@ -500,16 +507,27 @@ akbd_processevent(struct akbd_softc *sc,
                                if (ISSET(sc->sc_caps, CL_DOWN_RESET))
                                        CLR(sc->sc_caps, CL_DOWN_RESET);
                                else if (ISSET(sc->sc_caps, CL_DOWN_ADB)) {
-                                       akbd_input(sc, ISSET(sc->sc_caps,
-                                           CL_DOWN_LOGICAL) ?
-                                             ADBK_KEYDOWN(ADBK_CAPSLOCK) :
-                                             ADBK_KEYUP(ADBK_CAPSLOCK));
-                                       sc->sc_caps ^= CL_DOWN_LOGICAL;
+                                       if ((sc->sc_caps &
+                                           (CL_DOWN_LOGICAL|CL_WANT_KEYUP)) !=
+                                           (CL_DOWN_LOGICAL|CL_WANT_KEYUP))
+                                               akbd_synth_cl_updn(sc);
+                                       sc->sc_caps &= ~CL_WANT_KEYUP;
                                }
                        }
                } else {
+                       if (event->bytes[0] == 0xff) {
+                               if ((sc->sc_caps &
+                                   (CL_DOWN_LOGICAL|CL_WANT_KEYUP)) !=
+                                   (CL_DOWN_LOGICAL|CL_WANT_KEYUP))
+                                       akbd_synth_cl_updn(sc);
+                               sc->sc_caps &= ~CL_WANT_KEYUP;
+                       }
+                       else if (event->bytes[1] != 0xff) {
+                               sc->sc_caps |= CL_CHORDED;
+                       }
                        akbd_capslockwrapper(sc, event->bytes[0]);
                        akbd_capslockwrapper(sc, event->bytes[1]);
+                       sc->sc_caps &= ~CL_CHORDED;
                }
                break;
        default:
@@ -525,8 +543,28 @@ akbd_processevent(struct akbd_softc *sc,
 void
 akbd_capslockwrapper(struct akbd_softc *sc, int key)
 {
-       if (ADBK_KEYVAL(key) == ADBK_CAPSLOCK)
-               sc->sc_caps ^= CL_DOWN_ADB;
+       if (key == ADBK_CAPSLOCK) {
+               sc->sc_caps |= CL_DOWN_ADB|CL_WANT_KEYUP;
+       }
+       else if (key == ADBK_KEYUP(ADBK_CAPSLOCK)) {
+               sc->sc_caps &= ~(CL_DOWN_ADB|CL_DOWN_LOGICAL|CL_WANT_KEYUP);
+       }
+       else if (ISSET(sc->sc_caps, CL_CHORDED)) {
+               /*
+                * This is a no-op, to avoid the case where we have
+                * 0x39 + 0x38 keys chorded, where the first call to
+                * this func with 0x39 setting CL_WANT_KEYUP and the
+                * second call with 0x38 to undo it in the following
+                * else-statements!
+                */
+       }
+       else if (key != 0xff && key & 0x80) {
+               if (ISSET(sc->sc_caps, CL_WANT_KEYUP) &&
+                   !ISSET(sc->sc_caps, CL_DOWN_LOGICAL))
+                       akbd_synth_cl_updn(sc);
+       }
+       else if (!(key & 0x80))
+               sc->sc_caps &= ~CL_WANT_KEYUP;

        if (key != 0xff)
                akbd_input(sc, key);
@@ -582,3 +620,58 @@ akbd_input(struct akbd_softc *sc, int ke
                wskbd_input(sc->sc_wskbddev, type, val);
        }
 }
+
+static void
+akbd_synth_cl_updn(struct akbd_softc *sc)
+{
+       if (!ISSET(sc->sc_caps, CL_DOWN_ADB))
+               return;
+
+       akbd_input(sc, ISSET(sc->sc_caps, CL_DOWN_LOGICAL) ?
+             ADBK_KEYDOWN(ADBK_CAPSLOCK) : ADBK_KEYUP(ADBK_CAPSLOCK));
+       sc->sc_caps ^= CL_DOWN_LOGICAL;
+}
+
+#ifdef ADB_DEBUG_VERBOSE
+static void
+akbd_show_state(char const *p, struct akbd_softc *sc, adb_event_t *event,
+       int key)
+{
+       char scstate[64], *scsep;
+
+       bzero(&scstate, sizeof(scstate));
+       scsep = "";
+       if (sc->sc_caps & CL_DOWN_ADB) {
+               strlcat(scstate, "CL_DOWN_ADB", sizeof(scstate));
+               scsep = "|";
+       }
+       if (sc->sc_caps & CL_DOWN_LOGICAL) {
+               strlcat(scstate, scsep, sizeof(scstate));
+               strlcat(scstate, "CL_DOWN_LOGICAL", sizeof(scstate));
+               scsep = "|";
+       }
+       if (sc->sc_caps & CL_DOWN_RESET) {
+               strlcat(scstate, scsep, sizeof(scstate));
+               strlcat(scstate, "CL_DOWN_RESET", sizeof(scstate));
+               scsep = "|";
+       }
+       if (sc->sc_caps & CL_WANT_KEYUP) {
+               strlcat(scstate, scsep, sizeof(scstate));
+               strlcat(scstate, "CL_WANT_KEYUP", sizeof(scstate));
+               scsep = "|";
+       }
+       if (sc->sc_caps & CL_CHORDED) {
+               strlcat(scstate, scsep, sizeof(scstate));
+               strlcat(scstate, "CL_CHORDED", sizeof(scstate));
+       }
+       if (event && event->byte_count == 2)
+               printf("%s: %4s> %02x%02x state:%04x (%s)\n",
+                   sc->sc_dev.dv_xname, p,
+                   event->bytes[0], event->bytes[1],
+                   sc->sc_caps, scstate);
+       else
+               printf("%s: %4s> --%02x state:%04x (%s)\n",
+                   sc->sc_dev.dv_xname, p,
+                   event ? event->bytes[0] : key, sc->sc_caps, scstate);
+}
+#endif /* ADB_DEBUG_VERBOSE */













On 4/16/07, patrick keshishian <[hidden email]> wrote:

> Sorry for the cross-post to tech@, I'm attaching a diff for akbd.c
> that I think solves my problem.
>
> After looking at the source (akbd.c) and playing with it, with
> some of the debug printf()s turned on and a few of mine added
> (see ADB_DEBUG_VERBOSE) I saw that the events reported aren't
> very consistent. The code works as-is fine with the caps-lock
> functionality. First press and release turns caps-lock on and
> a subsequent press and release turns caps-lock off.
>
> However, if one swaps the caps-lock and control keys, (typical
> Unix keyboard layout, well... sorta), there are much opportunity
> for confusing the current code.
>
> I read the comment where it mentions that 0xffff serves as a
> key-up for the caps-lock key.  This seems to be true in most
> cases.  However, if one happens to chord the caps-lock key
> with any other key, the 0xffff doesn't always get delivered!
>
> It seems, from my tests, that sometimes the up-event of the
> chorded key suppresses the 0xffff delivery.  As you can see,
> this would confuse the current code into keeping the caps-lock
> key "locked", which isn't bad if it were not re-assigned to
> be the control key.
>
> There was one additional case I saw where even if the caps-lock
> key was not chorded, but the sequence of key presses of caps-lock
> and another key were close enough, the up-event for the other
> key would arrive first followed by 0xffff.
>
> Initially I came up with a pretty ugly and somewhat complicated
> patch, which seemed to solve the problem.  It introduced a new
> CL_DOWN* flag. Fortunately, I had to toss that patch as I noticed
> a few cases it didn't cover while composing the email to submit
> the patch.
>
> My current solution uses two additional flags, and it seems to
> be much cleaner than my first one.  Still it is ugly, but not
> much could I do given the inconsistent way events are delivered.
> I didn't dig too much down to the lower levels, where there may
> (or may not) have been a cleaner solution.
>
> Here is a brief description of what I did.  The two new flags
> added are CL_WANT_KEYUP and CL_CHORDED.
>
> CL_WANT_KEYUP is set whenever the caps-lock key-down event is
> observed.  It sets the state for the code to know that an up-
> event is expected.  If the expected 0xffff is received this
> flag is cleared.  However, if we don't see the 0xffff event,
> but see another key-up event (possibly the chorded key) we
> use CL_WANT_KEYUP state to set the CL_DOWN_LOGICAL state.
> If the 0xffff event were to arrive after the chorded key's
> up-event, we note that the CL_DOWN_LOGICAL _and_ CL_WANT_UPKEY
> state flags are set and we only unset the CL_WANT_UPKEY flag.
> In the case where the caps-lock key is kept depressed and
> the user continues to type (e.g., user has remapped caps-lock
> and ctrl keys, and is working in vi) the first key-down event
> will clear the CL_WANT_KEYUP event.
>
> The second flag, CL_CHORDED is used for when we get the caps-
> lock down and a chorded key-down event.  e.g., 0x39 0x38.  This
> case will invoke akbd_capslockwrapper() two times. Once for
> 0x39 (caps-lock down), which will set CL_WANT_KEYUP state and
> a second time for 0x38, which is a key-down event which normally
> would clear the CL_WANT_KEYUP state.  The CL_CHORDED state is
> set whenever we notice a chorded key sequence and this flag is
> used in akbd_capslockwrapper() to not unset the CL_WANT_KEYUP
> state, and so prevent future confusion of the code.
>
> I've tested this patch for a bit now, and I think it is working
> pretty well.  I don't have any other machines with adb keyboards
> to test it against, so I can't be 100% sure that it will not
> break anything else.
>
> Comments are of course welcome, just don't thrash (too hard)
> on me :-)
>
>
>
>
> On 4/15/07, Jun-ichiro itojun Hagino <[hidden email]> wrote:
> > > Originally I posted [1] about a key-repeat issue which Otto pointed me
> > > to documentation that helped me resolve the issue.  In the original
> > > post I also mentioned a strange problem I'm seeing with the Control
> > > key after I swap the CapsLock and Control keys.   Digging a bit more
> > > I think this may be have to do with how key events are generated.
> > > Google showed me a reference [2] that claimed ADB keyboard design
> > > flaw. Quoting the article:
> >
> >         iirc macppc ADB capslock code does some trick to avoid weirdness of
> >         capslock (to present proper keypress signals to upper layer).
> >         it was taken from netbsd (sys/arch/macppc/dev/akbd.c, CAPS_IS_CONTROL).
> >         if there's any mistake in the trick, it should be corrected.
>
> I couldn't find any reference to CAPS_IS_CONTROL in -current
> source tree.
>
> --patrick

Reply | Threaded
Open this post in threaded view
|

Re: caps-lock / control key swap weirdness (adb keyboard)

patrick keshishian
Screwing around some more I found a related set of key sequences
that my patch does not handle properly. These cases do not fall
in the normal use case, IMO. But I thought I would mention them
for the record.  An example would be the following key sequence:

        1. Press and hold down shift key
        2. Press and hold down caps-lock key
        3. Release shift key
        4. Press and hold down the 'h' key (or almost any other key)
        5. Release caps-lock key
           * This here already has got the code in a confused state
        6. Release 'h' key.

I am not sure if it is worth cludging up the code for these less
likely cases. In fact I'm not sure at all if it would be possible
to account for all possible variations.

While thinking about how I may solve this case, I ended up making
some minor changes to my original patch, by wrapping some of the
conditions checked in the if-statements into macros that aid read-
ability.  I also added some comments where I added code that set
and un-set the states.

I'm including the new patch which is functionally equivalent to the
one I sent out yesterday morning.

p.s., I don't know how to make the copy and paste process not
convert the tabs into spaces.  That's annoying!




Index: akbd.c
===================================================================
RCS file: /cvs/src/sys/dev/adb/akbd.c,v
retrieving revision 1.6
diff -u -p -r1.6 akbd.c
--- akbd.c      13 Mar 2007 20:56:56 -0000      1.6
+++ akbd.c      17 Apr 2007 08:42:53 -0000
@@ -59,6 +59,11 @@
  */
 int    akbdmatch(struct device *, void *, void *);
 void   akbdattach(struct device *, struct device *, void *);
+static void akbd_synth_cl_updn(struct akbd_softc *sc);
+#ifdef ADB_DEBUG_VERBOSE
+static void akbd_show_state(char const *p, struct akbd_softc *sc,
+       adb_event_t *event, int key);
+#endif

 /* Driver definition. */
 struct cfattach akbd_ca = {
@@ -476,6 +481,15 @@ akbd_rawrepeat(void *v)
 #define        CL_DOWN_ADB     0x01
 #define        CL_DOWN_LOGICAL 0x02
 #define        CL_DOWN_RESET   0x04
+#define        CL_WANT_KEYUP   0x08
+#define        CL_CHORDED      0x10
+#define CL_LOGICAL_FLUX        (CL_DOWN_LOGICAL|CL_WANT_KEYUP)
+
+/*
+ * Macro to test multiple bits set
+ */
+#define ARESET(t,f)    (((t)&(f)) == (f))
+#define CL_IN_FLUX(sc) (ARESET((sc)->sc_caps, CL_LOGICAL_FLUX))

 /*
  * Given a keyboard ADB event, decode the keycodes and pass them to wskbd.
@@ -500,16 +514,23 @@ akbd_processevent(struct akbd_softc *sc,
                                if (ISSET(sc->sc_caps, CL_DOWN_RESET))
                                        CLR(sc->sc_caps, CL_DOWN_RESET);
                                else if (ISSET(sc->sc_caps, CL_DOWN_ADB)) {
-                                       akbd_input(sc, ISSET(sc->sc_caps,
-                                           CL_DOWN_LOGICAL) ?
-                                             ADBK_KEYDOWN(ADBK_CAPSLOCK) :
-                                             ADBK_KEYUP(ADBK_CAPSLOCK));
-                                       sc->sc_caps ^= CL_DOWN_LOGICAL;
+                                       if (!CL_IN_FLUX(sc))
+                                               akbd_synth_cl_updn(sc);
+                                       sc->sc_caps &= ~CL_WANT_KEYUP;
                                }
                        }
                } else {
+                       if (event->bytes[0] == 0xff) {
+                               if (!CL_IN_FLUX(sc))
+                                       akbd_synth_cl_updn(sc);
+                               sc->sc_caps &= ~CL_WANT_KEYUP;
+                       }
+                       else if (event->bytes[1] != 0xff) {
+                               sc->sc_caps |= CL_CHORDED;
+                       }
                        akbd_capslockwrapper(sc, event->bytes[0]);
                        akbd_capslockwrapper(sc, event->bytes[1]);
+                       sc->sc_caps &= ~CL_CHORDED;
                }
                break;
        default:
@@ -525,8 +546,38 @@ akbd_processevent(struct akbd_softc *sc,
 void
 akbd_capslockwrapper(struct akbd_softc *sc, int key)
 {
-       if (ADBK_KEYVAL(key) == ADBK_CAPSLOCK)
-               sc->sc_caps ^= CL_DOWN_ADB;
+       if (key == ADBK_CAPSLOCK) {
+               sc->sc_caps |= CL_DOWN_ADB|CL_WANT_KEYUP;
+       }
+       else if (key == ADBK_KEYUP(ADBK_CAPSLOCK)) {
+               sc->sc_caps &= ~(CL_DOWN_ADB|CL_DOWN_LOGICAL|CL_WANT_KEYUP);
+       }
+       else if (ISSET(sc->sc_caps, CL_CHORDED)) {
+               /*
+                * This is a no-op, to avoid the case where we have
+                * 0x39 + 0x38 keys chorded, where the first call to
+                * this func with 0x39 setting CL_WANT_KEYUP and the
+                * second call with 0x38 to undo it in the following
+                * else-statements!
+                */
+       }
+       else if (key != 0xff && key & 0x80) {
+               /*
+                * If we want an up-event, and CL_DOWN_LOGICAL isn't
+                * yet set, synthesize the caps-lock up event
+                */
+               if (ISSET(sc->sc_caps, CL_WANT_KEYUP) &&
+                   !ISSET(sc->sc_caps, CL_DOWN_LOGICAL))
+                       akbd_synth_cl_updn(sc);
+       }
+       else if (!(key & 0x80)) {
+               /*
+                * On an down-event we should clear the CL_WANT_KEYUP
+                * state. This covers the case where the user is typing
+                * while holding down the caps-lock key.
+                */
+               sc->sc_caps &= ~CL_WANT_KEYUP;
+       }

        if (key != 0xff)
                akbd_input(sc, key);
@@ -582,3 +633,58 @@ akbd_input(struct akbd_softc *sc, int ke
                wskbd_input(sc->sc_wskbddev, type, val);
        }
 }
+
+static void
+akbd_synth_cl_updn(struct akbd_softc *sc)
+{
+       if (!ISSET(sc->sc_caps, CL_DOWN_ADB))
+               return;
+
+       akbd_input(sc, ISSET(sc->sc_caps, CL_DOWN_LOGICAL) ?
+             ADBK_KEYDOWN(ADBK_CAPSLOCK) : ADBK_KEYUP(ADBK_CAPSLOCK));
+       sc->sc_caps ^= CL_DOWN_LOGICAL;
+}
+
+#ifdef ADB_DEBUG_VERBOSE
+static void
+akbd_show_state(char const *p, struct akbd_softc *sc, adb_event_t *event,
+       int key)
+{
+       char scstate[64], *scsep;
+
+       bzero(&scstate, sizeof(scstate));
+       scsep = "";
+       if (sc->sc_caps & CL_DOWN_ADB) {
+               strlcat(scstate, "CL_DOWN_ADB", sizeof(scstate));
+               scsep = "|";
+       }
+       if (sc->sc_caps & CL_DOWN_LOGICAL) {
+               strlcat(scstate, scsep, sizeof(scstate));
+               strlcat(scstate, "CL_DOWN_LOGICAL", sizeof(scstate));
+               scsep = "|";
+       }
+       if (sc->sc_caps & CL_DOWN_RESET) {
+               strlcat(scstate, scsep, sizeof(scstate));
+               strlcat(scstate, "CL_DOWN_RESET", sizeof(scstate));
+               scsep = "|";
+       }
+       if (sc->sc_caps & CL_WANT_KEYUP) {
+               strlcat(scstate, scsep, sizeof(scstate));
+               strlcat(scstate, "CL_WANT_KEYUP", sizeof(scstate));
+               scsep = "|";
+       }
+       if (sc->sc_caps & CL_CHORDED) {
+               strlcat(scstate, scsep, sizeof(scstate));
+               strlcat(scstate, "CL_CHORDED", sizeof(scstate));
+       }
+       if (event && event->byte_count == 2)
+               printf("%s: %4s> %02x%02x state:%04x (%s)\n",
+                   sc->sc_dev.dv_xname, p,
+                   event->bytes[0], event->bytes[1],
+                   sc->sc_caps, scstate);
+       else
+               printf("%s: %4s> --%02x state:%04x (%s)\n",
+                   sc->sc_dev.dv_xname, p,
+                   event ? event->bytes[0] : key, sc->sc_caps, scstate);
+}
+#endif /* ADB_DEBUG_VERBOSE */




On 4/16/07, patrick keshishian <[hidden email]> wrote:

> I was reminded that I need to include the patch in-line.
> Hopefully this works this time.
>
> My apologies.
>
> --patrick
>
>
> Index: akbd.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/adb/akbd.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 akbd.c
> --- akbd.c      13 Mar 2007 20:56:56 -0000      1.6
> +++ akbd.c      16 Apr 2007 09:07:29 -0000
> @@ -59,6 +59,11 @@
>   */
>  int    akbdmatch(struct device *, void *, void *);
>  void   akbdattach(struct device *, struct device *, void *);
> +static void akbd_synth_cl_updn(struct akbd_softc *sc);
> +#ifdef ADB_DEBUG_VERBOSE
> +static void akbd_show_state(char const *p, struct akbd_softc *sc,
> +       adb_event_t *event, int key);
> +#endif
>
>  /* Driver definition. */
>  struct cfattach akbd_ca = {
> @@ -476,6 +481,8 @@ akbd_rawrepeat(void *v)
>  #define        CL_DOWN_ADB     0x01
>  #define        CL_DOWN_LOGICAL 0x02
>  #define        CL_DOWN_RESET   0x04
> +#define        CL_WANT_KEYUP   0x08
> +#define        CL_CHORDED      0x10
>
>  /*
>   * Given a keyboard ADB event, decode the keycodes and pass them to wskbd.
> @@ -500,16 +507,27 @@ akbd_processevent(struct akbd_softc *sc,
>                                 if (ISSET(sc->sc_caps, CL_DOWN_RESET))
>                                         CLR(sc->sc_caps, CL_DOWN_RESET);
>                                 else if (ISSET(sc->sc_caps, CL_DOWN_ADB)) {
> -                                       akbd_input(sc, ISSET(sc->sc_caps,
> -                                           CL_DOWN_LOGICAL) ?
> -                                             ADBK_KEYDOWN(ADBK_CAPSLOCK) :
> -                                             ADBK_KEYUP(ADBK_CAPSLOCK));
> -                                       sc->sc_caps ^= CL_DOWN_LOGICAL;
> +                                       if ((sc->sc_caps &
> +                                           (CL_DOWN_LOGICAL|CL_WANT_KEYUP)) !=
> +                                           (CL_DOWN_LOGICAL|CL_WANT_KEYUP))
> +                                               akbd_synth_cl_updn(sc);
> +                                       sc->sc_caps &= ~CL_WANT_KEYUP;
>                                 }
>                         }
>                 } else {
> +                       if (event->bytes[0] == 0xff) {
> +                               if ((sc->sc_caps &
> +                                   (CL_DOWN_LOGICAL|CL_WANT_KEYUP)) !=
> +                                   (CL_DOWN_LOGICAL|CL_WANT_KEYUP))
> +                                       akbd_synth_cl_updn(sc);
> +                               sc->sc_caps &= ~CL_WANT_KEYUP;
> +                       }
> +                       else if (event->bytes[1] != 0xff) {
> +                               sc->sc_caps |= CL_CHORDED;
> +                       }
>                         akbd_capslockwrapper(sc, event->bytes[0]);
>                         akbd_capslockwrapper(sc, event->bytes[1]);
> +                       sc->sc_caps &= ~CL_CHORDED;
>                 }
>                 break;
>         default:
> @@ -525,8 +543,28 @@ akbd_processevent(struct akbd_softc *sc,
>  void
>  akbd_capslockwrapper(struct akbd_softc *sc, int key)
>  {
> -       if (ADBK_KEYVAL(key) == ADBK_CAPSLOCK)
> -               sc->sc_caps ^= CL_DOWN_ADB;
> +       if (key == ADBK_CAPSLOCK) {
> +               sc->sc_caps |= CL_DOWN_ADB|CL_WANT_KEYUP;
> +       }
> +       else if (key == ADBK_KEYUP(ADBK_CAPSLOCK)) {
> +               sc->sc_caps &= ~(CL_DOWN_ADB|CL_DOWN_LOGICAL|CL_WANT_KEYUP);
> +       }
> +       else if (ISSET(sc->sc_caps, CL_CHORDED)) {
> +               /*
> +                * This is a no-op, to avoid the case where we have
> +                * 0x39 + 0x38 keys chorded, where the first call to
> +                * this func with 0x39 setting CL_WANT_KEYUP and the
> +                * second call with 0x38 to undo it in the following
> +                * else-statements!
> +                */
> +       }
> +       else if (key != 0xff && key & 0x80) {
> +               if (ISSET(sc->sc_caps, CL_WANT_KEYUP) &&
> +                   !ISSET(sc->sc_caps, CL_DOWN_LOGICAL))
> +                       akbd_synth_cl_updn(sc);
> +       }
> +       else if (!(key & 0x80))
> +               sc->sc_caps &= ~CL_WANT_KEYUP;
>
>         if (key != 0xff)
>                 akbd_input(sc, key);
> @@ -582,3 +620,58 @@ akbd_input(struct akbd_softc *sc, int ke
>                 wskbd_input(sc->sc_wskbddev, type, val);
>         }
>  }
> +
> +static void
> +akbd_synth_cl_updn(struct akbd_softc *sc)
> +{
> +       if (!ISSET(sc->sc_caps, CL_DOWN_ADB))
> +               return;
> +
> +       akbd_input(sc, ISSET(sc->sc_caps, CL_DOWN_LOGICAL) ?
> +             ADBK_KEYDOWN(ADBK_CAPSLOCK) : ADBK_KEYUP(ADBK_CAPSLOCK));
> +       sc->sc_caps ^= CL_DOWN_LOGICAL;
> +}
> +
> +#ifdef ADB_DEBUG_VERBOSE
> +static void
> +akbd_show_state(char const *p, struct akbd_softc *sc, adb_event_t *event,
> +       int key)
> +{
> +       char scstate[64], *scsep;
> +
> +       bzero(&scstate, sizeof(scstate));
> +       scsep = "";
> +       if (sc->sc_caps & CL_DOWN_ADB) {
> +               strlcat(scstate, "CL_DOWN_ADB", sizeof(scstate));
> +               scsep = "|";
> +       }
> +       if (sc->sc_caps & CL_DOWN_LOGICAL) {
> +               strlcat(scstate, scsep, sizeof(scstate));
> +               strlcat(scstate, "CL_DOWN_LOGICAL", sizeof(scstate));
> +               scsep = "|";
> +       }
> +       if (sc->sc_caps & CL_DOWN_RESET) {
> +               strlcat(scstate, scsep, sizeof(scstate));
> +               strlcat(scstate, "CL_DOWN_RESET", sizeof(scstate));
> +               scsep = "|";
> +       }
> +       if (sc->sc_caps & CL_WANT_KEYUP) {
> +               strlcat(scstate, scsep, sizeof(scstate));
> +               strlcat(scstate, "CL_WANT_KEYUP", sizeof(scstate));
> +               scsep = "|";
> +       }
> +       if (sc->sc_caps & CL_CHORDED) {
> +               strlcat(scstate, scsep, sizeof(scstate));
> +               strlcat(scstate, "CL_CHORDED", sizeof(scstate));
> +       }
> +       if (event && event->byte_count == 2)
> +               printf("%s: %4s> %02x%02x state:%04x (%s)\n",
> +                   sc->sc_dev.dv_xname, p,
> +                   event->bytes[0], event->bytes[1],
> +                   sc->sc_caps, scstate);
> +       else
> +               printf("%s: %4s> --%02x state:%04x (%s)\n",
> +                   sc->sc_dev.dv_xname, p,
> +                   event ? event->bytes[0] : key, sc->sc_caps, scstate);
> +}
> +#endif /* ADB_DEBUG_VERBOSE */