Suggestions or guidance in cleaning this code up?

I'm building a temperature monitor/controller for a kegerator with an LCD display and need some help cleaning this code up ....


/*
Purpose: Temperature Regulator for a Kegerator with LCD display.
Using a LM35 and Arduino to check temperature and display current temp to an LCD.
Also the Arduino will turn a Solid State Relay on/off within a defined temp range
-LM35 connected to 5V, Grd, Analog Pin 0
-Serial LCD connected to 5V, Grd, Digital Pin 1 (tx pin)
-SS Relay connected to Grd, Digital Pin 12
-Back Light Button connected to Digital Pin 3, Grd, 5V via 10K resistor (pin pulled HIGH in open state).

Most of this code was borrowed heavily from BRUTUS @ brutusweb.com .. thanks!
*/

#define ledPin 13 // LED connected to digital pin 13
#define lm35 0 // LM35 on Analog 0
#define blightPin 3 // Momentary switch for back light to digital pin 3
#define relayPin 12 // Relay connected to digital pin 12

int buttonState = 0 //state of back light button

void sendlcd(byte command) //Quick function to send LCD commands.
{
Serial.print(command, BYTE);
}

void setup() // run once, when the sketch starts
{
analogReference(INTERNAL); //using internal voltage reference ~1.1v
pinMode(ledPin, OUTPUT); // sets the digital pin as output
pinMode(lm35, INPUT); // sets the analog pin as input
pinMode(blightPin, INPUT); // sets the digital pin as input
pinMode(relayPin, OUTPUT); // sets the digital pin as output
digitalWrite(ledPin, HIGH);

delay(1000);

Serial.begin(9600);
delay(1000); //Let Ports Stabilize

sendlcd(27); //Reset Command For LCD
sendlcd(122);

delay(500); //Full Reset takes a bit

sendlcd(254); //Clear and Go Home on LCD
sendlcd(1);
digitalWrite(ledPin, LOW); //turn off LED
delay(250);
}

void loop() // run for eternity
{
long temp;
long temp1;
long temp2;
long temp_x;
int lm35_val=0;

digitalWrite(ledPin,HIGH);
delay(500); //After digital write, delay to allow stabilized analog read
lm35_val=analogRead(lm35); //read value of center leg of LM35
temp = lm35_val; //output voltage of LM35, 10mV = 1 Degree Celcius
temp1=temp/10; //creates true Celcius reading
temp2=temp%10; //modulus operation to calculate remainder for higher resolution reading

if (temp2 >= 5)
{
temp_x++;
}

sendlcd(254); //Clear LCD Screen
sendlcd(1);
Serial.println("Current Temp"); //Print "Current Temp" message
Serial.print(temp1); //Print Celsius Temperature
Serial.print("."); //Print decimal place
Serial.print(temp2); //Print "Tenths" place
sendlcd(223); //Print "degree" character
Serial.print("C"); //Print "C" for Celsius

digitalWrite(ledPin,LOW); //Turn off LED

/* Fahrenheit Conversion in progress (needs some help........)

Serial.print (((long)temp1 * 9)/ 5 + 32); // convert Celsius to Fahrenheit and print
Serial.print (223);
Serial.print ("F"); // print Fahrenheit label

*/

// To turn back light on LCD on for 10 seconds...

buttonState = digitalRead(blightPin); // read the back light switch input:

if (buttonState == LOW) // if the switch is closed:
{

sendlcd(27); //Turn back light on (ASCII for ESC)
sendlcd(42); //ASCII for *
sendlcd(255); // backlight value (0)=off (255)=full on
delay(10000); // wait 10 seconds
sendlcd(27); //Turn back light off
sendlcd(42);
sendlcd(0);
}

if ((temp1) > 7 ) //check temp is above x degrees
{
digitalWrite (relayPin, HIGH); //if temp is above x degrees turn pin "ON"
}
else if ((temp 1) < 4 ) //check temp is below x degrees
{
digitalWrite (relayPin, LOW); //if temp is below x degree turn pin "OFF"
}

delay(20000); //Check temp every 20 seconds

}

The arduino is reading an LM35 for the temperature and turning a Solid State Relay on and off depending on the set temperature, and finally displaying the temp. on a serial LCD panel....

