Code works, but not with a button

Hi I have tried my best to get this code to start with a button push. I am hoping to be able to repeat the sequence with a button push at any time.

I am building a model. There are several lights on. when the button is pushed, I want the code to run-turn off some lights, turn others on at the same time, than after 10 seconds (or so), the lights come back on one at a time, the others just turned on go off. This code works the way I want it on my Arduino Mega. But i have tried all kinds of button codes, libraries and made up stuff, but it won’t go. The primary problem is the timing trigger that I want to start with the button goes no matter if the button is pushed or not. I am stuck. This looks like it should be so easy, but there is something that I am just not getting.

This is the code that I worked out. (The “drydock” LEDs should be in an array, but one step at a time.) You can see things that I have tried commented out). I also know that the redundant "redPin"s need not be there. Thanks.

// Enterprise Refit Red Alert Sequence

int dryDock1 = 23; //floodlights
int dryDock2 = 25;
int dryDock3 = 27;
int dryDock4 = 29;
int dryDock5 = 31;
int dryDock6 = 33;
int dryDock7 = 35;
int redPin1 = 50; //red alert lights
int redPin2 = 52;
int redSwitch = 28;
int relayPin = 48;//relay
int redState = 1;
int redLength = 11000;

const unsigned long eventInterval0 = 10000;
const unsigned long eventInterval1 = 11000;
const unsigned long eventInterval2 = 11500;
const unsigned long eventInterval3 = 12000;
const unsigned long eventInterval4 = 12500;
const unsigned long eventInterval5 = 13000;
const unsigned long eventInterval6 = 13500;
const unsigned long eventInterval7 = 14000;
unsigned long previousTime = 0;
unsigned long currentTime = 0;


void setup() {
  Serial.begin(9600);
  pinMode(redPin1, OUTPUT);
  pinMode(redPin2, OUTPUT);
  pinMode(dryDock1, OUTPUT);
  pinMode(dryDock2, OUTPUT);
  pinMode(dryDock3, OUTPUT);
  pinMode(dryDock4, OUTPUT);
  pinMode(dryDock5, OUTPUT);
  pinMode(dryDock6, OUTPUT);
  pinMode(dryDock7, OUTPUT);
  pinMode(relayPin, OUTPUT);
  pinMode(redSwitch, INPUT);
}

void loop() {

  //  redState = digitalRead(redSwitch);  // check if button is pushed
  //  if (redState == HIGH)
  currentTime = millis();
  if (currentTime <= redLength)
  {
    digitalWrite(redPin1, HIGH);
    digitalWrite(redPin2, HIGH);
    digitalWrite(relayPin, HIGH);
    digitalWrite(dryDock1, LOW);
    digitalWrite(dryDock2, LOW);
    digitalWrite(dryDock3, LOW);
    digitalWrite(dryDock4, LOW);
    digitalWrite(dryDock5, LOW);
    digitalWrite(dryDock6, LOW);
    digitalWrite(dryDock7, LOW);
  }


  if (currentTime = millis())
    //    previousTime = currentTime;
    /* Updates frequently */
//    digitalWrite(redPin1, LOW);
//  digitalWrite(redPin2, LOW);
//  digitalWrite(relayPin, LOW);

  if (currentTime >= eventInterval1)
  {
    Serial.println("Ice Ice Baby1");
    { digitalWrite(redPin1, LOW);
      digitalWrite(redPin2, LOW);
      digitalWrite(relayPin, LOW);
      digitalWrite(dryDock1, HIGH);
    }
    /* Update the timing for the next time around */
    //  previousTime = currentTime;
  }
  /* This is the event */
  if (currentTime >= eventInterval2) {
    /* Event code */
    Serial.println("Ice Ice Baby2");

    //digitalWrite(dryDock1, HIGH);
    digitalWrite(dryDock2, HIGH);
    digitalWrite(redPin1, LOW);
    digitalWrite(redPin2, LOW);
    digitalWrite(relayPin, LOW);
  }

  /* Update the timing for the next time around */
  //  previousTime = currentTime;
  /* This is the event */
  if (currentTime >= eventInterval3) {
    /* Event code */
    Serial.println("Ice Ice Baby3");
    digitalWrite(dryDock3, HIGH);
    digitalWrite(redPin1, LOW);
    digitalWrite(redPin2, LOW);
    digitalWrite(relayPin, LOW);
  }
  /* Update the timing for the next time around */
  // previousTime = currentTime;
  /* This is the event */
  if (currentTime >= eventInterval4) {
    /* Event code */
    Serial.println("Ice Ice Baby4");
    digitalWrite(dryDock4, HIGH);
    digitalWrite(redPin1, LOW);
    digitalWrite(redPin2, LOW);
    digitalWrite(relayPin, LOW);
  }
  /* Update the timing for the next time around */
  //  previousTime = currentTime;
  /* This is the event */
  if (currentTime  >= eventInterval5) {
    /* Event code */
    Serial.println("Ice Ice Baby5");
    digitalWrite(dryDock5, HIGH);
    digitalWrite(redPin1, LOW);
    digitalWrite(redPin2, LOW);
    digitalWrite(relayPin, LOW);
  }
  /* Update the timing for the next time around */
  //   previousTime = currentTime;

  if (currentTime  >= eventInterval6) {
    /* Event code */
    Serial.println("Ice Ice Baby6");
    digitalWrite(dryDock6, HIGH);
    digitalWrite(redPin1, LOW);
    digitalWrite(redPin2, LOW);
    digitalWrite(relayPin, LOW);
  }
  /* Update the timing for the next time around */
  //   previousTime = currentTime;

  if (currentTime  >= eventInterval7) {
    /* Event code */
    Serial.println("Ice Ice Baby7");
    digitalWrite(dryDock7, HIGH);
    digitalWrite(redPin1, LOW);
    digitalWrite(redPin2, LOW);
    digitalWrite(relayPin, LOW);
    Serial.println("end");
    for (;;) {}
  } else
  { }
  //

}

