Code Check - DS18B20 temp controller

After a bit of help checking my code.

I am developing a temperature controller that will read from multiple DS18B20 and then activate a one of series of relays using the digital output from the arduino. Each DS18B20 is linked to a different relay. So it’s basically acting like multiple temp controllers. With the exception that the last relay is activated if any of other relays are on, and off when all the other relays are off.

I have tested it with 4 DS18B20 and the Serial Monitor and it seems to work fine. I understand my code is not brillante and a lot of the work could be better done using classes. However it works as far as I can tell. Can anyone spot any major floors in the code?

#include <OneWire.h>
#include <DallasTemperature.h>

// Data wire is plugged into pin 2 on the Arduino
#define ONE_WIRE_BUS_PIN 2

// Setup a oneWire instance to communicate with any OneWire devices
OneWire oneWire(ONE_WIRE_BUS_PIN);

// Pass our oneWire reference to Dallas Temperature.
DallasTemperature sensors(&oneWire);

// Assign the addresses of your 1-Wire temp sensors.
DeviceAddress Probe01 = { 
  0x28, 0x17, 0xD8, 0xEA, 0x05, 0x00, 0x00, 0xC7 }; //Blank DS18B20
DeviceAddress Probe02 = { 
  0x28, 0x19, 0x28, 0xEB, 0x05, 0x00, 0x00, 0xFD}; //DS18B20 with tape
//DeviceAddress Probe03 = {}; // ????
DeviceAddress Probe04 = { 
  0x28, 0x6B, 0xE3, 0x2A, 0x06, 0x00, 0x00, 0x0B }; //Black China
DeviceAddress Probe05 = { 
  0x28, 0xCF, 0xFC, 0x2A, 0x04, 0x00, 0x00, 0xD7 }; //Zac Wire with Wayki

//Define Temp Set Points
float SetPointTank1 = 21;
float SetPointTank2 = 21;
float SetPointTank3 = 21;
float SetPointTank4 = 21;
float SetPointTank5 = 21;
float SetPointFridge = 21;

//Asign Digital Pins to valves and SSR
int Valve1 = 7;   //Tank1 - Bright Beer Tank
int Valve2 = 8;   //Tank2 - 2000L Leaky Fermenter
int Valve3 = 9;   //Tank3 - Short China Fermenter
int Valve4 = 10;  //Tank4 - Black Clad Peruvian Fermenter
int Valve5 = 11;  //Tank5 - 2000L New China Fermenter
int Valve0 = 12;  //Fridge - Walk in cooler
int Pump = 13;    //Recirc Pump on Chiller (SSR)

void setup(void)
{  
  // initialize the digital pins as an output.
  pinMode(Valve0, OUTPUT);
  pinMode(Valve1, OUTPUT);     
  pinMode(Valve2, OUTPUT);     
  pinMode(Valve3, OUTPUT);     
  pinMode(Valve4, OUTPUT);     
  pinMode(Valve5, OUTPUT);     
  pinMode(Pump, OUTPUT);  

  // Start up the library
  sensors.begin();

  // set the resolution to 10 bit (Can be 9 to 12 bits .. lower is faster)
  sensors.setResolution(Probe01, 12);
  sensors.setResolution(Probe02, 12);
  //  sensors.setResolution(Probe03, 12);
  sensors.setResolution(Probe04, 12);  
  sensors.setResolution(Probe05, 12);  

  // start serial port
  Serial.begin(9600);

}

