IF &&

I have run into a problem with IF &&
I want a number of pins to go HIGH and LOW depending on the state of various buttons and count. The count is working fine, however if statebutton is LOW I do not want the action performing
I have tried the following

if ((count == 0) && (stateButton == 1))//test

using 1, HIGH TRUE to no avail.
The statebutton is to work as a master switch with nothing happening unless it is HIGH

if (count == 0)//test
{
count++;
digitalWrite(ledpin1, HIGH);
digitalWrite(ledpin2, LOW);
digitalWrite(ledpin3, LOW);
digitalWrite(E1, LOW);
digitalWrite(E2, HIGH);
digitalWrite(E3, LOW);
}

Sorry forgot my manners, any help or pointers much appreciated

There is nothing wrong with your improperly posted snippet. So, the problem is with the code that you did not post.

My guess is that you are comparing a pin number to HIGH or LOW, not the state read from the pin.

Sorry about the improperly posted snippet, please advise correct format and I will post complete code

Found it, complete code is attached

v1.1.ino (1.85 KB)

Read this before posting a programming question ...
posted at the top of the forum

use code tags </> like this:

int ledPin0 = 12;
int ledpin1 = 11;
int ledpin2 = 10;
int ledpin3 = 9;
int switchPin = 8;
int E1 = 7; //PIN6
int E2 = 6; //PIN7
int E3 = 5; //PIN9
int boost = 13;
int count = 0;
boolean lastButton;
boolean currentButton = false;
boolean ledOn = false;

void setup() {

  pinMode(boost, INPUT);
pinMode(ledPin0, OUTPUT);
  pinMode(switchPin, INPUT);
  pinMode(ledpin1, OUTPUT);
  pinMode(ledpin2, OUTPUT);
  pinMode(ledpin3, OUTPUT);
pinMode(E1, OUTPUT);
pinMode(E2, OUTPUT);
pinMode(E3, OUTPUT);
  count = 0;
}
//debounce function to stabilise the button
boolean debounce(boolean last)
{
  boolean current = digitalRead(switchPin);
  if (last != current)
  {
    delay(5);
    current = digitalRead(switchPin);
  }
  return current;  
  
}
void loop() {
  int stateButton = digitalRead(boost); 
 if(stateButton == 1) { //if is pressed
     digitalWrite(ledPin0, HIGH); //write 1 or HIGH to led pin
  } else { //if not pressed
     digitalWrite(ledPin0, LOW);  //write 0 or low to led pin
  lastButton = currentButton;
  currentButton = debounce(lastButton);
  if (lastButton == false && currentButton == true)
  {
    if ((count == 0) && (stateButton == 0))//test
    { 
      count++;
      digitalWrite(ledpin1, HIGH);
      digitalWrite(ledpin2, LOW);
      digitalWrite(ledpin3, LOW); 
digitalWrite(E1, LOW);
digitalWrite(E2, HIGH);
digitalWrite(E3, LOW);
    }

    else if ((count == 1) && (stateButton == 0))//test
    { 
      count++;
      digitalWrite(ledpin1, LOW); 
      digitalWrite(ledpin2, HIGH);
      digitalWrite(ledpin3, LOW);
digitalWrite(E1, HIGH);
digitalWrite(E2, LOW);
digitalWrite(E3, LOW);
    }

    else if ((count == 2) && (stateButton == 0))//test
    { 
      count = 0;
      digitalWrite(ledpin1, LOW);
      digitalWrite(ledpin2, LOW);
      digitalWrite(ledpin3, HIGH);
digitalWrite(E1, HIGH);
digitalWrite(E2, LOW);
digitalWrite(E3, HIGH);
  }
    }  
  } }

It is not necessary to attach code that is so short. Just use code tags and post it here.

int ledPin0 = 12;
int ledpin1 = 11;
int ledpin2 = 10;
int ledpin3 = 9;
int switchPin = 8;
int E1 = 7; //PIN6
int E2 = 6; //PIN7
int E3 = 5; //PIN9
int boost = 13;
int count = 0;
boolean lastButton;
boolean currentButton = false;
boolean ledOn = false;

