Comparing Floats.

My problem is I need to compare setpoint to temperatureC and make PIN13 high if the
temperatureC is less than the setpoint.

I have tested all the other code and it works fine.

My problem is in the RELAY1 Function at the end of the code. I replaced the code in that block with a simple led flashing code and it worked so I know my problem is to do with comparing the setpoint against temperature C.

I wrote the Roundsetpoint function to try and sort the problem but still dose not work.

Any thoughts?

Andy

#include <math.h>
#include <EEPROM.h>
#include <LiquidCrystal.h>
#include <HCMatrixKeypad.h>
LiquidCrystal lcd(12, 11, 5, 4, 3, 2);
#include <HCMatrixKeypad.h>

/* Sets how many times the same key must be scanned before it is accepted as 
   pressed (settled). If you get multiple key presses then increase this value 
   (max 255)*/
#define DEBOUNCE 10

#define C1 A2/* DIO for keypad column 1 */
#define C2 10 /* DIO for keypad column 2 */
#define C3 9  /* DIO for keypad column 3 */
#define C4 8  /* DIO for keypad column 4. Delete if there is no column 4! */
#define C5 7  /* DIO for keypad column 5. Delete if there is no column 5! */
#define C6 6  /* DIO for keypad column 6. Delete if there is no column 6! */

#define R1 A1 /* DIO for keypad row 1 */
HCMatrixKeypad Keypad(DEBOUNCE, C1, C2, C3, C4, C5, C6, ROWMARKER, R1); /* Uncomment for 6x1 keypad */
byte key = 0;
byte Left = 4;
byte Right = 3;
float setpoint;
float temperatureC;
int sensorPin = A0;
int Roundedtemp;
int Roundedsetpoint;
void setup()
{
    Serial.begin(9600);
    EEPROM.read (1); // make the eeprom or atmega328 memory address 1
    setpoint = EEPROM.read(1);
    lcd.begin(16, 2);
    lcd.print("Deg C");  
    lcd.setCursor(6, 0);
    lcd.print("SetP");
    pinMode(13, OUTPUT);
}


/* Main program */
void loop()
{
   relay1();
   gettemp();
   RoundSetpoint();
     /* Scans the keypad once. This line needs to be run repeatedly */
  Keypad.Scan();
  
  /* Has a new key been pressed  */ 
  if(Keypad.New_Key()) 
  {
    /* If so the send the key to the serial port */
       
    key = (Keypad.Read() % 10); /* 1's column is the keypad column number */
    incUP();
    incDown(); 
    delay(20);
    
    Serial.print (setpoint);
    
}

}

 void incUP()
 {
   
   if (Left == key) {
    key = 0;

    setpoint = setpoint+1;
    
   return;  
   }
 }
  void incDown()
  
  {
 
    if (Right == key) {
    key = 0;
    setpoint = setpoint-0.5;
    
   return;  
   }
 
 }
 void gettemp()
 {
   {
   
     int reading = analogRead(sensorPin);  
  // converting that reading to voltage, for 3.3v arduino use 3.3
 float voltage = reading * 3.3;
 voltage /= 1024.0; 
 // now print out the temperature
 float temperatureC = (voltage - 0.5) * 100 ;  //converting from 10 mv per degree wit 500 mV offset
                                             //to degrees ((voltage - 500mV) times 100)
 // now convert to Fahrenheit
 float temperatureF = (temperatureC * 9.0 / 3.3) + 32.0;
 lcd.setCursor(0, 1);
 lcd.print(temperatureC); 
 lcd.setCursor(6, 1);
 lcd.print (setpoint);
 EEPROM.write (1,setpoint);  
     
   }
 }

void relay1()
{
  {
 if (Roundedtemp <= Roundedsetpoint)
 {
 digitalWrite(13, HIGH);
 
 }
 else
 {
 digitalWrite(13, LOW);
 }
 return;
  }
   
}

void RoundSetpoint()
{
Roundedtemp = round(temperatureC);
Roundedsetpoint =round(setpoint);

}

Add some serial.print() statements to see what the actual values are.

As an aside, if you are comparing integer temperatures and setpoints, it doesn't make much sense to decrement the setpoint by 0.5.

andyedtec:
My problem is I need to compare setpoint to temperatureC and make PIN13 high if the
temperatureC is less than the setpoint.

