Toggling a push button within a control program

Hi, I would appreciate some help, please. I have very limited knowledge of programming but have managed to write a control program that opens and shuts a door depending on max and min air temperature using a DHT sensor. The main body of my program works well-using motor control relays and door limit switches.
I'm trying to incorporate a toggled button press switch which on a single press opens the door allowing entry and a second press closes it on exit. (obviously, the door would be always open above a set temperature)
My board is an Arduino Uno with the button input on pin 2 (dsw INPUT_PULLUP). My button toggle code works fine on its own and toggles pin 13 and 7 but will not work within my program structure. The program loops OK, but the button toggle has no effect.
I've included my complete code.

/*********************
  greenhouse relay motor control with toggle door switch on pin 2
 
  11/11/2019
*********************/
#define DHTPIN 4         // pin 4 sensor is connected to
#include <DHT.h>
#define DHTTYPE DHT22   // DHT 22  (AM2302)

// *************************************** motor (1) door control motor = in1,in2,enA ***************************
int led = 13;
int status = false;
int in1 = 12;
int in2 = 11;
int enApin = 10;
int dswPin = 2;     // door button switch open/close
int outPin = 7;       // TR1 base drive signal
int state = LOW;      // the current state of the output pin
int reading;           // the current reading from the input pin
int previous = HIGH;    // the previous reading from the input pin
long time = 0;         // the last time the output pin was toggled
long debounce = 200;
int vent = 9;              // window vent fan
int fan = 8;                // humidity fan
int buttonLpin = 6;         // left side door switch
int buttonRpin = 5;         // right hand door switch
int val = 0;
int ledPin = 13;            // door locked open led
volatile int buttonState = 0;
const uint32_t debounceTime = 10;  // 5 mSec, enough for most switches
int maxTemp2 = 28;     // max set at 28  for door
int minTemp2 = 24;     // min set at 24   "    "
int maxTemp = 18;      // max set at 18  for vent
int minTemp = 15;      // min set at 15  for vent
int maxHum  = 60;
int minHum  = 55;

DHT dht(DHTPIN, DHTTYPE);

void setup() {
  pinMode(outPin, OUTPUT);
  pinMode(ledPin, OUTPUT); // door locked open signal
  pinMode(dswPin, INPUT_PULLUP); // set the internal pull up, door button.
  pinMode (vent, OUTPUT);   // ventfan
  pinMode (enApin, OUTPUT);  // motor conrol module enable
  pinMode (in1, OUTPUT);     // motor control
  pinMode (in2, OUTPUT);     // motor control
  pinMode (fan, OUTPUT);     // humidity fan
  pinMode (buttonLpin, INPUT_PULLUP);         // left door switch
  pinMode (buttonRpin, INPUT_PULLUP);         // right door switch

  Serial.begin(9600);
  dht.begin();
}
void loop() {
  // *********** Door press toggle button...push to open / close  ***********

  reading = digitalRead(dswPin);   /// door lock open / close
  if (reading == HIGH && previous == LOW && millis() - time > debounce)
    if (state == HIGH)
      state = LOW;
    else
      state = HIGH;
  time = millis();

  { digitalWrite(ledPin, state);
    digitalWrite (outPin, state);
  }

  // **************************************************************
  delay(500); // Wait a few milliseconds between measurements.
  // Reading temperature or humidity takes about 250 milliseconds!
  // Sensor readings may also be up to 2 seconds 'old' (its a very slow sensor)
  float h = dht.readHumidity();
  // Read temperature as Celsius
  float t = dht.readTemperature();

  // Check if any reads failed and exit early (to try again).
  if (isnan(h) || isnan(t)) {
    Serial.println("Failed to read from DHT sensor!");
    return;
  }

  // ============================== VENT CHECK ============
  if (t > maxTemp) {
    digitalWrite(vent, HIGH);
  } else if (t < minTemp) {                         // if(h < minHum || t < minTemp) {
    digitalWrite(vent, LOW);
    // =============================== DOOR CHECK (temp2) OPENING FOR HIGH TEMPERATURE
  }
  if (t > maxTemp2) {
    val = digitalRead(buttonLpin);     // read the input pin
    digitalWrite(enApin, val);         // was in1 for grounded limit sw or 2 if using pull up
    delay( debounceTime );
    digitalWrite(in1, LOW);        // door  motor  >>>> OPEN   pin 12
    digitalWrite(in2, LOW);        //  "      "     "     "  pin 11

    // ======================================= door closing / for temp2 = LOW TEMPERATURE
  } else if (t < minTemp2) {
    val = digitalRead(buttonRpin);     // read the input pin
    digitalWrite(enApin, val);           // sets the output to in1 (if pull up)   (in2 for input grounded via sw)
    delay( debounceTime );
    digitalWrite(in1, HIGH);         // door  motor  >>>> CLOSE  pin 12
    digitalWrite(in2, HIGH);       //  "      "     "     "  pin 11

  }            //=================== checking humidity sensors to control fan if high fan on
  if (h > maxHum) {
    digitalWrite(fan, HIGH);
  } else if (h < minHum) {
    digitalWrite(fan, LOW);
  }
  Serial.print("Humidity: ");
  Serial.print(h);
  Serial.print(" %\t");
  Serial.print("Temperature: ");
  Serial.print(t);
  Serial.println(" *C ");
}