void setup() {

  pinMode(boost, INPUT);
pinMode(ledPin0, OUTPUT);
  pinMode(switchPin, INPUT);
  pinMode(ledpin1, OUTPUT);
  pinMode(ledpin2, OUTPUT);
  pinMode(ledpin3, OUTPUT);
pinMode(E1, OUTPUT);
pinMode(E2, OUTPUT);
pinMode(E3, OUTPUT);
  count = 0;
}
//debounce function to stabilise the button
boolean debounce(boolean last)
{
  boolean current = digitalRead(switchPin);
  if (last != current)
  {
    delay(5);
    current = digitalRead(switchPin);
  }
  return current;  
  
}
void loop() {
  int stateButton = digitalRead(boost); 
 if(stateButton == 1) { //if is pressed
     digitalWrite(ledPin0, HIGH); //write 1 or HIGH to led pin
  } else { //if not pressed
     digitalWrite(ledPin0, LOW);  //write 0 or low to led pin
  lastButton = currentButton;
  currentButton = debounce(lastButton);
  if (lastButton == false && currentButton == true)
  {
    if ((count == 0) && (stateButton == 0))//test
    { 
      count++;
      digitalWrite(ledpin1, HIGH);
      digitalWrite(ledpin2, LOW);
      digitalWrite(ledpin3, LOW); 
digitalWrite(E1, LOW);
digitalWrite(E2, HIGH);
digitalWrite(E3, LOW);
    }

    else if ((count == 1) && (stateButton == 0))//test
    { 
      count++;
      digitalWrite(ledpin1, LOW); 
      digitalWrite(ledpin2, HIGH);
      digitalWrite(ledpin3, LOW);
digitalWrite(E1, HIGH);
digitalWrite(E2, LOW);
digitalWrite(E3, LOW);
    }

    else if ((count == 2) && (stateButton == 0))//test
    { 
      count = 0;
      digitalWrite(ledpin1, LOW);
      digitalWrite(ledpin2, LOW);
      digitalWrite(ledpin3, HIGH);
digitalWrite(E1, HIGH);
digitalWrite(E2, LOW);
digitalWrite(E3, HIGH);
  }
    }  
  } }

The formatting of the code is abysmal. Tools + Auto Format would fix that.

Some of your comments are useless. What does the value of E1 have to do with PIN6?

There is nothing in your post about how the switches are wired. You are not using the internal pullup resistors, so we must assume that you have external resistors. Do you? How ARE the switches wired?

Why are you debouncing only one of the switches?

I can't see any relationship between the pin number variable's name, boost, and the pin state variable's name, stateButton. Did you just make the names up at random? Why not use names that look like they have a relationship, like boostPin and boostState?

switchPin is useless. It says nothing about the purpose of the switch.

PaulS:
It is not necessary to attach code that is so short. Just use code tags and post it here.

int ledPin0 = 12;

int ledpin1 = 11;
int ledpin2 = 10;
int ledpin3 = 9;
int switchPin = 8;
int E1 = 7; //PIN6
int E2 = 6; //PIN7
int E3 = 5; //PIN9
int boost = 13;
int count = 0;
boolean lastButton;
boolean currentButton = false;
boolean ledOn = false;

void setup() {

pinMode(boost, INPUT);
pinMode(ledPin0, OUTPUT);
  pinMode(switchPin, INPUT);
  pinMode(ledpin1, OUTPUT);
  pinMode(ledpin2, OUTPUT);
  pinMode(ledpin3, OUTPUT);
pinMode(E1, OUTPUT);
pinMode(E2, OUTPUT);
pinMode(E3, OUTPUT);
  count = 0;
}
//debounce function to stabilise the button
boolean debounce(boolean last)
{
  boolean current = digitalRead(switchPin);
  if (last != current)
  {
    delay(5);
    current = digitalRead(switchPin);
  }
  return current; 
 
}
void loop() {
  int stateButton = digitalRead(boost);
if(stateButton == 1) { //if is pressed
    digitalWrite(ledPin0, HIGH); //write 1 or HIGH to led pin
  } else { //if not pressed
    digitalWrite(ledPin0, LOW);  //write 0 or low to led pin
  lastButton = currentButton;
  currentButton = debounce(lastButton);
  if (lastButton == false && currentButton == true)
  {
    if ((count == 0) && (stateButton == 0))//test
    {
      count++;
      digitalWrite(ledpin1, HIGH);
      digitalWrite(ledpin2, LOW);
      digitalWrite(ledpin3, LOW);
digitalWrite(E1, LOW);
digitalWrite(E2, HIGH);
digitalWrite(E3, LOW);
    }

else if ((count == 1) && (stateButton == 0))//test
    {
      count++;
      digitalWrite(ledpin1, LOW);
      digitalWrite(ledpin2, HIGH);
      digitalWrite(ledpin3, LOW);
digitalWrite(E1, HIGH);
digitalWrite(E2, LOW);
digitalWrite(E3, LOW);
    }

else if ((count == 2) && (stateButton == 0))//test
    {
      count = 0;
      digitalWrite(ledpin1, LOW);
      digitalWrite(ledpin2, LOW);
      digitalWrite(ledpin3, HIGH);
digitalWrite(E1, HIGH);
digitalWrite(E2, LOW);
digitalWrite(E3, HIGH);
  }
    } 
  } }



