(Repost) Pushbutton Not Activating a Certain Function In My Code

Hi, so I'm making a 5V Solar Powerbank for a YouTube video, the solar and Powerbank part works fine, it uses a tiny microcontroller - DFRobot Beetle board to measure the Li-Ion battery's voltage, then displays how charged it is on a 4-pin Battery Capacity Indicator module, that part all works fine. What I'm trying to do is to be able to press a pushbutton once and get the display module going, then press it again to turn it off.
My issue in making this simple feature possible is coding for this, I'm having trouble figuring out what function name to call out to start and stop the batteryDisplay function from the module's TM1651 library.
Pasted below is my full code (including my attempts in coding for the feature), though my changes didn't bring the Battery Capacity Indicator module to turn off or on when I pushed the push button.

Code:

#include "TM1651.h"
#define CLK 10//pins definitions for TM1651 and can be changed to other ports
#define DIO 9
#define cellPin A0
#define buttonPin 11
int val = 0;
int value = 0; //Val used to store input of Switch or pin 7
int state = 0; //0 Shows LED off 1 is LED on
int old_val = 0; // Previous version of val
TM1651 batteryDisplay(CLK, DIO);

void setup()
{
  Serial.begin(9600);  // Debugging only
  batteryDisplay.init();
  batteryDisplay.set(7);//0 ~ 7 mean to different brightness;
  batteryDisplay.frame(FRAME_ON);//light the frame of the battery display or FRAME_OFF to turn off the frame of the battery display

  pinMode(cellPin, INPUT);
}

void loop()
{
  value = digitalRead(buttonPin); //Reads the value of the button and stores
  if ((value == HIGH) && (old_val == LOW))
  {
    state = 1 - state;  //Check if state has changed
    // delay(10);
  }
  old_val = value; // Val is now old val so its stored

  switch (state)
  {
    case 1:
      _start(); //starting battery display animation
      break;
    case 0:
      _stop(); //stopping battery display animation
      break;
  }


  charging(); //initializing function

  val = analogRead(cellPin);
  Serial.println(val);
  delay(1000);

}

