Page 1 of 1

Keyboard shift probolems

Posted: Fri Nov 03, 2006 9:27 am
by xarnze
I have writen a basic keyboard driver to get the key pressed and then outputit to the screen, but ive had a probolem when trying to add the shift keys, when ever i
press a key it now prints out the character from both keymaps, so if any of you get a chance to have a look at it for me i would be very greatful:

Code: Select all

void keyboard_handler(struct regs *r)

{

  unsigned char scancode;

  unsigned shift_state = 0;

  char asciicode;

  

  //read form the buffer

  scancode=inportb(0x60);



  if (scancode == (42|0x80) || scancode == (54|0x80))

    {

      //shift not pressed
      shift_state = 0;

    }

  else if (scancode == 42 || scancode == 54)

   {

      // shift
 pressed
      shift_state = 1;

   }

    else

    {

      if (shift_state)

      {

	asciicode = kbdus_upper[scancode];

      }

      else

      {

        asciicode = kbdus[scancode];

      }

      putch(asciicode);

    }

} 

Posted: Fri Nov 03, 2006 10:21 am
by gaf
Press a key it now prints out the character from both keymaps
Could it be that your code prints the make-code and break-code for each key ? Also make sure that 'shift_code' is either a global variable or a static local in your actual code.

regards,
gaf

Stoping it printing the breakcodes

Posted: Fri Nov 03, 2006 1:32 pm
by xarnze
Thanks, it now works fine when i press shift but still prints from both keymaps if shift is now pressed, you were right it is printing the breakcodes aswell, so how would i stop it fom printing the breakcodes?

Thatnks again for your help with this.

Posted: Sat Nov 04, 2006 6:48 am
by Midas
I'm not certain I quite understand your code - why are you ORing with 0x80 at the start? You're looking for an AND I think - ORing will mean that it will always set shift as being down. Here's some heavily commented sample code (may well not compile - I've just typed it OTOH).

Code: Select all

unsigned char scancode;
unsigned shift_state = 0;
char asciicode;

scancode = inportb(0x60); // Read from keyboard controller

if(scancode & 0x80)
{
    // If bit 15 is set, then this is a key break code
    switch(scancode)
    {
        case 0xAA:
            // 0xAA == 0x2A | 0x80 - the shift key break code
            shift_state = 0;
            break;
        /* Note that the switch statement is overkill for just the shift key, but
         * there may well be other keys for which you wish to do similar
         * things (ctrl, alt, perhaps others peculiar to your OS). */
    }
}
else
{
    // If bit 15 isn't set, then it's a key make code
    switch(scancode)
    {
        case 0x2A:
            /* 0x2A is the shift key's scancode - we want to handle it specially
             * as it isn't a printable character. */
            shift_state = 1;
            break;
        default:
            /* Where we handle the non-special cases - usually all the
             * printable characters. */
           
            /* I prefer, through laziness, to do it this way - I doubt there's
             * much of a performance difference either way. */
           asciicode = kbdus[scancode];
           if(shift_state)
               asciicode = kbdus_upper[scancode];

           putch(asciicode); /* We'd want to replace this later, to place the
                              * key into a buffer of some sort, then read from
                              * it; or (later still, perhaps) send the key in a 
                              * message to the active process. */
           break;
    }
}

outportb(0x20, 0x20); // Acknowldge IRQ

Posted: Sat Nov 04, 2006 9:00 am
by gaf
I'm not certain I quite understand your code - why are you ORing with 0x80 at the start?
From what I understand the line you're refering to compares the read-in scancode with the shift break-codes. The bitwise OR is only used to set the break-code bit for the comparison.

The code should actually work if you just add a last comparison before the table look-up:

Code: Select all

if(scancode == LEFT_SHIFT_BREAK || scancode == RIGHT_SHIFT_BREAK) 
{ 
    shift_state = 0; 
} 
else
{    
   if(scancode == LEFT_SHIFT_MAKE || scancode == RIGHT_SHIFT_MAKE) 
   { 
       shift_state = 1; 
   } 
   else 
   { 
       // TODO: Check if it's a make code -> if(~scancode &0x80)

       if(shift_state) 
       {
           asciicode = kbdus_upper[scancode]; 
       } 
       else 
       { 
           asciicode = kbdus[scancode]; 
       }
       putch(asciicode); 
    } 
}
Nevertheless I would recommend using a switch as Midas proposed. It will become much easier to read once you've added support for a few more modifier keys to your driver.

regards,
gaf

Fixed

Posted: Sat Nov 04, 2006 9:47 am
by xarnze
I got it working now, i just added a check to see if it was a make code or not as gaf said, but i am going to add in switch functions later, i just wanted to get the basics done first. i will use what Midas has done to help with the more complex parts, and setting up all of the other keys.
Thanks for all your help everyone.

Posted: Sat Nov 04, 2006 11:40 am
by Midas
gaf wrote:
I'm not certain I quite understand your code - why are you ORing with 0x80 at the start?
From what I understand the line you're refering to compares the read-in scancode with the shift break-codes. The bitwise OR is only used to set the break-code bit for the comparison.
Ah, right you are. Read it twice and managed to miss that. :oops: Thanks.