The formatting of the code is abysmal. Tools + Auto Format would fix that.

Some of your comments are useless. What does the value of E1 have to do with PIN6?

There is nothing in your post about how the switches are wired. You are not using the internal pullup resistors, so we must assume that you have external resistors. Do you? How ARE the switches wired?

Why are you debouncing only one of the switches?

There is a clue in my status as to why the formatting is abysmal...NEWBIE !
Comments are not useless PIN6 refers to pin 6 on a external component !
Yes I am using external pullups
Because only one switch requires debouncing

For a sketch that has so many criticisms and errors it works 95% orif I do away with the master switch it works 100% as desired !

Comments are not useless PIN6 refers to pin 6 on a external component !

The comment might be useful if it said that. Just saying PIN6 doesn't convey any usable information.

There is a clue in my status as to why the formatting is abysmal...NEWBIE !

There is NO reason to not properly indent code.

f I do away with the master switch it works 100% as desired !

What do your Serial.print() statements tell you is happening?

If nothing is to happen unless the master switch is pressed, I would expect to see code like:

const byte masterPin = 13;

void loop()
{
   int masterState = digitalRead(masterPin);
   if(masterState == LOW) // LOW means pressed
   {
      doEverything();
   }
}

with, of course, a doEveryting() function that does everything. That function would NOT have to worry about the state of the master switch.

I have gone back to the start and started a fresh as the old sketch worked fine on a simulator, but when it came to the real thing it failed terribly :frowning:

The outcome I need is this.

Pins 4-5-6 power a bi-colour led to display L_M_H setting selected with pin 11 with a pull up

Pins 8-9-10 when HIGH trigger a l298n to send 12v to a set of relays this is controlled by pin 12

The boost pin 12 is triggered by remote control relay with pull ups attached, if this relay is open I do not want anything to happen, with the exception of allowing the L_M_H setting to be changed.

When the boost pin 12 is momentary triggered the required relays on pins 8-9-10 are set to HIGH and LOW as required.

The sketch below groups the Low Medium and High settings together and performs everything together, which means when the remote control relay is triggered it cycles though the heat settings on each activation, when what is required is choose the setting once and let the remote relay trigger on pin 12 turn pins 8-9-10 HIGH or LOW

I have got to the stage I can not see the trees for the wood, please please help me

boolean lastButton;
boolean currentButton = false;
boolean ledOn = false;
int count = 0;
void setup() {
  pinMode(4, OUTPUT);//Boost
  pinMode(5, OUTPUT);//Low
  pinMode(6, OUTPUT);//Medium
  pinMode(7, OUTPUT);//High
  pinMode(8, OUTPUT);//Element 1
  pinMode(9, OUTPUT);//Element 2
  pinMode(10, OUTPUT);//Element 3
  pinMode(12, INPUT);// Boost
  pinMode(11, INPUT);// Setting

}
boolean debounce(boolean last)
{
  boolean current = digitalRead(12);
  if (last != current)
  {
    delay(5);
    current = digitalRead(12);
  }
  return current;

}

