Using functions

robecq · April 2, 2021, 9:14pm

Hi

I am trying to understand how to call functions correctly
I am using an example from this forum that uses a button latch to turn on and off an LED.
Putting this code into a function and then calling it works fine
I now have 4 buttons and 4 LEDs
I want to press one of the 4 buttons and the corresponding LED lights up. push it again and the LED turns off.
My code only works if all the buttons are pressed at once, not one at a time. Any suggestions on how I go about it?
my code:

uint32_t start, dbStart;
const byte btn1 = 2, btn2 = 3, btn3 = 4, btn4 = 5, led1 = 17, led2 = 16, led3 = 15, led4 = 14, dbTime = 15;
int  btn = 0;
int  led = 0;

bool pinState = true,
     btnState = true,
     prevBtnState = true,
     latch = false;
void setup() {
  Serial.begin(9600);
  Serial.println("ready:");
  pinMode(btn1, INPUT);
  pinMode(led1, OUTPUT);
  pinMode(btn2, INPUT);
  pinMode(led2, OUTPUT);
  pinMode(btn3, INPUT);
  pinMode(led3, OUTPUT);
  pinMode(btn4, INPUT);
  pinMode(led4, OUTPUT);

}

void loop() {
  button(btn1, led1); // better? method of calling the function
  button(btn2, led2);
  button(btn3, led3);
  button(btn4, led4);
  //btn = btn1;  led = led1; button(); // works if its the only line
  //btn = btn2;  led = led2; button();  // if these thre lines
  //btn = btn3;  led = led3; button();  // are included,
  //btn = btn4;  led = led4; button();  // then all buttons need to be pressed at once

}
void button(int btn, int led) {
  // debounce button ++++++++++++++++
  if (digitalRead(btn) != pinState)  // get state of pin 2
  {
    dbStart = millis();     // reset db timer
    pinState = !pinState;   // now they are equal, won't enter
  }                         // here again unless pin state changes
  if (millis() - dbStart > dbTime) // db timer has elapsed
    btnState = pinState;           // button state is valid
  //+++++++++++++++++++++++++++++++++++
  if (btnState != prevBtnState && btnState == HIGH) // btnState changed
  {
    // if button pressed and NOT latched
    if (!latch)
    {
      latch = true;    // prevents entry while latched
      digitalWrite(led, HIGH);
    }
    else
    {
      // if button pressed while latched,
      latch = false;
      digitalWrite(led, LOW);
    }
    Serial.print( "LED is ");
    if (latch)
    {
      Serial.println("ON");
    }
    else
    {
      Serial.println("OFF");
    }
  }
  prevBtnState = btnState;
}

Extra Question: I don't understand the significance of using the following line to label the dbStart integer

uint32_t start, dbStart;

instead of the more usual

unsigned long dbStart= 0;

PerryBebbington · April 2, 2021, 9:29pm

I am trying to understand how to call functions correctly

You are calling the function correctly.

The problem is the function has one set of timers to deal with 4 (or any number) of different possible inputs, so they get all mixed up.

First you call your function for button 1 and LED 1, so it does it's thing, then immediately afterwards for button 2, LED 2 then 3 then 4, but the timer is still dealing with 1, so it gets all messed up. Each button and LED needs its own timer.

When you have multple instances of very similar variables it's time to learn about arrays array - Arduino Reference as they should all be grouped together in an array.

Next thing is number things from 0, not 1. The first element of the array you will be using will be 0, then 1,2, 3 (not 4).

PerryBebbington · April 2, 2021, 9:34pm

With regards to uint32_t and unsigned long.

Unsigned long does not specify how many bits or bytes the variable uses. Depending on the architecture it will be 32 or 64 bits (or someone will come along and tell me I got that wrong!). The point is you are expected to know, it's not explicit.

With uint32_t you know you are getting 32 bits, guaranteed.

My preference is the uint32_t style because I know what I am getting but not everyone agrees, go with your own preference. Remember that char is for characters, if you use uint8_t or int8_t for characters sooner or later it will catch you out.

++Karma; // For trying to do things correctly and take the first steps from beginner to experienced programmer.

robecq · April 2, 2021, 10:46pm

Thanks for your reply.
I've created arrays for the buttons, leds and timer. (in your reply, you are only referring to an array for the timer yes?)
Button and LED array seem to work, but I am struggling with my timer array

//uint32_t start, dbStart;
const byte btn0 = 2, btn1 = 3, btn2 = 4, btn3 = 5, led0 = 17, led1 = 16, led2 = 15, led3 = 14, dbTime = 15;
int dbs0 = 0, dbs1 = 1000, dbs2 = 2000, dbs3 = 3000;
int  btn = 0;
int  led = 0;
int btnarray[4] = {btn0, btn1, btn2, btn3};
int ledarray[4] = {led0, led1, led2, led3};
int dbStart[4] = {dbs0, dbs1, dbs2, dbs3};

bool pinState = true,
     btnState = true,
     prevBtnState = true,
     latch = false;
