Atmega 328P Odd Keypad Problem

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

Hello,

 

I've been a reader of the forums for a while, but it was mostly just for curiosity. While on winter break from school I'm trying to put together a project. In the past I have used PicAxe's for projects, but I've long wanted to take the plunge into AVR's.

 

I'm programming using the Dragon, which is powered via its own powered USB hub. I am using a 328P, and I'm using the internal 8 MHz clock, and both the AVR and an LCD screen are powered via their own battery packs. So far I've gotten the serial LCD working (the Parallax 27977, the datasheet with commands is here: https://www.parallax.com/sites/d...) and also set it up to be able to use the "printf" command with it. The LCD is connected on PD0 and PD1.

 

My problem is trying to attach a 4x4 keypad and detect which number is pressed, and then send that information to the LCD. I had been working off of the user chunwwe's keypad code from here: http://www.avrfreaks.net/forum/c...

 

I'm trying to get the 4x4 to read like a standard one would, with keys mapped as:

[ 1, 2, 3, A,

  4, 5, 6, B,

  7, 8, 9, C,

  *, 0, #, D]

 

I have the 4x4 keypad connected with the 4 rows connected with: row 1 to PB0, row 2 to PB1, row 3 to PB2 and row 4 to PB3. The columns are: col 1 to PB4, col 2 to PB5, col 3 to PB6, and col 4 to PB7. There are 10k pulldown resistors on each of the rows (PB0 to PB3.) Im having the columns go high one at a time, and if a row also goes high (indicating a button press) then it should be able to pick the value of the button from the array.

 

 

With the attached code, I am able to read when the fourth column buttons are pressed, the "A," "B," "C," and "D" buttons read, but no others will. I have tried using different formatting in the number cases (instead of "%d" using "%s" or "%x") with no luck there either. 

 

So, here's the code, if anyone has any suggestions, please let me know. Thank you in advance!

#define F_CPU 8000000UL /* 8Mhz clock rate */
#define BAUD 2400
#define USART_BAUDRATE 2400
#define BAUD_PRESCALE (((F_CPU / (USART_BAUDRATE * 16UL))) - 1)
#define display_On 0x16 /*  Parralax cmd: Display on with no cursor and no blink */
#define first_position 0x80 /* Parralax cmd: Move cursor to line 0 position 0 */
#define form_feed 0x0C /* Parralax cmd: Clear all text, move to line 0 pos 0 */
#define line_feed 0x0A /* Parralax cmd: Cursor moved to next line */
#define backlight_on 0x11 /* Parralax cmd: LCD Backlight on */
#define backlight_off 0x12 /* Parralax cmd: LCD Backlight off */

#include <util/delay.h>
#include <avr/io.h>
#include <stdio.h>

static inline void ser_init() /* I'm told Static Inline functions are great for things that are called once (like initializing serial) */
{
	UCSR0B = (1 << TXEN0) | (1 << RXEN0); /* Serial Transmit and Recieve Enable */
	UCSR0C = (0 << USBS0) | (3 << UCSZ00); /* Set Stop Bit Length (1 Stop Bit) and Frame Length (8 data bits). No parity */
	UBRR0H = (unsigned char)(BAUD_PRESCALE >> 8); /* Load upper 8-bits of baud rate value into high byte of UBBR0 register */
	UBRR0L = (unsigned char)BAUD_PRESCALE; /* Load lower 8-bits of the baud rate value into low byte of UBBR0 register */
	while ( !( UCSR0A & (1 << UDRE0)));
	; /* Wait for an empty Transmit Buffer */
	UDR0 = display_On; /* Turn LCD on with no blink and no cursor */

}

/* Keypad reading.... */
unsigned char keypad(void)
{
	unsigned char key;	/* Used to store the key pressed on the 4x4 keypad matrix */
	unsigned char portb_pin;
	unsigned char keypad_column[4] = {7,6,5,4};	/* Pins b7, 6, 5, 4 of Port A control keypad columns */
	unsigned char keypad_row[4] = {3,2,1,0};	/* Pins b3, 2, 1, 0 of Port A control keypad rows */
	unsigned char keypad_array[4][4] = /* Array containing keypad values */
	{	{'1','2','3','A'},
	{'4','5','6','B'},
	{'7','8','9','C'},
	{'*','0','#','D'}	};
	unsigned char column;
	unsigned char row;
	DDRB = 0xF0;	/* Configure Port B, Pins b7 to b4 are output. Pins b3 to b0 are input */

	for (column=0; column<4; column++)
	{	//
		PORTB = (1<<(keypad_column[column]));
		_delay_ms(50);
		portb_pin = PINB;	/* Read the value from port B */

		for(row=0; row<4; row++)
		{
			if((portb_pin & (1<<(keypad_row[row])))==1)
			{
				key = keypad_array[column][row];	/* Search for the corresponding element in keypad array */
				return (key);	/* Return the answer, which key is pressed */
			}
		}
	}
}

int printCHAR(char character, FILE *stream) /* Has printchar become a stream */
{

	while ((UCSR0A & (1 << UDRE0)) == 0) {}; /* While the transmit flag is active, the serial buffer is a character */

	UDR0 = character;

	return 0;
}

FILE uart_str = FDEV_SETUP_STREAM(printCHAR, NULL, _FDEV_SETUP_RW); /* Sets up "printf" */

int main(void)
{
	stdout = &uart_str; /* Links stdout and stream from file we set up */
	unsigned char digit;	/* Temporary variable to hold the value of which key is pressed */
	ser_init(); /* Serial Initialize */

	UDR0 = backlight_on; /* Turn on LCD backlight */

	while(1)
	{
		digit = keypad();	/* Call the  function "keypad" to return the value of key pressed, and hold it with variable "digit" */

		switch(digit) /* Now evaluate value of "digit" to match with the LED pattern needed to be exported to Port B */
		{
			case '0':
			case '1':
			case '2':
			case '3':
			case '4':
			case '5':
			case '6':
			case '7':
			case '8':
			case '9':
			while ( !( UCSR0A & (1 << UDRE0))); /* Wait for serial buffer to clear, clear screen, turn on backlight, display text */
			UDR0 = form_feed;
			_delay_ms(5);
			UDR0 = backlight_on;
			printf("The # is:\r");
			printf("%d", digit);
			_delay_ms(50);
			break;

			case 'A':
			case 'B':
			case 'C':
			case 'D':
			case '*':
			case '#':
			while ( !( UCSR0A & (1 << UDRE0))); /* Wait for serial buffer to clear, clear screen, turn on backlight, display text */
			UDR0 = form_feed;
			_delay_ms(5);
			UDR0 = backlight_on;
			printf("The # is:\r");
			printf("%c", digit);
			_delay_ms(50);
			break;

		}

	}
	return(1);
}

 

This topic has a solution.
Last Edited: Wed. Jan 17, 2018 - 01:15 AM
This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'd suggest the root cause is this line:

if((portb_pin & (1<<(keypad_row[row])))==1)

take away the ==1 as the result will not always be 0 or 1 depending on the actual bit you're testing.

 

your code have a number of things that could be improved -

1. why pulldown resistors when you have pullups on chip?

2. to select a row, the port pin should be made an output and low. To deselect, it should be made an input. This is so multiple keypresses don't short the port pins as outputs.

3. your constant tables should be declared __flash and const.

4. where's the debounce?

 

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

Thank you for the suggestions!

 

Right now the keypad is a priority, and I am currently using short delays in lieu of a debounce (which is not optimal, but is just until I can make sure the keypad is working). Thank you for the __flash and const suggestion as well, I'll look into that as soon as I can get this problem working.

 

I took off the pulldowns, and am using the internal pullups (im pretty sure...) and I also removed the ==1 result. I also tried to address your #2 point. The resulting code now has the LCD just flash 

 

"The # is:

49 "

 

The closest I was able to get to reading the pad was the original code. Here are the affected lines from what I mangled from your suggestions:

/* Keypad reading.... *
/
unsigned char keypad(void)
{
	unsigned char key;	/* Used to store the key pressed on the 4x4 keypad matrix */
	unsigned char portb_pin;
	unsigned char keypad_column[4] = {7,6,5,4};	/* Pins b7, 6, 5, 4 of Port A control keypad columns */
	unsigned char keypad_row[4] = {3,2,1,0};	/* Pins b3, 2, 1, 0 of Port A control keypad rows */
	unsigned char keypad_array[4][4] = /* Array containing keypad values */
	{	{'1','2','3','A'},
	{'4','5','6','B'},
	{'7','8','9','C'},
	{'*','0','#','D'}	};
	unsigned char column;
	unsigned char row;
	DDRB = 0xF0;	/* Configure Port B, Pins b7 to b4 are output. Pins b3 to b0 are input */
	PORTB = 0x0F;

	for (column=0; column<4; column++)
	{	//
		PORTB = ~(1<<(keypad_column[column]));
		_delay_ms(50);
		portb_pin = PINB;	/* Read the value from port B */

		for(row=0; row<4; row++)
		{
			if((portb_pin & ((1<<keypad_row[row]))))
			{
				key = keypad_array[column][row];	/* Search for the corresponding element in keypad array */
				return (key);	/* Return the answer, which key is pressed */
			}
		}
	}
}

