It works, but is is good enough?

After much YouTube learning, I have managed to get my project working... Would anyone be kind enough to check my code to see if it is optimized or am I going a bit "arround the houses" to get my result.. Bassiclaly check I'm not doing anything wrong :slight_smile:
For example, I thought that I would be able to declare (thermo.temperature(RNOMINAL, RREF)) as a float at the begining. I'm concerned that I might be calling (thermo.temperature(RNOMINAL, RREF)) too many times.

#include <LiquidCrystal_I2C.h>
#include <Adafruit_MAX31865.h>

// Use software SPI: CS, DI, DO, CLK
Adafruit_MAX31865 thermo = Adafruit_MAX31865(10, 11, 12, 13);
// use hardware SPI, just pass in the CS pin
//Adafruit_MAX31865 thermo = Adafruit_MAX31865(10);

// The value of the Rref resistor. Use 430.0 for PT100 and 4300.0 for PT1000
#define RREF      4300.0
// The 'nominal' 0-degrees-C resistance of the sensor
// 100.0 for PT100, 1000.0 for PT1000
#define RNOMINAL  1000.0
//#define buzzer 7           //buzzer pin



LiquidCrystal_I2C lcd(0x27,16,2);  // set the LCD address to 0x3F for a 16 chars and 2 line display
int startDelay = 500;
int refreshDelay = 1000;
int redLed=2;
int yelLed=4;
int greenLed=3;
float MyTempReading;
float TOO_COLD = -1.0f;
float TOO_HOT = +10000.0f;
float lowTemp = 25.;        //lowest temp to work (100)
float safeLow = 27.;        //temp at start of warning
float safeHigh = 30;        //temp warning between safe low and maxtemp (160)
float maxTemp = 29.99;      //ultimate maximum temp (159.99)



int ledState = LOW;             // ledState used to set the LED
unsigned long previousMillis = 0;        // will store last time LED was updated
const long interval = 250;           // interval at which to blink (milliseconds)

const float pressure[] = {
  0.000, 0.038, 0.075, 0.114, 0.155, 0.196, 0.238, 0.282, 0.327, 0.373, // 100 ... 109
  0.421, 0.469, 0.520, 0.571, 0.624, 0.679, 0.734, 0.792, 0.851, 0.911, // 110 ... 119
  0.973, 1.037, 1.103, 1.170, 1.238, 1.309, 1.381, 1.456, 1.532, 1.609, // 120 ... 129
  1.689, 1.771, 1.855, 1.941, 2.029, 2.119, 2.211, 2.305, 2.402, 2.501, // 130 ... 139
  2.602, 2.705, 2.811, 2.919, 3.030, 3.143, 3.259, 3.377, 3.498, 3.622, // 140 ... 149
  3.748, 3.877, 4.009, 4.143, 4.281, 4.421, 4.564, 4.711, 4.860, 5.013, // 150 ... 159
  5.168                                                                 // 160
};

float pressureFromTemperature(long temp) {
  if (temp < (lowTemp)) return TOO_COLD;
  if (temp > (maxTemp)) return TOO_HOT;
  return pressure[temp-25];
}

