My brain hurts. Please help

I'm using an esp32-wroom board. I'm using platformIO in VS Code to program this. The project I'm attempting is this...

I have push button switch wired up the the controller using a pullup resistor wired to +3.3V and the other end going to ground. My ultimate goal is to have multiple switches that play a given animation when the button is pressed and released. I want to animation to continue until I press the button again at which point I want the display to clear. With my current code (posted below) when I press t he button the animation plays 1 frame then stops. Another thing I'd like to have is the ability to set how fast the animation plays via code. I have been around in circles with this code 100 different ways from Sunday and I just can't figure out what I'm doing wrong. Would someone be kind enough to help me out here and maybe give me some clues where I'm going wrong? Also if I'm doing something totally dumb in my code I'd love some direction to get it right.

#include <MD_MAX72xx.h>
#include <SPI.h>
#include <Arduino.h>
#include "animations.h"

// Define the number of devices we have in the chain and the hardware interface
#define MAX_DEVICES 4
#define HARDWARE_TYPE MD_MAX72XX::FC16_HW

#define CLK_PIN   18  // or SCK
#define DATA_PIN  23  // or MOSI
#define CS_PIN    5  // or SS

// Create a new instance of the MD_MAX72XX library
MD_MAX72XX mx = MD_MAX72XX(HARDWARE_TYPE, DATA_PIN, CLK_PIN, CS_PIN, MAX_DEVICES);

unsigned long previousMillis = 0;
const long interval = 75;
const int button1Pin = 2;
const int button2Pin = 4;
int currentFrame = 0;

void playAnimation1(); // Forward declaration
void buttons();       // Forward declaration

void setup() {
  mx.begin();
  mx.control(MD_MAX72XX::INTENSITY, 3); // Set intensity level (0-15)
  mx.clear();
  pinMode(button1Pin, INPUT_PULLUP);
  }

void loop() {
  buttons();
}

void playAnimation1() {
  // Ensure the number of columns does not exceed the array bounds
  int numColumns = sizeof(animation_03[0]) / sizeof(animation_03[0][0]);
  if (numColumns > 32) {
    numColumns = 32; // Limit to 32 columns if the array has more columns
  }
  for (int i = 0; i < numColumns; i++) {
    mx.setColumn(i, animation_03[currentFrame][i]);
  }
  currentFrame++;
  if (currentFrame >= sizeof(animation_03) / sizeof(animation_03[0])) {
    currentFrame = 0;
  }
}

  void buttons() {
  #define button1Pressed LOW // Button wired with pullup resistor
  uint32_t currentMillis = millis();
  static uint32_t lastMillis;
  const uint32_t debounceTime = 20;
  bool currentButton1State = digitalRead(button1Pin);
  static bool lastButton1State;
  bool animation1Playing = false;

  if (lastButton1State != currentButton1State) {
    if (currentMillis - lastMillis >= debounceTime) {
      lastButton1State = currentButton1State;
      if (currentButton1State == button1Pressed) {
        animation1Playing = true;
        playAnimation1();
        }
      }
    } else {
      animation1Playing = false;
      lastMillis = currentMillis;
    }
}

Where did you get this code? It looks fairly sophisticated but you ask a simple question.

What currently is determining the frame rate that you want eventually to adjust?

Try this:

Move the bool animation1Playing so it is a global variable.

Remove the call to playAnimation1() in the button code.

In the loop, if animation1Playing is true, call playAnimation1() to show the next frame. Time that with a millis() based mechanism like you see in the Blink Without Delay example in the IDE.

I cannot test this, but it looks like playAnimation1() will do the next frame, and repeat when it gets to the end.

a7

Hi alto.

This code is the result of things I've been reading and testing I'm doing. I'm not 100% newb to C++ but rather like only 99.7% newb to it :smiley:

Currently nothing is controlling my framerate. There's a global variable "interval" still in this code that I was trying to make work here. I forgot to delete that variable when my experiment failed.

I moved the animation1Playing bool to global and changed my loop like this...

void loop() {
  buttons();
  if(animation1Playing == true) {
    playAnimation1();
  } else {
    
  }
}

