[SOLVED] Trying to make a getch() function...

[t6q4]

Hello,
I am trying to make a getch() function for my os. I know I posted a thread like this some time ago, but I've rewritten my kernel since then and the method wasnt very flexible. Anyway, this is my code:

Code: Select all

//Includes somewhere here
//KBDUS defined here

u32int getch_progress=0;
char new_key;

/* Handles the keyboard interrupt */
void keyboard_handler(registers_t *r)
{
    unsigned char scancode;

    //Read from port 0x60 for getting input
    scancode = inportb(0x60);

//Check if key has been released or pressed
    if (scancode & 0x80)
    {
        //unused for now
    }
    else
    {
//Here, we set getch_process to 1
        getch_progress=1;
        new_key=kbdus[scancode]);
    }
}

/* Installs the keyboard handler into IRQ1 */
void init_keyboard()
{
    register_interrupt_handler(IRQ1, keyboard_handler);
}

char getch() 
{ 
   getch_progress=0;
   if(getch_progress==0) 
      while(getch_progress==0);	  
   kprintf("getch_progress=%u;\n",ex);
   getch_progress = 0;    
   return new_key; 
}

The trouble is, that getch_progress is never equal to 1, and therefore, never exits the loop. The kprintf() in the getch() function is never printed. How come then, that I have set getch_progress to 1 in the keyboard irq handler? I think I know why, that all variables are reset to their original value after the keyboard handler is executed, but does anyone know of a solution to this?


New problem, see below.

Thanks,
t6q4

[lukem95]

try the volatile keyword

[trolly]

the keyboard controler return the scancode +0 if the key is pressed and +128 if it's released. also you should check that the scancode is below 128


try:

Code: Select all

//Check if key has been released or pressed
    if (scancode < 128) 
    {
        //unused for now
    }
    else 
....

[einsteinjunior]

Hi,
My question may seem out of topic but why do you not write a keyboard driver that will store the pressed keys in a buffer?
Your getch () function will be more easier to code since it will only require you to read whats in the buffer at that time instead of applications accessing hardware directly .......it could be worse if its a multitasking os.
Thats just my opinion.

[Combuster]

A stab at the dark:

- Does your interrupt handler actually execute?

- Does register_interrupt_handler take an IRQ or an interrupt number? you'll need to set interrupt #33 (0x21) in the IDT to have it respond to IRQ 1.

- Have you unmasked the IRQ in the PIC

- Have you read the wiki's page on I_Cant_Get_Interrupts_Working

[codemastersnake]

Hi,
My question may seem out of topic but why do you not write a keyboard driver that will store the pressed keys in a buffer?
Your getch () function will be more easier to code since it will only require you to read whats in the buffer at that time instead of applications accessing hardware directly .......it could be worse if its a multitasking os.
Thats just my opinion.

I agree to that. I also use a buffer for implementing keyboard driver. It works fine for me.

[t6q4]

@Combuster - I forgot to put IRQ1 instead of 1 (and btw IRQ1 is usually Interrupt 8 unless you remap the PIC...)
Sorry, it was a very silly mistake...
I'm not going to go and implement a buffer now, because my OS is multitasking (well, it hopes to be in the near future), and I had to change the function type into an unsigned, otherwise it printed three horizontal lines on top of each other and an S. Now it doesnt print the right letter, just a random one, so if i pressed H it would print an n with a square block, and an L prints an S and a space.

Thanks for the help in finding the stupid silly bug, everyone...
But can anyone help with this new bug?

[AJ]

(and btw IRQ1 is usually Interrupt 8 unless you remap the PIC...)

If you are in PMode, you *have* to remap the PIC which is why Combuster assumes vector 0x21 - the first 32 interrupts are reserved for CPU exceptions.

Have you gone through Combusters' list of things to try and still can't get it working?

Cheers,
Adam

[t6q4]

A stab at the dark:

- Does your interrupt handler actually execute?

- Does register_interrupt_handler take an IRQ or an interrupt number? you'll need to set interrupt #33 (0x21) in the IDT to have it respond to IRQ 1.

- Have you unmasked the IRQ in the PIC

- Have you read the wiki's page on I_Cant_Get_Interrupts_Working

-Yes I solved that problem. It does set the variables and issues an EOI so my interrupt is working.
-That was my fix
-It was never masked - the handler IS called.

Combuster's solutions got the interrupt handler working as I said in the last post. The problem is I repeat, it does not kprintf(key); the right key, as I also said in my last problem.

I just want to make sure, that my first problem of getch_progress never equal to 1 is solved. I have a new problem, as seen in my above post. Thanks.

This is the code that prints the key:

Code: Select all

unsigned char k;
k=getch();
kprintf("%s",n);

Hope you understood the new problem now,
t6q4

[JamesM]

Well, have you printed out the scancode you recieve, and the numeric value of the output you recieve from getch, then compared them with your keyboard lookup table?

[AJ]

Also:

This is the code that prints the key:
Code:
unsigned char k;
k=getch();
kprintf("%s",n);

Try using %c instead of %s.

Cheers,
Adam

[Ready4Dis]

Ok, well first a few things I'd like to point out...
why are you checking if getch_progress == 0 then doing a while == 0? They are the same statement, just use the while loop. Also, make sure you use volatile keyword when creating the progress variable, I know it was mentioned, but it's a biggy (volatile int getch_progress;).

Code: Select all

#define SizeOfBuffer 1024
char KeyBuffer[SizeOfBuffer];
volatile int BufferStart=0, BufferEnd=0;

#define NextNum(n) (((n)+1)%SizeOfBuffer)

void keyboard_handler(registers_t *r) 
{ 
    unsigned char scancode; 

    //Read from port 0x60 for getting input 
    scancode = inportb(0x60); 

//Check if key has been released or pressed 
    if (scancode & 0x80) 
    { 
        //unused for now 
    } 
    else //Lets add it to our buffer as pressed
    { 
        if (NextNum(BufferEnd) == BufferStart) //We have a full buffer?!
           return; //Can't add it, lets just bail
        KeyBuffer[BufferEnd] = kbdus[scancode]);
        BufferEnd = NextNum(BufferEnd);
    }
} 

So, now we have code to fill a ring buffer (think of a circle with 1024 points, and keep following it around, if Start == End, it's empty, if End catches back up to Start, it's full, if they aren't equal there is some data).

Code: Select all

char kbhit(void)
{
   return (BufferStart != BufferEnd);
}

char getch(void)
{
   char Ret;
   while (BufferStart == BufferEnd); //Do nothing while waiting
   Ret = KeyBuffer[BufferStart];
   BufferStart = NextNum(BufferStart);
   return Ret;
}

So, now you've got a much simpler and more robust getch, however once you start multitasking this probably won't work anyways, because you will want to direct the keyboard strokes to whatever is the 'active' process (for example, a text editor). But, this helps so you don't miss any keystrokes (unless you use the entire buffer!). As a plus, you've got a kbhit function so you can check if a key is hit before getting it without blocking also.

Code is untested and typed in this little window, if there is a bug, I apologize, but it should give you a good idea on a 'simple' buffered keyboard driver.

[t6q4]

The scancode was not the problem. Thanks AJ, I used %c instead of %s and it worked!

@Ready4Dis - Thanks for your pseudocode, and I will try and make use of a buffer. About the if statement, I had it there for a few earlier bugs which I fixed myself. I forgot to remove the if statement, so i'll go do that now I have the volatiles in there already thanks to lukem95. i'll try to use shorter variable names in the future, but I dont really mind using long ones, as long as they're not TOO long (extreme example 1024 characters).