program

seems you messed up with the curly brackets {} in this part of the code...

  if (reading == HIGH && previous == LOW && millis() - time > debounce)
    if (state == HIGH)
      state = LOW;
    else
      state = HIGH;
  time = millis();

  { digitalWrite(ledPin, state);
    digitalWrite (outPin, state);
  }

500 is not a few seconds....

  delay(500); // Wait a few seconds between measurements.

I did nor read further.

Many thanks for taking a look at my post. 500 milliseconds, of course, my typing mistake. I've also deleted the curly brackets from the write statements which has made no difference. The "door open/close" code on its own works fine.
I'm more used to using pic assembly code from years back which seemed simpler for me to understand.

may be deleting them was not the right choice ? (you are not in Python, indentation does not dictate what is in the if and what is not. brackets do.)

what are you really trying to achieve when the condition

[color=blue](reading == HIGH && previous == LOW && millis() - time > debounce)[/color]

is evaluated to true?

I also don't understand the comment in    digitalWrite(enApin, val);        // was in1 for grounded limit sw or 2 if using pull up

Hi, J-M-L This statement ((reading == HIGH && previous == LOW && millis() - time > debounce) was copied from a toggle switch example snippet and works fine on its own. As does my door control code on its own. My problem... and where I need guidance, is to combine a door open and close feature using a press button switch toggling a door to open and close. The original circuit used a solid-state logic control module hence my abbreviations. I now use relays instead, and the door limit switches were configured differently so gnore my comments in the text of the code. The code program runs fine as it is.

So you copied this from somewhere and kept useless comments. Fine with me but does not answer the question

What should happen when that test is true ?

As a more general comment you are trying to merge two states machines so may be better to start with a piece of paper, define states and possible transitions upon which events a implement that

the variable you named "previous" is never changed anywhere in your code!
the purpose of this variable is to store the last button state so you can detect a change
in your button state. without up dating the "previous" variable you cant detect a change!

also since the delay(500) is in your loop section you are making the button detection wait too.
this will make your button very annoyingly insensitive to the user.

a more reasonable wait to help with bounce would be about 5.

please get into the habit of allways using brackets after your "if" statements.
this way you can use nested "if" statements and multiple lines after an "if".

if you are not familiar with the exclamation mark operator in my code below... it means opposite or NOT equal

here is a basic example to detect a simple button push and create a section that happens less frequently for your other stuff. hopefully you can take it from there.

  int sensorTimer;

void loop() {


  reading = digitalRead(dswPin);   /// door lock open / close
  if(reading!=previous){// check for change
    
        if(reading==LOW){// check for down
         state=!state; // set state to opposite
         Serial.println("button was pushed");
         digitalWrite(ledPin, state);
         digitalWrite (outPin, state);
         }
     previous=reading;
     }
 
 sensorTimer++;
 if(sensorTimer>100){sensorTimer=0;

 // everything here will happen every 500 miliseconds
 // so this is where you do your other stuff that happens 
 // with a longer delay !!!!!
 
 }
  delay(5);// makes main loop happen every 5 milliseconds 
}

taterking:
a more reasonable wait to help with bounce would be about 5.

here is a basic example to detect a simple button push and create a section that happens less frequently for your other stuff. hopefully you can take it from there.
...

I would "argue" that it's better to not mix debounce timing with how frequently you want to update something and I would rather suggest to stick to "conventional" millis() technics and let the loop() flow as fast as it can, implementing possibly the "poor's man debounce" with a delay() when you detect a change in the button).

void loop() {
  static unsigned long sensorTimer = 0;
  
  reading = digitalRead(dswPin);   /// door lock open / close
  if (reading != previous) { // check for change
    if (reading == LOW) { // check for down
      state = !state; // set state to opposite
      Serial.println("button was pushed");
      digitalWrite(ledPin, state);
      digitalWrite (outPin, state);
    }
    delay(15); // poor's man debounce
    previous = reading;
  }

  if (millis() - sensorTimer > 500) {

    // everything here will happen every 500 miliseconds
    // so this is where you do your other stuff that happens
    // with a longer delay !!!!!

    sensorTimer += 500;
  }
}

@ J_M_L (So you copied this from somewhere and kept useless comments. Fine with me but does not answer the question

What should happen when that test is true ? )

May I point out in respect of your comments! I am a complete novice regarding programming and this project has seen many different attempted versions of code over a period of several months, the comments were for my benefit and just got carried over. I also put together a test jig with motor drive and limit switches of which helped me prove the working of the project and correct any errors.
At 74 years of age, I've enjoyed learning something new but came here to learn more and needed guidance using a toggle button feature I struggled with. So from the replies, I've learnt a lot more and can say that everything is now working since I have a better understanding of coding. I don't mind constructive criticism, but your manner in the way you have replied has not been helpful.
Thank you to those who have offered their ideas and samples.

Jevray

@Jevray

I accept the criticism as I can see how you could interpret them negatively. It's a common bias of written communication / perception. I was trying to help you think about your code structure rather than giving you code that works. I found that the best way to remember something (whatever the age) is to actually do it by oneself and you were not far.

On the comments: Think about it for 2 seconds: You come here to ask for help. People will read your code. If the code shows comments that do not relate to the line of code, they will assume you made a mistake there. We have no clue what your code was a few months ago... -> Hence (to me) the minimum amount of work required — beginner or not — is to ensure the comments are appropriate, helpful, relevant. if not, just get rid of them before posting. You'll get better answers.

how did you decide to address the case when you push on the button top open the door and half a second later the temperature is such that it tiggers shutting the door? are you closing the door, or keep it open until the next button press (ie pressing the button means going to manual mode)?

@ J-M-L Thank you, The control system is for an outside greenhouse and is replacing the one I constructed and programmed many years ago using PIC 16F876 controller.
The entrance door has two single button push switches (one inside and one outside) both connected in parallel. Push to enter and the door opens and remains open until either the internal or external button is pressed a second time upon which it then closes. If the outside temperature is higher than the set point the vent will open and so does the door should there be a further increase, subject to the control parameters set within the program.

I have amended my code and tested it using the model testing platform and all works fine. However, I would imagine programmers looking over it could offer improvements.
I've posted my working program here:-

/*********************
  Greenhouse relay motor control with door toggling OPEN/CLOSE switch
  Vent fan 5v logic high drive signal output
  Humidity circulation fan ( via relay 4 when configured)
  15/11/2019
*********************/
#define DHTPIN 4         // pin 4 sensor is connected to
#include <DHT.h>
#define DHTTYPE DHT22   // DHT 22  (AM2302)

// *************************************** motor (1) door control motor = in1,in2,enA ***************************
int led = 13;
int status = false;
int in1 = 12;
int in2 = 11;
int enApin = 10;
int outPin = 7;       // external door OPEN indicator led
long time = 0;         // the last time the output pin was toggled
long debounce = 200;
int vent = 9;              // window vent fan
int fan = 8;                // humidity fan
int buttonLpin = 6;         // left side door switch
int buttonRpin = 5;         // right hand door switch
int val = 0;
int ledPin = 13;            // door locked open led
int buttonState = 0;
int lastButtonState = 0;
const int  buttonPin = 2;    // the pin that the pushbutton is attached to
int buttonPushCounter = 0;   // counter for the number of button presses
const uint32_t debounceTime = 10;  // 5 mSec, enough for most switches
int maxTemp2 = 28;     // max set at 28  for door
int minTemp2 = 24;     // min set at 24   "    "
int maxTemp = 18;      // max set at 18  for vent
int minTemp = 15;      // min set at 15  for vent
int maxHum  = 60;
int minHum  = 55;

DHT dht(DHTPIN, DHTTYPE);

void setup() {
  pinMode(outPin, OUTPUT);
  pinMode(ledPin, OUTPUT); // door locked open 
  pinMode(buttonPin, INPUT_PULLUP); // set the internal pull up, door button.
  pinMode (vent, OUTPUT);   // ventfan
  pinMode (enApin, OUTPUT);  // motor conrol module enable
  pinMode (in1, OUTPUT);     // motor control
  pinMode (in2, OUTPUT);     // motor control
  pinMode (fan, OUTPUT);     // humidity fan
  pinMode (buttonLpin, INPUT_PULLUP);         // left door switch
  pinMode (buttonRpin, INPUT_PULLUP);         // right door switch

  Serial.begin(9600);
  dht.begin();
}
void loop() {
  // *********** Door press toggle button...push to open / close  ***********
  
  buttonState = digitalRead(buttonPin);
  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) {
    // if the state has changed, increment the counter
    if (buttonState == HIGH) {
      // if the current state is HIGH then the button went from off to on:
      buttonPushCounter++;
 } else {
      // if the current state is LOW then the button went from on to off:
 }
    // Delay a little bit to avoid bouncing
    delay(50);
  }
  lastButtonState = buttonState;

  if (buttonPushCounter % 2 == 0) {
    digitalWrite(ledPin, HIGH);     // Uno on board led
    digitalWrite(outPin, HIGH);     // door lock open led (ON)
    digitalWrite(in1, LOW);         // motor direction
    digitalWrite(in2, LOW);         // motor direction
    delay(200);            // let motor direction relays settle before actuating 12v relay
    val = digitalRead(buttonLpin);
    val = digitalRead(buttonLpin);     // read the input pin
    digitalWrite(enApin, val);         // set 12v relay
    return;                            // return and loop
  } else {
    digitalWrite(ledPin, LOW);         // return to program,door lock open led off
    digitalWrite(outPin, LOW);     // door lock open led (OFF)
  }
  
  float h = dht.readHumidity();  // checking humidity
  // Read temperature as Celsius
  float t = dht.readTemperature();

  // ============================== VENT CHECK ============
  if (t > maxTemp) {
    digitalWrite(vent, HIGH);
  } else if (t < minTemp) {                         // if(h < min temp digitalWrite(vent, LOW);
    
    // =============================== DOOR CHECK (temp2) OPENING FOR HIGH TEMPERATURE
  }
  if (t > maxTemp2) {
    val = digitalRead(buttonLpin);     // read the door limit left micro switch
    digitalWrite(enApin, val);         // enable 12v switching relay (in3)
    delay( debounceTime );
    digitalWrite(in1, LOW);        // door  motor  >>>> OPEN   pin 12
    digitalWrite(in2, LOW);        //  "      "     "     "    pin 11

    // ======================================= door closing / for temp2 = LOW TEMPERATURE
  } else if (t < minTemp2) {
    val = digitalRead(buttonRpin);     // read the door limit right micro switch
    digitalWrite(enApin, val);           // sets the 12v switching relay (in3)
    delay( debounceTime );
    digitalWrite(in1, HIGH);         // door  motor  >>>> CLOSE  pin 12
    digitalWrite(in2, HIGH);       //  "      "     "     "  pin 11

  }            //=================== checking humidity sensors to control fan if high fan on
  if (h > maxHum) {
    digitalWrite(fan, HIGH);      // 5v output control line HIGH
  } else if (h < minHum) {
    digitalWrite(fan, LOW);       // 5v output control line LOW
  }

}

