Function for incrementing an integer value

Projects Programming
1

Hi, I've written a small sketch which increments the value of an integer (known as scene) every time a button is pressed. once the value of scene reaches <5 it loops back round to zero.

here is the sketch (which works)...


int scene = 0;

int buttonState = 1;
int lastButtonState = 1;

int buttonPin = 19;

void setup() {
  Serial.begin(9600);
  pinMode(buttonPin, INPUT_PULLUP);

}

void loop() {
  buttonState = digitalRead(buttonPin);

    if(buttonState != lastButtonState && buttonState == 0){
      scene++;
    }  
    lastButtonState = buttonState; 
    if(scene == 5){
      scene = 0;
    }
    Serial.println(scene);

    delay(20);
}

I would now like to put this into a function as I would like to use it many times for buttons on different pins. However, I'm new to functions and am struggling. For some reason instead of the value of scene incrementing once each time i press the button, it rapidly loops through values: 0 - 4 for the entire time that I hold the button down... and then stops again when I let go

here is my attempt at the function (not working): -

int scene = 0;        // i would like scene to remain global as other functions need to access this later

int getScene(int buttonPin){
  int buttonState = 1;
  int lastButtonState = 1;

  buttonState = digitalRead(buttonPin);

    if(buttonState != lastButtonState && buttonState == 0){
      scene++;
    }  
    lastButtonState = buttonState; 
    if(scene == 5){
      scene = 0;
    }
}

void setup(){
  Serial.begin(9600);
  pinMode(19, INPUT_PULLUP);
}

void loop(){
  getScene(19);

  Serial.println(scene);

  delay(20);
}

any help here would be massively appreciated.
Thanks

2

Every time you enter getScene(), you set buttonState and lastButtonState to 1.
Then you set buttonState to 0 each time when the button is pressed, so your if statement increments scene.

Also you say you are returning an int from the function, but you don't return anything. Probably should be void.

3

Thank you,
That makes sense... However, I just tried declaring those two variables without any value and then set it to change both values back to 1 after the if statement. and also changed the return type from int to void. but now for some reason I'm not getting any changes at all in the serial monitor when i press the button. It just stays at zero.

by the way, should this function be of return type: int as it is returning the value of scene? or am i misunderstanding things here?

here is the code after i tried changing it....
I
thanks again by the way

int scene = 0; //i would like scene to remain global as other functions need to access this later

void getScene(int buttonPin){
  int buttonState;
  int lastButtonState;

  buttonState = digitalRead(buttonPin);

    if(buttonState != lastButtonState && buttonState == 0){
      scene++;
    }  
    buttonState = 1;
    lastButtonState = 1;
     
    if(scene == 5){
      scene = 0;
    }
}

void setup(){
  Serial.begin(9600);
  pinMode(19, INPUT_PULLUP);
}

void loop(){
  getScene(19);

  Serial.println(scene); // button is on pin 19

  delay(20);
}
4

A method that returns 'void' should never be called 'get...' anything. Get methods are a standard name for methods that fetch a value.

Did you possibly mean, "next scene" ?