I have tested all the other code and it works fine.

My problem is in the RELAY1 Function at the end of the code. I replaced the code in that block with a simple led flashing code and it worked so I know my problem is to do with comparing the setpoint against temperature C.

I wrote the Roundsetpoint function to try and sort the problem but still dose not work.

Any thoughts?

Normally, one should not compare floats because roundoff errors can trick the comparison. For example, if you did “X = 10.0 / 3.0”, X would be 3.3333… but then X * 3 == 10.0 may return false because of rounding errors.

But, this isn’t your problem I don’t think.

You should be able to say “if temperature < setpoint then turn on 13” because if there IS any comparison error, it will be so minute that it won’t matter.

You may wish to use a deadband to keep your system from hunting or rapidly switching on and off as the temperature just barely goes above and below setpoint. For example, something like this:

if (temperature + 0.5) < setpoint then 13 = ON
if (temperature - 0.5) > setpoint then 13 = OFF

This will give you a 1 “unit” deadband that the temperature can drift within without changing pin 13.

Lastly, your degrees C to F code is wrong. You want F = (C * (9 / 5)) + 32 or F = (C * 1.8) + 32 (note 9/5 == 1.8).

The 9.0 / 3.3 is wrong. It’s supposed to be 9.0 / 5.0 (or just 1.8).

http://c-faq.com/fp/fpequal.html

One big problem is that the “float temperatureC” in gettemp() creates a new local variable that is DIFFERENT from the global variable of the same name.

Try this:

//#include <math.h>
#include <EEPROM.h>
#include <LiquidCrystal.h>
//#include <HCMatrixKeypad.h>
LiquidCrystal lcd(12, 11, 5, 4, 3, 2);

/* Sets how many times the same key must be scanned before it is accepted as
   pressed (settled). If you get multiple key presses then increase this value
   (max 255)*/
const int DEBOUNCE = 10;

const int C1 = A2; /* DIO for keypad column 1 */
const int C2 = 10; /* DIO for keypad column 2 */
const int C3 = 9;  /* DIO for keypad column 3 */
const int C4 = 8;  /* DIO for keypad column 4. Delete if there is no column 4! */
const int C5 = 7;  /* DIO for keypad column 5. Delete if there is no column 5! */
const int C6 = 6;  /* DIO for keypad column 6. Delete if there is no column 6! */
const int R1 = A1; /* DIO for keypad row 1 */
HCMatrixKeypad Keypad(DEBOUNCE, C1, C2, C3, C4, C5, C6, ROWMARKER, R1); /* Uncomment for 6x1 keypad */

const byte Left = 4;  // Column number of Left key
const byte Right = 3;  // Column number of Right key

float setpoint;
float temperatureC;

const int sensorPin = A0;

void setup() {
  Serial.begin(9600);
  EEPROM.read (1); // make the eeprom or atmega328 memory address 1
  setpoint = EEPROM.read(1);
  lcd.begin(16, 2);
  lcd.print("Deg C");
  lcd.setCursor(6, 0);
  lcd.print("SetP");
  pinMode(13, OUTPUT);
}


/* Main program */
void loop() {
  temperatureC = gettemp();

  lcd.setCursor(0, 1);
  lcd.print(temperatureC);
  lcd.setCursor(6, 1);
  lcd.print (setpoint);

  if (temperatureC < setpoint)
    digitalWrite(13, HIGH);
  else
    digitalWrite(13, LOW);

  /* Scans the keypad once. This line needs to be run repeatedly */
  Keypad.Scan();

  /* Has a new key been pressed  */
  if (Keypad.New_Key()) {
    /* If so the send the key to the serial port */

    /* 1's column is the keypad column number */
    byte key = (Keypad.Read() % 10);
    switch (key) {
      case Left:
        incUP();
        break;
      case Right:
        incDown();
        break;
    }
    delay(20);

    Serial.print (setpoint);
  }
}

void incUP() {
  setpoint = setpoint + 1;
  EEPROM.write (1, setpoint);
}

void incDown() {
  setpoint = setpoint - 1;
  EEPROM.write (1, setpoint);
}