void printPressureFor(long temp) {
  float p =  pressureFromTemperature(thermo.temperature(RNOMINAL, RREF)); //pressureFromTemperature(temp);
  Serial.print(F("T° = ")); Serial.print(temp);
  
  if (p <= TOO_COLD){ Serial.println(F(" \t➜ Too Cold"));
      lcd.clear();
      lcd.setCursor(0,0);     //Set cursor to character 0 on line 0
      lcd.print("Temp=  ");
      lcd.print(thermo.temperature(RNOMINAL, RREF));
      lcd.print((char)223);   //set Degrees symbol
      lcd.print("C");
      lcd.setCursor(0,1);     //Set cursor to character 2 on line 0
      lcd.print("P= ");
      lcd.print("Temp Too Cold");
      digitalWrite(redLed, LOW);
      digitalWrite(greenLed, LOW);
      //next blink the Yellow LED
        unsigned long currentMillis = millis();
        if(currentMillis - previousMillis >= interval) {
        // save the last time you blinked the LED
        previousMillis = currentMillis;  
        // if the LED is off turn it on and vice-versa:
        if (ledState == LOW)
          ledState = HIGH;
        else
          ledState = LOW;
          // set the LED with the ledState of the variable:
          digitalWrite(yelLed, ledState);
     }
  }

else if ((thermo.temperature(RNOMINAL, RREF))>= (safeLow) && (thermo.temperature(RNOMINAL, RREF))<=(safeHigh)){ 
  Serial.print(F("\t➜ P = "));
    Serial.println(p, 3);
      lcd.clear();
      lcd.setCursor(0,0);     //Set cursor to character 0 on line 0
      lcd.print("Temp=  ");
      lcd.print(thermo.temperature(RNOMINAL, RREF));
      lcd.print((char)223);   //set Degrees symbol
      lcd.print("C");
      lcd.setCursor(0,1);     //Set cursor to character 2 on line 0
      lcd.print("P= ");
      lcd.print(p, 3);
      digitalWrite(yelLed, LOW);
      digitalWrite(greenLed, LOW);
      //next blink the Yellow LED
        unsigned long currentMillis = millis();
        if(currentMillis - previousMillis >= interval) {
        // save the last time you blinked the LED
        previousMillis = currentMillis;  
        // if the LED is off turn it on and vice-versa:
        if (ledState == LOW)
          ledState = HIGH;
        else
          ledState = LOW;
          // set the LED with the ledState of the variable:
          digitalWrite(redLed, ledState);
     }
  }
  
  else if (p >= TOO_HOT){ Serial.println(F(" \t➜ Too Hot"));
      lcd.clear();
      lcd.setCursor(0,0);     //Set cursor to character 0 on line 0
      lcd.print("Temp=  ");
      lcd.print(thermo.temperature(RNOMINAL, RREF));
      lcd.print((char)223);   //set Degrees symbol
      lcd.print("C");
      lcd.setCursor(0,1);     //Set cursor to character 2 on line 0
      lcd.print("P= ");
      lcd.print("Temp Too Hot");
      digitalWrite(redLed, HIGH);
      digitalWrite(yelLed, LOW);
      digitalWrite(greenLed, LOW);
  }
  else {
    Serial.print(F("\t➜ P = "));
    Serial.println(p, 3);
      lcd.clear();
      lcd.setCursor(0,0);     //Set cursor to character 0 on line 0
      lcd.print("Temp=  ");
      lcd.print(thermo.temperature(RNOMINAL, RREF));
      lcd.print((char)223);   //set Degrees symbol
      lcd.print("C");
      lcd.setCursor(0,1);     //Set cursor to character 2 on line 0
      lcd.print("P= ");
      lcd.print(p, 3);
      digitalWrite(redLed, LOW);
      digitalWrite(yelLed, LOW);
      digitalWrite(greenLed, HIGH);
  }
}




void setup() {
  pinMode(redLed,OUTPUT);
  pinMode(yelLed,OUTPUT);
  pinMode(greenLed,OUTPUT);
  Serial.begin(9600);
  //Serial.println("Adafruit MAX31865 PT100 Sensor Test!");
  lcd.init();
  lcd.clear();         
  lcd.backlight();      // Make sure backlight is on
  thermo.begin(MAX31865_3WIRE);  // set to 2WIRE or 4WIRE as necessary

  lcd.setCursor(1,0);   //Set cursor to character 1 on line 0
  lcd.print("Coffee Boiler");

  lcd.setCursor(2,1);   //Move cursor to character 2 on line 1
  lcd.print("Temperature");

  delay(startDelay);

  lcd.clear();
  lcd.setCursor(2,0);   //Set cursor to character 2 on line 0
  lcd.print("To Pressure");

  lcd.setCursor(4,1);   //Move cursor to character 2 on line 1
  lcd.print("Sensor");

  delay(startDelay);
  }