void loop()  {
  int stateButton = digitalRead(11);
  if (stateButton == 1) { //if is pressed
    digitalWrite(4, HIGH); //write 1 or HIGH to led pin
  } else { //if not pressed
    digitalWrite(4, LOW);  //write 0 or low to led pin
    lastButton = currentButton;
    currentButton = debounce(lastButton);
    if (lastButton == false && currentButton == true)
    {
      if (count == 0)
      {
        count++;
        digitalWrite(5, HIGH);
        digitalWrite(6, LOW);
        digitalWrite(7, LOW);
        digitalWrite(8, LOW);
        digitalWrite(9, HIGH);
        digitalWrite(10, LOW);
      }

      else if (count == 1)
      {
        count++;
        digitalWrite(5, LOW);
        digitalWrite(6, HIGH);
        digitalWrite(7, LOW);
        digitalWrite(8, HIGH);
        digitalWrite(9, LOW);
        digitalWrite(10, LOW);
      }

      else if (count == 2)
      {
        count = 0;
        digitalWrite(5, LOW);
        digitalWrite(6, LOW);
        digitalWrite(7, HIGH);
        digitalWrite(8, HIGH);
        digitalWrite(9, LOW);
        digitalWrite(10, HIGH);
      }
    }
  }
}
  pinMode(4, OUTPUT);//Boost
  pinMode(5, OUTPUT);//Low
  pinMode(6, OUTPUT);//Medium
  pinMode(7, OUTPUT);//High
  pinMode(8, OUTPUT);//Element 1

Don't comment, give the pins names.

AWOL:

  pinMode(4, OUTPUT);//Boost

pinMode(5, OUTPUT);//Low
  pinMode(6, OUTPUT);//Medium
  pinMode(7, OUTPUT);//High
  pinMode(8, OUTPUT);//Element 1


Don't comment, give the pins names.

I was berated for doing that yesterday

AWOL:
Hope this helps

  pinMode(4, OUTPUT);//Boost

pinMode(5, OUTPUT);//Low
 pinMode(6, OUTPUT);//Medium
 pinMode(7, OUTPUT);//High
 pinMode(8, OUTPUT);//Element 1



Don't comment, give the pins names.
int ledPin0 = 4;
int ledPin1 = 5;
int ledPin2 = 6;
int ledPin3 = 7;
int setting = 8;
int E1 = 9;
int E2 = 10;
int E3 = 11;
int boost = 12;
boolean lastButton;
boolean currentButton = false;
boolean ledOn = false;
int count = 0;
void setup() {
  pinMode(ledPin0, OUTPUT);//Boost
  pinMode(ledPin1, OUTPUT);//Low
  pinMode(ledPin2, OUTPUT);//Medium
  pinMode(ledPin3, OUTPUT);//High
  pinMode(E1, OUTPUT);//Element 1
  pinMode(E2, OUTPUT);//Element 2
  pinMode(E3, OUTPUT);//Element
  pinMode(setting, INPUT);// Setting
  pinMode(boost, INPUT);// Boost


}
boolean debounce(boolean last)
{
  boolean current = digitalRead(boost);
  if (last != current)
  {
    delay(5);
    current = digitalRead(boost);
  }
  return current;

}
// the loop function runs over and over again forever
void loop()  {
  int stateButton = digitalRead(setting);
  if (stateButton == 1) { //if is pressed
    digitalWrite(ledPin0, HIGH); //write 1 or HIGH to led pin
  } else { //if not pressed
    digitalWrite(ledPin0, LOW);  //write 0 or low to led pin
    lastButton = currentButton;
    currentButton = debounce(lastButton);
    if (lastButton == false && currentButton == true)
    {
      if (count == 0)
      {
        count++;
        digitalWrite(ledPin1, HIGH);
        digitalWrite(ledPin2, LOW);
        digitalWrite(ledPin3, LOW);
        digitalWrite(E1, LOW);
        digitalWrite(E2, HIGH);
        digitalWrite(E3, LOW);
      }

      else if (count == 1)
      {
        count++;
        digitalWrite(ledPin1, LOW);
        digitalWrite(ledPin2, HIGH);
        digitalWrite(ledPin3, LOW);
        digitalWrite(E1, HIGH);
        digitalWrite(E2, LOW);
        digitalWrite(E3, LOW);
      }

      else if (count == 2)
      {
        count = 0;
        digitalWrite(ledPin1, LOW);
        digitalWrite(ledPin2, LOW);
        digitalWrite(ledPin3, HIGH);
        digitalWrite(E1, HIGH);
        digitalWrite(E2, LOW);
        digitalWrite(E3, HIGH);
      }
    }
  }

}

