Will this work? Programming help needed!

Hi,
I have written this code with help from other program examples that i have sourced and want to know if I am on the right track.
The program is for automatic control of an alcohol still I have.
The servos and relays etc are yet to be wired up, but I have a question about what I have written.
I have several sequences that are to be run in order and I am worried that when each sequence finishes it will move through the earlier sequences as well as the sequence it is up to.
Let me give you an example: (look at the code below…)

The equal sequence runs (equal int = 1) and times out, and the heads integer is then set to = 1.
When the loop comes around again, because equal int still = 1, will it start counting again?

Any help would be greatly appreciated. I thought I would ask the forum as I have a few weeks until I wire the sensors and relays etc.
Matt

int Tmid = 0;  //initialize variables, T is the input from the middle thermistor
int Ttop = 0;  //initialize variables, T is the input from the top thermistor

int waterTmid = 50;   // waterT is the initial temp to turn on water 
int safetyTmid = 85;  // the safety middle temp when the heater should turn off
int safetyTtop = 85;  // the safety top temp when the heater should turn off

int boil = 0;  // If boil is 0, then the boil has not started.  If it is 1, then the boil has started.
int equal = 0;  // If equal is 0, then the boil is not to temp.  If it is 1, then the boil is at temp and equalising.
int heads = 0;  // If heads is 0, then the equalisation has not finished.  If it is 1, then the heads has started.
int hearts = 0;  // If hearts is 0, then the heads have not finished.  If it is 1, then the hearts have started.
int shutdown = 0;  // If shut is 0, then the hearts has not finished.  If it is 1, then the shutdown will commence.
int overtemp = 0;  // If overtemp is 0, then all is well.  If it is 1, then the process will shutdown.

int start = 7;   // choose the input pin (for a pushbutton)
int switch1 = 5;     // variable for reading the B1 button pin status
int switch2 = 6;     // variable for reading the B1 button pin status
int val1;            // Variable for reading start button B1
int val2;            // Variable for reading stop button B2

long startone = 0;  // placeholder for the starting time for mash 1
long stopone = 0;  // placeholder for the stop time for mash 1
long remaining = 0; // difference between startone and stopone
int remainsecs = 0;  // convert to something close to a second like integer


void setup(){  // run setup once


pinMode(4, OUTPUT);  // initialize pin 4 as output, will be used to trigger heater relay circuit
pinMode(8, OUTPUT);  // initialize pin 8 as output, will be used to trigger pump relay circuit

pinMode(switch1, INPUT);
pinMode(switch2, INPUT);

Serial.begin(9600);  // sends serial data to pc, allow for reading the temp and status as reported by Serial.print
}