void charging() //for displaying battery lvl | through a charging animation (which is disabled here)
{
  if (val <= 550)
  {
    for (uint8_t level = 0; level < 1; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 600)
  {
    for (uint8_t level = 0; level < 2; level ++) /*the following lines of code see if battery's ADC analog value is greater than or equal to a certain value,
                                                 then makes the module display so many bars as a percentage accordingly.*/
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 650)
  {
    for (uint8_t level = 0; level < 3; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 700)
  {
    for (uint8_t level = 0; level < 4; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 750)
  {
    for (uint8_t level = 0; level < 5; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 800)
  {
    for (uint8_t level = 0; level < 6; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }



}

I usually get around such small things quicker than the problem I'm having now, but this time it really got me stuck, so that's why you're seeing my post on the forum.
If any of you could suggest a quick tip, code modification, or method to make this little feature work-in with my battery display, that would be great.

Any help would be much appreciated, thanks!

Edit:
I figured out the solution, what I should have instead done to make the button switch turn the TM1651 Battery display on and off was to add this:

void loop()
{
  indicatorOff();

void indicatorOff()
{
  for (uint8_t level = 0; level < 1; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  batteryDisplay.frame(FRAME_OFF);  
}

It's to give an option to this part of the code:

void loop()
{
  val = analogRead(cellPin);
  Serial.println(val);
  delay(200);

  indicatorOff();
  
  value = digitalRead(buttonPin); //Reads the value of the button and stores
  if ((value == HIGH) && (old_val == LOW))
  {
    state = 1 - state;  //Check if state has changed
    delayMicroseconds(10);
  }
  old_val = value; // Val is now old val so its stored
  switch (state)
  {
    case 1:
      charging(); //initiallizing 
      break;
    case 0:
      indicatorOff(); //deactivating
      break;
  }
}

to have a deactivating condition for the thing to switch to, so the display module won't show anything when the button is debounced to a low state.
Fully working updated code:

#include "TM1651.h"
#define CLK 10//pins definitions for TM1651 and can be changed to other ports
#define DIO 9
#define cellPin A0
#define buttonPin 11
int value = 0; //Val used to store input of Switch or pin 7
int state = 0; //0 Shows LED off 1 is LED on
int old_val = 0; // Previous version of val
int val = 0;
TM1651 batteryDisplay(CLK, DIO);


void setup()
{
  Serial.begin(9600);  // Debugging only
  batteryDisplay.init();
  batteryDisplay.set(7);//0 ~ 7 mean to different brightness;
  batteryDisplay.frame(FRAME_ON);//light the frame of the battery display or FRAME_OFF to turn off the frame of the battery display

  pinMode(cellPin, INPUT);

  pinMode(buttonPin, INPUT);
}

void indicatorOff()
{
  for (uint8_t level = 0; level < 1; level ++)
  {
    batteryDisplay.displayLevel(level);
    delayMicroseconds(5);
  }
  batteryDisplay.frame(FRAME_OFF);
}

void loop()
{
  val = analogRead(cellPin);
  Serial.println(val);
  delay(200);

  indicatorOff();

  value = digitalRead(buttonPin); //Reads the value of the button and stores
  if ((value == HIGH) && (old_val == LOW))
  {
    state = 1 - state;  //Check if state has changed
    delayMicroseconds(10);
  }
  old_val = value; // Val is now old val so its stored
  switch (state)
  {
    case 1:
      charging(); //initiallizing
      break;
    case 0:
      indicatorOff(); //deactivating
      break;
  }



}


void charging() //for displaying battery lvl | through a charging animation (which is disabled here)
{
  batteryDisplay.frame(FRAME_ON);
  if (val <= 550)
  {
    for (uint8_t level = 0; level < 1; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 600)
  {
    for (uint8_t level = 0; level < 2; level ++) /*the following lines of code see if battery's ADC analog value is greater than or equal to a certain value,
                                                 then makes the module display so many bars as a percentage accordingly.*/
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 650)
  {
    for (uint8_t level = 0; level < 3; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 700)
  {
    for (uint8_t level = 0; level < 4; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 750)
  {
    for (uint8_t level = 0; level < 5; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 800)
  {
    for (uint8_t level = 0; level < 6; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
}

I happened to find the solution myself, writing some code from my own experience and logic :wink:
Though, thanks to those of you who lead me in the right direction! :smiley:
I hope this will help anyone reading this who also wants a similar feature in their project.

You put this outside any conditional block, so it updates every time through loop(), regardless of any input conditions or changes.

Also, you commented out the delay that would debounce your switch.

Ok, I'll correct my code with the things you've pointed out so far, thx!

What exactly do you mean by "You put this outside any conditional block"?
I'm a little confused.

Google, "conditional block C++"

Ok, so should it look like this?:

if ((value == HIGH) && (old_val == LOW))
  {
    state = 1 - state;  //Check if state has changed
    old_val = value; // Val is now old val so its stored
    delay(10);
    }

This button code looks correct to me for detecting a button press state change depending on how the button is wired.

How is the button pin wired so that you are expecting HIGH when pressed? Are you using a pull down resistor?

It should be very easy for you to confirm with Serial output that the button is changing the state as you require or if you need to change the button reading code.

I'm having trouble figuring out what function name to call out to start and stop the batteryDisplay function from the module's TM1651 library.

If this is your real issue, please explain more about what you want to see with each button press.

I think that you may want to call charging() in one state, and possibly batteryDisplay.displayLevel(0); in the other.

EDIT: I do not think you should have this statement in loop() if you are going to call the display from the button. The comment is wrong, it is calling the function not "initializing" it whatever that means.

//charging(); //initializing void function

Hi, the button pin is wired correctly, to D11, while also having a pulldown resistor.
I checked that this portion of the code is working, but so far I'm only able to turn on/off the battery indicator module's frame LED calling the "batteryDisplay.frame(FRAME_ON);" to "batteryDisplay.frame(FRAME_OFF);", but that's not only what I want it to do.
So to confirm, yes, the button and code works fine, but I want to be able to turn the whole module's LEDs on and off.

If this is your real issue, please explain more about what you want to see with each button press.

The idea is that you press the button once to check the battery level, and press once again to turn the indicator off. Ya get what I mean?

That would make sense, but how would I do that? Would I need to type them side by side under each "case:"?

how would I do that? Would I need to type them side by side under each "case:"?

Try this

#include "TM1651.h"
#define CLK 10//pins definitions for TM1651 and can be changed to other ports
#define DIO 9
#define cellPin A0
#define buttonPin 11
int val = 0;
int value = 0; //Val used to store input of Switch or pin 7
int state = 0; //0 Shows LED off 1 is LED on
int old_val = 0; // Previous version of val
TM1651 batteryDisplay(CLK, DIO);

void setup()
{
  Serial.begin(9600);  // Debugging only
  batteryDisplay.init();
  batteryDisplay.set(7);//0 ~ 7 mean to different brightness;
  batteryDisplay.frame(FRAME_ON);//light the frame of the battery display or FRAME_OFF to turn off the frame of the battery display

  pinMode(cellPin, INPUT);
}

void loop()
{
  value = digitalRead(buttonPin); //Reads the value of the button and stores
  if ((value == HIGH) && (old_val == LOW))
  {
    state = 1 - state;  //Check if state has changed
    // delay(10);
  }
  old_val = value; // Val is now old val so its stored

  switch (state)
  {
    case 1:
      // _start(); //starting battery display animation
      charging()_;
      break;
    case 0:
      //_stop(); //stopping battery display animation
      batteryDisplay.displayLevel(0);
      break;
  }


  //charging(); //initializing function

  val = analogRead(cellPin);
  Serial.println(val);
  delay(1000);

}

void charging() //for displaying battery lvl | through a charging animation (which is disabled here)
{
  if (val <= 550)
  {
    for (uint8_t level = 0; level < 1; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 600)
  {
    for (uint8_t level = 0; level < 2; level ++) /*the following lines of code see if battery's ADC analog value is greater than or equal to a certain value,
                                                 then makes the module display so many bars as a percentage accordingly.*/
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 650)
  {
    for (uint8_t level = 0; level < 3; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 700)
  {
    for (uint8_t level = 0; level < 4; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 750)
  {
    for (uint8_t level = 0; level < 5; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
  if (val >= 800)
  {
    for (uint8_t level = 0; level < 6; level ++)
    {
      batteryDisplay.displayLevel(level);
      delayMicroseconds(5);
    }
  }
}

Thanks for your changes, but it still didn't quite work the way I wanted to.
What you did is you made only the charging(); battery display LEDs turn on and off, but the battery outer bar always stays lit.

I don't have the library or the display to actually troubleshoot so I'm basically working with the library source code.

Does this command do what you want?
batteryDisplay.frame(FRAME_OFF);

Ok, and no, that only turns off the frame, we need to find a way to also turn off this function: "charging();"
Would you by any chance know how that'd be done?

What if you combine the two commands

batteryDisplay.frame(FRAME_OFF);
batteryDisplay.displayLevel(0);
  if ( condition )
  {
    // lines inside of
    // a block of code
    // that executes on condition
  }

Also true for while (), anything that changes execution on a condition being met.

A while() loop, hmm, and how could I modify my code with that change? Thx.

That was an explanation of what is a conditional.
.