void setup() {
  Serial.begin(9600);
  Serial.println("ready:");
  pinMode(btn0, INPUT);
  pinMode(led0, OUTPUT);
  pinMode(btn1, INPUT);
  pinMode(led1, OUTPUT);
  pinMode(btn2, INPUT);
  pinMode(led2, OUTPUT);
  pinMode(btn3, INPUT);
  pinMode(led3, OUTPUT);

}

void loop() {
  for (int i = 0; i < 4 ; i++) {
    button (btnarray[i], ledarray[i], dbStart[i]);
    Serial.print(btnarray[i]); Serial.print("----"); Serial.print(ledarray[i]); Serial.print("----"); Serial.println(dbStart[i]);



  }
  //   button(btn1, led1); // better? method of calling the function
  //   button(btn2, led2);
  //   button(btn3, led3);
  //   button(btn4, led4);

}
void button(int btn, int led, int i) {
  Serial.print(btn); Serial.print("----"); Serial.print(led); Serial.print("----"); Serial.println(dbStart);
  dbStart = dbStart[i];

  // debounce button ++++++++++++++++
  if (digitalRead(btn) != pinState)  // get state of pin 2
  {
    dbStart = millis();     // reset db timer
    pinState = !pinState;   // now they are equal, won't enter
  }                         // here again unless pin state changes
  if (millis() - dbStart > dbTime) // db timer has elapsed
    btnState = pinState;           // button state is valid
  //+++++++++++++++++++++++++++++++++++
  if (btnState != prevBtnState && btnState == HIGH) // btnState changed
  {
    // if button pressed and NOT latched
    if (!latch)
    {
      latch = true;    // prevents entry while latched
      digitalWrite(led, HIGH);
    }
    else
    {
      // if button pressed while latched,
      latch = false;
      digitalWrite(led, LOW);
    }
    Serial.print( "LED is ");
    if (latch)
    {
      Serial.println("ON");
    }
    else
    {
      Serial.println("OFF");
    }
  }
  prevBtnState = btnState;
}

PerryBebbington · April 3, 2021, 8:10am

(in your reply, you are only referring to an array for the timer yes?)

No, I'm referring to an array for anything where you have multiple similar variables, so in your case buttons, LEDs and timers. You seem to have worked that out.

You've created arrays for LED and button pins then not used them in setup. You can use there there inside a for loop.

int dbStart[4]

Millis() is unsigned 32 bits, so anything to do with millis should be unsigned 32 bits, int is signed 16 bits.

I struggle to read other people's code (I sometime struggle to read my own!), your button function is a confused (or confusing to me!) mess of dealing with array variables and variables not in an array. Your function needs to be able to deal with any of the 4 buttons without the data from one being mixed up with the data from another. This means that if there is anything (such as debounce time out) that needs to be stored between function calls then there has to be variable for each instance of that thing, as you have with your arrays. You still have single instance variables that look like they should be 1 per button or LED, latch for example.

You define:

int dbStart[4] = {dbs0, dbs1, dbs2, dbs3};

as an array then try to use it as a single, non-array variable:

  dbStart = dbStart[i];

and

    dbStart = millis();     // reset db timer

I suggest you start again with your button function and think about what it needs to do and build it a step at a time. There's too much wrong with it for me to suggest small changes that will fix it. I think you need to understand arrays better so you don't make mistakes like the one just above here.

You're on the right track, keep going.

robecq · April 3, 2021, 8:53am

Thanks for the feedback, I started looking at functions, but am now brushing up on arrays. I'll get there eventually, and post back here when I do.

UKHeliBob · April 3, 2021, 9:23am

This example sketch uses a struct to hold everything we need to know about a button and its associated LED such as pin number and states.

struct dataLayout
{
  byte buttonPin;
  byte ledPin;
  byte currentButtonState;
  byte previousButtonState;
};

Then it creates an array of structs and puts in the values for each button and LED

dataLayout data[4] =
{
  {A3, 3, HIGH, HIGH},
  {A2, 5, HIGH, HIGH},
  {A1, 6, HIGH, HIGH},
  {A0, 9, HIGH, HIGH},
};

From now on in the code we can refer to an individual button/LED pair as data[array index number] and the value of a named data item with a dot after the button/LED indentifier.

For instance data[0].currentButtonState would refer to the current buttonstate of the first button/LED pair bearing in mind that arrays are indexed starting at zero

Using this method

  for (int x = 0; x < 4; x++)
  {
    pinMode(data[x].buttonPin, INPUT_PULLUP);
    pinMode(data[x].ledPin, OUTPUT);
  }

sets the pinModes() for each button/LED pair

Then in loop() we can determine the current state of each button, compare it with its previous state and take action if needed. Because we are using an array the code to do this can be in a for loop and only needs to be written once

Here is the full example

const byte buttonPins[] = {A3, A2, A1, A0};
const byte ledPins[] = {3, 5, 6, 9};

struct dataLayout
{
  byte buttonPin;
  byte ledPin;
  byte currentButtonState;
  byte previousButtonState;
};

