# What have I done wrong in program code simulation

#### Eng Emmanuel

##### New Member
Hello,
I have created a simple circuit of Automatic temperature controller with the keypad interface and the microcontroller (PIC16F877A) output goes to the Fan (whenever temperature is high) and Heater (whenever temperature is low), but the simulation does not work perfectly. I am using a MICROC PRO COMPILER with PROTEUS 8.0.
My code:
C:
char keypadPort at PORTD;

sbit LCD_RS at RB4_bit;
sbit LCD_EN at RB5_bit;
sbit LCD_D4 at RB0_bit;
sbit LCD_D5 at RB1_bit;
sbit LCD_D6 at RB2_bit;
sbit LCD_D7 at RB3_bit;

sbit LCD_RS_Direction at TRISB4_bit;
sbit LCD_EN_Direction at TRISB5_bit;
sbit LCD_D4_Direction at TRISB0_bit;
sbit LCD_D5_Direction at TRISB1_bit;
sbit LCD_D6_Direction at TRISB2_bit;
sbit LCD_D7_Direction at TRISB3_bit;

void main() {
unsigned short rd,kp;
unsigned int Temp_Ref;
float Ref_Temp,rd1;
unsigned char TxT[4],TxT1,InTemp;
TRISA0_bit=1;
TRISC0_bit=0;
TRISC1_bit=0;
LCD_Init();
Lcd_Cmd(_LCD_CLEAR);
Lcd_Cmd(_LCD_CURSOR_OFF);
Lcd_Out(1,4,"AUTOMATIC");
delay_ms(1000);
Lcd_Out(2,1,"TEMPERATURE CONTROL");
Lcd_Cmd(_LCD_SHIFT_LEFT);
delay_ms(2000);
Lcd_Cmd(_LCD_CLEAR);
PORTC.RC0=0;
PORTC.RC1=0;

START:
Lcd_Cmd(_LCD_CLEAR);
Lcd_Out(1,1,"Enter Temp. Ref");
Temp_Ref=0;
Lcd_Out(2,1,"Temp. Ref:");
while(1){
do
while(!kp);
if(kp==15)break;
if(kp>3 && kp<12)kp=kp-2;
if(kp==14)kp=0;
if(kp==13) goto START;
Lcd_Chr_Cp(kp+'0');
Temp_Ref=(10*Temp_Ref)+kp;}
Lcd_Cmd(_LCD_CLEAR);
Lcd_Out(1,1,"Temp. Ref:");
IntToStr(Temp_Ref,TxT1);
InTemp=Ltrim(TxT1);
Lcd_Out_Cp(InTemp);
Lcd_Out(2,1,"Press # to cont.");
kp=0;
while(kp!=15)
{
do
while(!kp); goto START_PGM;
}

START_PGM:
while(1) {
Lcd_Cmd(_LCD_CLEAR);
rd1=(rd*5000.0)/1024.0;
Ref_Temp=rd1/10.0;
FloatToStr(Ref_Temp,TxT);
Lcd_Out(1,13,TxT);
Lcd_Out(1,15,'C');
if(Ref_Temp>Temp_Ref){
PORTC.RC0=0,
PORTC.RC1=1;}
if(Ref_Temp<Temp_Ref){
PORTC.RC0=1,
PORTC.RC1=0;}
if(Ref_Temp==Temp_Ref){
PORTC.RC0=0,
PORTC.RC1=0;}  }

}
Simulation:

Last edited by a moderator:

#### Ian Rogers

##### User Extraordinaire
Forum Supporter
First... Its very bad C practice to use goto and lables... You will also run into trouble in real time with two port writes to inductive loads one after the other. I would use PORTC = 1 or 2 instead.

I'm very surprised that MikroC allows "if(Ref_Temp>Temp_Ref)" as they are different variables, You'll need a cast for one or the other one is a float and the other an int.

try
if((int)Ref_Temp>Temp_Ref){ blah blah }

I also really hate the way MikroC allows non case sensitivity... For portability, you can switch it off.

#### Eng Emmanuel

##### New Member
First... Its very bad C practice to use goto and lables... You will also run into trouble in real time with two port writes to inductive loads one after the other. I would use PORTC = 1 or 2 instead.

I'm very surprised that MikroC allows "if(Ref_Temp>Temp_Ref)" as they are different variables, You'll need a cast for one or the other one is a float and the other an int.

try
if((int)Ref_Temp>Temp_Ref){ blah blah }

I also really hate the way MikroC allows non case sensitivity... For portability, you can switch it off.
Thanks, let me try it.

#### Beau Schwabe

##### Active Member
It is a bad idea to drive your relays directly from a micro.... you should use a mosfet or transistor buffer. Additionally a back emf diode should be used across each relay to prevent latch-up in the micro.

Last edited:

#### Pommie

##### Well-Known Member
Code:
        kp=Keypad_Key_Click();
while(!kp);
This will wait forever if kp equals zero.
Assuming you want to wait until kp has a value other than zero try,
Code:
  while(!kp)               //note - NO semicolon
