Push button input not working with LEDs as intended.

Using Arduino Programming Questions
1

I'm trying to write a program that starts an led sequence (using a 74hc595n shift register) as soon as the Arduino is powered on and during that led sequence if you push the button I want the led sequence to stop completely. Instead of stopping though, I get a strange outcome where one led will flash on the 2nd led (Binary 0100 0000)and then the 7th (Binary 0000 0010) back and forth for the entire time that you're pressing the button. Once the button is released the sequence goes back to normal. Any reason for this?

Here is a link to the video of my issue on Youtube:

Here is the code:

//Constant Global variables
const int dataPin = 5;
const int latchPin = 6;
const int clockPin = 7;

//NON-Constant Global variables
bool seq_running = true;
bool pwr_on;

//Single LED attached to pin 12.
int led = 12;

//Button Pin attached to pin 2.
int buttonPin = 2;

//Reads buttonPin state (either HIGH or LOW).
int buttonState;
int buttonPrev = 1;

//Use for delay
int d = 100;


//variables for making clear whether the button is pressed or not.
byte pressed = 0;
byte notpressed = 1;

//SETUP FUNCTION
void setup()
{
  Serial.begin(9600);
  pinMode(dataPin, OUTPUT);
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
  
  pinMode(led, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP); 
}

//LOOP FUNCTION
void loop()
{  
  buttonState = digitalRead(buttonPin);
  byte x = B10000000;
  byte shutoff = B00000000;

  //Check if the Arduino is powered on and running.
  if(Serial)
  {
    pwr_on = true;
      if(pwr_on == true)
      {
        for(byte i = 0; i <= 6; i++)
        {
          x = x >> 1;
          shiftDataOut(x);
          delay(d);
          changeBtnMsg();
          if(buttonPrev == notpressed && buttonState == pressed)
          {
            if(seq_running == true)
            {
              x = shutoff;
              shiftDataOut(x);
              Serial.println("Sequence has been terminated.");
            }
          }
        }

      x = B00000001;
  
      for(int i = 7; i >= 1; i--)
      {
        x = x << 1;
        shiftDataOut(x);
        delay(d);
        changeBtnMsg();
        if(buttonPrev == notpressed && buttonState == pressed)
        {
            if(seq_running == true)
            {
              x = shutoff;
              shiftDataOut(x);
              Serial.println("Sequence has been terminated.");
            }
        }
      }
     }
   }
   else
   { 
     pwr_on = false;
   }
}



//Function to easier handle shift register data shifting to output pins. 
void shiftDataOut(byte data)
{
  digitalWrite(latchPin, LOW);
  shiftOut(dataPin, clockPin, LSBFIRST, data);
  digitalWrite(latchPin, HIGH);
}





//Custom function for changing what the serial monitor says about the button.
//Makes it easier to understand the button's logic.
void changeBtnMsg()
{
  if(buttonState == pressed)
  {
    Serial.println("Button Pressed(HIGH)");
  }
  else
  {
    Serial.println("Button NOT Pressed(LOW)");
  }
}
2

this does not check if your arduino is powered and running...

  //Check if the Arduino is powered on and running.
  if (Serial)

if your arduino is not powered, you can be sure it won't be running anyway.... :slight_smile:
so you can get rid of all this stuff

3

Oh ok thank you for letting me know that. Would that have anything to do with why my program isn't working the way it's supposed to?

4

I don't think so, just useless stuff coming in the way of understanding what you do.

I would go with a simpler code structure, something like this:

const byte  buttonPin = 2;
const uint32_t animationTiming = 100;

void takeOneStepIfNeeded(bool forceStep = false)
{
  static uint32_t lastStep;
  if (forceStep || (millis() - lastStep >= animationTiming)) {
    lastStep = millis();
    // ***********************
    // HERE LIGHT UP NEXT LED
    // TO DO ...
    // ***********************
  }
}

void ledsOff()
{
  // ***********************
  // HERE TURNS ALL LEDs OFF
  // TO DO ...
  // ***********************
}

void setup()
{
  pinMode(buttonPin, INPUT_PULLUP);
  takeOneStepIfNeeded(true); // force turning ON the first LED to get going
}

void loop()
{
  if (digitalRead(buttonPin) != LOW) takeOneStepIfNeeded(); // take a step if button not pressed
  else {
    ledsOff(); // otherwise keep LEDs OFF
  }
}

(typed here so untested)

you need to write the code that light up the next LED in the function takeOneStepIfNeeded() and code that turns all LEDs off in ledsOff()

5

void takeOneStepIfNeeded(bool forceStep = false)
{
static uint32_t lastStep;
if (forceStep || (millis() - lastStep >= animationTiming)) {
lastStep = millis();
// ***********************
// HERE LIGHT UP NEXT LED
// TO DO ...
// ***********************
}
}

Could you explain the code for me? I'm having a hard time understanding. if you declare lastStep without a value, doesn't that default it to 0? Also, why do you assign lastStep to millis() inside the if statement and not before? I'm just trying to figure this out. I'm new and still learning Arduino. Thanks for the responses btw!

6

Could you explain the code for me? I'm having a hard time understanding. if you declare lastStep without a value, doesn't that default it to 0?

this is a function so local variable would be transient (go away at the end of the function call) but in this case I declared lastStep as static which basically make it sticky and initialized to 0. So the first time I call the function from setup() I use 'true' as parameter so I force taking one step and light up a LED which also initialize lastStep to the value of millis().

Next time I call this function in the loop, I don't use true as parameter, I have no parameter at all meaning the function uses its default value which is false and so I enter the if only if animationTiming milliseconds have elapsed since last time I took a step.

7