Maybe not Solved YET: 4X3 Keypad interfacing

Go To Last Post
51 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Below is the program i am using to interface keypad. Can someone help me understand what is wrong in this code. I can only see "#" being displayed. 

 

#ifndef F_CPU
#define F_CPU 1000000UL // 1 MHz clock speed
#endif

#define D0 eS_PORTB0
#define D1 eS_PORTB1
#define D2 eS_PORTB2
#define D3 eS_PORTB3
#define D4 eS_PORTB4
#define D5 eS_PORTB5
#define D6 eS_PORTB6
#define D7 eS_PORTB7
#define RS eS_PORTD5
#define EN eS_PORTD7
#define RW eS_PORTD6
// #define KEYPAD PORTC  //KEYPAD IS ATTACHED ON PORTC

#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>
#include "lcd.h"
uint8_t key,keypressed;

   void main()
   {
	   DDRB = 0xFF;
	   DDRD = 0xFF;
	   int i;
	   Lcd8_Init();
	   DDRC|= 0XF0;
	   PORTC|= 0X0F;
	   _delay_ms(5000);
	   while(1)
		{
			for(i=0;i<10;i++){

			if (PINC!=0b11111111)//in any of column pins goes high execute the loop
			{
// 				int i;
// 				char num[8] = "*PINC";
// 				unsigned char keypressed = 0;
//
// 				for ( i = 0; i < 8; ++i )
// 				keypressed |= (num[i] == '1') << (7 - i);

				if (PINC==0b00010001)
				{
					Lcd8_Set_Cursor(0,0);
					Lcd8_Write_String("1");//if row1 and column1 is high show “1”

				}
				if (PINC==0b00010010)
				{

					Lcd8_Set_Cursor(0,0);
					Lcd8_Write_String("4");// if row1 and column2 is high show “4”

				}
				if (PINC==0b00010100)
				{
					Lcd8_Set_Cursor(0,0);
					Lcd8_Write_String("7");// if row1 and column3 is high show “7”

				}
				if (PINC==0b00011000)
				{
					Lcd8_Set_Cursor(0,0);
					Lcd8_Write_String("*");//if row1 and column4 is high show “*”

				}

				if (PINC==0b00100001)
				{
					Lcd8_Set_Cursor(0,0);
					Lcd8_Write_String("2");// if row2 and column1 is high show “2”

				}
				if (PINC==0b00100010)
				{
					Lcd8_Set_Cursor(0,0);
					Lcd8_Write_String("5");// if row2 and column2 is high show “5”

				}
				if (PINC==0b00100100)
				{
					Lcd8_Set_Cursor(0,0);
					Lcd8_Write_String("8");// if row2 and column3 is high show “8”

				}
				if (PINC==0b00101000)
				{
					Lcd8_Set_Cursor(0,0);
					Lcd8_Write_String("0");// if row2 and column4 is high show “0”

				}

				if (PINC==0b01000001)
				{
					Lcd8_Set_Cursor(0,0);
					Lcd8_Write_String("3");

				}
				if (PINC==0b01000010)
				{
					Lcd8_Set_Cursor(0,0);
					Lcd8_Write_String("6");

				}
				if (PINC==0b01000100)
				{
					Lcd8_Set_Cursor(0,0);
					Lcd8_Write_String("9");

				}
				if  (PINC==0b01001000);
				{
					Lcd8_Set_Cursor(0,0);
					Lcd8_Write_String("#");

				}
			}

			else PINC == 0b11111111;
			return 0;
			}

		}
   }

 

Last Edited: Thu. Apr 20, 2017 - 02:06 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

3x4's have been done to death on the internet - suggest you just look at one of the many existing examples.

 

The point surely is that you do not look for somplete 8 bit patterns in the input such as:

if (PINC==0b00101000)

but you "walk a bit" across the outputs then check each bit of the input in turn to see if it is seen coming back in there.

 

because AVRs have pull-ups rather than pull-downs it's most usual to use a 0 bit rather than a 1 bit as your "walking bit". You then check the inputs for seeing it coming back in and can then detect one (or more) intersections between the output and the input.

 

In your code you have:

	   DDRC|= 0XF0;
	   PORTC|= 0X0F;

which suggest you are using C4..C7 as your outputs and C0..C3 as your inputs?

 