kp=Keypad_Key_Click();
Mike.
BTW, if you correctly indent your code it is much easier to follow and spot mistakes.
Here is your code indented with the unused labels removed.
EDIT, I was wrong START is used!!! and jumps out of a while loop - eeek.
Code:
void main(){
unsigned short rd,kp;
unsigned int Temp_Ref;
float Ref_Temp,rd1;
unsigned char TxT[4],TxT1,InTemp;
TRISA0_bit=1;
TRISC0_bit=0;
TRISC1_bit=0;
LCD_Init();
Lcd_Cmd(_LCD_CLEAR);
Lcd_Cmd(_LCD_CURSOR_OFF);
Lcd_Out(1,4,"AUTOMATIC");
delay_ms(1000);
Lcd_Out(2,1,"TEMPERATURE CONTROL");
Lcd_Cmd(_LCD_SHIFT_LEFT);
delay_ms(2000);
START:
Lcd_Cmd(_LCD_CLEAR);
PORTC.RC0=0;
PORTC.RC1=0;
Lcd_Cmd(_LCD_CLEAR);
Lcd_Out(1,1,"Enter Temp. Ref");
Temp_Ref=0;
Lcd_Out(2,1,"Temp. Ref:");
while(1){
do
while(!kp);
if(kp==15)
break;
if(kp>3 && kp<12)
kp=kp-2;
if(kp==14)
kp=0;
if(kp==13)
goto START;            //Eeek, you can't do this
Lcd_Chr_Cp(kp+'0');
Temp_Ref=(10*Temp_Ref)+kp;
}
Lcd_Cmd(_LCD_CLEAR);
Lcd_Out(1,1,"Temp. Ref:");
IntToStr(Temp_Ref,TxT1);
InTemp=Ltrim(TxT1);
Lcd_Out_Cp(InTemp);
Lcd_Out(2,1,"Press # to cont.");
kp=0;
while(kp!=15){
do
while(!kp);
goto START_PGM;
}
while(1){
Lcd_Cmd(_LCD_CLEAR);
rd1=(rd*5000.0)/1024.0;
Ref_Temp=rd1/10.0;
FloatToStr(Ref_Temp,TxT);
Lcd_Out(1,13,TxT);
Lcd_Out(1,15,'C');
if(Ref_Temp>Temp_Ref){
PORTC.RC0=0,
PORTC.RC1=1;
}
if(Ref_Temp<Temp_Ref){
PORTC.RC0=1,
PORTC.RC1=0;
}
if(Ref_Temp==Temp_Ref){
PORTC.RC0=0,
PORTC.RC1=0;
}
}
}

#### rjenkinsgb

##### Well-Known Member

Added bits commented & spaced out, I've not re-indented it all..

I don't see a loop that prevents the whole thing exiting though. Should it also have a while() outside the for()..

Edit - forgot to clear the flag for later passes..

C:
void main(){
unsigned short rd,kp;
unsigned int Temp_Ref;
float Ref_Temp,rd1;
unsigned char TxT[4],TxT1,InTemp;

unsigned short restart_flag; // Extra variable

TRISA0_bit=1;
TRISC0_bit=0;
TRISC1_bit=0;
LCD_Init();
Lcd_Cmd(_LCD_CLEAR);
Lcd_Cmd(_LCD_CURSOR_OFF);
Lcd_Out(1,4,"AUTOMATIC");
delay_ms(1000);
Lcd_Out(2,1,"TEMPERATURE CONTROL");
Lcd_Cmd(_LCD_SHIFT_LEFT);
delay_ms(2000);

for(;;) { // Wrap the whole thing in an endless loop
restart_flag = false;

//     START:

Lcd_Cmd(_LCD_CLEAR);
PORTC.RC0=0;
PORTC.RC1=0;
Lcd_Cmd(_LCD_CLEAR);
Lcd_Out(1,1,"Enter Temp. Ref");
Temp_Ref=0;
Lcd_Out(2,1,"Temp. Ref:");
while(1){
do
while(!kp);
if(kp==15)
break;
if(kp>3 && kp<12)
kp=kp-2;
if(kp==14)
kp=0;
if(kp==13)
{
restart_flag = true; // Set the flag
break;
}
//        goto START;            //Eeek, you can't do this

Lcd_Chr_Cp(kp+'0');
Temp_Ref=(10*Temp_Ref)+kp;
}

if(restart_flag)    // Check the flag bit
break;            // And break out the FOR loop if set

Lcd_Cmd(_LCD_CLEAR);
Lcd_Out(1,1,"Temp. Ref:");
IntToStr(Temp_Ref,TxT1);
InTemp=Ltrim(TxT1);
Lcd_Out_Cp(InTemp);
Lcd_Out(2,1,"Press # to cont.");
kp=0;
while(kp!=15){
do
while(!kp);

restart_flag = true;        // And again
//        goto START_PGM;
}

if(restart_flag)    // Check the flag bit
break;            // And break out the FOR loop if set

while(1){
Lcd_Cmd(_LCD_CLEAR);
rd1=(rd*5000.0)/1024.0;
Ref_Temp=rd1/10.0;
FloatToStr(Ref_Temp,TxT);
Lcd_Out(1,13,TxT);
Lcd_Out(1,15,'C');
if(Ref_Temp>Temp_Ref){
PORTC.RC0=0,
PORTC.RC1=1;
}
if(Ref_Temp<Temp_Ref){
PORTC.RC0=1,
PORTC.RC1=0;
}
if(Ref_Temp==Temp_Ref){
PORTC.RC0=0,
PORTC.RC1=0;
}
}

}    // Close the extra loop
}

Last edited:

#### Pommie

##### Well-Known Member
If you indent it (and find the hidden closing braces at the end of lines) then it doesn't exit.

For the first part of the code that has to repeat if key 13 is pressed, I thought something like,
Code:
   //     START:
kp=13;
while(kp!=15){
if(kp==13){
Lcd_Cmd(_LCD_CLEAR);
Lcd_Out(1,1,"Enter Temp. Ref");
Temp_Ref=0;
Lcd_Out(2,1,"Temp. Ref:");
}
if(kp>3 && kp<12)
kp=kp-2;
if(kp==14)
kp=0;
Lcd_Chr_Cp(kp+'0');
Temp_Ref=(10*Temp_Ref)+kp;
}
Once it gets past this part it never goes back and so another while forever loop does the temperature monitor/adjust.

Mike.