I'm sure that its something that I'm misinterpreting, but I appreciate the suggestions.

Last Edited: Sun. Jan 14, 2018 - 10:24 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

First rule of debugging - change one thing at a time. You just solved one problem and created more!

To clear a port bit it is: PORTB &= ~

Also note that the bits you read back will be inverted, so you code needs to consider that.

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

I finally got it to work. It doesn't have the __flash constants or the debounce, but I figured I'd post it anyways in case it will make someone else's life easier down the road. So, even though it's not at all polished, here is an LCD and 4x4 keypad read on the 328P. Thank you for the suggestions, Kartman.

 

/*
 * LCD_Keypad_WORKS.c
 *
 * Created: 1/14/2018 8:44:55 PM
 * Author : rawri
 */ 
/*
 * Keypad.c
 *
 * Created: 1/13/2018 10:26:01 AM
 * Author : rawri
 */ 
#define F_CPU 8000000UL /* 8Mhz clock rate */
#define BAUD 2400
#define USART_BAUDRATE 2400
#define BAUD_PRESCALE (((F_CPU / (USART_BAUDRATE * 16UL))) - 1)
#define display_On 0x16 /*  Parralax cmd: Display on with no cursor and no blink */
#define first_position 0x80 /* Parralax cmd: Move cursor to line 0 position 0 */
#define form_feed 0x0C /* Parralax cmd: Clear all text, move to line 0 pos 0 */
#define line_feed 0x0A /* Parralax cmd: Cursor moved to next line */
#define backlight_on 0x11 /* Parralax cmd: LCD Backlight on */
#define backlight_off 0x12 /* Parralax cmd: LCD Backlight off */

