Two different codes work independently but not when combined into one sketch

Hello, I am very new to electronics and programming but have been working steadily through various tutorials introducing basic circuits. I have reached a stage where I have a fully functional circuit and program that reads the current temperature with a sensor, outputs it to an LCD display and changes the colour of a bi-colour LED between red and green based on the current temperature. I also have another circuit on the same breadboard and powered by the same Arduino, which toggles a blue LED on and off with one push button switch. Again, this works exactly as intended when I run the program independently from the temperature sensing sketch.

My issue is that I cannot seem to combine the two functions into one sketch. When I try to run the sketch below, the temperature sensing side works as usual, but the push button blue LED side does not- the LED remains either constantly on or off depending on whether I set the int ledState HIGH or LOW and the button does not toggle it on or off. As far as I can see, I’m not using the same variables for multiple things, so I’m assuming there’s some other issue that I just can’t recognise. Any help to spot my error would be very much appreciated. I’ve also attached a Fritzing diagram of my current prototyping layout- I apologise in advance for its inelegance, again it’s something I’m very new to!

/*
  
This sketch reads the current temperature in °C 
and prints a reading to the LCD, while a bi-colour LED lights up
red or green depending on a specific temperature range. A push button switch toggles another blue LED on and off independently (to build on in future development).

  The circuit:
 * LCD RS pin to digital pin 12
 * LCD Enable pin to digital pin 11
 * LCD D4 pin to digital pin 5
 * LCD D5 pin to digital pin 4
 * LCD D6 pin to digital pin 3
 * LCD D7 pin to digital pin 2
 * LCD R/W pin to ground
 * LCD VSS pin to ground
 * LCD VCC pin to 5V
 * Pot wiper to LCD VO pin (pin 3)
 * DS18B20 data pin to digital pin 7 via 4.7k resistor 
 * Bi-colour LED to digital pins 9 and 10 via 330R resistors
 * Blue LED attached from pin 6 to ground via 220R resistor
 * Push button attached from pin 8 to 5V
 * 10K resistor attached from pin 8 to ground

 */

// Include the library code for the temperature sensor:
#include <LiquidCrystal.h>
#include <OneWire.h>
#include <DallasTemperature.h>

// Data wire is plugged into pin 7 on the Arduino:
#define ONE_WIRE_BUS 7

// Setup a oneWire instance to communicate with any OneWire device:
OneWire oneWire(ONE_WIRE_BUS);

// Pass the oneWire reference to Dallas Temperature:
DallasTemperature sensors(&oneWire);

// Initialize the LCD library with the numbers of the interface pins:
LiquidCrystal lcd(12, 11, 5, 4, 3, 2);

//Bi-colour LED set up and ideal temperature range parameters:
int redPin = 10;
int greenPin = 9;
int temperature_ideal = 24.99;

//Blue LED and push button set up:
const int buttonPin = 8;     // the number of the push button pin
const int bluePin =  6;      // the number of the blue LED pin

// Variables that will change:
int ledState = LOW;         // the current state of the blue LED output pin
int buttonState;             // the current reading from the push button input pin
int lastButtonState = LOW;   // the previous reading from the push button input pin

// The following variables are long's because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an integer:
long lastDebounceTime = 0;  // the last time the output pin was toggled
long debounceDelay = 50;    // the debounce time; increase if the output flickers


void setup() {
   // Start up the sensor library:
  sensors.begin();
  // set up the LCD's number of columns and rows:
  lcd.begin(16, 2);
  // Print message to the LCD:
  lcd.print("Temperature is:");
  //Set LED output:
  pinMode(redPin, OUTPUT);
  pinMode(greenPin, OUTPUT);
  pinMode(buttonPin, INPUT);
  pinMode(bluePin, OUTPUT);
 
}

void loop() {
  // call sensors.requestTemperatures() to issue a global temperature
  // request to all devices on the bus:
  sensors.requestTemperatures(); // Send the command to get temperatures
  // set the cursor to column 0, line 1
  // (note: line 1 is the second row, since counting begins with 0):
  lcd.setCursor(0, 1);
   lcd.print(sensors.getTempCByIndex(0)); 
    delay(1000); //Set conditions to display the two different colours:
    if(sensors.getTempCByIndex(0) > temperature_ideal)
    {setColor(255, 0);}
    else {setColor(0, 225);}
    delay(1000);

  // read the state of the switch into a local variable:
  int reading = digitalRead(buttonPin);

  // check to see if you just pressed the button 
  // (i.e. the input went from LOW to HIGH),  and you've waited 
  // long enough since the last press to ignore any noise:  

  // If the switch changed, due to noise or pressing:
  if (reading != lastButtonState) {
  // reset the debouncing timer:
    lastDebounceTime = millis();
    
   // toggle the ledState variable each time the button is pressed: 
      if (reading == HIGH) {
      ledState = !ledState;
    } 
  } 
  
  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:
    buttonState = reading;
  }
  
  
  // set the blue LED using the state of the button:
  digitalWrite(bluePin, ledState);

  // save the reading.  Next time through the loop,
  // it'll be the lastButtonState:
  lastButtonState = reading;  
}

