Switching LED's on and off

Hi all,

Very, very new to this. I’ve set up this code for LED’s to chase each other and a switch. Basically I want the program to run and the LED’s to chase when the switch is in one state, and for all the LED’s to be off when the switch is in the other state.

Currently, switching the switch starts the LED’s chasing fine, but when the switch is switched back, the LED’s stop chasing but the last lit LED stays on instead of them all turning off, flipping the switch starts them chasing again.

What am I missing?

Thanks,
Paul.

int ed2 = 3;
int ed3 = 4;
int ed4 = 5;

unsigned long previousMillis = 0;
const long interval  = 500;
const long interval2 = 1000;
const long interval3 = 1500;
const long interval4 = 1501;
void setup() {
  //start serial connection
  Serial.begin(9600);
  pinMode (ed2,  OUTPUT);
  pinMode (ed3,  OUTPUT);
  pinMode (ed4,  OUTPUT);
  pinMode(9, INPUT_PULLUP);
}

void loop() {
  // put your main code here, to run repeatedly:
  unsigned long currentMillis = millis();
  //read the pushbutton value into a variable
  int sensorVal = digitalRead(9);
  //print out the value of the pushbutton
  Serial.println(sensorVal);

  if (sensorVal == HIGH) {

    if (currentMillis - previousMillis > interval && currentMillis - previousMillis < interval2) {
      digitalWrite (ed2, HIGH); digitalWrite (ed3, LOW); digitalWrite (ed4, LOW);
    }

    if (currentMillis - previousMillis > interval2 && currentMillis - previousMillis < interval3) {
      digitalWrite (ed2, LOW); digitalWrite (ed3, HIGH); digitalWrite (ed4, LOW);
    }

    if (currentMillis - previousMillis > interval3) {
      digitalWrite (ed2, LOW); digitalWrite (ed3, LOW); digitalWrite (ed4, HIGH);
    }
    if (currentMillis - previousMillis > interval4) {
      previousMillis = currentMillis;
    }

    else if (sensorVal == LOW) {
      digitalWrite (ed2, LOW);
      digitalWrite (ed3, LOW);
      digitalWrite (ed4, LOW);
    }
  }
}

You need to move your else if down a bit. As it is, it’s contained within the if it’s supposed to be the else for.

Welcome paulbre.
++Karma; // For posting your code correctly on your first post.

In addition to what bill said you don’t need else if, just else. If sensorVal is not HIGH it must be LOW, no need to test it again.

PerryBebbington:
Welcome paulbre.
++Karma; // For posting your code correctly on your first post.

In addition to what bill said you don't need else if, just else. If sensorVal is not HIGH it must be LOW, no need to test it again.

Hi PerryBebbington

Thanks for the positive Karma, glad my first post has been received well :slight_smile:
I tried as you say, it does indeed switch off as it should, but when on, the LED's chase as they should, but are very dim.

Dim suggests not enough current, you have told us nothing about how they are wired or powered so difficult to help.

Post a schematic, hand drawn and photographed is fine if you don't have software for creating schematics.

wildbill:
You need to move your else if down a bit. As it is, it's contained within the if it's supposed to be the else for.

Hi wildbill,
Right on the money! Added in the extra set of {} to contain the else if separately and its now all working :slight_smile: Thanks for the assist.

PerryBebbington:
Dim suggests not enough current, you have told us nothing about how they are wired or powered so difficult to help.

Post a schematic, hand drawn and photographed is fine if you don't have software for creating schematics.

Please forgive the terrible schematic, tried the Fusion360 electrical design package and not sure how to input I/O ports.

The LED's are connected to pins 3, 4, and 5 on an adafruit trinket pro 3v. Switch is connected to pin 9. GPIO's output 3.3v at 20mA according to the spec sheet. Each resistor is 220ohms.

I'm not understanding how if I do it your way, they are dim, but wildbills way yields the same switching on/off result, but the LED's are at full brightness?

Each LED should have it's own resistor.

3V3 at the pin - about 1V8 across the LEDs gives 1V5 / 220 Ohms gives about 6.8mA shared between 2 LEDs, so about 3.4mA per LED.

I don't understand your comment about a difference between my way and Bill's.

Ahh, apologies, should have done a double check on my actual circuit before hitting send.
Each LED has its own 220ohm resistor already.

I think I've found where the issue is, if I change the else if statement to:

  if (sensorVal == LOW); {

The LED's are dim,

if I type it as:

  if (sensorVal == LOW) {

The LED's are bright.

Ok, but I meant get rid of the if, you just need the else without the test.

} else {
// Code...

Ahh, I see what you mean now, just tried it and it works great :slight_smile:

Thanks ever so much to you both, I’d been scratching my head on it for the last day or two (only really started on looking at coding a little over a week ago). Think I’m going to be posting a few more questions up on here before long as I’d like to include sound too :smiley:

  if (sensorVal == LOW); {

The only code being run because sensorVal is LOW is the semicolon. Any code in the { } afterwards is always run, whatever the state of sensorVal

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.