it's a great start

I'm a bit confused on what drives what, you could possibly make things easier to read with functions like activateMotorToOpenDoor() or activateMotorToCloseDoor() etc

pins could be also declared as "const byte" and using meaningful names + adding the word 'pin' in the variable name always help make code readable.

you have two lines doing the same thing

    val = digitalRead(buttonLpin);
    val = digitalRead(buttonLpin);     // read the input pin

I'm still unclear what happens if I press the button to open the door and t° drops to less than minTemp2? would that close the door on me?

@ J-M-L whoops, duplicated instruction. The line looks at the left-hand door limit switch when the door open button is pressed. When the door is fully open the microswitch operates, and the code shuts off the door motor.

I'm still unclear what happens if I press the button to open the door and t° drops to less than minTemp2? would that close the door on me?

The door toggle code on a button press keeps the door open irrespective of the temperature, the "return" statement loops back until a second button press "else" allows the program to continue. So until the door button is pressed again the door will stay open for all time, until a second press "toggles" and runs the door motor in reverse when the close stop limit switch will shut off the drive motor.
I agree my statement comments could be more descriptive, I will keep this in mind. I have some time to spend before I finally install, so I will look at improving the code structures as advised.
(minTemp2?) is celsius. There are two Temp parameters one for the door and another for the vent. ( see int=) in code.

I see

you rely on the modulo 2 to not go into the second part

J-M-L:
I see

you rely on the modulo 2 to not go into the second part

Yes, that's it, and everything else works fine, maybe the code needs streamlining but it would still give me the same result. When I finally put it all together I will use a nano board. All my previous projects used PIC micro's plus discrete components all on perf board! Thank goodness for Arduino's and their ability to link modules together. In the meantime, I'm still learning.
Regards Jevray.

Good work - agree with the “if it works, it works” :slight_smile: