PSOC 5LP thermostat

Tip / Sign in to post questions, reply, level up, and achieve exciting badges. Know more

cross mob
naalc_3270511
Level 1
Level 1

Hello everyone,

I am working a Thermostat program using the PSOC 5lp, a 100k thermistor, and a 100k resistor. The analog input is going into pin 3.1. The UART prompts the user to enter a target temp then it calculates the error and turns on a heater resistor or a fan respectively to bring the thermistor to the target temp.

Here's the problem:

After enter a target temp it correctly outputs the expected data ie. current temp, temp error, and the logic correctly turns on the fan or the heater. But, if I enter a new target temp for some reason the temp data becomes negative  Tempdata= -27894 and current temp becomes negative  Current Temp=-946. Can't figure out what I did wrong. Here's my main.c code:

#include "project.h"

#include "stdio.h"

char string[50];

int StoredVal=0;

int tempdata=0;

int main(void)

{

    CyGlobalIntEnable; /* Enable global interrupts. */

    /* Place your initialization/startup code here (e.g. MyInst_Start()) */

    UART_Start();

    ADC_Start();

  

  

 

  

    for(;;)

    {

        ADC_StartConvert();

        tempdata=ADC_GetResult16();

        char digit=UART_GetChar();

    

     if(digit && digit !=13)

      {

  

      digit=digit-48; //converts ASIC to integer//49-48=1

      int integercon=(StoredVal*10)+digit;

      StoredVal=integercon;

      sprintf(string,"You entered %d \r\n",StoredVal);

      UART_PutString(string);

  

      }

      

    

     

  

     if(digit==13)

      {

     

        int SetTemp=0;

        SetTemp=StoredVal;

       int tempc=0;

       tempc=(tempdata*(47.0/1446.0))-40.07399723;

            int temperror=SetTemp-tempc;

            CyDelay(1000);

            sprintf(string," Tempdata= %d \r\n New Target Temp=%d \r\n Current Temp=%d \r\n Temp error=%d \r\n",tempdata, SetTemp, tempc, temperror);

            UART_PutString(string);

          

      if(StoredVal<50 || StoredVal>120)

        {

            sprintf(string,"The number you entered = %d. Please enter a number greater than 50, but less than 120 then press enter.\r\n",StoredVal);

            UART_PutString(string);

            StoredVal=0;

         

      

        }

        if(temperror>3) //heater on

        {

            sprintf(string,"Temp error is %d and the heater is on. Press Enter again to set Temp back to 0.\r\n",temperror);

            UART_PutString(string);

        Heater_Write(0);

            StoredVal=0;

          

         }

        if(temperror<0) //heater off

        {

            sprintf(string,"Temp error is %d and the heater is off. Press Enter again to set Temp back to 0.\r\n",temperror);

            UART_PutString(string);

        Heater_Write(1);

          StoredVal=0;

        }

          

       }   

       

      

      }

  

  

   }

0 Likes
1 Solution
MotooTanaka
Level 9
Level 9
Distributor - Marubun (Japan)
First comment on blog Beta tester First comment on KBA

Hi,

I read your code.

Probably the first problem is that you defined string as a length of 50,

but your message strings are much longer, so you caused memory over run.

I added following code to test the real length of the message(s)

========================

void test_str_len(void)

{

    int tempdata, SetTemp, tempc, temperror ;

    tempdata = 25 ;

    SetTemp = 22 ;

    tempc = 23 ;

    temperror = 3 ;

   

            sprintf(string," Tempdata= %d \r\n New Target Temp=%d \r\n Current Temp=%d \r\n Temp error=%d \r\n",tempdata, SetTemp, tempc, temperror);

            UART_PutString(string);

sprintf(str, "Length of string was %d\n", strlen(string)) ;

print(str) ;   

                sprintf(string,"The number you entered = %d. Please enter a number greater than 50, but less than 120 then press enter.\r\n",StoredVal);

                UART_PutString(string);

sprintf(str, "Length of string was %d\n", strlen(string)) ;

print(str) ;

                sprintf(string,"Temp error is %d and the heater is on. Press Enter again to set Temp back to 0.\r\n",temperror);

                UART_PutString(string);

sprintf(str, "Length of string was %d\n", strlen(string)) ;

print(str) ;

                sprintf(string,"Temp error is %d and the heater is off. Press Enter again to set Temp back to 0.\r\n",temperror);

                UART_PutString(string);

sprintf(str, "Length of string was %d\n", strlen(string)) ;

print(str) ; 

}

========================

The result was

========================

Length of string was 73

The number you entered = 0. Please enter a number greater than 50, but less than 120 then press enter.

Length of string was 104

Temp error is 3 and the heater is on. Press Enter again to set Temp back to 0.

Length of string was 80