void loop(){ // loop this as long as Arduino has power or until reset button is pressed

delay(1000); // 1 second delay between loops                                                                    ????????????????????????????????????????????


// Temp display

Ttop = analogRead(2);  // get voltage from mid thermistor
Serial.print("Top: ");  // print midthermistor value to pc, this is not really temp but a number between 1 and 1024
Serial.print(Ttop);
Serial.print("\n");


Tmid = analogRead(3);  // get voltage from top thermistor
Serial.print("Mid: ");  // print thermistor value to pc, this is not really temp but a number between 1 and 1024
Serial.print(Tmid);
Serial.print("\n");






// Start

  if (boil == 0){                                    // if process has not started
  Serial.print("Press Start button...");
  val1 = digitalRead(switch1);                       // read input value and store it in val
  if (val1 == HIGH) {                                // check if the advance button is pressed
   boil = 1;    }

// Boil

if (boil == 1){  // Boil process
  digitalWrite(4, HIGH);  // Heat on
  digitalWrite(8, LOW);  // Water off
Serial.println("Boiling...");  // this prints out every second 
}


// Water on

if (Tmid > waterTmid){ // if mid temp is more than safety temp, do this loop
    digitalWrite(4, HIGH);  // Heat on
    digitalWrite(8, HIGH); // turn on cooling water
    Serial.println("Heat down, Water on");  // this prints out when water is on
     equal = 1;                                                                                                     
                                                                                                                  // Servo movement needed!!!!!!!!!!!!!!!!!!!!!!
   
   
     startone = millis(); // start mash clock, reads milliseconds from this moment
     stopone = startone + 3600000; // calculate the end of mash, 3.6 million milliseconds or 60 minutes
}

                                                                                              



// Equal

if (equal == 1){  // Equal process
  digitalWrite(4, HIGH);  // Heat on
  digitalWrite(8, HIGH);  // Water on
  
  
  
  long curtime = millis(); // set variable curtime to the elapsed time since program started
  remaining = (-1)*(curtime - stopone); // caculate how many milliseconds until mash is complete
  remainsecs = (int)(remaining/1000); // convert into human readable second like intervals but not exact
  Serial.print(remainsecs);  // print it to screen
  Serial.print(" - ");
  Serial.print("Remaining");
  Serial.print("\n"); 
  
  if (val1 == HIGH) {                                // check if the advance button is pressed
   heads = 1; }  

  if (curtime > stopone){  // if we reach the end of the mash time
   heads = 1;  }
 }
  
                                                                                                     

// Collect heads

if (heads == 1){  // Heads process
  digitalWrite(4, HIGH);  // Heat on
  digitalWrite(8, HIGH);  // Water on
  Serial.println("Collecting heads...");  // this prints out every second 
  if (val1 == HIGH) {                                // check if the advance button is pressed
   hearts = 1;    }

                                                                                                                 // Stepping motor needed!!!!!!!!! 
}



// Collect hearts

if (hearts == 1){  // Hearts process
  digitalWrite(4, HIGH);  // Heat on
  digitalWrite(8, HIGH);  // Water on
Serial.println("Collecting hearts...");  // this prints out every second 

                                                                                                                    // Stepping motor needed!!!!!!!!! 
}


// Shutdown

if (shutdown == 1){  // if process is complete, loop this
  digitalWrite(4, LOW);  // Heat off
  digitalWrite(8, LOW);  // Water off
Serial.println("Process completed");  // this prints out every second 
}


if (Tmid > safetyTmid){ // if mid temp is more than safety temp, do this loop
    digitalWrite(4, LOW);  // turn off heat and pump
    digitalWrite(8, LOW);
    Serial.println("Overtemp Mid");  // this prints out every second                                               ****need a timer to shut off water!!!!
    shutdown = 1; 
}

if (Ttop > safetyTtop){ // if top temp is more than safety temp, do this loop
    digitalWrite(4, LOW);  // turn off heat and pump
    digitalWrite(8, LOW);
    Serial.println("Overtemp Mid");  // this prints out every second 
    overtemp = 1;
}


if (overtemp == 1){  // overtemp shutdown 
    digitalWrite(4, LOW); // Turn on heat
    digitalWrite(8, HIGH); // Turn on water
    Serial.println("Overtemp Shutdown!");  // Prints boil on screen                                                ****need a timer to shut off water!!!!
    


// Stop Button

  val2 = digitalRead(switch2);                       // read input value and store it in val
  if (val2 == HIGH) {                                // check if the button is pressed
   overtemp = 1;    }





}

    
  }
}

Moderator edit:
</mark> <mark>[code]</mark> <mark>

</mark> <mark>[/code]</mark> <mark>
tags added.

"When the loop comes around again, because equal int still = 1, will it start counting again?"
Yes, unless you clear it back to 0 at some point.

You can also put some checks around items and only initialize them if they haven't been already:

if (equal == 0){
startone = millis(); // start mash clock, reads milliseconds from this moment
stopone = startone + 3600000; // calculate the end of mash, 3.6 million milliseconds or 60 minutes
}
then equal == 1, this part is skipped.

I suggest you Google “State machine”.

int boil = 0;  // If boil is 0, then the boil has not started.  If it is 1, then the boil has started.
int equal = 0;  // If equal is 0, then the boil is not to temp.  If it is 1, then the boil is at temp and equalising.
int heads = 0;  // If heads is 0, then the equalisation has not finished.  If it is 1, then the heads has started.
int hearts = 0;  // If hearts is 0, then the heads have not finished.  If it is 1, then the hearts have started.
int shutdown = 0;  // If shut is 0, then the hearts has not finished.  If it is 1, then the shutdown will commence.