float gettemp() {
  // converting an analog reading to a voltage,
  // for 3.3v arduino use 3.3
  float voltage = (analogRead(sensorPin) * 3.3) / 1024.0;

  //converting from 10 mv per degree C with 500 mV offset
  //to degrees ((voltage - 500mV) times 100)
  return (voltage - 0.5) * 100;
}

For temperature, float variables are really not appropriate. Not only do you have this rounding issue, but you also lose precision using floats. A better idea would be to store temperature in thousandths of a degree and keep it in a int or a long depending on the range of temperatures.

The only time float variables are useful is when you have data that ranges over a wide range. Temperature does not, at least not with anything you are using to measure with an Arduino. If you want to keep up with kilometers to three decimal places, then use meters instead. Want meters to 6 decimal places, use micrometers. But keep everything in integers to keep the math working. The only time you need floats is when you have kilometers to six decimal places AND micrometers to six places in the same calculation. Then there's not enough digits to represent both numbers in one format and you use floats. And even then, careful use of math can still avoid them.

John thanks that worked great.

Could you please explain your changes for me please.

Thanks

Andy

johnwasser:
One big problem is that the “float temperatureC” in gettemp() creates a new local variable that is DIFFERENT from the global variable of the same name.

Try this:

//#include <math.h>

#include <EEPROM.h>
#include <LiquidCrystal.h>
//#include <HCMatrixKeypad.h>
LiquidCrystal lcd(12, 11, 5, 4, 3, 2);

/* Sets how many times the same key must be scanned before it is accepted as
  pressed (settled). If you get multiple key presses then increase this value
  (max 255)*/
const int DEBOUNCE = 10;

const int C1 = A2; /* DIO for keypad column 1 /
const int C2 = 10; /
DIO for keypad column 2 /
const int C3 = 9;  /
DIO for keypad column 3 /
const int C4 = 8;  /
DIO for keypad column 4. Delete if there is no column 4! /
const int C5 = 7;  /
DIO for keypad column 5. Delete if there is no column 5! /
const int C6 = 6;  /
DIO for keypad column 6. Delete if there is no column 6! /
const int R1 = A1; /
DIO for keypad row 1 /
HCMatrixKeypad Keypad(DEBOUNCE, C1, C2, C3, C4, C5, C6, ROWMARKER, R1); /
Uncomment for 6x1 keypad */

const byte Left = 4;  // Column number of Left key
const byte Right = 3;  // Column number of Right key

float setpoint;
float temperatureC;

const int sensorPin = A0;

void setup() {
  Serial.begin(9600);
  EEPROM.read (1); // make the eeprom or atmega328 memory address 1
  setpoint = EEPROM.read(1);
  lcd.begin(16, 2);
  lcd.print(“Deg C”);
  lcd.setCursor(6, 0);
  lcd.print(“SetP”);
  pinMode(13, OUTPUT);
}

/* Main program */
void loop() {
  temperatureC = gettemp();

lcd.setCursor(0, 1);
  lcd.print(temperatureC);
  lcd.setCursor(6, 1);
  lcd.print (setpoint);

if (temperatureC < setpoint)
    digitalWrite(13, HIGH);
  else
    digitalWrite(13, LOW);

/* Scans the keypad once. This line needs to be run repeatedly */
  Keypad.Scan();

/* Has a new key been pressed  /
  if (Keypad.New_Key()) {
    /
If so the send the key to the serial port */

/* 1’s column is the keypad column number */
    byte key = (Keypad.Read() % 10);
    switch (key) {
      case Left:
        incUP();
        break;
      case Right:
        incDown();
        break;
    }
    delay(20);

Serial.print (setpoint);
  }
}

void incUP() {
  setpoint = setpoint + 1;
  EEPROM.write (1, setpoint);
}

void incDown() {
  setpoint = setpoint - 1;
  EEPROM.write (1, setpoint);
}

float gettemp() {
  // converting an analog reading to a voltage,
  // for 3.3v arduino use 3.3
  float voltage = (analogRead(sensorPin) * 3.3) / 1024.0;

//converting from 10 mv per degree C with 500 mV offset
  //to degrees ((voltage - 500mV) times 100)
  return (voltage - 0.5) * 100;
}

Thank you for your input, the info on the deadband is great.
Andy

Krupski:
Normally, one should not compare floats because roundoff errors can trick the comparison. For example, if you did “X = 10.0 / 3.0”, X would be 3.3333… but then X * 3 == 10.0 may return false because of rounding errors.