andi968,

Here's an example of your code with changes to both formatting and symbol names.

The symbols names I've used don't have to necessarily match PaulS's but they should be consistently applied across your code.

Note that proper use of symbols can reduce the need for some comments, and that doing so keeps such comments from getting out of sync with the code and misleading debugging efforts.

const uint8_t   pinLED_BOOST        =  4;
const uint8_t   pinLED_LOW          =  5;
const uint8_t   pinLED_MEDIUM       =  6;
const uint8_t   pinLED_HIGH         =  7;
const uint8_t   pinBUTTON_SET       =  8;
const uint8_t   pinLED_ELEMENT_1    =  9;
const uint8_t   pinLED_ELEMENT_2    = 10;
const uint8_t   pinLED_ELEMENT_3    = 11;
const uint8_t   pinBUTTON_BOOST     = 12;

const uint8_t   LED_OFF             = LOW;
const uint8_t   LED_ON              = HIGH;

const uint8_t   BUTTON_UP           = LOW;
const uint8_t   BUTTON_DOWN         = HIGH;

boolean lastButton;
boolean currentButton = false;
boolean ledOn = false;
int count = 0;

void setup()
{
    pinMode(pinLED_BOOST,       OUTPUT);
    pinMode(pinLED_LOW,         OUTPUT);
    pinMode(pinLED_MEDIUM,      OUTPUT);
    pinMode(pinLED_HIGH,        OUTPUT);
    pinMode(pinLED_ELEMENT_1,   OUTPUT);
    pinMode(pinLED_ELEMENT_2,   OUTPUT);
    pinMode(pinLED_ELEMENT_3,   OUTPUT);

    pinMode(pinBUTTON_SET,      INPUT);
    pinMode(pinBUTTON_BOOST,    INPUT);
}

boolean debounce(boolean last)
{
    boolean current = digitalRead(pinBUTTON_BOOST);
    if (last != current)
    {
        delay(5);
        current = digitalRead(pinBUTTON_BOOST);
    }

    return current;
}

void loop() 
{
    int stateButton = digitalRead(pinBUTTON_SET);
    if (stateButton == BUTTON_DOWN)
    {
        digitalWrite(pinLED_BOOST, LED_ON);
    }
    else
    {
        digitalWrite(pinLED_BOOST, LED_OFF);

        lastButton      = currentButton;
        currentButton   = debounce(lastButton);

        if ( lastButton == false && currentButton == true )
        {
            if ( count == 0 )
            {
                count++;

                digitalWrite(pinLED_LOW,        LED_ON);
                digitalWrite(pinLED_MEDIUM,     LED_OFF);
                digitalWrite(pinLED_HIGH,       LED_OFF);

                digitalWrite(pinLED_ELEMENT_1,  LED_OFF);
                digitalWrite(pinLED_ELEMENT_2,  LED_ON);
                digitalWrite(pinLED_ELEMENT_3,  LED_OFF);
            }
            else if ( count == 1 )
            {
                count++;

                digitalWrite(pinLED_LOW,        LED_OFF);
                digitalWrite(pinLED_MEDIUM,     LED_ON);
                digitalWrite(pinLED_HIGH,       LED_OFF);

                digitalWrite(pinLED_ELEMENT_1,  LED_ON);
                digitalWrite(pinLED_ELEMENT_2,  LED_OFF);
                digitalWrite(pinLED_ELEMENT_3,  LED_OFF);
            }
            else if ( count == 2 )
            {
                count = 0;

                digitalWrite(pinLED_LOW,        LED_OFF);
                digitalWrite(pinLED_MEDIUM,     LED_OFF);
                digitalWrite(pinLED_HIGH,       LED_ON);

                digitalWrite(pinLED_ELEMENT_1,  LED_OFF);
                digitalWrite(pinLED_ELEMENT_2,  LED_OFF);
                digitalWrite(pinLED_ELEMENT_3,  LED_ON);
            }
        }
    }
}

I was berated for doing that yesterday

Berated for giving the pins names or not giving them names ?

UKHeliBob:
Berated for giving the pins names or not giving them names ?

Giving them names, I always give them names to remind me where they go.

lloyddean:
andi968,

Here's an example of your code with changes to both formatting and symbol names.