void loop() {
  uint16_t rtd = thermo.readRTD();
  
  //int MyTempReading = (thermo.temperature(RNOMINAL, RREF));   //as below but does not allow intergers (eg 0.00, only 00)
  float MyTempReading = (thermo.temperature(RNOMINAL, RREF));   //set the temp reading to a local variable to save calling multiple times
  
  //Serial.print("RTD value: "); Serial.println(rtd);
  float ratio = rtd;
  ratio /= 32768;
  //Serial.print("Ratio = "); Serial.println(ratio,8);
  //Serial.print("Resistance = "); Serial.println(RREF*ratio,8);
  Serial.print("Temperature = "); Serial.println(); printPressureFor(MyTempReading);
    
   

  // Check and print any faults
  uint8_t fault = thermo.readFault();
  if (fault) {
    Serial.print("Fault 0x"); Serial.println(fault, HEX);
    if (fault & MAX31865_FAULT_HIGHTHRESH) {
      Serial.println("RTD High Threshold"); 
      lcd.clear();
      lcd.setCursor(0,0);   //Move cursor to character 2 on line 1
      lcd.print("RTD High Threshold");
    }
    if (fault & MAX31865_FAULT_LOWTHRESH) {
      Serial.println("RTD Low Threshold"); 
      lcd.clear();
      lcd.setCursor(0,0);   //Move cursor to character 2 on line 1
      lcd.print("RTD Low Threshold");
    }
    if (fault & MAX31865_FAULT_REFINLOW) {
      Serial.println("REFIN- > 0.85 x Bias"); 
      lcd.clear();
      lcd.setCursor(0,0);   //Move cursor to character 2 on line 1
      lcd.print("RTD Low Threshold");
    }
    if (fault & MAX31865_FAULT_REFINHIGH) {
      Serial.println("REFIN- < 0.85 x Bias - FORCE- open"); 
      lcd.clear();
      lcd.setCursor(0,0);   //Move cursor to character 2 on line 1
      lcd.print("REFIN- < 0.85 x Bias - FORCE- open");
    }
    if (fault & MAX31865_FAULT_RTDINLOW) {
      Serial.println("RTDIN- < 0.85 x Bias - FORCE- open"); 
      lcd.clear();
      lcd.setCursor(0,0);   //Move cursor to character 2 on line 1
      lcd.print("RTDIN- < 0.85 x Bias - FORCE- open");
    }
    if (fault & MAX31865_FAULT_OVUV) {
      Serial.println("Under/Over voltage"); 
      lcd.clear();
      lcd.setCursor(0,0);   //Move cursor to character 2 on line 1
      lcd.print("Under/Over voltage");
    }
    thermo.clearFault();
  }
  Serial.println();
  delay(refreshDelay);
}

At a glance mind you it appears like nice code so far.

I don't think there's any harm in calling thermo.temperature() frequently, but usually since all the code in the function is running at the same time (on a human scale) it is preferred to take stock once, that is solicit inputs and values from sensor and so forth.

If thermo.temperature() return a float, you can simply place

  float currentTemperature = thermo.temperature(RNOMINAL, RREF);

at the top of the function code and use that variable everywhere in the rest of the function body.

I don't know what you tried that didn't work.

Oh wait, I see you tried that… you need to bookmark "variable scope in c++" for study time. Google that.

In the meantime, moving the declaration of the variable you want to use so it is outside any function blocks will make it visible to all functions that care to use it. Two edges of the same sword - global variables in larger projects can be a problem and thus are sorta discouraged. In these small sketches not so much a matter of concern.

So declare a global variable, and use the therm.temperature() function whenever you think you need to get a new value from the sensor.

HTH

a7

I will have a look into that, I'm up early for a radio rally, so hopefully I'll get chance tomorrow if not it'll wait... especially after a bit more Google :slight_smile:

Thanks

Any code performing the way You want is a good code.
Any code can be written in numerous other ways and be made that very delicate that only "experts" would understand and You wouldn't manage to change anything. No need for such.....

You could use the F() macro (first example) a bit more in your program to save dynamic memory.

You seem to be using Software SPI on the Hardware SPI pins. That is a bit wasteful of the hardware.

Using the hardware should work:

// Use software SPI: CS, DI, DO, CLK
// Adafruit_MAX31865 thermo = Adafruit_MAX31865(10, 11, 12, 13);
// use hardware SPI, just pass in the CS pin
Adafruit_MAX31865 thermo = Adafruit_MAX31865(10);

Oh yes, thanks for the reminder, i was supposed to look into the wiring up of hardware pins on a Nano and give it a try...
My task was to research the difference between using software and hardware pins, then research the wiring of it (assuming that it's better to use hardware) < answered by you>

Typically I got lost in the rest of the project!

In your case, the wiring is unchanged. The difference is that the software version uses CPU cycles to imitate the hardware that is just sitting idle.