My biggest problem is trying to fix the code on calling the back-light on/off..... there is a momentary switch connected that, when press, should turn on the LCD back-light for 10 seconds
How can I avoid the use of Delay's and have the arduino listen for a button press while still running the temperature checking code?

Any pointers are much appreciated!! Thanks!

Hey silverhalo--

It sounds like a cool project.

Doing multiple things at once is a classic problem. Your instinct not to use delay is a good one. Delay is fine when doing one thing, but when you are trying to manage multiple things it really gets in the way.

My suggestion is to rewrite your loop function. Instead of making a series of things that happen with delay()s in between, instead note what time it is and do things only when they are supposed to be done. This requires a little bit of bookkeeping using state variables, but should become clear with practice.

For example, where you have roughly this now:

void loop()
{
  .. turn on the LED
  .. check the temperature
  .. display the temperature
  .. check the button state
  .. if (button is down)
  .. { turn on back light; wait 10 seconds; turn off back light; }
  .. set the relay according to temperature
  .. wait 20 seconds
}

do this instead

unsigned long last_temperature_check_time = 0;
unsigned long last_lcd_turn_on_time = 0;
void loop()
{
  unsigned long time = millis();
  if (time - last_temperature_check_time > 20000) // has it been 20 seconds?
  {
    .. turn on the LED
    .. check the temperature
    .. display the temperature
    .. set the relay according to temperature
   last_temperature_check_time = time;
  }
  .. check the button state
  .. if (button is down)
  { last_lcd_turn_on_time = time;  .. turn on back light;  }
  else if (time - last_lcd_turn_on_time > 10000) // has it been 10 seconds?
  { .. turn off back light. }
}

Does that make any sense?

Regards,

Mikal

I thought millis() were the way to go, but I'm still trying to comprehend them though. From what I understand is basically it starts a count from when the sketch first starts and counts until it overflows.. which is supposedly around 49-50 days... and from that steady count you can time events to happen off of it.
So in the sketch I would have a variable that would compare itself with the millis count and if it has been more than 20 seconds, the "if" loop would run and the current count of the millis would be the new variable to compare to 20 seconds later..etc..etc...
Am I close??
So what happens when the millis reset to zero and the last saved variable is a rather large number?

Thanks again for your pointers!!

Silverhalo, your analysis is exactly right. And if you follow the example in my previous post, the rollover of millis won't harm you because of the way integer arithmetic works in C. Happily, the expression

 if (time - last_temperature_check_time > 20000) // has it been 20 seconds?

works just fine even if "time" has rolled over.

Mikal

Here is my first rewrite... it verifies okay but I haven't had a chance to see if works with the hardware yet.

/*
Purpose: Temperature Regulator for a Kegerator with LCD display.
Using a LM35 and Arduino to check temperature and display current temp to an LCD.
Also the Arduino will turn a Solid State Relay on/off within a defined temp range
-LM35 connected to 5V, Grd, Analog Pin 0
-Serial LCD connected to 5V, Grd, Digital Pin 1 (tx pin)
-SS Relay connected to Grd, Digital Pin 12
-Back Light Button connected to Digital Pin 3, Grd, 5V via 10K resistor (pin pulled HIGH in open state).
*/

#define ledPin 13 // LED connected to digital pin 13
#define lm35 0 // LM35 on Analog 0
#define blightPin 3 // Momentary switch for back light to digital pin 3
#define relayPin 12 // Relay connected to digital pin 12

unsigned long last_temperature_check_time = 0;
unsigned long last_lcd_turn_on_time = 0;

void sendlcd(byte command) //Quick function to send LCD commands.
{
Serial.print(command, BYTE);
}

void setup() // run once, when the sketch starts
{
analogReference(INTERNAL); //using internal voltage reference ~1.1v
pinMode(ledPin, OUTPUT); // sets the digital pin as output
pinMode(lm35, INPUT); // sets the analog pin as input
pinMode(blightPin, INPUT); // sets the digital pin as input
pinMode(relayPin, OUTPUT); // sets the digital pin as output
digitalWrite(ledPin, HIGH);

delay(1000);

Serial.begin(9600);
delay(1000); //Let Ports Stabilize

sendlcd(27); //Reset Command For LCD
sendlcd(122);

delay(500); //Full Reset takes a bit

sendlcd(254); //Clear and Go Home on LCD
sendlcd(1);
digitalWrite(ledPin, LOW); //turn off LED
delay(250);
}

