Page 1 of 1
[SOLVED] Trying to make a getch() function...
Posted: Tue Apr 01, 2008 10:55 am
by 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
Posted: Tue Apr 01, 2008 11:17 am
by lukem95
try the volatile keyword
Posted: Tue Apr 01, 2008 11:33 am
by 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
....
Posted: Tue Apr 01, 2008 12:41 pm
by 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.
Posted: Tue Apr 01, 2008 12:52 pm
by 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
Posted: Tue Apr 01, 2008 2:12 pm
by codemastersnake
einsteinjunior wrote: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.
Posted: Wed Apr 02, 2008 3:48 am
by 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?
Posted: Wed Apr 02, 2008 3:55 am
by AJ
t6q4 wrote:(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
Posted: Wed Apr 02, 2008 4:12 am
by t6q4
Combuster wrote: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
Posted: Wed Apr 02, 2008 4:23 am
by 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?
Posted: Wed Apr 02, 2008 4:56 am
by 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
Re: Trying to make a getch() function...
Posted: Wed Apr 02, 2008 5:43 am
by 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.
Posted: Wed Apr 02, 2008 7:05 am
by 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).