But, this isn’t your problem I don’t think.

You should be able to say “if temperature < setpoint then turn on 13” because if there IS any comparison error, it will be so minute that it won’t matter.

You may wish to use a deadband to keep your system from hunting or rapidly switching on and off as the temperature just barely goes above and below setpoint. For example, something like this:

if (temperature + 0.5) < setpoint then 13 = ON
if (temperature - 0.5) > setpoint then 13 = OFF

This will give you a 1 “unit” deadband that the temperature can drift within without changing pin 13.

Lastly, your degrees C to F code is wrong. You want F = (C * (9 / 5)) + 32 or F = (C * 1.8) + 32 (note 9/5 == 1.8).

The 9.0 / 3.3 is wrong. It’s supposed to be 9.0 / 5.0 (or just 1.8).

andyedtec: Thank you for your input, the info on the deadband is great. Andy

Here's a nice simple debounced pushbutton reader:

uint8_t getButton (uint8_t port, uint8_t wait)
{
#define DEB 1000
    uint16_t deb = DEB; // define & set debounce count
    if (digitalRead (port)) { // return immediately if button not pushed
        return 0;
    }
    while (deb--) { // read port "debounce" times
        if (digitalRead (port)) { // the button bounced, reset "deb"
            deb = DEB;
        }
    }
    if (wait) { // optional wait for button to be released
        while (!digitalRead (port));
    }
    return 1;
}

All you do is wire a N.O button from any pin to ground, then set that pin pinMode (x, INPUT_PULLUP); Call the function with the pin number. It will return 1 if the button is pressed and 0 if not. If you want the code to wait until the button is detected, THEN RELEASED, set "wait" to 1. If you want it to return as soon as a debounced button is detected, set "wait" to 0.

A debounce count of 1000 is a good number for most buttons... long enough to handle lots of bouncing (like snap-action microswitches) but fast enough not to be "slow". Adjust if necessary (higher = wait longer).

JohnWasser is correct - the declaration of temperatureC in gettemp is hiding the one in the main class.

On the topic of floats, IMO:

My personal rule is to always treat floats and doubles as an approximation of some value. Hopefully a close approximation, but nevertheless never exact. They have a tolerance which must be kept in mind.

(Obviously, many particular values do have an exact floating point representation and can be tested for equality - but it’s best not to rely on that is what I’m saying.)

This means that floats and doubles usually cannot be usefully checked for equality, and any arithmetic done with them will accumulate small errors. But it’s fine to check them for greater than/less than - just remember there’s a tiny amount of slop.

With respect to their use, floats must never be used for things that must be counted exactly. Using a float number to count dollars is always a mistake. I once worked on a project where time was accounted for in hours, and they switch to floats so they could track it in ten-minute intervals. Huge mistake. IN the context of arduino programming, it would be a mistake to count the steps of a stepper as 24ths of a rotation using a float.

What floats are good for is measurements of physical quantities (which your sensors can never managed exactly, anyway), and statistical and trigonometric calculations.

Now as to the code, I see a couple of things:

Variables that start with an uppercase letter. Please don’t do this - programmers use variables like this for class names.

  if(Keypad.New_Key()) 
  {
    key = (Keypad.Read() % 10); /* 1's column is the keypad column number */
    incUP();
    incDown();
… 
}

This looks like you are unconditionally incing up and then incing down. That’s not what happens, because theres a check in the two methods, but the method names don’t indicate that.

It would be very, very much nicer to pass ‘key’ into the methods rather than to stuff it into a variable. This means not using the mechanism of setting it to zero to manage the logic. But this logic is sufficiently simple that I don;t see why you need to have separate functions at all. How about:

    switch(key) {
    case Left:
        setpoint += 1; 
        break;
    case Right:
        setpoint -= 0.5; 
        break;
    }

To make this work, Left and Right need to be declared like so:

const byte Left = 4;
const byte Right = 3;

I don’t know why you are bothering to round the temp and setpoint (ok, setpoint is rounded because it’s in EEPROM - fair enough). And it’s weird that you decrement the setpoint by .5 and round it: this means that every second decrement will do nothing. the gettemp function does three separate things (getting the temperature, converting it to F, writing to the LCD).

