What have I done wrong in program code simulation

Eng Emmanuel · Sep 12, 2019

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;
ADCON1=0b11000000;
  TRISA0_bit=1;
  TRISC0_bit=0;
  TRISC1_bit=0;
  ADC_Init();
  LCD_Init();
  Keypad_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
  kp=Keypad_Key_Click();
  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
  kp=Keypad_Key_Click();
  while(!kp); goto START_PGM;
  }
 
  START_PGM:
  while(1) {
  Lcd_Cmd(_LCD_CLEAR);
  rd=ADC_Read(0);
  rd1=(rd*5000.0)/1024.0;
  Ref_Temp=rd1/10.0;
  FloatToStr(Ref_Temp,TxT);
  Lcd_Out(2,1,"TEMP. READ: ");
  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:

Ian Rogers · Sep 12, 2019

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 · Sep 12, 2019

Ian Rogers said:
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 · Oct 7, 2019

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.

Pommie · Oct 7, 2019

One bad mistake is,

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;
    ADCON1=0b11000000;
    TRISA0_bit=1;
    TRISC0_bit=0;
    TRISC1_bit=0;
    ADC_Init();
    LCD_Init();
    Keypad_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
        kp=Keypad_Key_Click();
        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
        kp=Keypad_Key_Click();
        while(!kp);
        goto START_PGM;
    }
    while(1){
        Lcd_Cmd(_LCD_CLEAR);
        rd=ADC_Read(0);
        rd1=(rd*5000.0)/1024.0;
        Ref_Temp=rd1/10.0;
        FloatToStr(Ref_Temp,TxT);
        Lcd_Out(2,1,"TEMP. READ: ");
        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 · Oct 8, 2019

How about something like:

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

    ADCON1=0b11000000;
    TRISA0_bit=1;
    TRISC0_bit=0;
    TRISC1_bit=0;
    ADC_Init();
    LCD_Init();
    Keypad_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
        kp=Keypad_Key_Click();
        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
        kp=Keypad_Key_Click();
        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);
        rd=ADC_Read(0);
        rd1=(rd*5000.0)/1024.0;
        Ref_Temp=rd1/10.0;
        FloatToStr(Ref_Temp,TxT);
        Lcd_Out(2,1,"TEMP. READ: ");
        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
}

Pommie · Oct 8, 2019

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:");
        }    
        kp=Keypad_Key_Click();
        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.

What have I done wrong in program code simulation

Eng Emmanuel

New Member

Ian Rogers

User Extraordinaire

Eng Emmanuel

New Member

Beau Schwabe

Active Member

Pommie

Well-Known Member

rjenkinsgb

Well-Known Member

Pommie

Well-Known Member

Similar threads

What have I done wrong in program code simulation

Eng Emmanuel

New Member

Ian Rogers

User Extraordinaire

Eng Emmanuel

New Member

Beau Schwabe

Active Member

Pommie

Well-Known Member

rjenkinsgb

Well-Known Member

Pommie

Well-Known Member

Similar threads

Privacy & Transparency

Privacy & Transparency