Oh, yes, the “for” statement at the end is just so I can stop the print. I don’t intend it to be used in the final sketch.

-Jim

Right off the bat, I think this isn’t doing what you want.

This sets current time to millis() then always returns true. Compare is == not =

Also redSwitch is set up as an input. But you never “look” at it.

-jim lee

Hello,
my recommendation is to build a small time controlled FSM by using the SWITCH/CASE statement.
eg
switch (event) {
case Event1: makeLedBusiness( Event1); break;
case Event2: makeLedBusiness(Event2); break;
.
.
.

There’s a lot of stuff in there that is dubious, But I think the central fact of the switch not being digitalRead() from happoened while they were trying to debug this, notice the first two lines of loop being commented out.

What I would suggest OP do is make a new sketch, with just button press functionality in it. I susopect that when they wire the switch to it the way it is in this project, it will randomly report that it is pressed while sitting there unpressed, because the wire from the switch floats when the button isn’t pressed. Any time you see pinMode(switchpin,INPUT) in code written by a novice, you can predict that twill be what they ask about; people seem to instinctively wire switches with one side on 5V, other end to an INPUT and expect it to read 0 if switch isnt closed and 1 if it is. When switch is closed, that’s correct. But when it’s not - there’s no more reason for it to read 0 than to read 1, and it will pick up ambient electromagnetic noise like an antenna and transition randomly.

The problem with “floating” inputs is widely discussed

The usual solution is to have the other side of the switch connected to ground instead of the positive supply, and to set the pin INPUT_PULLUP not INPUT. You see this design pattern EVERYWHERE if you take apart consumer electronics - never the other way around. Even though there are a fair number of microcontrollers with internal pulldowns too (ex: stm32), you never see anyone doing it that way in a product.

Thanks, Jim. I don’t refer to the switch because no matter how I tried to incorporate it, it would never work for me. Normally, as I thought I understood it, you say, look at the button, notice its state high or low. If low, wait. When the state changes to high, then do the thing. But I need that button to also start the time stamp that the rest refers to. Then I want it to wait for the button press to let it start again.

I’ll need to do some more research on that.

I have my buttons pulled down to ground via 10K resistors.

You should always show a good actual wiring image too.

I don’t know if this qualifies as ‘good’, but here is one.

Try something like:

  redState = digitalRead(redSwitch);  // check if button is pushed
  if (redState == HIGH)
    currentTime = millis();
  if (millis()-currentTime <= redLength)

if (currentTime = millis())

Assume you fixed the above problem.

You should have commented the above line otherwise the line after would or would not be executed.

Some of the LEDs in the front are not connected or not connected properly.


You should study the State Machine programming technique.


Where are things right now ?

FYI,

Hi. There is no change, and the LEDs in the foreground are not a part of this section. I guess I should have pulled all the extraneous stuff (it is all part of a rig for lighting my Star Trek model).

I can get a button to turn on the reds/off the spots, and after a length of time, restore what was there, but all the whites at once. I want to sequence them on, not just “pop”.

I am also studying state machines to see if I can figure out using that. I am using external pulldown method.

Please show us the code as it is right now.

// Integer variables


int dryDock1 = 23;
int dryDock2 = 25;
int dryDock3 = 27;
int dryDock4 = 29;
int dryDock5 = 31;
int dryDock6 = 33;
int dryDock7 = 35;
int redPin1 = 50;
int redPin2 = 52;
int redSwitch = 28;
int relayPin = 48;
int redState = 1;
int redLength = 11000;

const unsigned long eventInterval0 = 10000;
const unsigned long eventInterval1 = 11000;
const unsigned long eventInterval2 = 11500;
const unsigned long eventInterval3 = 12000;
const unsigned long eventInterval4 = 12500;
const unsigned long eventInterval5 = 13000;
const unsigned long eventInterval6 = 13500;
const unsigned long eventInterval7 = 14000;
unsigned long previousTime = 0;
unsigned long currentTime = 0;


void setup() {
  Serial.begin(9600);
  pinMode(redPin1, OUTPUT);
  pinMode(redPin2, OUTPUT);
  pinMode(dryDock1, OUTPUT);
  pinMode(dryDock2, OUTPUT);
  pinMode(dryDock3, OUTPUT);
  pinMode(dryDock4, OUTPUT);
  pinMode(dryDock5, OUTPUT);
  pinMode(dryDock6, OUTPUT);
  pinMode(dryDock7, OUTPUT);
  pinMode(relayPin, OUTPUT);
  pinMode(redSwitch, INPUT);
}

void loop() {

    redState = digitalRead(redSwitch);  // check if button is pushed
    if (redState == HIGH)
  currentTime =- millis();
  if (currentTime <= redLength)
  {
    digitalWrite(redPin1, HIGH);
    digitalWrite(redPin2, HIGH);
    digitalWrite(relayPin, HIGH);
    digitalWrite(dryDock1, LOW);
    digitalWrite(dryDock2, LOW);
    digitalWrite(dryDock3, LOW);
    digitalWrite(dryDock4, LOW);
    digitalWrite(dryDock5, LOW);
    digitalWrite(dryDock6, LOW);
    digitalWrite(dryDock7, LOW);
  }


  if (currentTime = millis())
    //    previousTime = currentTime;
    /* Updates frequently */
    digitalWrite(redPin1, LOW);
  digitalWrite(redPin2, LOW);
  digitalWrite(relayPin, LOW);

  if (currentTime >= eventInterval1)
  {
    Serial.println("Ice Ice Baby1");
    { digitalWrite(redPin1, LOW);
      digitalWrite(redPin2, LOW);
      digitalWrite(relayPin, LOW);
      digitalWrite(dryDock1, HIGH);
    }
    /* Update the timing for the next time around */
    //  previousTime = currentTime;
  }
  /* This is the event */
  if (currentTime >= eventInterval2) {
    /* Event code */
    Serial.println("Ice Ice Baby2");

    //digitalWrite(dryDock1, HIGH);
    digitalWrite(dryDock2, HIGH);
    digitalWrite(redPin1, LOW);
    digitalWrite(redPin2, LOW);
    digitalWrite(relayPin, LOW);
  }

  /* Update the timing for the next time around */
  //  previousTime = currentTime;
  /* This is the event */
  if (currentTime >= eventInterval3) {
    /* Event code */
    Serial.println("Ice Ice Baby3");
    digitalWrite(dryDock3, HIGH);
    digitalWrite(redPin1, LOW);
    digitalWrite(redPin2, LOW);
    digitalWrite(relayPin, LOW);
  }
  /* Update the timing for the next time around */
  // previousTime = currentTime;
  /* This is the event */
  if (currentTime >= eventInterval4) {
    /* Event code */
    Serial.println("Ice Ice Baby4");
    digitalWrite(dryDock4, HIGH);
    digitalWrite(redPin1, LOW);
    digitalWrite(redPin2, LOW);
    digitalWrite(relayPin, LOW);
  }
  /* Update the timing for the next time around */
  //  previousTime = currentTime;
  /* This is the event */
  if (currentTime  >= eventInterval5) {
    /* Event code */
    Serial.println("Ice Ice Baby5");
    digitalWrite(dryDock5, HIGH);
    digitalWrite(redPin1, LOW);
    digitalWrite(redPin2, LOW);
    digitalWrite(relayPin, LOW);
  }
  /* Update the timing for the next time around */
  //   previousTime = currentTime;

  if (currentTime  >= eventInterval6) {
    /* Event code */
    Serial.println("Ice Ice Baby6");
    digitalWrite(dryDock6, HIGH);
    digitalWrite(redPin1, LOW);
    digitalWrite(redPin2, LOW);
    digitalWrite(relayPin, LOW);
  }
  /* Update the timing for the next time around */
  //   previousTime = currentTime;

  if (currentTime  >= eventInterval7) {
    /* Event code */
    Serial.println("Ice Ice Baby7");
    digitalWrite(dryDock7, HIGH);
    digitalWrite(redPin1, LOW);
    digitalWrite(redPin2, LOW);
    digitalWrite(relayPin, LOW);
    Serial.println("end");
  //  for (;;) {}
  } else
  { }
  //

}

This results in no button control. it goes by itself.
If I comment out the line

suggested, the switch works, but only as long as I hold it down. And never again.

Do you know what the difference is in these two statements ?

Do something when a switch becomes HIGH.

Do something when a switch is HIGH.

You still have this:

if (currentTime = millis())         ←

Just wondering…

Why use eternal pull downs, when internal pull ups are available?

-jim lee

It’s just the way I learned. Besides the components, is there an advantage?