It's still behaving in the same manor of playing 1 frame per button push. It seems as if my animation1Playing flag is getting set to true when I press the button then resetting itself to false before the function completes.

Lastly do you think I'm incorrectly using millis() in my buttons function? I'm just curious since you referred me to an example file.

Hi Delta_G.

I'm sure there's a good reason, but I can't figure it out. Why shouldn't I have the #define button1Pressed LOW inside my buttons() function? If I move it out it just gives me a error. When I put in LOW at the end with it outside of the function the editor is recommending I use "LONG_WIDTH".

Also I moved the animation1Playing variable to global and I get the same behavior.

The buttons() function isn't playing the animation. It's reading the state of buttons and setting variables. The playAnimation1() function is doing the playing... well it should if I could figure out what's going wrong at least :slight_smile:

That was my bad about an error with the #define statement. I put it outside of the function and it's fine. Here is my updated code. Also I see your point about using the buttons function set the animation1Playing bool to true...or false. Is it really a big deal though? I just picked the name "buttons()" out of my backside. I could have well named the function frogs(). Since that function is looking to see if a button is pressed and an animation should happen when it is I just though it logical to call the animation function from there. Anywho... Here's the code. Still playing one frame every time I press the button :frowning:

#include <MD_MAX72xx.h>
#include <SPI.h>
#include <Arduino.h>
#include "animations.h"

// Define the number of devices we have in the chain and the hardware interface
#define MAX_DEVICES 4
#define HARDWARE_TYPE MD_MAX72XX::FC16_HW

#define CLK_PIN   18  // or SCK
#define DATA_PIN  23  // or MOSI
#define CS_PIN    5  // or SS

// Create a new instance of the MD_MAX72XX library
MD_MAX72XX mx = MD_MAX72XX(HARDWARE_TYPE, DATA_PIN, CLK_PIN, CS_PIN, MAX_DEVICES);

const long interval = 75;
const int button1Pin = 2;
const int button2Pin = 4;
int currentFrame = 0;
bool animation1Playing = false;

void playAnimation1(); // Forward declaration
void buttons();       // Forward declaration

void setup() {
  Serial.begin(115200);
  mx.begin();
  mx.control(MD_MAX72XX::INTENSITY, 3); // Set intensity level (0-15)
  mx.clear();
  pinMode(button1Pin, INPUT_PULLUP);
  }

void loop() {
  buttons();
  if(animation1Playing == true) {
    playAnimation1();
  } else {
    animation1Playing = false;
  }
}

void playAnimation1() {
  // Ensure the number of columns does not exceed the array bounds
  int numColumns = sizeof(animation_03[0]) / sizeof(animation_03[0][0]);
  if (numColumns > 32) {
    numColumns = 32; // Limit to 32 columns if the array has more columns
  }
  for (int i = 0; i < numColumns; i++) {
    mx.setColumn(i, animation_03[currentFrame][i]);
  }
  currentFrame++;
  if (currentFrame >= sizeof(animation_03) / sizeof(animation_03[0])) {
    currentFrame = 0;
  }
}
  #define button1Pressed LOW

  void buttons() {
  #define button1Pressed LOW // Button wired with pullup resistor
  uint32_t currentMillis = millis();
  static uint32_t lastMillis;
  const uint32_t debounceTime = 20;
  bool currentButton1State = digitalRead(button1Pin);
  static bool lastButton1State;
  
  if (lastButton1State != currentButton1State) {
    if (currentMillis - lastMillis >= debounceTime) {
      lastButton1State = currentButton1State;
      if (currentButton1State == button1Pressed) {
        animation1Playing = true;
        }
      }
    } else {
      animation1Playing = false;
      lastMillis = currentMillis;
    }
}

I forgot to remove the #define from buttons() before I copied. It's gone from inside the function now though

I'm defiantly soaking up your input here. I totally get your point about making the functions logical in what they do. I want to get this code working how it's arranged now then see about changing it up to include your suggestions in regards to the button function. I just feel if I were to try that now I'd never find what is breaking my current situation.

So I did a few changes and it's defiantly a step forward. Now when I press and hold the button the animation plays properly (well sorta, more on that in a second). When I release the button the animation stops and holds the current frame on the display.