#include <util/delay.h>
#include <avr/io.h>
#include <stdio.h>

static inline void ser_init() /* I'm told Static Inline functions are great for things that are called once (like initializing serial) */
{
	UCSR0B = (1 << TXEN0) | (1 << RXEN0); /* Serial Transmit and Recieve Enable */
	UCSR0C = (0 << USBS0) | (3 << UCSZ00); /* Set Stop Bit Length (1 Stop Bit) and Frame Length (8 data bits). No parity */
	UBRR0H = (unsigned char)(BAUD_PRESCALE >> 8); /* Load upper 8-bits of baud rate value into high byte of UBBR0 register */
	UBRR0L = (unsigned char)BAUD_PRESCALE; /* Load lower 8-bits of the baud rate value into low byte of UBBR0 register */
	while ( !( UCSR0A & (1 << UDRE0)));
	; /* Wait for an empty Transmit Buffer */
	UDR0 = display_On; /* Turn LCD on with no blink and no cursor */
	
}

unsigned char keypad(void){	// Define the function to return which key is pressed
	unsigned char key;	//which key is pressed?
	unsigned char portb_pin;
	
	unsigned char keypad_column[4] = {7,6,5,4};	//Pins b7, 6, 5, 4 of Port B control keypad columns
	unsigned char keypad_row[4] = {3,2,1,0};	// Pins b3, 2, 1, 0 of Port B control keypad rows
	unsigned char keypad_array[4][4] ={	//Array contains all
		{'1','2','3','A'},
		{'4','5','6','B'},
		{'7','8','9','C'},
		{'*','0','#','D'}
	};
	unsigned char column;
	unsigned char row;

	DDRB = 0xF0;	//Configure Port B, Pins b7 to b4 are output. Pins b3 to b0 are input.
	for (column=0; column<4; column++){	//
		PORTB = ~(1<<(keypad_column[column]));
		_delay_ms(20);
		portb_pin = PINB;	//read the value from port B
		for(row=0; row<4; row++){
			if((portb_pin & (1<<(keypad_row[row])))==0) {
				key = keypad_array[column][row];	// search for the corresponding element in keypad array
				return (key);	// return the answer, which key is pressed?
			}
		}
	}
}