void setColor(int red, int green)
{
  #ifdef COMMON_ANODE
    red = 255 - red;
    green = 255 - green;
  #endif
  analogWrite(redPin, red);
  analogWrite(greenPin, green); 

  }

The delay(1000) statement is only giving your code one opportunity to read the switch every second. If it’s not pressed at that exact instant it will be missed. You need to do your timing using millis() instead of delay(). See:

File --> Examples --> 02.Digital --> BlinkWithoutDelay

int temperature_ideal = 24.99;

Do you understand what an integer is?

Your code is VERY difficult to follow. Put every { on a line BY ITSELF. Put every } on a line BY ITSELF. Use Tools + Auto Format to properly indent the code.

   delay(1000); //Set conditions to display the two different colours:

That comment has nothing to do with that code.

Comments could, and should in this case, go on lines by themselves.

What, exactly, is buttonState for? It does not take three variables (reading, buttonState, and lastButtonState) to debounce a switch and determine that a change has, or has not, taken place.

Debouncing the switch is hardly necessary, when you have two seconds worth of delay() in loop(). Fred Flintstone switches don't bounce THAT long.

Use Ctrl-T in the IDE to reformat your code to a common C style. Also, move your comments off to the right side of the screen so the reader doesn't have to wade through the comments to see the code. Those will make it easier for us to read your code. Lastly, this code:

  if ((millis() - lastDebounceTime) > debounceDelay) {
     buttonState = reading;                        // whatever the reading is, it's been there for longer
  }                                                // than debounce delay, use it as the actual current state:

is supposed to do what? You never use buttonState, so why the if statement block?

Many thanks for the replies already, I'll work to clean up the code and make it more legible based on the advice here and update again when I've done so.

int temperature_ideal = 24.99; Well that's particularly embarrassing! Not sure where the .99 came from, it should of course just be 24.

I think part of the problem is that due to the programs both working individually, I've assumed every line of code is proper and correct. As I said before, this code is bolted together from other people's examples and tutorials online and adapted for my own purposes, and I left the comments in largely as they appeared in the originals so as to help me remember and learn what the lines of code are doing. On that subject, is there a proper etiquette for citing code from examples that are in the public domain that I should use? As the code is mixture of adaptations, original lines and lines that are verbatim from existing programs, I'm not sure how I would do this.

Hi,
The OPs Fritzy

Tom... :slight_smile:

Ok, so I’ve finally made some progress. It wasn’t that the push button/blue LED part of the circuit wasn’t working, it was indeed down to the delay function for the LCD display that was causing the apparent issue- if I held down the button for long enough the LED would eventually light up. I’d forgot that the delay affected everything in the loop, not just the LCD part- schoolboy error! Thanks for everybody’s help with this. I’ve rewrote the sketch with a different method for the push button (thanks for pointing out debouncing was indeed unnecessary) and it now works, albeit with the minor issue that I still do have to hold down the push button briefly to toggle the blue LED on and off:

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

#define ONE_WIRE_BUS 7

OneWire oneWire(ONE_WIRE_BUS);

DallasTemperature sensors(&oneWire);

LiquidCrystal lcd(12, 11, 5, 4, 3, 2);

int redPin = 10;
int greenPin = 9;
int temperature_ideal = 24;

const int buttonPin = 8;
const int bluePin =  6;

int blueState = LOW;
int buttonState = 0;
int lastButtonState = LOW;
int buttonPushCounter = 0;

void setup()
{

  sensors.begin();
  lcd.begin(16, 2);
  lcd.print("Temperature is:");
  pinMode(redPin, OUTPUT);
  pinMode(greenPin, OUTPUT);
  pinMode(buttonPin, INPUT);
  pinMode(bluePin, OUTPUT);
  Serial.begin(9600);
}

void loop()
{

  sensors.requestTemperatures();
  lcd.setCursor(0, 1);
  lcd.print(sensors.getTempCByIndex(0));
  if (sensors.getTempCByIndex(0) > temperature_ideal)
  {
    setColor(255, 0);
  }
  else
  {
    setColor(0, 225);
    delay(25);
  }


  buttonState = digitalRead(buttonPin);
  if (buttonState != lastButtonState)
  {
    if (buttonState == HIGH)
    {
      buttonPushCounter++;
      Serial.println("on");
      Serial.print("number of button pushes:  ");
      Serial.println(buttonPushCounter);
    }
    else
    {
      Serial.println("off");
    }

  }

  lastButtonState = buttonState;
  if (buttonPushCounter % 2 == 0)
  {
    digitalWrite(bluePin, HIGH);
  }
  else
  {
    digitalWrite(bluePin, LOW);
  }

}
void setColor(int red, int green)
{
#ifdef COMMON_ANODE
  red = 255 - red;
  green = 255 - green;
#endif
  analogWrite(redPin, red);
  analogWrite(greenPin, green);

}

I’m now investigating further as to how to use the LCD without a delay function at all- I’ve seen that there are already some forum post on this subject. Again, thanks for everybody’s help for pointing me in the right direction and helping me organise my code more clearly. If there are any other recommendations for improving my formatting please do let me know.

I've seen that there are already some forum post on this subject.

All of which involve some variation on the blink without delay example.