//new loop with millis... sweet

void loop() // run till the end of days or the beer runs out...
{
unsigned long time = millis();
if
(time - last_temperature_check_time > 20000) //has it been 20 seconds?
{
digitalWrite(ledPin,HIGH); //turn on LED for running routine status

//Check Temperature routine
long temp;
long temp1;
long temp2;
long temp_x;
int lm35_val=0;

delay(500); //After digital write, delay to allow stabilized analog read
lm35_val=analogRead(lm35); //read value of center leg of LM35
temp = lm35_val; //output voltage of LM35, 10mV = 1 Degree Celcius
temp1=temp/10; //creates true Celcius reading
temp2=temp%10; //modulus operation to calculate remainder for higher resolution reading

if (temp2 >= 5)
{
temp_x++;
}

//Send temperature to LCD routine
sendlcd(254); //Clear LCD Screen
sendlcd(1);
Serial.println("Current Temp"); //Print "Current Temp" message
Serial.print(temp1); //Print CelsuisTemperature
Serial.print("."); //Print decimal place
Serial.print(temp2); //Print "Tenths" place
sendlcd(223); //Print degree charater
Serial.print("C"); //Print "C" for Celsuis

digitalWrite(ledPin,LOW); //Turn off LED

//Turn Relay on/off routine
if ((temp1) > 7 ) //check temp is above x degrees
{
digitalWrite (relayPin, HIGH); //if temp is above x degrees turn pin "ON"
}
else if ((temp1) < 4 ) //check temp is below x degrees
{
digitalWrite (relayPin, LOW); //if temp is below x degree turn pin "OFF"
}
}

//Check Backlight Button State

last_lcd_turn_on_time = digitalRead(blightPin); // read the back light switch input:

if (last_lcd_turn_on_time = time) // turn on back light;
{

sendlcd(27); //ASCII ESC
sendlcd(42); //ASCII *
sendlcd(255); //Val (0)=off, Val(255)=full on

}

else if (time - last_lcd_turn_on_time > 10000) // has it been 10 seconds?
{
sendlcd(27); //Turn back light off
sendlcd(42);
sendlcd(0);
}

}

Does it look okay syntax wise? Why do the "if" and "else if" lines do not need to have a ";" on them? Still a bit iffy on the backlight code.. not sure if I have the "read switch input" part written correctly...

I can't wait to go home and see how this new code works...
I'll post some pictures of the entire hardware setup when I'm done so everyone can get the full effect of the beer kegerator magnificence!!

Thanks again Mikal for your input...

Silverhalo--