int printCHAR(char character, FILE *stream) /* Has printchar become a stream */
{
	

	while ((UCSR0A & (1 << UDRE0)) == 0) {}; /* While the transmit flag is active, the serial buffer is a character */

	UDR0 = character;

	return 0;
}

FILE uart_str = FDEV_SETUP_STREAM(printCHAR, NULL, _FDEV_SETUP_RW); /* Sets up "printf" */


int main(void)
{
	stdout = &uart_str; /* Links stdout and stream from file we set up */
	int digit;	/* Temporary variable to hold the value of which key is pressed */
	ser_init(); /* Serial Initialize */
	
	UDR0 = backlight_on; /* Turn on LCD backlight */
	
	

	
	while(1)
	{
		digit = keypad();	/* Call the  function "keypad" to return the value of key pressed, and hold it with variable "digit" */
		
		switch(digit) /* Now evaluate value of "digit" to match with the LED pattern needed to be exported to Port B */
		{
			case '0':
			case '1':		
			case '2':
			case '3':
			case '4':
			case '5':
			case '6':
			case '7':
			case '8':
			case '9':
			while ( !( UCSR0A & (1 << UDRE0))); /* Wait for serial buffer to clear, clear screen, turn on backlight, display text */
			UDR0 = form_feed;
			_delay_ms(5);
			UDR0 = backlight_on;
			printf("The # is:\r");
			printf("%c", digit);
			_delay_ms(50);
			break;
			
			case 'A':
			case 'B':
			case 'C':
			case 'D':
			case '*':
			case '#':
			while ( !( UCSR0A & (1 << UDRE0))); /* Wait for serial buffer to clear, clear screen, turn on backlight, display text */
			UDR0 = form_feed;
			_delay_ms(5);
			UDR0 = backlight_on;
			printf("The # is:\r");
			printf("%c", digit);
			_delay_ms(50);
			break;
			
			
		}
		
		
		
	}
	return(1);
}

 

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

It ‘works’ but nowhere near as robust as it could be. Try measuring the current of the cpu whilst pressing a few buttons on the keypad simultaneously.

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

That is true, it is not as robust as it could be. I'm pretty sure that I said that in my last post. It took me quite a while to get to that point though, and instead of just letting the post die I figured I'd at least post the working code. I do plan on working on Debounce and looking into multiple key presses, but as my code was just about getting it working I sort of figured that it was outside the scope of this post...I wasn't really trying to turn this post into a repository for this project, but if you think it'll help I will

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

This is the way to mark your topic as 'Solved': http://www.avrfreaks.net/comment...