Temp error is 3 and the heater is off. Press Enter again to set Temp back to 0.

Length of string was 81

You entered 232

========================

So you need to set the length of string at least 104 or longer, I would say 128 to be safe.

Meantime, "sontaku"ing your code, I would organize the code something like below

as I want to have "gathering command entered" and "measuring the temp and take care of the heater" separated.

(modified main.c)

=======================

#include "project.h"

#include "stdio.h"

// char string[50];

#define STR_LEN 128

char string[STR_LEN+1] ;

int str_index = 0 ;

/**

* get_str()

* acquire a string from UART and if 13 (CR) is entered

* complete the string and return 1

* if the string is getting too long,

* complete the string at STR_LEN and return -1

* otherwise return 0

*/

int get_str(void)

{

    uint8_t c ;

    int result = 0 ;

   

    while(UART_GetRxBufferSize()) {

        c = UART_GetByte() ;

        if (c == 13) {

            string[str_index] = 0 ;

            str_index = 0 ;

            result = 1 ;

            break ;

        } else {

            string[str_index] = c ;

            str_index++ ;

            if (str_index >= STR_LEN) {

                string[STR_LEN] = 0 ;

                str_index = 0 ;

                result = -1 ;

                break ;

            }

        }

    }

    return( result ) ;

}

int main(void)

{

    int SetTemp = 60 ;

    int tempdata ;

    int tempc ;

    int temperror ;

    int StoredVal ;

   

    CyGlobalIntEnable; /* Enable global interrupts. */

    /* Place your initialization/startup code here (e.g. MyInst_Start()) */

    UART_Start();

    ADC_Start();

   

    for(;;)

    {

        if (get_str()) {

            if (strlen(string)) {

                sscanf(string, "%d", &StoredVal) ;

                sprintf(string, "StoredVal is now %d\r\n", StoredVal) ;

                UART_PutString(string) ;

            }

            if(StoredVal <50 || StoredVal >120)

            {

                sprintf(string,"The number you entered = %d. Please enter a number greater than 50, but less than 120 then press enter.\r\n",StoredVal);

                UART_PutString(string);

            } else {

                SetTemp = StoredVal ;

                sprintf(string, "SetTemp is now %d\r\n", SetTemp) ;

                UART_PutString(string) ;

            }

        }

           

        ADC_StartConvert();

        ADC_IsEndConversion(ADC_WAIT_FOR_RESULT) ; /* recommended */

        tempdata=ADC_GetResult16();

       

        tempc=(tempdata*(47.0/1446.0))-40.07399723;

        temperror=SetTemp-tempc;

   

        CyDelay(1000);           

        sprintf(string," Tempdata= %d \r\n New Target Temp=%d \r\n Current Temp=%d \r\n Temp error=%d \r\n",tempdata, SetTemp, tempc, temperror);

        UART_PutString(string);

        if(temperror>3) //heater on

        {

            sprintf(string,"Temp error is %d and the heater is on. Press Enter again to set Temp back to 0.\r\n",temperror);

            UART_PutString(string);              

            Heater_Write(0);

        }

        if(temperror<0) //heater off

        {

            sprintf(string,"Temp error is %d and the heater is off. Press Enter again to set Temp back to 0.\r\n",temperror);

            UART_PutString(string);               

            Heater_Write(1); 

        }

    }

}

=======================

moto

View solution in original post

0 Likes
2 Replies
MotooTanaka
Level 9
Level 9
Distributor - Marubun (Japan)
First comment on blog Beta tester First comment on KBA

Hi,

I read your code.

Probably the first problem is that you defined string as a length of 50,

but your message strings are much longer, so you caused memory over run.

I added following code to test the real length of the message(s)

========================

void test_str_len(void)

{

    int tempdata, SetTemp, tempc, temperror ;

    tempdata = 25 ;

    SetTemp = 22 ;

    tempc = 23 ;

    temperror = 3 ;

   

            sprintf(string," Tempdata= %d \r\n New Target Temp=%d \r\n Current Temp=%d \r\n Temp error=%d \r\n",tempdata, SetTemp, tempc, temperror);

            UART_PutString(string);

sprintf(str, "Length of string was %d\n", strlen(string)) ;

print(str) ;   

                sprintf(string,"The number you entered = %d. Please enter a number greater than 50, but less than 120 then press enter.\r\n",StoredVal);

                UART_PutString(string);

sprintf(str, "Length of string was %d\n", strlen(string)) ;

print(str) ;

                sprintf(string,"Temp error is %d and the heater is on. Press Enter again to set Temp back to 0.\r\n",temperror);

                UART_PutString(string);

sprintf(str, "Length of string was %d\n", strlen(string)) ;

print(str) ;

                sprintf(string,"Temp error is %d and the heater is off. Press Enter again to set Temp back to 0.\r\n",temperror);

                UART_PutString(string);

sprintf(str, "Length of string was %d\n", strlen(string)) ;

print(str) ; 

}