Yet after this you never actually do anything to change the state of the outputs and only read PINC looking for complete 8 bits patterns (of which 4 bits are actually output).

 

If you don't know how to read/set individual bits in a port then read the "bit manipulation 101" thread in the tutorial form here. And after that just google "4x3 AVR" and look at some of the 100's of existing projects that already show how to do this.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Also, PINC is read several times for a single set of tests.

That might be okay, but being sure of that might require major thinking.

Simpler to read PINC once and test the copy.

International Theophysical Year seems to have been forgotten..
Anyone remember the song Jukebox Band?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

In general, when posting, tell AVR model, clock speed, toolchain, and version.

 

While you state 4x3 the code looks a lot like 4x4.

 

Does your keypad have diodes or not?  [that will get into multiple-press detection] 

 

clawson wrote:
google "4x3 AVR" and look at some of the 100's of existing projects that already show how to do this.

Looks like a nice one:  http://extremeelectronics.co.in/...

App note (up to 8x8): http://www.atmel.com/images/doc2...

Three app notes:  http://www.atmel.com/products/mi...

 

Oh, yeah, once you get it "working" then you'll need to debounce...

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Tue. Mar 21, 2017 - 06:33 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you for the information.I understood what you were trying to say and came up with this code below. It doesnt work. can you please let me know possible mistakes made. 

#ifndef F_CPU
#define F_CPU 1000000UL // 1 MHz clock speed
#endif

#define D0 eS_PORTB0
#define D1 eS_PORTB1
#define D2 eS_PORTB2
#define D3 eS_PORTB3
#define D4 eS_PORTB4
#define D5 eS_PORTB5
#define D6 eS_PORTB6
#define D7 eS_PORTB7
#define RS eS_PORTD5
#define EN eS_PORTD7
#define RW eS_PORTD6
// #define KEYPAD PORTC  //KEYPAD IS ATTACHED ON PORTC

#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>
#include "lcd.h"

void col_init(void)
{
	DDRD = 0X0F;
	PORTD = 0X70;
	_delay_ms(5);
}

void row_init(void)
{
	DDRD = 0X78;
	PORTD = 0X87;
	_delay_ms(5);
}

unsigned char read_key(void)
{
	unsigned char value;
	col_init();
	value = 0;
	
	if(!(PINC & 0X10))
	{
		value = 0X01;
	}
	else if(!(PINC & 0X20))
	{
		value = 0X02;
	}
	else if(!(PINC & 0X40))
	{
		value = 0X03;
	}
	
	row_init();
	
	if(!(PINC & 0X01))
	{
		value += 0X00;
	}
	else if(!(PINC & 0X02))
	{
		value += 0X03;
	}
	else if(!(PINC & 0X04))
	{
		value += 0X06;
	}
	else if(!(PINC & 0X08))
	{
		value += 0X00;
	}
	
	_delay_ms(50);
	return value;
		
}

int main (void)
{
	unsigned char keypressed;
	DDRB = 0xFF;
	DDRD = 0xFF;
	Lcd8_Init();
	keypressed = 0X00;
	col_init;
	while(1)
	{
		if(!(PINC == 0X70))
		{
			keypressed = read_key();
		}
			Lcd8_Set_Cursor(1,1);
			Lcd8_Write_String(keypressed);
	}
}

Thanks in advance. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you for your input. I tried some things and made changes. Please check for any mistakes made.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sorry about that. I am using Atmega 644PV, 1MHZ clock, 4X3 keypad interface, 16X2 LCD display and atmel studio 7.0.

 

I also looked at the links you provided me with but got stuck somewhere during returning the value. But tried to come up with this new code as shown above. Please have a look and let me know for any mistakes. 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Also, i have used PORT B for data lines of LCD, PORT D for EN,RS and RW of LCD and Port c PC0....PC6 for 4X3 keypad.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

And have you turned of JTAG?

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I used 

 MCUCSR = (1<<JTD);
    MCUCSR = (1<<JTD);

in the code and it is throwing up an error saying MCUCSR undeclared. Do i have to include any library files specifically for this?? or am i suppose to declare??

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

MCUCSR undeclared

Does you chip have MCUCSR or is it called something else?

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

shamili1992 wrote:
Please check for any mistakes made.

Now you have no manipulation of DDRC or PORTC, yet you expect PINC to have a particular value?

 

You haven't told us the wiring of your keypad, as far as I can see.

 

