Program won't exit function

I'm working on this small project for a to do list. It's similar to that of Mike Boyd's if you've seen it. I'm just testing something with some pushbuttons.

I have 9 RGB LED's, with a WS2812, connected together for some animations.
4 pushbuttons are used as toggle switches for the first 4 LEDs. When I press the first button the first led turns green, if pressed again it is red, same goes for the next 3 LEDs.

When all the LEDs are green I want to play a little animation I made of a 3 LED's lighting up, kinda like a block moving across the LEDs. I only want this to happen ONCE, there and back and that's it.

For some reason when the program enters the function for the animation it just never stops. I thought returning at the end would stop it but it doesn't.

Here is the code:

#include <Arduino.h>
#include <FastLED.h>

#define LED_PIN 7
#define NUM_LEDS 9
#define DELAY 50
#define BLOCK 3

int blockCounter = 0;

int blockAnimation(int blockSize, int delayTime);

CRGB leds[NUM_LEDS];

int buttonPins[4] = {2, 3, 4, 5};
int buttonVals[4];
int buttonOlds[4] = {1};
bool ledsOn[4] = {false};
bool allDone;
int buttonVal;
int buttonOld = 1;
bool ledOn = true;

void setup() {
  // put your setup code here, to run once:
  FastLED.addLeds<WS2812, LED_PIN, RGB>(leds, NUM_LEDS);
  for(int i = 0; i < 4; i++) {
    pinMode(buttonPins[i], INPUT);
    digitalWrite(buttonPins[i], HIGH);
  }
  
  Serial.begin(9600);
}

void loop() {
  //blockAnimation(BLOCK, DELAY);
  for(int i = 0; i < 4; i++)
    buttonVals[i] = digitalRead(buttonPins[i]);

  for(int i = 0; i < 4; i++) {
    if(buttonVals[i] == 0 && buttonOlds[i] == 1) {
      if(!ledsOn[i]) {
        ledsOn[i] = true;
        leds[i] = CRGB::Green;
      } else {
        ledsOn[i] = false;
        leds[i] = CRGB::Red;
      }
    }

    buttonOlds[i] = buttonVals[i];
    FastLED.show();
  }

  
  for(int i = 0; i < 4; i++) {
    if(ledsOn[i])
      allDone = true;
    else
      allDone = false;
  }
  
  Serial.println(blockCounter);
  //Figure out how to make it short
  if(allDone) {
    blockAnimation(BLOCK, DELAY);
    allDone = false;
  }
  
  
  
}

int blockAnimation(int blockSize, int delayTime) {
    // Forwards block moves of size BLOCK
  for(int i = BLOCK-2; i < NUM_LEDS; i++) {
    for(int j = 0; j < BLOCK; j++){
      leds[i-j] = CRGB::Magenta;
      FastLED.show();
    }
    delay(DELAY);
    leds[i-(BLOCK-1)] = CRGB::Black;
    FastLED.show();
  }
  delay(DELAY);
  leds[NUM_LEDS - 2] = CRGB::Black;
  FastLED.show();
  delay(DELAY);
  
  //Backwards movement of block size BLOCK
  for(int i = NUM_LEDS - (BLOCK-1); i >= 0; i--) {
    for(int j = BLOCK - 1; j >= 0; j--){
      leds[i+j] = CRGB::Magenta;
      FastLED.show();
    }
    delay(DELAY);
    leds[i+(BLOCK-1)] = CRGB::Black;
    FastLED.show();
  }
  delay(DELAY);
  leds[1] = CRGB::Black;
  FastLED.show();
  delay(DELAY);
  return 0;
}

This does the same thing as:

    if(ledsOn[3])
      allDone = true;
    else
      allDone = false;

I doubt that is what you want it to do. It's not commented so I have no idea what that is. As far as the non-returning function, I suspect you have written outside of array bounds with your indexing expressions. Analyze them to make sure.

You can do that by substituting print statements of the variables instead of the colour assignments.

1 Like

For debugging I wrote two macros that shorten the code you have to type to get detailed serial output

especially the dbgi-macro offers slow output in loops that are running at high speed.
Inserting such debug-output will guide you to that place where your function stcucks

//#define debugTxtVar(abc, def) Serial.print("<  "#abc" = "); Serial.print(def); Serial.println('>')

// using the #define "statement" to make the compiler "write" code
// #define can be used to make the compiler replace text that describes something with (almost) anything else
// easy example 
// let's assume IO-pin number 4 shall be set to work as output 
// the command for defining an IO-pin to be configured as output is
//
// pinMode(4,OUTPUT); 
// the number "4" says nothing about he purpose of this IO-pin

// let's assume the IO-pin shall switch On/Off a buzzer

// you would have to add a comment to explain
// pinMode(4,OUTPUT); // IO-pin 4 is buzzer

// the compiler needs the number. To make the code easier to read and understand
// #define BuzzerPin 4
// So the command looks like this
// pinMode(BuzzerPin,OUTPUT);

// the descriptive word "BuzzerPin" gets replaced through the compiler by a "4"
// so what the compiler compiles is still 'pinMode(4,OUTPUT);'

// the #define can do much more than just replace a word by a single number
// it can even take parameters like in this example


#define dbg(myFixedText, variableName) \
  Serial.print( F(#myFixedText " "  #variableName"=") ); \
  Serial.println(variableName);
// usage: dbg("1:my fixed text",myVariable);
// myVariable can be any variable or expression that is defined in scope

#define dbgi(myFixedText, variableName,timeInterval) \
  do { \
    static unsigned long intervalStartTime; \
    if ( millis() - intervalStartTime >= timeInterval ){ \
      intervalStartTime = millis(); \
      Serial.print( F(#myFixedText " "  #variableName"=") ); \
      Serial.println(variableName); \
    } \
  } while (false);
// usage: dbgi("2:my fixed text",myVar,myInterval);
// myVar can be any variable or expression that is defined in scope
// myInterval is the time-interval which must pass by before the next
// print is executed

If you have questions about how to use it. Just post the questions.
I'm interested in improving the documentation and explanation how it works
based on the real questions others have

best regards Stefan

while the conditions for setting "allDone" may not be correct, i believe the problem is despite allDone being set to true after blockAnimation() is called, the conditions for setting allDone have not changed

in other words, do you need additional logic to only call blockAnimation() once?

with allDone initialized to true

allDone &= ledsOn [i];

so that allDone is set false if any led is not true

I recommend using the boolean AND in this case, not the bitwise AND:
allDone &&= ledsOn [i];

Yep, I changed my logic and that fixed it. Thanks for the help!