Are these mutually exclusive states? It looks like it, as you won’t be “not started” and “not finished” at the same time. So you really want one variable rather than a whole lot of different ones. Something like:

enum { BOIL, EQUAL, HEADS, HEARTS, SHUTDOWN };

int state = BOIL;

So you start off in the first state. Then when that is done you add 1 to it and move onto the next state. And so on.

Thanks for the replies!!

Nick, you are right in thinking that each state is mutually exclusive, when the boil is finished I wish to move to the next state and so on...
I am unfamiliar with the enum command, but I assume this is placed in the void setup?? is this right?
Also, could give an axample of adding a 1 to the 'int state = BOIL;' command?

I apologise for my questions and lack of knowledge, I have been spending a lot of time researching this language but it is still fairly new to me!
Thanks

enum { BOIL, EQUAL, HEADS, HEARTS, SHUTDOWN };

int state = BOIL;

void setup ()
{
}

void loop ()
{
 state++; //next state 
 if (state > SHUTDOWN)
   exit (0);  // done!
  
   //blah blah
 
}

Thanks, I have adjusted the code, could you have a quick look and let me know what you think?
Thanks

#include <Morse.h>

int Tmid = 0;  //initialize variables, T is the input from the middle thermistor
int Ttop = 0;  //initialize variables, T is the input from the top thermistor

int waterTmid = 50;   // waterT is the initial temp to turn on water 
int safetyTmid = 85;  // the safety middle temp when the heater should turn off
int safetyTtop = 85;  // the safety top temp when the heater should turn off

//int boil = 0;  // If boil is 0, then the boil has not started.  If it is 1, then the boil has started.
//int equal = 0;  // If equal is 0, then the boil is not to temp.  If it is 1, then the boil is at temp and equalising.
//int heads = 0;  // If heads is 0, then the equalisation has not finished.  If it is 1, then the heads has started.
//int hearts = 0;  // If hearts is 0, then the heads have not finished.  If it is 1, then the hearts have started.
int manshutdown = 0;  // If shut is 0, then the hearts has not finished.  If it is 1, then the shutdown will commence.
int overtemp = 0;  // If overtemp is 0, then all is well.  If it is 1, then the process will shutdown.




enum { BEGIN, BOIL, EQUAL, HEADS, HEARTS, SHUTDOWN, };

int state = BEGIN;







int start = 7;   // choose the input pin (for a pushbutton)
int switch1 = 5;     // variable for reading the B1 button pin status
int switch2 = 6;     // variable for reading the B1 button pin status
int val1;            // Variable for reading start button B1
int val2;            // Variable for reading stop button B2





long startone = 0;  // placeholder for the starting time for mash 1
long stopone = 0;  // placeholder for the stop time for mash 1
long remaining = 0; // difference between startone and stopone
int remainsecs = 0;  // convert to something close to a second like integer



void setup(){  // run setup once


pinMode(4, OUTPUT);  // initialize pin 4 as output, will be used to trigger heater relay circuit
pinMode(8, OUTPUT);  // initialize pin 8 as output, will be used to trigger pump relay circuit

pinMode(switch1, INPUT);
pinMode(switch2, INPUT);

Serial.begin(9600);  // sends serial data to pc, allow for reading the temp and status as reported by Serial.print
}