So I tried putting in some code to allow me to set the framerate of the animation. I tried using the currentMillis - lastMillis and comparing it with the interval time, but that had no effect on the framerate. I can set interval to 1000000 and the animation just zooms through as fast as it can. Here's my last code if you wouldn't mind taking a new peek :slight_smile:

#include <MD_MAX72xx.h>
#include <SPI.h>
#include <Arduino.h>
#include "animations.h"

// Define the number of devices we have in the chain and the hardware interface
#define MAX_DEVICES 4
#define HARDWARE_TYPE MD_MAX72XX::FC16_HW

#define CLK_PIN   18  // or SCK
#define DATA_PIN  23  // or MOSI
#define CS_PIN    5  // or SS

// Create a new instance of the MD_MAX72XX library
MD_MAX72XX mx = MD_MAX72XX(HARDWARE_TYPE, DATA_PIN, CLK_PIN, CS_PIN, MAX_DEVICES);

const int button1Pin = 2;
const int button2Pin = 4;
int currentFrame = 0;
bool animation1Playing = false;

int interval = 1000;

void playAnimation1(); // Forward declaration
void buttons();       // Forward declaration

void setup() {
  mx.begin();
  mx.control(MD_MAX72XX::INTENSITY, 3); // Set intensity level (0-15)
  mx.clear();
  pinMode(button1Pin, INPUT_PULLUP);
  }

void loop() {
    buttons();
    if(animation1Playing == true) {
    playAnimation1();
  } else {
    animation1Playing = false;
  }
}

void playAnimation1() {
  unsigned long currentMillis = millis();
  unsigned long lastMillis = 0;
  
  int numColumns = sizeof(animation_03[0]) / sizeof(animation_03[0][0]);
  if (numColumns > 32) {
    numColumns = 32; // Limit to 32 columns if the array has more columns
  }
  for (int i = 0; i < numColumns; i++) {
    mx.setColumn(i, animation_03[currentFrame][i]);
  }
  if (currentMillis - lastMillis >= interval){
  currentFrame++;
  if (currentFrame >= sizeof(animation_03) / sizeof(animation_03[0])) {
    currentFrame = 0;
    }
  }
}

  #define button1Pressed LOW

  void buttons() {
  unsigned long currentMillis = millis();
  unsigned long lastMillis = 0;
  const uint32_t debounceTime = 20;
  bool currentButton1State = digitalRead(button1Pin);
  bool lastButton1State = HIGH;
  
  if (lastButton1State != currentButton1State) {
    if (currentMillis - lastMillis >= debounceTime) {
      lastButton1State = currentButton1State;
      if (currentButton1State == button1Pressed) {
        animation1Playing = true;
        }
      }
    } else {
      animation1Playing = false;
      lastMillis = currentMillis;
    }
}

I

This does not make any sense with the resistor connected to 3V3 and ground.

Other end of the switch. The resistor goes from 1 post of switch to +3.3v and to pin2. The other end of the switch goes to ground. That's a pullup configuration. When you press the button pin2 gets 0v when it's unpressed pin2 gets 3.3v

Like