I thought that link I gave was pretty darned good introduction/how-to.

 

I usually use [about] 2.5ms per column.  That give it time to overcome capacitance and such and settle down.  At the end of the time period, the row is read, the previous column turned off, and the new column energized.  I take the readings from all the rows and combine the values for a complete pass of all columns, and enter into my debounce mechanism.  That results in a word of bit flags for current state and a word of bit flags for new presses.  (In some apps I'll create or calculate another word for falling edges.)

 

Is JTAG enabled?

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

i am sorry that i did not specify the pins i used for keypad. It's PC0-PC3 for rows and PC4-PC6 for columns. 

 

I have disabled JTAG by using MCUCR . 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

shamili1992 wrote:
i am sorry that i did not specify the pins i used for keypad. It's PC0-PC3 for rows and PC4-PC6 for columns. I have disabled JTAG by using MCUCR .

In the code you posted for comment:

theusch wrote:
Now you have no manipulation of DDRC or PORTC, yet you expect PINC to have a particular value?

...and there is no mention of MCUCR.

 

And when you do show that timed sequence, you need to tell the optimization settings for your compiler.

 

 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
#ifndef F_CPU
#define F_CPU 1000000UL // 1 MHz clock speed
#endif

#define D0 eS_PORTB0
#define D1 eS_PORTB1
#define D2 eS_PORTB2
#define D3 eS_PORTB3
#define D4 eS_PORTB4
#define D5 eS_PORTB5
#define D6 eS_PORTB6
#define D7 eS_PORTB7
#define RS eS_PORTD5
#define EN eS_PORTD7
#define RW eS_PORTD6
// #define KEYPAD PORTC  //KEYPAD IS ATTACHED ON PORTC

#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>
#include "lcd.h"


void col_init(void)
{
	DDRD = 0X0F;
	PORTD = 0X70;
	_delay_ms(5);
}

void row_init(void)
{
	DDRD = 0X78;
	PORTD = 0X87;
	_delay_ms(5);
}

unsigned char read_key(void)
{
	unsigned char value;
	col_init();
	value = 0;
	
	if(!(PINC & 0X10))
	{
		value = 0X01;
		_delay_ms(50);
	}
	else if(!(PINC & 0X20))
	{
		value = 0X02;
		_delay_ms(50);
	}
	else if(!(PINC & 0X40))
	{
		value = 0X03;
		_delay_ms(50);
	}
	
	row_init();
	
	if(!(PINC & 0X01))
	{
		value += 0X00;
		_delay_ms(50);
	}
	else if(!(PINC & 0X02))
	{
		value += 0X03;
		_delay_ms(50);
	}
	else if(!(PINC & 0X04))
	{
		value += 0X06;
		_delay_ms(50);
	}
	else if(!(PINC & 0X08))
	{
		value += 0X00;
		_delay_ms(50);
	}
	
	_delay_ms(50);
	return value;
		
}

int main (void)
{
	unsigned char keypressed;
	MCUCR = (1<<JTD);
	MCUCR = (1<<JTD);
	
	DDRB = 0xFF;
	DDRD = 0xFF;
	Lcd8_Init();
	keypressed = 0X00;
	Lcd8_Set_Cursor(0,0);
	Lcd8_Write_String("Press the key");
	Lcd8_Cmd(0xC0);
	_delay_ms(500);
	col_init;
	while(1)
	{
		if(!(PINC == 0X70))
		{
			keypressed = read_key();
		}
			Lcd8_Set_Cursor(1,1);
			Lcd8_Write_String(keypressed);
	}
}

this is the updated code disabling JTAG.

 

Also, considering manipulation, i have initialized for rows separately and columns separately. Isn't that enough?? 

What else has to be done?? 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

shamili1992 wrote:
i have initialized for rows separately and columns separately. Isn't that enough??
shamili1992 wrote:
It's PC0-PC3 for rows and PC4-PC6 for columns.
shamili1992 wrote:
i have used PORT B for data lines of LCD, PORT D for EN,RS and RW of LCD and Port c PC0....PC6 for 4X3 keypad.

So you now are using a different port?

 

And as I said, you do one column at a time...

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

In your code you have

	col_init;

This will not call the function, you forgot the parenthesis. But this is not the problem, you have to understand how keypads work.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have bene the same ports since the beginning. I haven't changed any ports. And yes i will check with one column and get back. Thanks you!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you for your suggestions. I have understood how keypad works as a hardware but I am finding it difficult to put it all in the program. But i am working on it. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The secret is that you only make one row low and an output at one time.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

shamili1992 wrote:
I have bene the same ports since the beginning. I haven't changed any ports.

Read and think about what I quotes above.  Early code and discussion said that port C is used for keypad interfacing.

 

Later code does nothing with port C.  And the "init" routines deal with port D.  Which earlier you said w2as used for something else.

 

You have been given links to two good articles on matrix keypad interfacing.  One has GCC code.  Study them.

 

[and you have never given us a wiring diagram, nor told whether there are diodes in your keypad, nor whether you intend to handle multiple presses]

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you for all your help. I was able to figure it out. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

hello everyone!!

Could someone tell me how can polling be done with 4X3 keypad interfacing to atmega? I am trying to input date through keypad. 

Thanks in advance. Any help is appreciated.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If we assume you keypad works and gives us keycodes for each keypress, then it is a matter of solving the program logic that has nothing to do with the keypad or AVR. Sit down with a pencil and paper and map out the steps to get a keypress and decode it the way you want.

You want to input date - how? What feedback do you have for the user? An LCD?

Also consider that you are not the first person in the world to do this - I'm sure Google has some examples. Have you done some research rather than blindly asking on a forum?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:

If we assume you keypad works and gives us keycodes for each keypress, then it is a matter of solving the program logic that has nothing to do with the keypad or AVR. Sit down with a pencil and paper and map out the steps to get a keypress and decode it the way you want.

You want to input date - how? What feedback do you have for the user? An LCD?

Also consider that you are not the first person in the world to do this - I'm sure Google has some examples. Have you done some research rather than blindly asking on a forum?

Yes your assumption is right and feedback is through LCD. Keypad works for each keypress and i am able to put in the numbers through keypad. But my program isn't coming out of the loop after certain limit. Did some research but no luck on it. So wanted to make sure if there was any other way to do it. Well, Thanks for your suggestion. 

Last Edited: Thu. Apr 20, 2017 - 03:13 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

How do we know what your program is and what the problem might be? If you post the code, then we may be able to assist.

How i usually would structure my code would be having the keypad scanned in a timer isr and the keys put into a queue. My menu code can then read a keypress from the queue and process it in a state machine. Simples.

Last Edited: Thu. Apr 20, 2017 - 08:49 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
How do we know what your program is and what the problem might be? If you post the code, then we may be able to assist. How i usually would structure my code would be having the keypad scanned in a timer isr and the keys put into a queue. My menu code can then read a keypress from the queue and process it in a state machine. Simples.

below is the code i have been using. 

do 
					{
						do 
						{
							keypressed=read_key();
							
						} while (keypressed=0XFF);
						
						buffer[i]=keypressed;
						i++;
						
					} while (i<8);
					
					buffer[i]='\0';
					Lcd8_Set_Cursor(2,1);
					Lcd8_Write_String(buffer);

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It looks like you enter 8 keys and then it finishes.

What do you want it to do?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:

It looks like you enter 8 keys and then it finishes.

What do you want it to do?

 

Yes i am entering 8 numbers(date) and displaying it. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

So where is the problem?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1
} while (keypressed=0XFF);