void loop(){ // loop this as long as Arduino has power or until reset button is pressed

delay(1000); // 1 second delay between loops                                                                    ????????????????????????????????????????????





// Temp display

Ttop = analogRead(2);  // get voltage from mid thermistor
Serial.print("Top: ");  // print midthermistor value to pc, this is not really temp but a number between 1 and 1024
Serial.print(Ttop);
Serial.print("\n");


Tmid = analogRead(3);  // get voltage from top thermistor
Serial.print("Mid: ");  // print thermistor value to pc, this is not really temp but a number between 1 and 1024
Serial.print(Tmid);
Serial.print("\n");






// Start

  if (state == BEGIN){                                    // if process has not started
  Serial.print("Press Start button...");
  val1 = digitalRead(switch1);                       // read input value and store it in val
  if (val1 == HIGH) {                                // check if the advance button is pressed
    state++; //next state     }




// Boil

if (state == BOIL){  // Boil process
  digitalWrite(4, HIGH);  // Heat on
  digitalWrite(8, LOW);  // Water off
Serial.println("Boiling...");  // this prints out every second 
}


// Water on

if (Tmid > waterTmid){ // if mid temp is more than safety temp, do this loop
    digitalWrite(4, HIGH);  // Heat on
    digitalWrite(8, HIGH); // turn on cooling water
    Serial.println("Heat down, Water on");  // this prints out when water is on
    state++;                       //next state                                                                                                     
                                                                                                                     // Servo movement needed!!!!!!!!!!!!!!!!!!!!!!
     
     
     
     startone = millis(); // start mash clock, reads milliseconds from this moment
     stopone = startone + 3600000; // calculate the end of mash, 3.6 million milliseconds or 60 minutes
}

                                                                                              



// Equal

if (state == EQUAL){  // Equal process
  digitalWrite(4, HIGH);  // Heat on
  digitalWrite(8, HIGH);  // Water on
  
  
  
  long curtime = millis(); // set variable curtime to the elapsed time since program started
  remaining = (-1)*(curtime - stopone); // caculate how many milliseconds until mash is complete
  remainsecs = (int)(remaining/1000); // convert into human readable second like intervals but not exact
  Serial.print(remainsecs);  // print it to screen
  Serial.print(" - ");
  Serial.print("Remaining");
  Serial.print("\n"); 
  
  if (val1 == HIGH) {                                // check if the advance button is pressed
   state++;                       //next state   }  

  if (curtime > stopone){  // if we reach the end of the mash time
   state++;                       //next state    }
 }
  
  
                                                                                                        



// Collect heads

if (state == HEADS){  // Heads process
  digitalWrite(4, HIGH);  // Heat on
  digitalWrite(8, HIGH);  // Water on
  Serial.println("Collecting heads...");  // this prints out every second 
  if (val1 == HIGH) {                                // check if the advance button is pressed
   state++;                       //next state     }

                                                                                                                 // Stepping motor needed!!!!!!!!! 
}



// Collect hearts

if (state == HEARTS){  // Hearts process
  digitalWrite(4, HIGH);  // Heat on
  digitalWrite(8, HIGH);  // Water on
Serial.println("Collecting hearts...");  // this prints out every second 

                                                                                                                    // Stepping motor needed!!!!!!!!! 
}


// Shutdown

if (manshutdown == 1 ){  // if process is complete, loop this
  Serial.println("Process completed");  // this prints out every second
  digitalWrite(4, LOW);  // Heat off
   delay(100000);
  digitalWrite(8, LOW);  // Water off
 
}


if (Tmid > safetyTmid){ // if mid temp is more than safety temp, do this loop
    digitalWrite(4, LOW);  // turn off heat and pump
    digitalWrite(8, LOW);
    Serial.println("Overtemp Mid");  // this prints out every second                                               ****need a timer to shut off water!!!!
    overtemp = 1;  
}

if (Ttop > safetyTtop){ // if top temp is more than safety temp, do this loop
    digitalWrite(4, LOW);  // turn off heat and pump
    digitalWrite(8, LOW);
    Serial.println("Overtemp Mid");  // this prints out every second 
    overtemp = 1; 
}


if (overtemp == 1){  // overtemp shutdown 
    digitalWrite(4, LOW); // Turn on heat
    digitalWrite(8, HIGH); // Turn on water
    Serial.println("Overtemp Shutdown!");  // Prints boil on screen                                                ****need a timer to shut off water!!!!
    delay(100000);
    digitalWrite(8, LOW);

// Stop Button

  val2 = digitalRead(switch2);                       // read input value and store it in val
  if (val2 == HIGH) {                                // check if the button is pressed
   manshutdown = 1;    }





}

  }}}

The braces on your if statements don't seem to make sense. Could you hit contol-t in the editor, check that the blocks are what you wanted, and post the code again?

There are some lines with closing braces in comments.

  if (val1 == HIGH) {                                // check if the advance button is pressed
   state++;                       //next state   }  

  if (curtime > stopone){  // if we reach the end of the mash time
   state++;                       //next state    }
 }
  if (val1 == HIGH) {                                // check if the advance button is pressed
   state++;                       //next state     }

A control-t (or command-t, if you're a Mac user) would have told you that you had a brace mismatch.

In this bit:

   // Equal

    if (state == EQUAL){  // Equal process
...
      if (val1 == HIGH) {                                // check if the advance button is pressed
        state++;                       //next state   
      }

You aren’t reading val1, it is just the value left over from way back. Plus is “val1” a useful data name? How about “buttonB1Press”?

What’s with the blank lines? key got stuck?

It looks better than the original, I’m glad you used the state machine. Now for readability, put each state into a function, like this:

//#include <Morse.h>

int Tmid = 0;  //initialize variables, T is the input from the middle thermistor
int Ttop = 0;  //initialize variables, T is the input from the top thermistor

int waterTmid = 50;   // waterT is the initial temp to turn on water 
int safetyTmid = 85;  // the safety middle temp when the heater should turn off
int safetyTtop = 85;  // the safety top temp when the heater should turn off

int manshutdown = 0;  // If shut is 0, then the hearts has not finished.  If it is 1, then the shutdown will commence.
int overtemp = 0;  // If overtemp is 0, then all is well.  If it is 1, then the process will shutdown.

enum { 
  BEGIN, BOIL, EQUAL, HEADS, HEARTS, SHUTDOWN, };

int state = BEGIN;

int start = 7;   // choose the input pin (for a pushbutton)
int switch1 = 5;     // variable for reading the B1 button pin status
int switch2 = 6;     // variable for reading the B1 button pin status
int val1;            // Variable for reading start button B1
int val2;            // Variable for reading stop button B2

long startone = 0;  // placeholder for the starting time for mash 1
long stopone = 0;  // placeholder for the stop time for mash 1
long remaining = 0; // difference between startone and stopone
int remainsecs = 0;  // convert to something close to a second like integer

void setup()
{  // run setup once

  pinMode(4, OUTPUT);  // initialize pin 4 as output, will be used to trigger heater relay circuit
  pinMode(8, OUTPUT);  // initialize pin 8 as output, will be used to trigger pump relay circuit

  pinMode(switch1, INPUT);
  pinMode(switch2, INPUT);

  Serial.begin(9600);  // sends serial data to pc, allow for reading the temp and status as reported by Serial.print
}

void beginState ()
{
  Serial.print("Press Start button...");
  val1 = digitalRead(switch1);                       // read input value and store it in val
  if (val1 == HIGH) 
  {                                // check if the advance button is pressed
    state++; //next state    
  }
}  // end of beginState

void boilState ()
{
  digitalWrite(4, HIGH);  // Heat on
  digitalWrite(8, LOW);  // Water off
  Serial.println("Boiling...");  // this prints out every second 
}  // end of boilState

void equalState ()
{

  digitalWrite(4, HIGH);  // Heat on
  digitalWrite(8, HIGH);  // Water on

  long curtime = millis(); // set variable curtime to the elapsed time since program started
  remaining = (-1)*(curtime - stopone); // caculate how many milliseconds until mash is complete
  remainsecs = (int)(remaining/1000); // convert into human readable second like intervals but not exact
  Serial.print(remainsecs);  // print it to screen
  Serial.print(" - ");
  Serial.print("Remaining");
  Serial.print("\n"); 

  if (val1 == HIGH) 
  {                                // check if the advance button is pressed
    state++;                       //next state  
  }  

  if (curtime > stopone)
  {  // if we reach the end of the mash time
    state++;                       //next state    
  }
}  // end of equalState

void headsState ()
{
  digitalWrite(4, HIGH);  // Heat on
  digitalWrite(8, HIGH);  // Water on
  Serial.println("Collecting heads...");  // this prints out every second 
  if (val1 == HIGH)
  {                                // check if the advance button is pressed
    state++;                       //next state     
  }

  // Stepping motor needed!!!!!!!!! 
}  // end of headsState

void heartsState ()
{
  digitalWrite(4, HIGH);  // Heat on
  digitalWrite(8, HIGH);  // Water on
  Serial.println("Collecting hearts...");  // this prints out every second 

  // Stepping motor needed!!!!!!!!! 
}  // end of heartsState

void loop()
  { // loop this as long as Arduino has power or until reset button is pressed

  delay(1000); // 1 second delay between loops                                                                    ?????????????????????????????????
  // Temp display

  Ttop = analogRead(2);  // get voltage from mid thermistor
  Serial.print("Top: ");  // print midthermistor value to pc, this is not really temp but a number between 1 and 1024
  Serial.print(Ttop);
  Serial.print("\n");

  Tmid = analogRead(3);  // get voltage from top thermistor
  Serial.print("Mid: ");  // print thermistor value to pc, this is not really temp but a number between 1 and 1024
  Serial.print(Tmid);
  Serial.print("\n");

  // state machine

  switch (state)
  {
  case BEGIN:  
    beginState (); 
    break;

  case BOIL:   
    boilState (); 
    break;

  case EQUAL:  
    equalState (); 
    break;

  case HEADS:  
    headsState (); 
    break;

  case HEARTS: 
    heartsState (); 
    break;

  }  // end of switch


  // Water on

  if (Tmid > waterTmid)
  { // if mid temp is more than safety temp, do this loop
    digitalWrite(4, HIGH);  // Heat on
    digitalWrite(8, HIGH); // turn on cooling water
    Serial.println("Heat down, Water on");  // this prints out when water is on
    state++;                       //next state                                                                                                  
    startone = millis(); // start mash clock, reads milliseconds from this moment
    stopone = startone + 3600000; // calculate the end of mash, 3.6 million milliseconds or 60 minutes
  }  // end of temperature too high



  // Shutdown

  if (manshutdown == 1 )
  {  // if process is complete, loop this
    Serial.println("Process completed");  // this prints out every second
    digitalWrite(4, LOW);  // Heat off
    delay(100000);
    digitalWrite(8, LOW);  // Water off
  }  // end of manual shutdown

  if (Tmid > safetyTmid)
  { // if mid temp is more than safety temp, do this loop
    digitalWrite(4, LOW);  // turn off heat and pump
    digitalWrite(8, LOW);
    Serial.println("Overtemp Mid");  // this prints out every second                                               ****need a timer to shut off water!!!!
    overtemp = 1;  
  }  // end of mid temperature test

  if (Ttop > safetyTtop)
  { // if top temp is more than safety temp, do this loop
    digitalWrite(4, LOW);  // turn off heat and pump
    digitalWrite(8, LOW);
    Serial.println("Overtemp Mid");  // this prints out every second 
    overtemp = 1; 
  }  // end of top temperature test

  if (overtemp == 1)
  {  // overtemp shutdown 
    digitalWrite(4, LOW); // Turn on heat
    digitalWrite(8, HIGH); // Turn on water
    Serial.println("Overtemp Shutdown!");  // Prints boil on screen                                                ****need a timer to shut off water!!!!
    delay(100000);
    digitalWrite(8, LOW);

    // Stop Button

    val2 = digitalRead(switch2);                       // read input value and store it in val
    if (val2 == HIGH) 
    {                                // check if the button is pressed
      manshutdown = 1;    
    }  

  }// end of overtemp

} // end of loop

It isn’t perfect by any means, but it shows the idea of moving things like processing each state into their own function where you can more clearly see what it is doing. Now you can inspect each state function and make sure it does what you want, and glance over the main loop to see if that looks right. I see a few problems, I’ll let you find them. :slight_smile:

Even with Nick's surgery, loop is still huge - try moving some more of that stuff into separate functions - the state machine piece would be one obvious candidate, but you'll need others too ideally.

The spaces make it a bit easier for me to digest the information and search through it. I was planning on removing the spaces when complete.
Thanks again Nick, the changes look great. I am planning on reading and learning from the changes over the next couple of days.....