LED function interfering with toggle switch operation

Projects Programming
1

Hello I have a momentary toggle switch that turns on two LED's that is working correctly for left and right barrels. I also have a blinking function on a separate switch controlling the same LED's. The problem I am having is when I have the blink function operational the LED toggle if statement will only flash once and not turn on even though the Serial print shows the toggle switch working. At the end of the blink function if the switch is turned off the LED's are set to turn off and what I have found is I have to comment out the LED's digitalWrite(lBarrel, LOW) and digitalWrite(rBarrel, LOW) in order for the toggle switch to operate correctly but then when I turn off the blink switch the LED's will sometimes still be on which would be normal without the digitalWrite LOW. How can I do a work around or is it possible. Thanks RBixler.

const int LEDsw = 7;     // Barrel LED switch
const int staticsw = 8;  // Momentary switch to change LED from on to off

int LEDswState = 0;          // set switch state and start at 0/LOW
int LEDstate = 0;            // create and set the LED state to LOW to start
const long ledInterval = 50;   // LED flashing time
unsigned long prevMillis = 0;  // Declare previous time

int staticState = LOW;  // Declare set static switch state and start at 0/LOW
int staticSwitchNew;
int staticSwitchOld = 1;


void setup() {
  Serial.begin(9600);  // start serial comunications

  // Set pin modes for all input and output controls
  pinMode(LEDsw, INPUT);
  pinMode(staticsw, INPUT);
  pinMode(lBarrel, OUTPUT);
  pinMode(rBarrel, OUTPUT);

}

void loop() {
  unsigned long currentMillis = millis();  // Declare current time
  LEDswState = digitalRead(LEDsw);         // Read the LED switch
  staticSwitchNew = digitalRead(staticsw);

   if (staticSwitchOld == 0 && staticSwitchNew == 1){
    if (staticState == 0) {
      digitalWrite(lBarrel, HIGH);
      digitalWrite(rBarrel, HIGH);
      staticState = 1;
    } else {
      digitalWrite(lBarrel, LOW);
      digitalWrite(rBarrel, LOW);
      staticState = 0;
    }
  }
  staticSwitchOld = staticSwitchNew;
 Delay(50);

barrelLEDs();

} // End main loop

/********** Functions ***********/
void barrelLEDs() {
  unsigned long currentMillis = millis();
  // Check if the LED switch is on
  if (LEDswState == HIGH) {
    LEDswState = HIGH;

    if (currentMillis - prevMillis >= ledInterval) {
      //Save the last time you blinked the led
      prevMillis = millis();
      // if the led is off turn it on and vice-versa
      if (LEDstate == LOW) {
        LEDstate = HIGH;
      } else {
        LEDstate = LOW;
      }  // End millis() control
    }    // End LED state
    digitalWrite(lBarrel, LEDstate);
    digitalWrite(rBarrel, LEDstate);
  
  } else {
    LEDswState = LOW;
    //digitalWrite(lBarrel, LOW);
    //digitalWrite(rBarrel, LOW);
  }  // End LED switch state


}  // End LED control
2

:thinking:

3

Hello LarryD I switched the LEDSwitchState and still same issue.

    LEDswState = HIGH;
} else {
LEDswState = HIGH;
}
4

...

5
  • Show us the changes.
6 
const int LEDsw = 7;     // Barrel LED switch
const int staticsw = 8;  // Momentary switch to change LED from on to off

int LEDswState = 0;          // set switch state and start at 0/LOW
int LEDstate = 0;            // create and set the LED state to LOW to start
const long ledInterval = 50;   // LED flashing time
unsigned long prevMillis = 0;  // Declare previous time

int staticState = LOW;  // Declare set static switch state and start at 0/LOW
int staticSwitchNew;
int staticSwitchOld = 1;


void setup() {
  Serial.begin(9600);  // start serial comunications

  // Set pin modes for all input and output controls
  pinMode(LEDsw, INPUT);
  pinMode(staticsw, INPUT);
  pinMode(lBarrel, OUTPUT);
  pinMode(rBarrel, OUTPUT);

}