5 
    if(scene >= 5){

is a much safer way to do that.

6

'lastButtonState' is declared local to the function, its value is discarded when the function returns. To make it persistent, you need to also declare it 'static'.

    buttonState = 1;

Why? It's unused for the rest of the function, it's assigned a value every time the function is called.

7

Your problem is that the variables do not keeps it values between function calls. In such case any state variables are useless.

Open any C/C++ textbook and read about static variables

8

Yes, at least.

Then realize that the function is called with argument that will be used for many buttons, but those (now static) variables will be incorrect, as there is only one copy of them and you are trying to use it for all buttons.

So the hole gets deeper - each button value will need its own copies of any variables intended to track its state.

So you may want to read about arrays, too.

Or if you just want to get on with it, see if a button handling library will make life simpler at the expense of all the fun of figuring this out and writing your own code.

I rarely use libraries, never for buttons. But that's the fun I like to have so it all comes down to what you think of as fun. :expressionless:

But you shoukd learn about arrays if you haven't done already.

a7

9

It worked before because 'lastButtonState' was a global variable. If you change it back to a global your code should work again:

int lastButtonState = 1;

int getScene(int buttonPin){
  int buttonState = 1;
  buttonState = digitalRead(buttonPin);

    if(buttonState != lastButtonState && buttonState == 0) {
      scene++;
    }  
    lastButtonState = buttonState; 
    if(scene == 5){
      scene = 0;
    }
}

Another options to make it 'static' instead of global.

int getScene(int buttonPin){
  int buttonState = 1;
  static int lastButtonState = 1;

  buttonState = digitalRead(buttonPin);

    if(buttonState != lastButtonState && buttonState == 0){
      scene++;
    }  
    lastButtonState = buttonState; 
    if(scene == 5){
      scene = 0;
    }
}
10

Some button state handling happens here even if the state doesn't change. It's harmless in this case, but sloppy.

int getScene(int buttonPin){
  int buttonState = 1;
  static int lastButtonState = 1;

  buttonState = digitalRead(buttonPin);

    if(buttonState != lastButtonState && buttonState == 0){
      scene++;
    }  
    lastButtonState = buttonState; 
    if(scene == 5){
      scene = 0;
    }
}

you should only do that stuff when the button actually registers:

int getNextScene(int buttonPin) {
  int buttonState = 1;
  static int lastButtonState = 1;

  buttonState = digitalRead(buttonPin);
  if (buttonState != lastButtonState && buttonState == 0) {
    lastButtonState = buttonState;
    scene++;
    if (scene == 5) {
      scene = 0;
    }
  }
}

Also don't use 0 and 1 for pin states. It's not completely portable. Use the defined constants "LOW" and "HIGH".

11

That will only work once. After that buttonState is either == lastButtonState or != 0. You have to set lastButtonState even if buttonState is != 0.

A working alternative is:

  if (buttonState != lastButtonState) 
  {
    // Just changed
    lastButtonState = buttonState;
    if (buttonState == LOW) // changed to 'pressed'
      scene++;
    if (scene >= 5) 
   {
      scene = 0;
    }
  }
12

really appreciate all the help guys,
decided to go with alto777's option of using global arrays. But also took note of aargs advice on using HIGH's/ LOW's instead of 1's and 0's and also changed the function name from getScene to sceneSelect. I will reading up on static variables over the next few days as it's something i'm not familiar with... but in this instance I think (with help from you guys) I have a good solution.

which is this: -

  int scene[8] = {0, 0, 0, 0, 0, 0, 0, 0};
  int lastButtonState[8] = {HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH};
  int buttonState[8] = {HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH};

void sceneSelect(int buttonPin, int potNum){
  buttonState[potNum] = digitalRead(buttonPin);
    if(buttonState[potNum] != lastButtonState[potNum] && buttonState[potNum] == LOW){
      scene[potNum]++;
    }  
    if(scene[potNum] >= 5){
      scene[potNum] = 0;
    }
  lastButtonState[potNum] = buttonState[potNum];
}

void setup(){
  Serial.begin(9600);
  pinMode(19, INPUT_PULLUP);
}

void loop(){
  sceneSelect(19, 0);

  Serial.println(scene[0]);

  delay(20);
}

I can currently control 8 different scenes using these arrays simply by changing sceneSelect's arguments and obviously I can extend that if need be. Please feel free to point out anything else which you feel as though would be worth changing as any advice is much appreciate.

thanks again

13

Yes, or acknowledge both rising and falling edges, but only use one. As in the Arduino button example sketches... IIRC... oh, wait that's what you posted...

14

What does this mean? When the value becomes less than 5? How do you keep incrementing something until it is less then 5?

1 Like
15

By incrementing it 32763 times or so… :wink:

a7

16

fair enough

17

It is quite possible that your sketch in #12 is fine, but when I placed in the wokwi to play with and started adding to it to see it really working I did change a few things that did not look right.

Before I ran the code at all. Just to make it look more like I had written it. :expressionless:

I added another array to hold pin numbers, so sceneSelect() only needs an index and looks up the corresponding pin.

Dunno what potNum signifies, it looks like an index over 8 somethings.

I put it on an UNO, just because.Try it out here, you will see a few tweaks, again not necessarily fixing any errors:


scene select wokwi simulation

// https://forum.arduino.cc/t/function-for-incrementing-an-integer-value/1045878

# define NPINS  4   // actual number realized in wiring

const int buttonPin[8] = {3, 4, 5, 6,}; // just four four now

int scene[8] = {0, 0, 0, 0, 0, 0, 0, 0};

bool dirt = false;   // just to keep printing to a minumum

int lastButtonState[8] = {HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH};
int buttonState[8] = {HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH, HIGH};

void sceneSelect(int potNum)
{
  buttonState[potNum] = digitalRead(buttonPin[potNum]);
  if (buttonState[potNum] != lastButtonState[potNum]) {
    if (buttonState[potNum] == LOW) {
      scene[potNum]++;
      if (scene[potNum] >= 5) scene[potNum] = 0;

      dirt = true;    // we need to print out all the scene values as there has been a change
    }

    lastButtonState[potNum] = buttonState[potNum];
  }
}

void setup()
{
  Serial.begin(9600);
  Serial.println("function for incrementing an integer value\n");

  for (unsigned char tt = 0; tt < NPINS; tt++)
    pinMode(buttonPin[tt], INPUT_PULLUP);
}

void loop()
{
  for (unsigned char tt = 0; tt < NPINS; tt++)
    sceneSelect(tt);

  if (dirt) {
    for (unsigned char tt = 0; tt < NPINS; tt++) {
      Serial.print(scene[NPINS - 1 - tt]);  // printing was backwards
      Serial.print(" ");
    }
    Serial.println();

    dirt = false;
  }
  
  delay(20);    // effective global debounce!
}

HTH

a7

18

Thanks Alto777,
Very much appreciated. And also, thanks for introducing me to wokwi. it looks very useful :slight_smile:

19

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.