void loop(void)
{

  sensors.requestTemperatures(); // Send the command to get temperatures

  float currentTempTank1;
  float currentTempTank2;
  float currentTempTank3;
  float currentTempTank4;
  float currentTempTank5;
  float currentTempFridge;

  currentTempTank1 = sensors.getTempC(Probe01);
  currentTempTank2 = sensors.getTempC(Probe02);
  //  currentTempTank3 = sensors.getTempC(Probe03);
  currentTempTank4 = sensors.getTempC(Probe04);
  currentTempFridge = sensors.getTempC(Probe05);

  /* Check Temp on Tank 1 */
  if ( currentTempTank1 <= SetPointTank1 )
  {
    Serial.print("Tank 1 Temperature is too LOW! It is: ");
    Serial.println(currentTempTank1);
    digitalWrite(Valve1, LOW);
  }

  if ( currentTempTank1 > SetPointTank1 )
  {
    Serial.print("Tank 1 Temperature is too HOT! It is: ");
    Serial.println(currentTempTank1);
    digitalWrite(Valve1, HIGH);
  }  

  /*Check Temp on Tank 2*/
  if ( currentTempTank2 <= SetPointTank2 )
  {
    Serial.print("Tank 2 Temperature is too LOW! It is: ");
    Serial.println(currentTempTank2);
    digitalWrite(Valve2, LOW);
  }

  if ( currentTempTank2 > SetPointTank2 )
  {
    Serial.print("Tank 2 Temperature is too HOT! It is: ");
    Serial.println(currentTempTank2);
    digitalWrite(Valve2, HIGH);
  }  

  /* Check Temp on Tank 3 NOT ACTIVE
   if ( currentTempTank3 <= SetPointTank3 )
   {
   Serial.print("Tank 3 Temperature is too LOW! It is: ");
   Serial.println(currentTempTank3);
   digitalWrite(Valve3, LOW);
   }
   
   if ( currentTempTank3 > SetPointTank3 )
   {
   Serial.print("Tank 3 Temperature is too HOT! It is: ");
   Serial.println(currentTempTank3);
   digitalWrite(Valve3, HIGH);
   }  */

  /* Check Temp on Tank 4 */
  if ( currentTempTank4 <= SetPointTank4 )
  {
    Serial.print("Tank 4 Temperature is too LOW! It is: ");
    Serial.println(currentTempTank4);
    digitalWrite(Valve4, LOW);
  }

  if ( currentTempTank4 > SetPointTank4 )
  {
    Serial.print("Tank 4 Temperature is too HOT! It is: ");
    Serial.println(currentTempTank4);
    digitalWrite(Valve4, HIGH);
  }  

  /* Check Temp on Tank 5 NOT ACTIVE 
   if ( currentTempTank5 <= SetPointTank5 )
   {
   Serial.print("Tank 5 Temperature is too LOW! It is: ");
   Serial.println(currentTempTank5);
   digitalWrite(Valve5, LOW);
   }
   
   if ( currentTempTank5 > SetPointTank5 )
   {
   Serial.print("Tank 5 Temperature is too HOT! It is: ");
   Serial.println(currentTempTank5);
   digitalWrite(Valve5, HIGH);
   }    */

  /* Check Temp on Walk In Cooler */
  if ( currentTempFridge <= SetPointFridge )
  {
    Serial.print("Fridge Temperature is too LOW! It is: ");
    Serial.println(currentTempFridge);
    digitalWrite(Valve0, LOW);
  }

  if ( currentTempFridge > SetPointFridge )
  {
    Serial.print("Fridge Temperature is too HOT! It is: ");
    Serial.println(currentTempFridge);
    digitalWrite(Valve0, HIGH);
  }    

  /*Check if any valves are open. If yes, turn pump on, in No, pump off*/
  if ( digitalRead(7) == LOW && digitalRead(8) == LOW && digitalRead(9) == LOW && digitalRead(10) == LOW && digitalRead(11) == LOW && digitalRead(12) == LOW )
  {
    Serial.println("Pump OFF");
    digitalWrite(Pump, LOW);
  }

  if ( digitalRead(7) == HIGH || digitalRead(8) == HIGH || digitalRead(9) == HIGH || digitalRead(10) == HIGH || digitalRead(11) == HIGH || digitalRead(12) == HIGH )
  {
    Serial.println("Pump ON");
    digitalWrite(Pump, HIGH);
  }  

  // Wait 20 Secods and repeat loop 
  delay(2000);
}

The eventual aim is to connect the arduino to a Raspberry Pi and use that as a web server and control interface.

Can anyone spot any major floors in the code?

The complete lack of arrays is one.

z4zacharia:
Can anyone spot any major floors in the code?

//Define Temp Set Points

float SetPointTank1 = 21;
float SetPointTank2 = 21;
float SetPointTank3 = 21;
float SetPointTank4 = 21;
float SetPointTank5 = 21;
float SetPointFridge = 21;