void loop() {
  unsigned long currentMillis = millis();  // Declare current time
  LEDswState = digitalRead(LEDsw);         // Read the LEd switch
  staticSwitchNew = digitalRead(staticsw);

   if (staticSwitchOld == 0 && staticSwitchNew == 1){
    if (staticState == 0) {
      digitalWrite(lBarrel, HIGH);
      digitalWrite(rBarrel, HIGH);
      staticState = 1;
    } else {
      digitalWrite(lBarrel, LOW);
      digitalWrite(rBarrel, LOW);
      staticState = 0;
    }
  }
  staticSwitchOld = staticSwitchNew;

barrelLEDs();

} // End main loop

/********** Functions ***********/
void barrelLEDs() {
  unsigned long currentMillis = millis();
  // Check if the LED switch is on
  if (LEDswState == LOW) {
    LEDswState = HIGH;

    if (currentMillis - prevMillis >= ledInterval) {
      //Save the last time you blinked the led
      prevMillis = millis();
      // if the led is off turn it on and vice-versa
      if (LEDstate == LOW) {
        LEDstate = HIGH;
      } else {
        LEDstate = LOW;
      }  // End millis() control
    }    // End LED state
    digitalWrite(lBarrel, LEDstate);
    digitalWrite(rBarrel, LEDstate);
  
  } else {
    LEDswState = LOW;
    //digitalWrite(lBarrel, LOW);
    //digitalWrite(rBarrel, LOW);
  }  // End LED switch state

}  // End LED control
7

After changing the code when I turn off the blink switch sometimes the LED's stay on unless I digitalWrite LOW.

8

I also tried xfpd's solution and still the same problem.

9
  • How are your switches wired ?
10

My switches are wired with a external pull down resister and 1 connection to the arduino pins 7 and 8 and other connection to 5V.

11

SwitchesAndLEDs
SwitchesAndLEDs1224×1108 280 KB

  • So you are wired as S1 above ?
12

Yes

13

If I use the internal pullup resistor will I have to do a digitalWrite to LOW at the end of my function.

15
  • You can use the S1 example, it's just not recommended especially with long wires.
16

My wires to the switches and LED's are only 3 inches long. I have been using pull down resistors for a long time, I can change to using pullup and de-solder the pull down resistors with no problem. With the blink function without delay I need to digitalWrite the LED's low otherwise sometimes when I turn off the LED switch the LED"s will stay on. Is there another way to monitor the LED's off state without digitalWrite LOW.

17

I also tried rewriting my toggle switch if statement this way and the LED's would flash once when turned on without commenting out the digitalWrite LOW in the blink function.

if ((staticSwitchOld == 0 && staticSwitchNew == 1) && (LEDswState == LOW)){
    if (staticState == 0) {
      digitalWrite(lBarrel, HIGH);
      digitalWrite(rBarrel, HIGH);
      staticState = 1;
    } else {
      digitalWrite(lBarrel, LOW);
      digitalWrite(rBarrel, LOW);
      staticState = 0;
    }
  }
  staticSwitchOld = staticSwitchNew;
18

You have a number of problems with the logic.

This

  if (LEDswState == LOW) {
    LEDswState = HIGH;

    if (currentMillis - prevMillis >= ledInterval) {
      //Save the last time you blinked the led
      prevMillis = millis();
      // if the led is off turn it on and vice-versa
      if (LEDstate == LOW) {
        LEDstate = HIGH;
      } else {
        LEDstate = LOW;
      }  // End millis() control
    }    // End LED state
    digitalWrite(lBarrel, LEDstate);
    digitalWrite(rBarrel, LEDstate);
  
  } else {
    LEDswState = LOW;
    //digitalWrite(lBarrel, LOW);
    //digitalWrite(rBarrel, LOW);
  }  // End LED switch st

Has toggling checked and executed earlier than the blinking timing mechanism. The code never gets a chance to see the timer expire, or does only occasionally and weirdly.

It also writes LEDstate to the LEDs sometimes, but never changes the value.

Lastly for now, LEDswState is over-used as the receiver of the digitalRead() in one place, and as the variable to handle toggling in another. They fight, LEDs lose.

This will blink the LEDs if you comment out the clobberation of LEDswState that happens when you do the digitalRead(). You need another variable to check the button and turn on and off blinking, if that is in fact the point:

void barrelLEDs() {
  static unsigned long prevMillis = 0;  // Declare previous time
  unsigned long currentMillis = millis();

// possibly check here to see if we should even be thinking about blinking?

// ???  return;  // not in blinking mode

// and don't change anything if it isn't time
    if (currentMillis - prevMillis < ledInterval) return;  // ALL DONE! not time to blink the LEDs
    prevMillis = currentMillis;

Serial.print("            barrel switch  ");
Serial.println(LEDswState);

  // Check if the LED switch is on
  if (LEDswState == LOW) {
    LEDswState = HIGH;
  } else {
    LEDswState = LOW;
  }

// write the LEDs with their new value:

  digitalWrite(lBarrel, LEDswState);
  digitalWrite(rBarrel, LEDswState);
}

I'd fix it all except life is too short, and even if I did, it would then be my code, not yours.

The nice thing is that code does exactly what you write in this case, so analysis and printing for checking variables and flow through the code will get you some good information.

Oh, nothing you posted compiled. I made obvious fixes. And you are using INPUT mode on the switches - are you using a pullup or pulldown resistor? If not, anything can happen and all observations are without meaning.

I switched you over to INPUT_PULLUP mode and wired my switches between the pin and ground.

a7

19

Note my switch pins are D7 and D8 and I do not have both pins connected together the switch pins are connected separately to 5V with a pull down resistor not just one connection to 5V

20

I also forgot to add the barrel LED's in the code I put in my recent post this is the full code Sorry!

const int LEDsw = 7;     // Barrel LED switch
const int staticsw = 8;  // Momentary switch to change LED from on to off

// Declare barrel LED output pins
const int lBarrel = 5;  // Left barrel
const int rBarrel = 6;  // Right barrel


int LEDswState = 0;          // set switch state and start at 0/LOW
int LEDstate = 0;            // create and set the LED state to LOW to start
const long ledInterval = 50;   // LED flashing time
unsigned long prevMillis = 0;  // Declare previous time

int staticState = LOW;  // Declare set static switch state and start at 0/LOW
int staticSwitchNew;
int staticSwitchOld = 1;


void setup() {
  Serial.begin(9600);  // start serial comunications

  // Set pin modes for all input and output controls
  pinMode(LEDsw, INPUT);
  pinMode(staticsw, INPUT);
  pinMode(lBarrel, OUTPUT);
  pinMode(rBarrel, OUTPUT);

}

void loop() {
  unsigned long currentMillis = millis();  // Declare current time
  LEDswState = digitalRead(LEDsw);         // Read the LEd switch
  staticSwitchNew = digitalRead(staticsw);

   if (staticSwitchOld == 0 && staticSwitchNew == 1){
    if (staticState == 0) {
      digitalWrite(lBarrel, HIGH);
      digitalWrite(rBarrel, HIGH);
      staticState = 1;
    } else {
      digitalWrite(lBarrel, LOW);
      digitalWrite(rBarrel, LOW);
      staticState = 0;
    }
  }
  staticSwitchOld = staticSwitchNew;

barrelLEDs();

} // End main loop

/********** Functions ***********/
void barrelLEDs() {
  unsigned long currentMillis = millis();
  // Check if the LED switch is on
  if (LEDswState == HIGH) {
    LEDswState = HIGH;

    if (currentMillis - prevMillis >= ledInterval) {
      //Save the last time you blinked the led
      prevMillis = millis();
      // if the led is off turn it on and vice-versa
      if (LEDstate == LOW) {
        LEDstate = HIGH;
      } else {
        LEDstate = LOW;
      }  // End millis() control
    }    // End LED state
    digitalWrite(lBarrel, LEDstate);
    digitalWrite(rBarrel, LEDstate);
  
  } else {
    LEDswState = LOW;
    //digitalWrite(lBarrel, LOW);
    //digitalWrite(rBarrel, LOW);
  }  // End LED switch state

}  // End LED control
21

You still have the timing inside the toggling, and you still are clobbering LEDswState every time you loop, which is very often.

  LEDswState = digitalRead(LEDsw);         // Read the LEd switch

Say when the blinking is supposed to happen. When the switch is closed, or on and off each time you press, or what?

You need to have the switch act on the variable that will say whether to blink. It is unlikely to be the same variable you use to read the switch, unless the switch is a real switch on and off, not a pushbutton toggle.

The code fights with itself over who does, and does not write to the LEDs.

Perhaps you clu,d describe in a few words how this is supposed to work: what kind each switch is and how the switches should control what is happening on the LEDs.


You probably won't again, but a good way to not mess up is to look at the code that is running, then use the IDE Copy for Forum tool to grab up the sketch in its entirety and come back here and paste it.

a7