Re: [OpenBIOS] [PATCH] get-key-map implementation

[Mark Cave-Ayland]

On 21/12/12 01:40, Amadeusz Sławiński wrote:
> This patch caused some issues while I was doing some tests. It seemed to
> go in infinte loop.
>
> Anyway, do we even need to remap control as command?
> > From what I tested it boots fine in verbose when I pass it '-v' in boot-args
>
> -boot order=d -prom-env 'boot-args=-v'
> and I'm pretty sure other arguments can also be passed this way.

Yes, it's fine to use -prom-env in this way.
> Still because we need get-key-map, some comments inline.
> > Index: drivers/adb_kbd.c
> >
> > --- drivers/adb_kbd.c	(revision 1078)
> > +++ drivers/adb_kbd.c	(working copy)
> > @@ -24,7 +24,7 @@
> >   #include "libc/byteorder.h"
> >   #include "libc/vsprintf.h"
> >   #include "kbd.h"
> >
> > +#include<math.h>
> >   #include "adb_bus.h"
> >   #include "adb_kbd.h"
> >
> > @@ -42,11 +42,13 @@
> >   }
> >
> > static void keyboard_read(void);
> > +static void getKeyMap(void);
> >
> > NODE_METHODS( keyboard ) = {
> >    { "open",		keyboard_open		},
> >    { "close",		keyboard_close		},
> >    { "read",               keyboard_read		},
> >
> > { "get-key-map",        getKeyMap		},
> > };
> >
> > /* VT100 escape sequences */
> >
> > @@ -566,3 +568,65 @@
> >    }
> >    PUSH(i);
> >   }
> >
> > +// The implementation of the get-key-map word.
> > +// Returns an array of 32 bytes. Certain bits
> > +// are used to determine which keys are being
> > +// held down.
> >
> > +static void getKeyMap(void)
> > +{
> > #define sizeOfArray 8   // 8 bytes (32 bits) in size
> > uint32_t keyPushed, *keyMapArray, offset;
> > const uint32_t modifierKeyIndex = 7;
> > const uint32_t commandKeyValue = pow(2,28);
> > const uint32_t shiftKeyValue = pow(2,30);
> > const uint32_t theAKeyValue = pow(2,27);
>
> pow() needs another header, can't you just give it numbers it wants
> 0x10000000;
> 0x40000000;
> 0x08000000;
> or at least
> 1<<28;
> 1<<30;
> 1<<27;
> both seem clearer to me seeing as it is encoded value
> > keyMapArray = (uint32_t *) malloc(sizeOfArray);
>
> malloc() ... is it freed anywhere?
> I'm pretty sure it just eats memory on each call, there may be not much
> calls, but still
> > if(!keyMapArray)        // if failed to allocate memory
> > {
> >   printk("Failed to allocate memory for keyMapArray!\n");
> >   PUSH(0);
> >   return;
> > }
> > // Set all the elements to zero
> > int index;
> > for(index = 0; index<  sizeOfArray; index++)
> > {
> >  keyMapArray[index] = 0;
> > }
> > feval("key?");
> > if(POP() != -1)         // if no key was pushed
> > {
> >   PUSH((uint32_t)keyMapArray);  // returns the address of keyMapArray
> >   return;
> > }
> > feval("key");
> > keyPushed = POP();
> > if(keyPushed>  0&&  keyPushed<  27) // control key combination
> > {
> >  offset = 1;
> >  keyMapArray[modifierKeyIndex] = commandKeyValue;
> > }
>
> I think that pretending that some keys are others is really bad idea
> and when you boot emulated OS you will be still missing key, it should
> just be fixed in emulator.
> Also I haven't tested this but there is chance that sdl backend works
> properly (ie it uses meta keys which from what I read are mapped to
> command on Macs).
> > else if(keyPushed>  64&&  keyPushed<  91)    // shift key combination
> > {
> >  offset = 65;
> >  keyMapArray[modifierKeyIndex] = shiftKeyValue;
> > }
> > else     // just a letter is being held down
> > {
> >  offset = 97;
> > }
> > // Determines the value and location of a bit in the array.
> > *keyMapArray = theAKeyValue>>  (keyPushed - offset);

I'm much more interested to know where you found this algorithm? BootX 
source has this function:

static long TestForKey(long key)
{
   long keyNum;
   long bp;
   char tc;

if (gOFVersion < kOFVersion3x) {
     switch(key) {
     case 'a' :         keyNum =   7; break;
     case 's' :         keyNum =   6; break;
     case 'v' :         keyNum =  14; break;
     case 'y' :         keyNum =  23; break;
     case kCommandKey : keyNum =  48; break;
     case kOptKey     : keyNum =  61; break;
     case kShiftKey   : keyNum =  63; break;
     case kControlKey : keyNum =  49; break;
     default : keyNum = -1; break;
     }
   } else {
     switch(key) {
     case 'a' :         keyNum =   3; break;
     case 's' :         keyNum =  17; break;
     case 'v' :         keyNum =  30; break;
     case 'y' :         keyNum =  27; break;
     case kCommandKey : keyNum = 228; break;
     case kOptKey     : keyNum = 229; break;
     case kShiftKey   : keyNum = 230; break;
     case kControlKey : keyNum = 231; break;
     case kDeleteKey  : keyNum = 45; break;
     default : keyNum = -1; break;
     }

// Map the right modifier keys on to the left.
     gKeyMap[28] |= gKeyMap[28] << 4;
   }

if (keyNum == -1) return 0;

bp = keyNum & 7;
   tc = gKeyMap[keyNum >> 3];

return (tc & (1 << bp)) != 0;
}

Is keyNum the hardware scancode? If so, can it be related to the 
translation tables in drivers/adb_kdb.c?

ATB,

Mark.