[help] 16*2 character lcd problem

i am trying to use a 16*2 lcd with atmega16 at 1mhz internal oscillator. i write the code in c language using winavr but it is not working. any help ??

#include<avr\io.h>

#include<util\delay.h>


#define F_CPU 1000000UL

#define dataport PORTD
#define comm PORTB

#define rs 0
#define rw 1
#define en 2


void ready(void)
{
	unsigned char address;
	DDRD = 0x00;
	comm &= ~(1<<rs);
	comm |= (1<<rw);
	do
	{
		comm &= ~(1<<rs);
		_delay_ms(15);
		address = dataport;
		comm |= (1<<en);
	}while((address & 0x80) == 0x80);
	}
	
	
void command_write(int i, unsigned char* cmd)
{
	ready();
	DDRD = 0xFF;
	dataport = cmd[i];
	comm &= ~(1<<rs);
	comm &= ~(1<<rw);
	comm |= (1<<en);
	_delay_ms(15);
	comm &= ~(1<<en);
	DDRD = 0x00;
	}


void data_write(unsigned char dat)
{
	ready();
	DDRD = 0xFF;
	dataport = dat;
	comm |= (1<<rs);
	comm &= ~(1<<rw);
	comm |= (1<<en);
	_delay_ms(15);
	comm &= ~(1<<en);
	DDRD = 0x00;
	}
	

void data_string(unsigned char* var)
{
    while(*var)
    data_write(*var++);
	}


void lcd_initial(void)
{
	unsigned char a[] = {0x38, 0x0E, 0x01, 0x06, 0x04, 0x80};
	int k;
	for(k=0; k<7; k++)
	{
		command_write(k, a);
		}
	}
	

int main(void)
{
	unsigned char string[] = "Hello";

	DDRB = 0x00;
	
	lcd_initial();
	
	data_string(string);
	
	while(1)
	{
	}
	
	}

hi rahul,
from ur code i see that u have defined PORTB as input port
DDRB = 0x00;
in this case, u can only read from that port and cannot modify its port pins which u are doing in the command_write and command_read routine. this is not accepted by the uC.
note that DDRB only determines the input/output status of the pins and not the value of the pins. So in the main loop change the statement
DDRB = 0x00 to DDRB = 0xff
Also as ur data lines are all output, u need not repeat the statement
DDRD = 0xff and DDRD = 0x00.
just initialise the data direction of PORTD once in the main loop as
DDRD = 0xff; //all output
and replace the statements in command routines to
PORTD = 0xff; and PORTD = 0x00; instead of
DDRD = 0xff; and DDRD = 0x00;

good luck.


P.S.
if u have defined any port as input then the only way to read that port is to use the PINA/B/C/D command.i,e. say if the PORTA is defined as input:
DDRA = 0x00;
then to read the status of PORTA in variable 'a' u have to write the statement:
a = PINA; ///right method
and not
a = PORTA; //error

thanks for the reply. i correct the mistake of DDRB = 0x00 to DDRB = 0xff. about the DDRD, for busy flag i need to read the values from the port so i have to change it again and again.

still the same problem, boxes are shown by the lcd, no character.

also is the delay of 15 ms enough or not ??

Update: on avrbeginners.net i found a c code for lcd, there is 'asm volatile ("nop");' statement instead of delay. can u also tell me the purpose of this command ??

problem seems to be here..

do
        {      //instead of this
                //comm &= ~(1<<rs);
                comm &=~(1<<en);
                //_delay_ms(15); //Not needed :)
                address = dataport;
                comm |= (1<<en);
        }while((address & 0x80) == 0x80);

Try and tell me..

still the same result. boxes on the screen.

is this command
address = dataport ;
translated as
address = PORTD;
valid?
I thought to read from the port u have to use PIND i.e.
address = PIND;
anyways, will try it out and let u know.

good coding !!!

we found that the lcd was damaged so we are going to purchare a new one today and try to get it worked. i also change the delay from 15 ms to 100 ms.