void loop(void)
{

sensors.requestTemperatures(); // Send the command to get temperatures

float currentTempTank1;
  float currentTempTank2;
  float currentTempTank3;
  float currentTempTank4;
  float currentTempTank5;
  float currentTempFridge;

/* Check Temp on Tank 1 */
  if ( currentTempTank1 <= SetPointTank1 )
  {
    Serial.print("Tank 1 Temperature is too LOW! It is: ");
    Serial.println(currentTempTank1);
    digitalWrite(Valve1, LOW);
  }

if ( currentTempTank1 > SetPointTank1 )
  {
    Serial.print("Tank 1 Temperature is too HOT! It is: ");
    Serial.println(currentTempTank1);
    digitalWrite(Valve1, HIGH);
  }




The eventual aim is to connect the arduino to a Raspberry Pi and use that as a web server and control interface.

Not wanting to be too pedantic, but:

You comment says

// set the resolution to 10 bit

but you have actually set them to 12bit. If you are serious about that, and that would be quite legitimate, there is no need to set it. The default is 12bit.

The lines

float currentTempTank1;

can go up the top with the other floats. there is no need to talk about them every trip round the loop. They can also be written

float currentTempTank1, currentTempTank2, currentTempTank3;

which reduces the amount of scrolling.

Similarly, while I recognise that you might want to change them, there is no apparent need for all those setpoints if they are all the same. One will do.

I suspect that conditionals are too demanding by virtue of being inelegant, thereby causing all valves to operate every loop cycle, which could be about a hundred times more than thay need to. Same goes for the pump.

I would be inclined to change the conditionals to something like

if ( currentTempTank1 < (SetPoint - 0.25))
do this

if ( currentTempTank1 > (SetPoint + 0.25))
do that

It would keep the temperature within 0.25 of the setpoint and probably make things a hell of a lot quieter.

Finally, your comment at the end says wait 20 seconds but the command says wait 2 seconds. I believe, if the conditionals are changed, a 2 second loop, which usually does nothing, would be fine anyway.

And I don’t see the need for a Raspberry pie. Arduino can surely do what you want on its own.

PaulS:
The complete lack of arrays is one.

I have not done much in the way of coding since learning C++ in Univiersity about 15 years ago. So what is the benifit of using arrays in an application like this?

Thanks Nick, I know my code isn’t that elegant, and I appreciate the help. Some of the comments are indeed incorrect, need to update those.

The setpoints are set individually as they will be different in the future. And I the idea is that I can build an interface of some kind to be able to set them locally or via the web.

Nick_Pyner:
I suspect that conditionals are too demanding by virtue of being inelegant, thereby causing all valves to operate every loop cycle, which could be about a hundred times more than thay need to. Same goes for the pump.

I would be inclined to change the conditionals to something like

if ( currentTempTank1 < (SetPoint - 0.25))
do this

if ( currentTempTank1 > (SetPoint + 0.25))
do that

It would keep the temperature within 0.25 of the setpoint and probably make things a hell of a lot quieter.

I was looking to build in something like this, but with this example what happens if the temp is with the range?

Ultimately what I would like to be able to do with this, is data logging, pre programed temperature changes, a web interface, a local screen interface. Hence why I was thinking of using the Raspberry Pi, but if I can all of this with an arduino that would be cool.

z4zacharia:
The setpoints are set individually as they will be different in the future. And I the idea is that I can build an interface of some kind to be able to set them locally or via the web.

OK, that settles that...

I was looking to build in something like this, but with this example what happens if the temp is with the range?

Nothing - which I hope is your intention.

Ultimately what I would like to be able to do with this, is data logging, pre programed temperature changes, a web interface, a local screen interface. Hence why I was thinking of using the Raspberry Pi, but if I can all of this with an arduino that would be cool.

All easily doable, I'm sure. I don't know anything about pre-programmed temperature changes, but I bet that is the easiest bit. I imagine an on-board RTC and a an alarm system would handle that. What you have in mind is really quite routine for Arduino but be aware that, if you are using a Uno, you are likely to need to change up to a Mega. This is not because of all those extra pins but because of the extra memory you need for those smart facilities.

You could do all that stuff with an arduino, but be aware that it's not much of a web server - if you want fancy pages you're probably better off hosting it elsewhere. Which you may want to do anyway, depending on what you mean by data logging.

You could store a huge amount of data on an SD card, but if you want to keep a lot of history, it'll likely be a bit slow serving it up. You could push it to an on-line service such as xively, or to your Pi.

Personally, I'd be inclined to keep the web gui and data storage on separate hardware and let the arduino just handle the temperature reading and controlling the relays. If you keep it all on an arduino, you'll have to step up to a Mega as Nick says, so for around the same price you can get a Pi and it's associated infrastructure.

Thanks Guys, I already have the Pi, so I might go that route. If I can get my head around the code etc. The data logging is probably best pushed to an online server as it will be more usefull to me that way.
Thanks again.