dataLayout data[4] =
{
  {A3, 3, HIGH, HIGH},
  {A2, 5, HIGH, HIGH},
  {A1, 6, HIGH, HIGH},
  {A0, 9, HIGH, HIGH},
};

void setup()
{
  Serial.begin(115200);
  while (!Serial);
  for (int x = 0; x < 4; x++)
  {
    pinMode(data[x].buttonPin, INPUT_PULLUP);
    pinMode(data[x].ledPin, OUTPUT);
  }
}

void loop()
{
  for (int x = 0; x < 4; x++)
  {
    data[x].currentButtonState = digitalRead(data[x].buttonPin);
    if (data[x].currentButtonState == LOW)
    {
      if (data[x].currentButtonState != data[x].previousButtonState)
      {
        digitalWrite(data[x].ledPin, !digitalRead(data[x].ledPin));
      }
    }
    data[x].previousButtonState = data[x].currentButtonState;
  }
}

You could do the same with individual arrays for each of the values but using a struct makes it much neater

robecq · April 3, 2021, 10:45am

Yay! I got it to work. Thanks for your help @Perry! I totally understand what you mean regarding trying to read others code, and I've tried to clean it up a bit, But I can see that UKHeliBob, has what looks like an even cleaner method. I have some tiding to do, I'm sure there is still useless code doing nothing hiding in there.

my final? code:

const byte btn0 = 2, btn1 = 3, btn2 = 4, btn3 = 5, led0 = 17, led1 = 16, led2 = 15, led3 = 14, dbTime = 15; //should this also be in an array?

int  btn = 0;
int  led = 0;
int btnarray[4] = {btn0, btn1, btn2, btn3};
int ledarray[4] = {led0, led1, led2, led3};
unsigned long dbStartarray[4] = {0, 0, 0, 0};
int pinStateArray[4] = {0, 0, 0, 0};
int btnStateArray[4] = {0, 0, 0, 0};
int prevBtnStateArray[4] = {0, 0, 0, 0};
int latchArray[4] = {0, 0, 0, 0};


void setup() {
  Serial.begin(9600);
  Serial.println("ready:");

  pinMode(btn0, INPUT);
  pinMode(led0, OUTPUT);
  pinMode(btn1, INPUT);
  pinMode(led1, OUTPUT);
  pinMode(btn2, INPUT);
  pinMode(led2, OUTPUT);
  pinMode(btn3, INPUT);
  pinMode(led3, OUTPUT);

}

void loop() {
  for (int i = 0; i < 4 ; i++) {
      button(btnarray[i], ledarray[i], i);

  }
}
    //__________________________________________________________________________________
    void button(int btn, int led, int i ) {

      unsigned long dbStart = dbStartarray[i];

      bool             pinState = pinStateArray[i],
                       btnState = btnStateArray[i],
                       prevBtnState = prevBtnStateArray[i],
                       latch = latchArray[i];


      // debounce button ++++++++++++++++
      if (digitalRead(btn) != pinState)  // get state of pin 
      {
        dbStart = millis();     // reset db timer
        pinState = !pinState;   // now they are equal, won't enter
      }                         // here again unless pin state changes
      if (millis() - dbStart > dbTime) // db timer has elapsed
        btnState = pinState;           // button state is valid
      //+++++++++++++++++++++++++++++++++++
      if (btnState != prevBtnState && btnState == HIGH) // btnState changed
      {
        // if button pressed and NOT latched
        if (!latch)
        {
          latch = true;    // prevents entry while latched
          digitalWrite(led, HIGH);
        }
        else
        {
          // if button pressed while latched,
          latch = false;
          digitalWrite(led, LOW);
        }
        Serial.print(i);
        Serial.print( "  LED is ");
        if (latch)
        {
          Serial.println("ON");
        }
        else
        {
          Serial.println("OFF");
        }

        

      }
      prevBtnState = btnState;

      dbStartarray[i] = dbStart;
      pinStateArray[i] = pinState;
      btnStateArray[i] = btnState;
      prevBtnStateArray[i] = prevBtnState;
      latchArray[i] =   latch;
    }

UKHeliBob · April 3, 2021, 11:02am

Good for getting it working

Now try the version in reply #6 and note how much neater it is by using an array of structs

PerryBebbington · April 3, 2021, 6:05pm

Yay! I got it to work.

Well done ! ! !

Thanks for your help @Perry!

You are welcome!

my final? code:

Err, no. Not unless you decide this hobby is not for you and you never try to improve it. Code can always be improved.

Follow UKHB's advice on using structs, which is your next lesson.

You are still not taking advantage of your arrays in setup().

system · August 1, 2021, 6:05pm

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

Topic		Replies	Views
1 button press , 1 pulse output (no long press) w/ Multiple IN & OUT Programming	31	282	January 26, 2026
Blink without delay, two intervals 240615 Programming	22	435	December 15, 2024
Understanding Concepts About Arrays General Guidance	27	252	November 23, 2025
Millis with multiple buttons Programming	20	370	September 12, 2024
Problem with calling function Programming	15	850	August 17, 2023

Using functions

Related topics