The symbols names I've used don't have to necessarily match PaulS's but they should be consistently applied across your code.

Note that proper use of symbols can reduce the need for some comments, and that doing so keeps such comments from getting out of sync with the code and misleading debugging efforts.

Thank you so much for tidying my mess up, I still can not see where I am going wrong.
Think I need to goto Specsavers

andi968:
Giving them names, I always give them names to remind me where they go.

I would be interested to see where that happened. Can you please provide a link ?

lloyddean:
andi968,

Here's an example of your code with changes to both formatting and symbol names.

The symbols names I've used don't have to necessarily match PaulS's but they should be consistently applied across your code.

Note that proper use of symbols can reduce the need for some comments, and that doing so keeps such comments from getting out of sync with the code and misleading debugging efforts.

A huge thank you Lloyddean thanks to you clean my code up, I could see the error of my ways. I had spent a week trying and trying and thanks to you I got there in the end
Many Thanks

const uint8_t   pinLED_Green          =  5;
const uint8_t   pinLED_Red       =  6;
const uint8_t   pinLED_BOOST        =  7;
const uint8_t   pinBUTTON_BOOST     = 8;
const uint8_t   pinLED_ELEMENT_1    =  9;
const uint8_t   pinLED_ELEMENT_2    = 10;
const uint8_t   pinLED_ELEMENT_3    = 11;
const uint8_t   pinBUTTON_SET       = 12;
const uint8_t   LED_OFF             = LOW;
const uint8_t   LED_ON              = HIGH;
const uint8_t   BUTTON_UP           = LOW;
const uint8_t   BUTTON_DOWN         = HIGH;
boolean lastButton;
boolean currentButton = false;
boolean ledOn = false;
int count = 0;

void setup()
{
  pinMode(pinLED_Green,         OUTPUT);
  pinMode(pinLED_Red,      OUTPUT);
  pinMode(pinLED_BOOST,       OUTPUT);
  pinMode(pinLED_ELEMENT_1,   OUTPUT);
  pinMode(pinLED_ELEMENT_2,   OUTPUT);
  pinMode(pinLED_ELEMENT_3,   OUTPUT);
  pinMode(pinBUTTON_SET,      INPUT);
  pinMode(pinBUTTON_BOOST,    INPUT);
}

boolean debounce(boolean last)
{
  boolean current = digitalRead(pinBUTTON_BOOST);
  if (last != current)
  {
    delay(5);
    current = digitalRead(pinBUTTON_BOOST);
  }

  return current;
}

void loop()
{
  int stateButton = digitalRead(pinBUTTON_SET);
  if (stateButton == BUTTON_DOWN)
  {
    digitalWrite(pinLED_BOOST, LED_ON);
  }
  else
  {
    digitalWrite(pinLED_BOOST, LED_OFF);

    lastButton      = currentButton;
    currentButton   = debounce(lastButton);

    if ( lastButton == false && currentButton == true )
    {
      if ( count == 0 )
      {
        count++;
        digitalWrite(pinLED_ELEMENT_1,  LED_OFF);
        digitalWrite(pinLED_ELEMENT_2,  LED_ON);
        digitalWrite(pinLED_ELEMENT_3,  LED_OFF);
        digitalWrite(pinLED_Red,  LED_ON);
        digitalWrite(pinLED_Green,  LED_ON);
      }
      else if ( count == 1 )
      {
        count++;
        digitalWrite(pinLED_ELEMENT_1,  LED_ON);
        digitalWrite(pinLED_ELEMENT_2,  LED_OFF);
        digitalWrite(pinLED_ELEMENT_3,  LED_OFF);
        digitalWrite(pinLED_Red,  LED_OFF);
        digitalWrite(pinLED_Green,  LED_ON);
      }
      else if ( count == 2 )
      {
        count = 0;
        digitalWrite(pinLED_ELEMENT_1,  LED_ON);
        digitalWrite(pinLED_ELEMENT_2,  LED_OFF);
        digitalWrite(pinLED_ELEMENT_3,  LED_ON);
        digitalWrite(pinLED_Red,  LED_ON);
        digitalWrite(pinLED_Green,  LED_OFF);
      }
    }
  }
}

The OP was not berated, but I took it that he was told to use useful names, not just any names.

The names used were not useful to me, and are unlikely to be useful to the OP six months from now.