C51 LCD help needed

CODE in C51

without comments it is not 'code' but 'scribbles'

also sticking multiple statements in the same line makes it hard/impossible to read

Erik

Have you checked it?

Don't create functions without any return type declaration. They will default to return int. If they are expected to not return anything, always write

void function_name(...) {
    ...
}

COMMENTED CODE

#include <reg52.h>
#include <stdio.h>

sbit RS=P1^6;  // define Rs
sbit E =P1^4;  // define E
sbit RW=P1^5;  // define  RW

pausa(int j) //delay
{ int i;
  for (i=0;i<j;i++)
  continue;
}


variaE()  // read control bus and data bus
{
  E =1;
    ;
  E =0;
 }


void delay(int m) // delay
{
        int j;
        for(j=0;j<=m;j++)
        continue;
}

LCDWI (char n)//write instructions for LCD
{
  RW =0;
  RS=0;
  E =0;
  P0=n;
  E =1;
  E =0;
  pausa(50);
}


LCDWD(char n) // write data in LCD
{
  RW=0;
  RS=1;
  E=0;
  P0=n;
  E=1;
  E=0;
  pausa(50);
}

print_LCD(char a,char *s)
{
LCDWI(a);
while(*s != 0);
LCDWD(*s++);
}


i_LCD() // initialization LCD
{
  RS=0;
  E=0;
  P0=0x30;variaE();delay(10);variaE();delay(1);variaE();delay(1);
  P0=0x20;variaE();variaE();variaE(); //???????
  LCDWI(0x38);
  LCDWI(0x0c);
  LCDWI(0x06);
  LCDWI(1);
  delay(50);
}


void main()
{
 i_LCD();
 print_LCD(0x80,"LCD Thermometer");   //To print string on the first line, we can use address 0x80, and message from LCD "LCD Thermometer"

}

That is _not_ commented code.

Comments should add something.

sbit RS=P1^6;  // define Rs

What use is "define Rs"? Why not instead tell exactly what Rs is?

variaE()  // read control bus and data bus
{
  E =1;
    ;
  E =0;
 }

Exactly where do you see a read in that function. To my eyes it performs two assigns. I would assume that it pulses the E control signal (don't you find the name E a bit non-descriptive by the way?) And control bus or data bus - are there two buses? Don't you mean that you latch any written instruction or data values into the LCD?

What do you think the ; in the middle does? A nop statement to take a bit extra time? The compiler will ignore it.

pausa(int j) //delay

Don't you think people can deduce that it is a delay from the function name? But what kind of delay? Delaying for ms? For seconds? Delay until the LCD has processed any written data? What does parameter j mean? By the way - use a real delay that synchronizes with the hardware. You really don't know how long delay pausa(100) will generate. It will depend on compiler settings, compiler version, crystal frequency, any changes to the call three, ...

LCDWI (char n)//write instructions for LCD
{
  RW =0;
  RS=0;
  E =0;
  P0=n;
  E =1;
  E =0;
  pausa(50);
}

Write instructions for LCD? What instructions, and how? RS=0 - is that read or write? How can I tell from the name? You seem to generate a pulse of E (E=1; E=0;) - but why do you have a function variaE() to do this, if you don't use the function? Why shorten the write of instructions to WI? And where in the name can I see that you write instructions (do you mean commands?) instead of data?

Are there any timing requirements? How do I know what they are? How do I know that your code fulfills them? And once more - what does pausa(50) do?

LCDWD(char n) // write data in LCD

Same comments as for LCDWI.

print_LCD(char a,char *s)

So what does "a" mean? Can I guess that it is an address? What values are allowed?
What does "s" mean? What character sets or length, ... are allowed?
Any special requirements to call this function?
Does it take long to execute?

i_LCD() // initialization LCD

Wouldn't it be better with a function name that said init_LCD() instead of having to rely on a comment to make it obvious? And without seeing the comment - how can you know in main() that the call i_LCD() initializes the display?

What does the comment "???????" mean? That you are slightly unsure about what the code does? How come? Didn't you write it? What does the datasheet say that you have to do to initialize the display? Are you fulfilling the datasheet requirements?

LCDWI(0x38);
LCDWI(0x0c);
LCDWI(0x06);
LCDWI(1);

Where do you see any comments that tells what the above "magic" does? Don't you think that you should have named the constants so that a reader could see if you set four or eight-bit mode or turn on/off the cursor, ...?

LCDWI() ends with a pausa(50). So what is the reason for another pausa(50) at the end of i_LCD()?

So how can you say that you posted commented code?