void playAnimation1() {
  static unsigned long currentMillis;

  currentMillis = millis();
  unsigned long lastMillis = 0;

But. Since you will be ending up with a good free running loop, a common habit is to have a global variable
unsinged long currentMillis;

and as the very first thing you do in your loop() function say

void loop() {

  currentMillis = millis();

which can literally be the only call to millis() in the entire sketch.

Everyone will be using the same value of the system timer which is millis(), it keeps all the code on the same page.

That value can be used in your button function, which is now the area to fix up so it turns on or off the animation with each button press.

I use now as that variable's name, currentMillis always struck me as unfriendly and pretentious. But any name would work, pick your own.

Sry I did not look close at the button thing; at the glance I gave, it seemed plausible.

a7

Allright. I'm completely confused with the whole millis thing. What Delta said makes sense. I was just going back to 0 every time because I wasn't setting lastMillis to currentMillis.

Where I'm confused now though. So I'm trying to use the millis timer in 2 functions. Do I need to declare currentMillis = millis(); in both functions? When I tried doing that in a global manor it just didn't work and gave an error. Also I'm not sure how to properly use it in my functions or where to properly place it. Also when I include the line currentMillis = millis(); as the first line in my loop it's a big fat error saying not declared.

Here's what I got. I'm somewhat positive I'm misusing the millis throughout my program and that's the root of my whole problem. With this current code when I press the button the animation appears and seems to advance 1 frame then stops. Additional button presses have no effect. I'm really starting to feel overly stupid here. I'm sure it's something obvious staring me in the face and I'm just not seeing it.

p.s @Delta_G We all better hope that autocorrect never starts training AI models hahaha

#include <MD_MAX72xx.h>
#include <SPI.h>
#include <Arduino.h>
#include "animations.h"

// Define the number of devices we have in the chain and the hardware interface
#define MAX_DEVICES 4
#define HARDWARE_TYPE MD_MAX72XX::FC16_HW

#define CLK_PIN   18  // or SCK
#define DATA_PIN  23  // or MOSI
#define CS_PIN    5  // or SS

// Create a new instance of the MD_MAX72XX library
MD_MAX72XX mx = MD_MAX72XX(HARDWARE_TYPE, DATA_PIN, CLK_PIN, CS_PIN, MAX_DEVICES);

const int button1Pin = 2;
const int button2Pin = 4;

int currentFrame = 0;
bool animation1Playing = false;

unsigned long now = 0;
unsigned long lastMillis = 0;
int frameRate = 100;

void playAnimation1(); // Forward declaration
void buttons();       // Forward declaration

void setup() {
  mx.begin();
  mx.control(MD_MAX72XX::INTENSITY, 3); // Set intensity level (0-15)
  mx.clear();
  pinMode(button1Pin, INPUT_PULLUP);
  }

void loop() {
  now = millis();
    buttons();
    if(animation1Playing == true) {
    playAnimation1();
  } else {
    animation1Playing = false;
  }
}

void playAnimation1() {
  //unsigned long now = millis();
  // unsigned long lastMillis = 0;
  
  int numColumns = sizeof(animation_03[0]) / sizeof(animation_03[0][0]);
  if (numColumns > 32) {
    numColumns = 32; // Limit to 32 columns if the array has more columns
  }
  for (int i = 0; i < numColumns; i++) {
    mx.setColumn(i, animation_03[currentFrame][i]);
  }
  if (now - lastMillis >= frameRate){
  lastMillis = now;
  currentFrame++;
  if (currentFrame >= sizeof(animation_03) / sizeof(animation_03[0])) {
    currentFrame = 0;
    }
  }
}

  #define button1Pressed LOW

  void buttons() {
  //unsigned long now = millis();
  // unsigned long lastMillis = 0;
  const int debounceTime = 20;
  bool currentButton1State = digitalRead(button1Pin);
  bool lastButton1State = HIGH;
  
  if (lastButton1State != currentButton1State) {
    if (now - lastMillis >= debounceTime) {
      lastMillis = now;
      lastButton1State = currentButton1State;
      if (currentButton1State == button1Pressed) {
        animation1Playing = true;
        }
      }
    } else {
      animation1Playing = false;
      currentButton1State == lastButton1State;
    }
}

OK I included static unsigned long lastMillis = 0; in both my buttons() and playAnimation1() functions. Now I'm back to the animation playing while the button is pressed and held. On a positive note I'm now able to adjust my global int frameRate = 50; variable and adjust how fast the animation plays. Thank God for small victories :smiley:

1 Like

I'm about to change gears, but what you need to do instead is called state change detection, so that the relevant variable is toggled (turned true if false and vice versa) when the button is pressed (goes from up to down).

Nothing happens to the variable while the button is being held, and nothing happens when the button is released (goes from down to up).

There's an example of the pattern in the IDE sketch 02. Digital / StateChangeDetection.

Post the code that almost works except for this last detail, it is probably a realtively small change but with big difference.

a7

@alto777

I looked over the state change example. It looks like a pretty simple concept. Looking through my code though, it looks like I am basically doing the same thing. For some reason though my program doesn't seem to agree with my state changes :confused: Anyway here's my most current code....

#include <MD_MAX72xx.h>
#include <SPI.h>
#include <Arduino.h>
#include "animations.h"

// Define the number of devices we have in the chain and the hardware interface
#define MAX_DEVICES 4
#define HARDWARE_TYPE MD_MAX72XX::FC16_HW

#define CLK_PIN   18  // or SCK
#define DATA_PIN  23  // or MOSI
#define CS_PIN    5  // or SS

// Create a new instance of the MD_MAX72XX library
MD_MAX72XX mx = MD_MAX72XX(HARDWARE_TYPE, DATA_PIN, CLK_PIN, CS_PIN, MAX_DEVICES);

const int button1Pin = 2;
const int button2Pin = 4;

int currentFrame = 0;
bool animation1Playing = false;
int frameRate = 50;
unsigned long now = 0;

void playAnimation1(); // Forward declaration
void buttons();       // Forward declaration

void setup() {
  Serial.begin(115200); // TESTING
  mx.begin();
  mx.control(MD_MAX72XX::INTENSITY, 3); // Set intensity level (0-15)
  mx.clear();
  pinMode(button1Pin, INPUT_PULLUP);
  }

void loop() {
  now = millis();
    buttons();
    if(animation1Playing == true) {
    playAnimation1();
  } else {
    animation1Playing = false;
  }
}

void playAnimation1() {
  static unsigned long lastMillis = 0;
  
  int numColumns = sizeof(animation_01[0]) / sizeof(animation_01[0][0]);
  if (numColumns > 32) {
    numColumns = 32; // Limit to 32 columns if the array has more columns
  }
  for (int i = 0; i < numColumns; i++) {
    mx.setColumn(i, animation_01[currentFrame][i]);
  }
  if (now - lastMillis >= frameRate){
  lastMillis = now;
  currentFrame++;
  if (currentFrame >= sizeof(animation_01) / sizeof(animation_01[0])) {
    currentFrame = 0;
    }
  }
}

  #define button1Pressed LOW

  void buttons() {
  static unsigned long lastMillis = 0;
  const int debounceTime = 20;
  bool currentButton1State = digitalRead(button1Pin);
  bool lastButton1State = HIGH;
  
  if (lastButton1State != currentButton1State) {
    if (now - lastMillis >= debounceTime) {
      lastMillis = now;
      lastButton1State = currentButton1State;
      if (currentButton1State == button1Pressed) {
        animation1Playing = true;
        }
      }
    } else {
      animation1Playing = false;
      currentButton1State == lastButton1State;
    }
}

Defiantly not. Look close. There's at least one obvious error, and I am too tired to confirm the structure looks familiar but isn't correct.

a7

I'm using platformIO on VS Code. Control+T doesn't give me anything to format code. This is my current buttons() function. It looks exactly like the example you have in terms of where the braces are.

void buttons() {
  static unsigned long lastMillis = 0;
  const int debounceTime = 20;
  bool currentButton1State = digitalRead(button1Pin);
  bool lastButton1State = HIGH;

  
  if (lastButton1State != currentButton1State) {
    if (now - lastMillis >= debounceTime) {
      lastMillis = now;
      lastButton1State = currentButton1State;
      if (currentButton1State == button1Pressed) {
        animation1Playing = true;
        }
      }
    } else {
      animation1Playing = false;
      currentButton1State = lastButton1State;
    }
}

I figured out auto formatting in VS Code. Also I moved the else statement up the chain one set of brackets at a time. It didn't work properly anywhere.

At this point I'm thinking this code is just un-fixable. It'd probably be best if I start from scratch doing one function at a time. With this I've been bouncing between all the functions breaking stuff and trying to fix it and it's just confusing me more. I'll take your advice only having things dealing with button actions in the buttons() function. I think I have a better grasp now where I should be using global vs local variables also.

I was sitting here with pen and paper writing the steps down. It dawned on me where I went off course with the braces right before you posted that. You're right those braces can really gum up the works. ugg.

I feel like I've learned quite a bit in this thread and I thank you and @alto777 for that. I do think I'm going to re-write this sketch though and be more 'logical' about my functions and their actions. You guys sure are patient! :slight_smile:

1 Like

You haven't read far or wide or deep. We can and do run out of patience. :expressionless:

But as long as there is effort and progress and we see listening and heeding, it lasts.

I took @Delta_G's code and changed it a bit to suit my own habits.

The major difference is the debounce timing. I move that to the first thing the function checks. "is it too soon to even look at the (possibly bouncing) button again?".

Try it here:

Wokwi_badge UA Single Button Reference Sketch


// https://wokwi.com/projects/402305903980081153
// https://forum.arduino.cc/t/my-brain-hurts-please-help/1277379

bool animation1Playing;
uint32_t currentMillis;

const int button1Pin = 6;
const int theLED = 7;

void setup() {
  pinMode(button1Pin, INPUT_PULLUP);
  pinMode(theLED, OUTPUT);
}

void loop() {
  currentMillis = millis();

  buttons();

  if (animation1Playing) digitalWrite(theLED, HIGH);

  if (!animation1Playing) digitalWrite(theLED, LOW);
}


# define PRESST LOW  // for switches puuled up and wired to ground

void buttons() {
  static uint32_t lastMillis;
  const uint32_t debounceTime = 20;
  static bool lastButton1State;

// too soon to even look?
  if (currentMillis - lastMillis < debounceTime) return;

  bool currentButton1State = digitalRead(button1Pin) == PRESST;  // true is pressed

  if (lastButton1State != currentButton1State) {

    if (currentButton1State) {
      animation1Playing = !animation1Playing;
    }

    lastButton1State = currentButton1State;
    lastMillis = currentMillis;   // so we don't even look again until
  } 
}

In @Delta_G's code, this edge handling got changed, I believe it is flawed and will follow the button state, not the button change:

      if (currentButton1State == button1Pressed) {
        animation1Playing = true;                      
      } else {
        animation1Playing = false;
        lastMillis = currentMillis;
      }

I think lastMillis needs to be advanced when animation1Playing is set true as well as when set false - debounce both edges.

Compare to this, which toggles on the rising edge and ignores the falling edge:

  if (lastButton1State != currentButton1State) {

    if (currentButton1State) {
      animation1Playing = !animation1Playing;
    }

    lastButton1State = currentButton1State;
    lastMillis = currentMillis;   // so we don't even look again until
  } 

I will say I had mucho trouble getting this right, feeling my inner Joe Biden I guess, and may well not have yet. It is a bitch, for something that has rolled off my fingers too many times. I think I get in trouble working from other ppl's code - the pattern can look correct, at a glance, and familiar, but if all the parts aren't just right, you sunk.

Having said all we have, and while I think it is important to code this kind of thing for one's self at least once, and I can't believe I am caving, but...

The ezButton library doesn't suck and is, in fact, EZ to use. I'll post the wokwi simulation I have that shows the least one needs to know about it, and say aloud there is no shame in using it, and spending the energy on the real problems.

a7

Here is ezButton minimal sketch the Universe let me find in the mess which is my filing system.

ezButton does have one odd flaw - it really doesn't like it if you do not call its service method loop(), which serves more or less the same purpose as the frequent calls to buttons() in the code we've been working here. But that should not be an issue in a free running no delay no blocking sketch.

Wire a button between pin 3 and ground, try it. The si mulation I found was larded up with things proving my assertion about the flaw it has, and I have a few fish to fry (well ,get ready for frying) which I hope to have done before she who must not be kept waiting swings by to grab me up for the beach. Another day I would do anything it took to put on auto-repeat.

# include "ezButton.h"

ezButton someButton (3, INPUT_PULLUP);

void setup() {
  Serial.begin(9600);
  Serial.println("Hello ezButton World!\n");

  someButton.setDebounceTime(50);
}

int count = 1;
int toggle;

void loop() {
  someButton.loop();

  if (someButton.isPressed()) { Serial.print("you pressed "); count++; Serial.println(count); }

  if (someButton.isReleased()) { Serial.print("          you unpressed ");  Serial.println(count);}
}

HTH

a7