I'm rather guessing that "=" was originally destined to become "==" in fact ;-)

 

PS a lot of people find this counter-intuitive (I know I do!) but you can avoid this kind of error by reversing the test:

} while (0xFF == keypressed);

On the occasion you mis-type that as:

} while (0xFF = keypressed);

the compiler simply won't let you get away with making that mistake (you cannot assign a variable to a constant!).

Last Edited: Thu. Apr 20, 2017 - 12:58 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

} while (keypressed=0XFF);

I'm rather guessing that "=" was originally destined to become "==" in fact ;-)

 

PS a lot of people find this counter-intuitive (I know I do!) but you can avoid this kind of error by reversing the test:

} while (0xFF == keypressed);

On the occasion you mis-type that as:

} while (0xFF = keypressed);

the compiler simply won't let you get away with making that mistake (you cannot assign a variable to a constant!).

You were right. That was one of the problems. Now i can see each key press is printed on LCD. But i m trying to take all the numbers and print it at once. But i donot see that happening. It is printing each time i press a key. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Your read_key() routine appears to be this:

unsigned char read_key(void)
{
	unsigned char value;
	col_init();
	value = 0;
	
	if(!(PINC & 0X10))
	{
		value = 0X01;
		_delay_ms(50);
	}
	else if(!(PINC & 0X20))
	{
		value = 0X02;
		_delay_ms(50);
	}
	else if(!(PINC & 0X40))
	{
		value = 0X03;
		_delay_ms(50);
	}

etc.

 

So that seems to be setting the returned value to low numbers like 0x01, 0x02, 0x03 etc. But then you do:

buffer[i]=keypressed;

and then finally:

Lcd8_Write_String(buffer);

So how on earth is that going to work? There is nothing here to convert 0x01, 0x02, 0x03 into '1', '2', '3' or whatever. Did you mistakenly think that 0x01, 0x02 etc in a buffer[] was going to appear as "123" ? It won't.

 

If you want the result of read_key to be "printable" then consider:

keypressed = read_key() + '0';

where the + '0' will convert 1,2,3 into '1', '2', '3' etc.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

 

unsigned char mydata[4][3] PROGMEM =
{
	{0X31,0X32,0X33},
	{0X34,0X35,0X36},
	{0X37,0X38,0X39},
	{0X2A,0X30,0X23}
};


void col_init(void)
{
	DDRC = 0X0F;
	PORTC = 0X70;
	_delay_ms(5);
}

void row_init(void)
{
	DDRC = 0XF0;
	PORTC = 0X0F;
	_delay_ms(5);
}

unsigned char read_key(void)
{
	unsigned char col, key;
	col_init();
	col = 0;
	
	if(!(PINC & 0X10))
	{
		col = 0;
		_delay_ms(50);
	}
	else if(!(PINC & 0X20))
	{
		col = 1;
		_delay_ms(50);
	}
	else if(!(PINC & 0X40))
	{
		col = 2;
		_delay_ms(50);
	}
	
	
	row_init();
	
	if(!(PINC & 0X01))
	{
		key=mydata[0][col];
		key=&(mydata[0][col]);
		key=pgm_read_byte(&(mydata[0][col]));
		_delay_ms(50);
	}
	else if(!(PINC & 0X02))
	{
		key=mydata[1][col];
		key=&(mydata[1][col]);
		key=pgm_read_byte(&(mydata[1][col]));
		_delay_ms(50);
	}
	else if(!(PINC & 0X04))
	{
		key=mydata[2][col];
		key=&(mydata[2][col]);
		key=pgm_read_byte(&(mydata[2][col]));
		_delay_ms(50);
	}
	else if (!(PINC & 0X08))
	{
		
		key=mydata[3][col];
		key=&(mydata[3][col]);
		key=pgm_read_byte(&(mydata[3][col]));
		_delay_ms(50);
	}
	
	_delay_ms(50);
	return key;
	
}

This is the routine i am using. I am able to see 1,2,3, ... etc., on my LCD when i press the respective keys.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The problem i am facing is that the loop does not exit after 8 numbers which is suppose to happen.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok, I understand, you have an ASCII map of the keyboard.

But the Lcd8_Write_String function is after the loop, so if something gets printed, it's because the loop has been exited.

Are you sure Lcd8_Write_String is completely debugged and working as it should?

 

edit:

Btw, you have residues from debugging in your program?

if(!(PINC & 0X01))
	{
		key=mydata[0][col];             //these lines seem to be
		key=&(mydata[0][col]);          //from previous versions, right?
		key=pgm_read_byte(&(mydata[0][col]));
		_delay_ms(50);
	}

You should comment out those lines, they are no longer doing anything useful.

Last Edited: Thu. Apr 20, 2017 - 09:50 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Where do you reset the value in i? It will work one time through, but drop through on the next. Of course, I'm just guessing as you've not shown enough of your code.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yeah, do...while loops always execute at least once, so failing to initialize i might explain the reported behavior.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

El Tangas wrote:

Ok, I understand, you have an ASCII map of the keyboard.

But the Lcd8_Write_String function is after the loop, so if something gets printed, it's because the loop has been exited.

Are you sure Lcd8_Write_String is completely debugged and working as it should?

 

edit:

Btw, you have residues from debugging in your program?

if(!(PINC & 0X01))
	{
		key=mydata[0][col];             //these lines seem to be
		key=&(mydata[0][col]);          //from previous versions, right?
		key=pgm_read_byte(&(mydata[0][col]));
		_delay_ms(50);
	}

You should comment out those lines, they are no longer doing anything useful.

Yes, I have used Lcd_write_string earlier and it works fine. and those lines are not from previous versions. That was the way described in Atmel website in order to use matrix and read values from it for each key pressed.

El Tangas wrote:

Yeah, do...while loops always execute at least once, so failing to initialize i might explain the reported behavior.

I tried to print a string "hello" to check if the loop is being executed and found it doesn't but after i see "hello" being displayed on the screen, i am still able to read every key press. 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thank you for your responses.

Well, i have to go through some steps before reaching this point. So here's the complete code i have been using. and i guess the previous if loop that i have been using is causing the key press to be displayed. But not sure how to go with it.

#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>
#include <stdio.h>
#include <avr/interrupt.h>
#include "lcd.h"                            // include LCD library
#include <avr/pgmspace.h>
#define PROGMEM
#define PGM_P const char*
#define pgm_read_byte(addr)(*(const unsigned char*)(addr))
void col_init(void);
void row_init(void);
unsigned char read_key(void);

#ifndef F_CPU
#define F_CPU 16000000UL                    // set the CPU clock
#endif

#define BAUD 4800                           // define baud
#define BAUDRATE ((F_CPU)/(BAUD*16UL)-1)    // set baudrate value for UBRR
unsigned char mydata[4][3] PROGMEM =
{
	{0X31,0X32,0X33},
	{0X34,0X35,0X36},
	{0X37,0X38,0X39},
	{0X2A,0X30,0X23}
};


void col_init(void)
{
	DDRC = 0X0F;
	PORTC = 0X70;
	_delay_ms(5);
}

void row_init(void)
{
	DDRC = 0XF0;
	PORTC = 0X0F;
	_delay_ms(5);
}

unsigned char read_key(void)
{
	unsigned char col, key;
	col_init();
	col = 0;
	
	if(!(PINC & 0X10))
	{
		col = 0;
		_delay_ms(50);
	}
	else if(!(PINC & 0X20))
	{
		col = 1;
		_delay_ms(50);
	}
	else if(!(PINC & 0X40))
	{
		col = 2;
		_delay_ms(50);
	}
	
	
	row_init();
	
	if(!(PINC & 0X01))
	{
		key=mydata[0][col];
		key=&(mydata[0][col]);
		key=pgm_read_byte(&(mydata[0][col]));
		_delay_ms(50);
	}
	else if(!(PINC & 0X02))
	{
		key=mydata[1][col];
		key=&(mydata[1][col]);
		key=pgm_read_byte(&(mydata[1][col]));
		_delay_ms(50);
	}
	else if(!(PINC & 0X04))
	{
		key=mydata[2][col];
		key=&(mydata[2][col]);
		key=pgm_read_byte(&(mydata[2][col]));
		_delay_ms(50);
	}
	else if (!(PINC & 0X08))
	{
		
		key=mydata[3][col];
		key=&(mydata[3][col]);
		key=pgm_read_byte(&(mydata[3][col]));
		_delay_ms(50);
	}
	
	_delay_ms(50);
	return key;
	
}







int main (void)
{
	unsigned char keypressed;
	MCUCR = (1<<JTD);
	MCUCR = (1<<JTD);
	
	DDRB = 0xFF;
	DDRD = 0xFF;
	Lcd8_Init();
	Lcd8_Cmd(0x80);
	Lcd8_Write_String("Press the key");
	_delay_ms(1200);
	Lcd8_Clear();
	col_init();
	while(1)
	{

		keypressed = read_key();
		Lcd8_Cmd(0x80);
		Lcd8_Write_String("Key pressed is :");
		Lcd8_Set_Cursor(2,1);
		Lcd8_Write_Char(keypressed);
		
		if (keypressed == 0X33)
		{
			char buffer[8], i=0;
			Lcd8_Cmd(0x80);
			Lcd8_Write_String("Select a crop:");
			while(1)
			{
				keypressed = read_key();
			Lcd8_Set_Cursor(2,1);
			Lcd8_Write_Char(keypressed);
				
				if (keypressed == 0X31)
					{
						Lcd8_Cmd(0x80);
						Lcd8_Write_String("Planting date ");
						
						do
							{
								do 
								{
									keypressed = read_key();
								} while (keypressed == 0XFF);
								 
								buffer[i] = keypressed;
								i++;
								
								}while(i<8);
									
										buffer[i]='\0';
										
										Lcd8_Write_String(buffer);
										
										
									
		
							
					}
			}
	}
}
}



 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

http://www.fourwalledcubicle.com/AVRCodeSamples.php

Is this menu system of use to you?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
http://www.fourwalledcubicle.com... Is this menu system of use to you?

I am not sure actually. I had a look at it. It has lots of things in it or may be i am not looking it in a right way. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

As highlighted above - you may well do:

			char buffer[8], i=0;

where i is set to 0. But then you:

			while(1)

and within that:

								buffer[i] = keypressed;
								i++;

and nothing ever sets i back to 0 again. Once you have consumed the buffer:

										Lcd8_Write_String(buffer);

then you should set i = 0.

 

BTW what is up with the indentation in your code? The whole point of indenting C is to make the flow of execution obvious to the reader. Your code goes out of the way to obfuscate this!! Either forget indentation all together:

int main (void)
{
unsigned char keypressed;
MCUCR = (1<<JTD);
MCUCR = (1<<JTD);

DDRB = 0xFF;
DDRD = 0xFF;
Lcd8_Init();
Lcd8_Cmd(0x80);
Lcd8_Write_String("Press the key");
_delay_ms(1200);
Lcd8_Clear();
col_init();
while(1)
{

keypressed = read_key();
Lcd8_Cmd(0x80);
Lcd8_Write_String("Key pressed is :");
Lcd8_Set_Cursor(2,1);
Lcd8_Write_Char(keypressed);

if (keypressed == 0X33)
{
char buffer[8], i=0;
Lcd8_Cmd(0x80);
Lcd8_Write_String("Select a crop:");
while(1)
{
keypressed = read_key();
Lcd8_Set_Cursor(2,1);
Lcd8_Write_Char(keypressed);

if (keypressed == 0X31)
{
Lcd8_Cmd(0x80);
Lcd8_Write_String("Planting date ");

do
{
do 
{
keypressed = read_key();
} while (keypressed == 0XFF);

buffer[i] = keypressed;
i++;

}while(i<8);

buffer[i]='\0';

Lcd8_Write_String(buffer);
}
}
}
}
}

or do it properly...

int main (void)
{
    unsigned char keypressed;
    MCUCR = (1<<JTD);
    MCUCR = (1<<JTD);
    
    DDRB = 0xFF;
    DDRD = 0xFF;
    Lcd8_Init();
    Lcd8_Cmd(0x80);
    Lcd8_Write_String("Press the key");
    _delay_ms(1200);
    Lcd8_Clear();
    col_init();
    while(1)
    {
        
        keypressed = read_key();
        Lcd8_Cmd(0x80);
        Lcd8_Write_String("Key pressed is :");
        Lcd8_Set_Cursor(2,1);
        Lcd8_Write_Char(keypressed);
        
        if (keypressed == 0X33)
        {
            char buffer[8], i=0;
            Lcd8_Cmd(0x80);
            Lcd8_Write_String("Select a crop:");
            while(1)
            {
                keypressed = read_key();
                Lcd8_Set_Cursor(2,1);
                Lcd8_Write_Char(keypressed);
                
                if (keypressed == 0X31)
                {
                    Lcd8_Cmd(0x80);
                    Lcd8_Write_String("Planting date ");
                    
                    do
                    {
                        do
                        {
                            keypressed = read_key();
                        } while (keypressed == 0XFF);
                        
                        buffer[i] = keypressed;
                        i++;
                        
                    }while(i<8);
                    
                    buffer[i]='\0';
                    
                    Lcd8_Write_String(buffer);
                }
            }
        }
    }
}

Normally I would have used the "indent" command in Linux to fix that indentation but as I am using a Windows only machine I actually did that online by pasting your code into:

 

http://courses.cs.washington.edu...

 

(hint: make sure the Language radio buttons are set correctly before you start!)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

As highlighted above - you may well do:

			char buffer[8], i=0;

where i is set to 0. But then you:

			while(1)

and within that:

								buffer[i] = keypressed;
								i++;

and nothing ever sets i back to 0 again. Once you have consumed the buffer:

										Lcd8_Write_String(buffer);

then you should set i = 0.

 

 

Thank you. I made the changes you mentioned but yet no change in the results. Now once "planting date" is displayed, no keypress is being displayed.

And sorry about the indent. I will keep this in mind.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

In that case rather than trying to print the whole string of 8 keys using:

Lcd8_Write_String(buffer);

Print what the 8 values you have collected are using:

int n;
for (n=0; n < 8; n++) {
    Lcd8_Num(buffer[i]);
    Lcd8_Write_Char(',');
}

which should show you the numeric value of the 8 bytes in buffer[]

 

Of course this is assuming your LCD library has functions called Lcd8_Num() and Lcd8_Write_Char() but as "Lcd8_" is quite an unusual name I think I may have found the site that shows that it DOES have these functions.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
 buffer[i]='\0';

this will overwrite the end of buffer. buffer is declared as 8 chars so the index can be 0..7 

i will be equal to 8 when it drops out of the loop.

I'd suggest you rewrite the code using a state machine and make it a bit more user friendly.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

In that case rather than trying to print the whole string of 8 keys using:

Lcd8_Write_String(buffer);

Print what the 8 values you have collected are using:

int n;
for (n=0; n < 8; n++) {
    Lcd8_Num(buffer[i]);
    Lcd8_Write_Char(',');
}

which should show you the numeric value of the 8 bytes in buffer[]

 

Of course this is assuming your LCD library has functions called Lcd8_Num() and Lcd8_Write_Char() but as "Lcd8_" is quite an unusual name I think I may have found the site that shows that it DOES have these functions.

I do not have Num function in my LCD library but i tried to print each character for every key press. But even after 8 keys, it just goes on printing as i keep going. 

do
{
    do{
    keypressed=read_key();
    }while(keypresses==0XFF);
    
    buffer[i]=keypressed;
    Lcd8_Write_Char(keypressed);
    i++;
    
}while(i<8);
i=0;

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

shamili1992 wrote:
I do not have Num function in my LCD library
In that case:

char numbuff[10];

for (n = 0; n < 8; n++) {
    itoa(buffer[i], numbuff, 10);
    Lcd8_Write_string(numbuff);
    Lcd8_Write_char(',');
}

should achieve much the same.

 

What I am suggesting here is that you don't try to print the return value of read_key() as character but you convert the numeric value to ASCII strings and print those. itoa() will achieve that (itoa = integer to ascii)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

In that case:

char numbuff[10];

for (n = 0; n < 8; n++) {
    itoa(buffer[i], numbuff, 10);
    Lcd8_Write_string(numbuff);
    Lcd8_Write_char(',');
}

should achieve much the same.

 

What I am suggesting here is that you don't try to print the return value of read_key() as character but you convert the numeric value to ASCII strings and print those. itoa() will achieve that (itoa = integer to ascii)

 

Thank you so much for all your help. I really appreciate it. I tried this as well. But i dont see anything on my LCD. I am sorry. I am trying too . 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well if that doesn't produce output either it means it's never getting out of the do{}..while() loop. When you did this:

do
{
    do{
    keypressed=read_key();
    }while(keypresses==0XFF);
    
    buffer[i]=keypressed;
    Lcd8_Write_Char(keypressed);
    i++;
    
}while(i<8);
i=0;

Did you ever see any output from Lcd8_Write_Char(). If not then it's clearly stuck in:

    do{
    keypressed=read_key();
    }while(keypresses==0XFF);

so that suggests the routine ALWAYS returns 0xFF.

 

But you have an LCD here! Why not pepper the read_key() routine with calls to Lcd8_Write_String("got this far") and see exactly what is happening in that code - is it following the path of execution you expect it to? If your LCD permits you to print the value of key variables at various stages then do those variables contain the values you expect them to have?

 

This is the whole nature of debugging - you annotate code so you can follow it's path, see it's data, check when either the data values or the flow of execution deviate from what you expect.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

 If your LCD permits you to print the value of key variables at various stages then do those variables contain the values you expect them to have?

 

This is the whole nature of debugging - you annotate code so you can follow it's path, see it's data, check when either the data values or the flow of execution deviate from what you expect.

 

Yes, my LCD permits to print the value of key variables at various stages and those variables contain the values i expect them to have. In this program itself i use read_key() function thrice to get to "planting_date" and it works fine.

 

Thank you. I have been trying to debug the way you suggested.