It's getting better! Just a couple of things:

  1. Anytime you have a delay in the loop with a pushbutton, you run the risk that you will miss a button push. Can you explain why you need the delay(500)? What exactly needs to "settle" after you turn the LED on? Can't we just delete that line?

  2. This code is a little screwy:

     last_lcd_turn_on_time = digitalRead(blightPin);      // read the back light switch input:

     if (last_lcd_turn_on_time = time)                    // turn on back light;
     {

"last_lcd_turn_on_time" is supposed to store the millis() time that the LCD was last turned on, but this expression is assigning it the state of the switch pin. I think what you want instead of these three lines is

    if (digitalRead(blightPin) == LOW) // turn on back light
    {
      last_lcd_turn_on_time = time;

I'm interested to see how this all turns out!

Mikal

Okay, I fixed, uploaded and ran the code... it works... almost.

/*
Purpose: Temperature Regulator for a Kegerator with LCD display.
Using a LM35 and Arduino to check temperature and display current temp to an LCD.
Also the Arduino will turn a Solid State Relay on/off within a defined temp range
      -LM35 connected to 5V, Grd, Analog Pin 0
      -Serial LCD connected to 5V, Grd, Digital Pin 1 (tx pin)
      -SS Relay connected to Grd, Digital Pin 12
      -Back Light Button connected to Digital Pin 3, Grd, 5V via 10K resistor (pin pulled HIGH in open state).
 */


#define ledPin 13                  // LED connected to digital pin 13
#define lm35 0                          // LM35 on Analog 0
#define blightPin 3                  // Momentary switch for back light to digital pin 3
#define relayPin 12                  // Relay connected to digital pin 12


unsigned long last_temperature_check_time = 0;
unsigned long last_lcd_turn_on_time = 0;

void sendlcd(byte command)            //Quick function to send LCD commands.
 {
  Serial.print(command, BYTE);
 }


void setup()                  // run once, when the sketch starts
{
  analogReference(INTERNAL);      //using internal voltage reference ~1.1v
  pinMode(ledPin, OUTPUT);            // sets the digital pin as output
  pinMode(lm35, INPUT);            // sets the analog pin as input
  pinMode(blightPin, INPUT);            // sets the digital pin as input
  pinMode(relayPin, OUTPUT);      // sets the digital pin as output
  digitalWrite(ledPin, HIGH);
 
  
  delay(1000);
  
  Serial.begin(9600);  
  delay(1000);                  //Let Ports Stabilize
  
  sendlcd(27);                  //Reset Command For LCD
  sendlcd(122);
  
  delay(500);                  //Full Reset takes a bit
  
    
  
  sendlcd(254);                  //Clear and Go Home on LCD
  sendlcd(1);
  digitalWrite(ledPin, LOW);            //turn off LED
  delay(250);
}





//new loop with millis

void loop()                  // run till the end of days or the beer runs out...
{
      unsigned long time = millis();
      if (time - last_temperature_check_time > 20000)      //has it been 20 seconds?
      {
                 
                 
                 digitalWrite(ledPin,HIGH);               //turn on LED for running routine status

            
            //Check Temperature routine
            long temp;
              long temp1;
              long temp2;
             long temp_x;
             int lm35_val=0;
  
              
              delay(500);                  //Delay to allow stabilized analog read.. not needed?
              lm35_val=analogRead(lm35);      //read value of center leg of LM35
             temp = lm35_val;                  //output voltage of LM35, 10mV = 1 Degree Celcius
              temp1=temp/10;                  //creates true Celcius reading
             temp2=temp%10;                  //modulus operation to calculate remainder for higher resolution reading
  
              if (temp2 >= 5)
                    {
                      temp_x++;
                    }
  
  
            //Send temperature to LCD routine
              sendlcd(254);                          //Clear LCD Screen
              sendlcd(1);
              Serial.println("Current Temp");            //Print "Current Temp" message
              Serial.print(temp1);                  //Print CelsuisTemperature
              Serial.print(".");                  //Print decimal place
              Serial.print(temp2);                  //Print "Tenths" place
              sendlcd(223);                          //Print degree charater
              Serial.print("C");                  //Print "C" for Celsuis

            digitalWrite(ledPin,LOW);            //Turn off LED

            
               //Turn Relay on/off routine
               if ((temp1) > 7 )                   //check temp is above x degrees
                            {
                       digitalWrite (relayPin, HIGH);      //if temp is above x degrees turn pin "ON"
                            } 
              else if ((temp1) < 4 )                    //check temp is below x degrees
                           {
                       digitalWrite (relayPin, LOW);      //if temp is below x degree turn pin "OFF"
                           } 
      }
      
      //Check Backlight Button State

      
      if (digitalRead(blightPin) == LOW)              // turn on back light
                {
              last_lcd_turn_on_time = time; 

             
            sendlcd(27);                          //ASCII ESC
                sendlcd(42);                          //ASCII *
               sendlcd(255);                            //Val (0)=off, Val(255)=full on
                
             }

        else if (time - last_lcd_turn_on_time > 10000)       // has it been 10 seconds?
              {
            sendlcd(27);                          //Turn back light off
                sendlcd(42);
               sendlcd(0);
             }

}

the problem I can see is that it is not waiting 20 seconds to run the check temperature routine. The LCD is actually refreshing itself about every 1/2 sec...... the delay(500).... and when I remove the delay the LCD freaks out cause it is refreshing the data in milliseconds.... quite fast... too fast for the LCD panel. Also the LED looks like it is steady on... which I really assume it is turning off and on so fast it looks like it is just stuck ON.

So.... what to do??? I think it needs to be able to update the "last_temperature_check_time" variable with a new value after each time it checks. I think that it sees the variable as 0 and so it always thinks it is less than 20 seconds... constantly..... am I close????

I thought it should be something like this but it doesn't seem to change anything...

unsigned long time = millis();
      if (time - last_temperature_check_time > 20000)      //has it been 20 seconds?
      {    last_temperature_check_time == time;

am I missing something??

(by the way... the backlight works killer!)

Close!

   last_temperature_check_time = time;

(Only one = sign. See my post #2.) Once that's fixed, see if you can remove the 500ms delay.

Mikal

Absolutely Brilliant! I'm so excited!!!!!
I don't know how I missed that part in your previous post... (I'm much more comfortable with the hardware (hand's-on) than software (head's-on)), but everything works great..
I got rid of the delay and it seems to be just fine....

Here are some pics of everything so far.... will install this to kegerator tomorrow and give here a proper go!

I designed and etched the circuit board myself... it is still a v.1 and needs to be updated for the LCD connections....

![](http://www.robertreznik.com/beer/circuit_board.jpg/img]






)

Here is the rest of the project... I want to do a write up on Instructables when I'm finished....




New Belguim FAT TIRE on tap...... my fav!

Thanks again for all your help Mikal!!!
If you are ever in New Mexico... stop by and have a beer!!

Excellent news!

Hey, I live only 475 miles from Carlsbad! How much farther do I have to drive to get my beer? :slight_smile:

Ok, now you can be the instructor for a while. (I'm uncomfortable with what you describe as the "hands on" stuff.) How did you build that "control panel"?

Nice job! And nice persistence!

Mikal

I designed the board in Adobe Illustrator, not the best choice for design but I am decent with vector software so it works for me. Since there were few components it was easy to look up their spec. sheets and get their dimensions for correct layout.... I do have a copy of EAGLE pcb design software but I am slow to learn it.
The PCB etching is always the fun part for me, its so cool to create something out of nothing... and have it work! Plus you can create custom graphic and text on it for bragging rights..... wicked!
I did an Instructable earlier this year on building a Distortion pedal for a guitar from scratch that has a detailed write up on etching you own circuit boards....check it out here...

I would trade "coding" for "soldering" any day.... but in this microprocessor day and age, one must know how to use both.

(Oh yeah, I'm in Albuquerque....... a bit less than 275 miles from Carlsbad.... )

Thanks. Tell you what: you do my soldering and I'll help with your coding any day. :slight_smile:

I'm specifically interested in how you made the little acrylic (?) display/button module.

Mikal

That's the easy part... I love working with acrylics!
I either cut the sheets to size with a straight edge and utility knife or rip it on my table saw (which is fast but tiny molten drops of plastic fly around and do not feel very nice on your skin!!).
To do the inside cuts I just use a dremel with a cut-off wheel, it makes a mess since it melts the plastic also but it is effective with practice. I also employee utility knives and files to carve/shape the opening to the size I need, a bit of medium sand paper works okay to soften the edges and burrs.
I always leave the protective paper or plastic on to keep the acrylic from getting scratched up until the part where I bend and shape it.
To mold the plastic to shape there are several tools out there, including some professional heating elements designed for melting, but I just use a $20 heat-gun.
I lay the plastic over the edge of a fire-resistant table and draw the gun back and forth along the line in which I plan to make the bend. If you keep the heat gun in one place too long you will start to "boil" the plastic and eventual burn it..
The technique is much like that for spray painting an object, start the stroke off the object, follow across the line and finish off the object... that way you will evenly heat the surface area without any hot spots.
I make sure the line where I am going to bend the object is right on the tables edge so when the plastic is hot enough it will naturally start to bend downward. I know then it is soft enough to bend and complete the bend by hand.
Just hold the bend in place for 10 seconds until it is cooled enough to hold its own shape.

I have another instructable about building an acrylic laptop stand I did a few years ago, but I used a blowtorch instead.... 'cause I'm crazy!!

Hope this helps!!

Here is the published product.....
http://www.instructables.com/id/Digital_Thermostatic_Beer_Refreshment_Regulator/

Your project looks great. Thanks for flowing up with the instructables article.

One additional idea I that I came up while discussing his kegerator was to estimate the amount beer left in the keg. You could use something like sparkfun product 8685 (the forum won't let me post a link) to weigh the keg and with some trial and error I think you could get pretty close to the percentage beer left in the keg. Hint: 1 gallon of water = 8lbs

Clicky link for pressure sensor linked above

Looks like you'd have to focus the weight of the keg onto a small spot for that to work.

void sendlcd(byte command)            //Quick function to send LCD commands.
{
 Serial.print(command, BYTE);
}

kinda silly to call a function just to call a function, this is more efficient

#define sendlcd(command) Serial.print(command, BYTE)

and you still use it with sendlcd(120);