please let me know if there is any other mistake.

the code so far with all the change ----

#include<avr\io.h>

#include<util\delay.h>


#define F_CPU 1000000UL

#define dataport PORTD
#define comm PORTB

#define rs 0
#define rw 1
#define en 2


void ready(void)
{
        unsigned char address;
        DDRD = 0x00;
        comm &= ~(1<<rs);
        comm |= (1<<rw);
        do
        {
                comm &= ~(1<<en);
                //_delay_ms(100);
                address = PIND;
                comm |= (1<<en);
        }while((address & 0x80) == 0x80);
        }
       
       
void command_write(int i, unsigned char* cmd)
{
        DDRD = 0xFF;
        dataport = cmd[i];
        comm &= ~(1<<rs);
        comm &= ~(1<<rw);
        comm |= (1<<en);
        _delay_ms(100);
        comm &= ~(1<<en);
        ready();
        }


void data_write(unsigned char dat)
{
        DDRD = 0xFF;
        dataport = dat;
        comm |= (1<<rs);
        comm &= ~(1<<rw);
        comm |= (1<<en);
        _delay_ms(100);
        comm &= ~(1<<en);
        ready();
        }
       

void data_string(unsigned char* var)
{
    while(*var)
    data_write(*var++);
        }


void lcd_initial(void)
{
        unsigned char a[] = {0x38, 0x0E, 0x01, 0x06, 0x04, 0x80};
        int k;
        for(k=0; k<7; k++)
        {
                command_write(k, a);
                }
        }
       

int main(void)
{
        unsigned char string[] = "Hello";

        DDRB = 0xFF;
       
        lcd_initial();
       
        data_string(string);
       
        while(1)
        {
        }
       
        }

there is no need of delay between En (H->L) just add one nop instead of that big delay.
en will only latch the value from data port to internal LCD controller, so unless the pulse is not given data will not be sent to LCD controller.

if you want to put delay then you can have it after the enable pulse.

here is the new code. but still boxes only in first row.

#include<avr\io.h>

#include<util\delay.h>


#define F_CPU 1000000UL

#define dataport PORTD
#define comm PORTB

#define rs 0
#define rw 1
#define en 2


void ready(void)
{
        unsigned char address;
        DDRD = 0x00;
        comm &= ~(1<<rs);
        comm |= (1<<rw);
        do
        {
                comm &= ~(1<<en);
                asm volatile ("nop");
				asm volatile ("nop");
				asm volatile ("nop");
				asm volatile ("nop");
                address = PIND;
                comm |= (1<<en);
        }while((address & 0x80) == 0x80);
        }
       
       
void command_write(int i, unsigned char* cmd)
{
        DDRD = 0xFF;
        dataport = cmd[i];
        comm &= ~(1<<rs);
        comm &= ~(1<<rw);
        comm |= (1<<en);
        asm volatile ("nop");
		asm volatile ("nop");
		asm volatile ("nop");
		asm volatile ("nop");
        comm &= ~(1<<en);
        ready();
        }


void data_write(unsigned char dat)
{
        DDRD = 0xFF;
        dataport = dat;
        comm |= (1<<rs);
        comm &= ~(1<<rw);
        comm |= (1<<en);
        asm volatile ("nop");
		asm volatile ("nop");
		asm volatile ("nop");
		asm volatile ("nop");
        comm &= ~(1<<en);
        ready();
        }
       

void data_string(unsigned char* var)
{
    while(*var)
    data_write(*var++);
        }


void lcd_initial(void)
{
        unsigned char a[] = {0x38, 0x0E, 0x01, 0x06, 0x04, 0x80};
        int k;
        for(k=0; k<7; k++)
        {
                command_write(k, a);
                }
        }
       

int main(void)
{
        unsigned char string[] = "Hello";

        DDRB = 0xFF;
       
        lcd_initial();
       
        data_string(string);
       
        while(1)
        {
        }
       
        }