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

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] htt
p://thread.gmane.org/gmane.os.openbsd.ppc/1143

[2] http://www.l
inuxjournal.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

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

On 4/14/07, Miod Vallat (on the road) <miodonline.fr> 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

> 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

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 <itojunitojun.org> 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]

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 <pkeshishgmail.com> 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 <itojunitojun.org> 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

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 <pkeshishgmail.com> 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 */