========================

The result was

========================

Length of string was 73

The number you entered = 0. Please enter a number greater than 50, but less than 120 then press enter.

Length of string was 104

Temp error is 3 and the heater is on. Press Enter again to set Temp back to 0.

Length of string was 80

Temp error is 3 and the heater is off. Press Enter again to set Temp back to 0.

Length of string was 81

You entered 232

========================

So you need to set the length of string at least 104 or longer, I would say 128 to be safe.

Meantime, "sontaku"ing your code, I would organize the code something like below

as I want to have "gathering command entered" and "measuring the temp and take care of the heater" separated.

(modified main.c)

=======================

#include "project.h"

#include "stdio.h"

// char string[50];

#define STR_LEN 128

char string[STR_LEN+1] ;

int str_index = 0 ;

/**

* get_str()

* acquire a string from UART and if 13 (CR) is entered

* complete the string and return 1

* if the string is getting too long,

* complete the string at STR_LEN and return -1

* otherwise return 0

*/

int get_str(void)

{

    uint8_t c ;

    int result = 0 ;

   

    while(UART_GetRxBufferSize()) {

        c = UART_GetByte() ;

        if (c == 13) {

            string[str_index] = 0 ;

            str_index = 0 ;

            result = 1 ;

            break ;

        } else {

            string[str_index] = c ;

            str_index++ ;

            if (str_index >= STR_LEN) {

                string[STR_LEN] = 0 ;

                str_index = 0 ;

                result = -1 ;

                break ;

            }

        }

    }

    return( result ) ;

}

int main(void)

{

    int SetTemp = 60 ;

    int tempdata ;

    int tempc ;

    int temperror ;

    int StoredVal ;

   

    CyGlobalIntEnable; /* Enable global interrupts. */

    /* Place your initialization/startup code here (e.g. MyInst_Start()) */

    UART_Start();

    ADC_Start();

   

    for(;;)

    {

        if (get_str()) {

            if (strlen(string)) {

                sscanf(string, "%d", &StoredVal) ;

                sprintf(string, "StoredVal is now %d\r\n", StoredVal) ;

                UART_PutString(string) ;

            }

            if(StoredVal <50 || StoredVal >120)

            {

                sprintf(string,"The number you entered = %d. Please enter a number greater than 50, but less than 120 then press enter.\r\n",StoredVal);

                UART_PutString(string);

            } else {

                SetTemp = StoredVal ;

                sprintf(string, "SetTemp is now %d\r\n", SetTemp) ;

                UART_PutString(string) ;

            }

        }

           

        ADC_StartConvert();

        ADC_IsEndConversion(ADC_WAIT_FOR_RESULT) ; /* recommended */

        tempdata=ADC_GetResult16();

       

        tempc=(tempdata*(47.0/1446.0))-40.07399723;

        temperror=SetTemp-tempc;

   

        CyDelay(1000);           

        sprintf(string," Tempdata= %d \r\n New Target Temp=%d \r\n Current Temp=%d \r\n Temp error=%d \r\n",tempdata, SetTemp, tempc, temperror);

        UART_PutString(string);

        if(temperror>3) //heater on

        {

            sprintf(string,"Temp error is %d and the heater is on. Press Enter again to set Temp back to 0.\r\n",temperror);

            UART_PutString(string);              

            Heater_Write(0);

        }

        if(temperror<0) //heater off

        {

            sprintf(string,"Temp error is %d and the heater is off. Press Enter again to set Temp back to 0.\r\n",temperror);

            UART_PutString(string);               

            Heater_Write(1); 

        }

    }

}

=======================

moto

0 Likes

Moto,

Good catch!

Another way to prevent string overruns (aside from increasing the receiving string length) is to use the snprintf() function.  It takes an additional input variable of the size of the receiving string as the second parameter.  The "snprintf" is considered by ANSI as a 'safe' function because a size test is performed when executing the function.  Therefore, it should be protected against "array overrun" faults which can be susceptible to viruses attacks.

I use snprintf() all the time in place of sprintf() for the very reason that if I undersized the receiving string, I'd get CPU crashes that usually result in a "Unhandled Int Vector" and/or watchdog timeout.

The return value of snprintf() is the number of chars transferred.  You can use this as a sanity check to test if the receiving string was large enough.  If the size fails, you could throw an "ASSERT".  Here's a snippet from a coding website about the return value:

"Return Value

The number of characters that would have been written if n had been sufficiently large, not counting the terminating null character.

If an encoding error occurs, a negative number is returned.

Notice that only when this returned value is non-negative and less than n, the string has been completely written."

Hope this helps.

Len

Len
"Engineering is an Art. The Art of Compromise."