Left and Right are constants, but a crucial fact like the voltage being 3.3 is buried down in the code.

I also have a personal rule that every variable or function name measuring or returning a physical quantity or currency must have the unit of measurement at the end. That ship wouldn’t have crashed into Mars iof they had done this.

You call relay1, which sets the relay, in the loop before finding out what the temp is. This means that it will always think the temp is 0 degrees the first time it is run.

You reset the setpoint and write it to eeprom every time the temperature is read rather than every time the setpoint is changed. This is a bad idea - eeprom can be written to only a limited number of times.

You measure the temperature in F and therefore the setpoint in F, but the temperature is displayed in C.

You include HCMAtrixKeypad twice.

C to F convesion is 9/5, not 9/3.3 . (3.3 is your arduino voltage, this was just a moment of duh)

Anyway. Here’s my rewrite. I’m not sure it will compile (because I don’t have your various include files), but I think it’s a bit clearer.

#include <math.h>
#include <EEPROM.h>
#include <LiquidCrystal.h>
#include <HCMatrixKeypad.h>
LiquidCrystal lcd(12, 11, 5, 4, 3, 2);
#include <HCMatrixKeypad.h>

/* Sets how many times the same key must be scanned before it is accepted as 
   pressed (settled). If you get multiple key presses then increase this value 
   (max 255)*/
#define DEBOUNCE 10

#define C1 A2/* DIO for keypad column 1 */
#define C2 10 /* DIO for keypad column 2 */
#define C3 9  /* DIO for keypad column 3 */
#define C4 8  /* DIO for keypad column 4. Delete if there is no column 4! */
#define C5 7  /* DIO for keypad column 5. Delete if there is no column 5! */
#define C6 6  /* DIO for keypad column 6. Delete if there is no column 6! */

#define R1 A1 /* DIO for keypad row 1 */
HCMatrixKeypad Keypad(DEBOUNCE, C1, C2, C3, C4, C5, C6, ROWMARKER, R1); /* Uncomment for 6x1 keypad */

const int sensorPin = A0;
const int relayPin = 13;
const float arduinoVoltage = 3.3f;

// keypad constants
const byte keyLeft = 4;
const byte keyRight = 3;

const float temperatureSlop = .5f; 

// EEPROM setpoint
int setpoint;

void setup()
{
    Serial.begin(9600);
    EEPROM.read (1); // make the eeprom or atmega328 memory address 1
    setpoint = EEPROM.read(1);
    lcd.begin(16, 2);
    lcd.print("Deg C");  
    lcd.setCursor(6, 0);
    lcd.print("SetP");
    pinMode(13, OUTPUT);
}

/* Main program */
void loop()
{
  adjustSetpoint(); // reads the keypad
  
  float temperatureF = readTemperatureF();
  
  showStatus(temperatureF); // we could add stuff here to write only if the status has changed, but meh
  
  if(temperatureF < setpoint - temperatureSlop) {
    digitalWrite(relayPin, HIGH);
  }
  else  if(temperatureF > setpoint + temperatureSlop) {
    digitalWrite(relayPin, LOW);
  }
  // else leave the relay as it is.

}

void adjustSetpoint() {
  Keypad.Scan();
  if(!Keypad.New_Key()) {
    return;
  }
  
  // ok. a key is pressed
  
  byte keyColumn = Keypad.Read() % 10;
  delay(20);
  
  switch(keyColumn) {
  case keyLeft:
    setpoint ++;
    break;
    
  case keyRight:
    setpoint --;
    break;
    
  default: 
    // setpoint not changed
    return;
  }

  // setpoint has been changed. Save it.  
  EEPROM.write (1,setpoint);  
}

float readTemperatureF() {
  float value = analogRead(sensorPin);
  value /= 1024; // scale to 0-1
  value *= arduinoVoltage; // convert to voltage
  value = (voltage - 0.5f) * 100;  //converting from 10 mv per degree wit 500 mV offset
                                   //to degrees ((voltage - 500mV) times 100)
  value = (value * 9f/5f) + 32; // convert to F                                 
  return value;
}

void showStatus(float temperatureF) {
 lcd.setCursor(0, 1);
 lcd.print(temperatureF); 
 lcd.setCursor(6, 1);
 lcd.print (temperatureF);
}