Variable Scope Help... I think

I am trying to control a variety of motorized actuators using a function rather than brute force writing out the valve operation in the main loop. I have adapted code from Adafruits tutorial on pull-up resistors and am using the button wiring at suggested. Using the tutorials code it works great. I believe that in my current adaptation I am having a variable scope issue but I cannot identify it.

When uploaded to my arduino mega, the LED on pin 13 used as my demo is off to start and then will turn on with a button press. It will not turn off again when the button is pressed again.

Please help and thank you!

Code below

int Actuator5ButtonPin = 5;              // switch is connected to pin 2
int Actuator5Switch = 13;

//int val;                        // variable for reading the pin status
//int val2;                       // variable for reading the delayed/debounced status
int Actuator5ButtonState;                // variable to hold the button state

int Actuator5ButtonAction = 0;              // Is the light on or off?

void setup() {
  pinMode(Actuator5ButtonPin, INPUT);    // Set the switch pin as input
  pinMode(Actuator5Switch, OUTPUT);      // Set to control the actuator switch
  
  Serial.begin(9600);                // Set up serial communication at 9600bps
  Actuator5ButtonState = digitalRead(Actuator5ButtonPin);   // read the initial state
}

void loop(){
  switchtoggle(Actuator5ButtonPin, Actuator5ButtonState, Actuator5ButtonAction, Actuator5Switch);
}

void switchtoggle(int ActuatorButtonPin, int ActuatorButtonState, int ActuatorButtonAction, int ActuatorSwitch){
  int val = digitalRead(ActuatorButtonPin);      // read input value and store it in val
  delay(10);                         // 10 milliseconds is a good amount of time
  int val2 = digitalRead(ActuatorButtonPin);     // read the input again to check for bounces
  if (val == val2) {                 // make sure we got 2 consistant readings!
    if (val != ActuatorButtonState) {          // the button state has changed!
      if (val == LOW) {                // check if the button is pressed
        if (ActuatorButtonAction == 0) {          // is the light off?
          ActuatorButtonAction = 1;               // turn light on!
          digitalWrite(ActuatorSwitch, HIGH);
        } else {
          ActuatorButtonAction = 0;               // turn light off!
          digitalWrite(ActuatorSwitch, LOW);
        }
      }
    }
    ActuatorButtonState = val;                 // save the new state in our variable
  }
}

First things first how is your switch wired?

Your results are being trapped in the function. To modify global variables from within a function, you need to pass the addresses of the variables to the function.

Try this:-

int Actuator5ButtonPin = 5;              // switch is connected to pin 2
int Actuator5Switch = 13;

//int val;                        // variable for reading the pin status
//int val2;                       // variable for reading the delayed/debounced status
int Actuator5ButtonState;                // variable to hold the button state

int Actuator5ButtonAction = 0;              // Is the light on or off?

void setup()
{
    pinMode(Actuator5ButtonPin, INPUT);    // Set the switch pin as input
    pinMode(Actuator5Switch, OUTPUT);      // Set to control the actuator switch

    Serial.begin(9600);                // Set up serial communication at 9600bps
    Actuator5ButtonState = digitalRead(Actuator5ButtonPin);   // read the initial state
}

void loop()
{
    switchtoggle(&Actuator5ButtonPin, &Actuator5ButtonState, &Actuator5ButtonAction, &Actuator5Switch);
}

void switchtoggle(int* pActuatorButtonPin, int* pActuatorButtonState, int* pActuatorButtonAction, int* pActuatorSwitch)
{
    int val = digitalRead(*pActuatorButtonPin);      // read input value and store it in val
    delay(10);                         // 10 milliseconds is a good amount of time
    int val2 = digitalRead(*pActuatorButtonPin);     // read the input again to check for bounces
    if (val == val2)                   // make sure we got 2 consistant readings!
    {
        if (val != *pActuatorButtonState)            // the button state has changed!
        {
            if (val == LOW)                  // check if the button is pressed
            {
                if (*pActuatorButtonAction == 0)            // is the light off?
                {
                    *pActuatorButtonAction = 1;               // turn light on!
                    digitalWrite(*pActuatorSwitch, HIGH);
                }
                else
                {
                    *pActuatorButtonAction = 0;               // turn light off!
                    digitalWrite(*pActuatorSwitch, LOW);
                }
            }
        }
        *pActuatorButtonState = val;                 // save the new state in our variable
    }
}

I'm a bit rushed at the moment, but I don't think I made any mistakes.
(I'm sure that someone will quickly point it out if I did. :smiley: )

    ActuatorButtonState = val;                 // save the new state in our variable

This variable is local to the function. If you want to update the global Actuator5ButtonState variable you must return a value to the calling program and call the function like this

Actuator5ButtonState = switchtoggle(Actuator5ButtonPin, Actuator5ButtonState, Actuator5ButtonAction, Actuator5Switch);

You should also be able to simplify the function by doing

     if (val == LOW) 
      {                // check if the button is pressed
        digitalWrite(ActuatorSwitch, !digitalRead(ActuatorSwitch));
      }

Do you really need all the variables sent to the function ? What is the purpose of ActuatorButtonAction ?

Incidentally, with numbered variables I can see the use of arrays in your future

This works:

int Actuator5ButtonPin = 5;              // switch is connected to pin 2
int Actuator5Switch = 13;

//int val;                        // variable for reading the pin status
//int val2;                       // variable for reading the delayed/debounced status
int Actuator5ButtonState;                // variable to hold the button state

int Actuator5ButtonAction = 0;              // Is the light on or off?

void setup() {
  pinMode(Actuator5ButtonPin, INPUT_PULLUP);    // Set the switch pin as input
  pinMode(Actuator5Switch, OUTPUT);      // Set to control the actuator switch

  Serial.begin(9600);                // Set up serial communication at 9600bps
  Actuator5ButtonState = digitalRead(Actuator5ButtonPin);   // read the initial state
}

void loop() {
  switchtoggle(Actuator5ButtonPin, Actuator5ButtonState, Actuator5ButtonAction, Actuator5Switch);
}

void switchtoggle(int ActuatorButtonPin, int ActuatorButtonState, int ActuatorButtonAction, int ActuatorSwitch) {
  int val = digitalRead(ActuatorButtonPin);      // read input value and store it in val
  delay(10);                         // 10 milliseconds is a good amount of time
  int val2 = digitalRead(ActuatorButtonPin);     // read the input again to check for bounces
  if (val == val2) {                 // make sure we got 2 consistant readings!
    if (val != ActuatorButtonState) {          // the button state has changed!
     Actuator5ButtonState = val;                 // save the new state in our variable
      if (val == LOW) {                // check if the button is pressed
        if (ActuatorButtonAction == 0) {          // is the light off?
          Actuator5ButtonAction = 1;               // turn light on!
          digitalWrite(ActuatorSwitch, HIGH);
        } else {
          Actuator5ButtonAction = 0;               // turn light off!
          digitalWrite(ActuatorSwitch, LOW);
        }
      }
    }
  }
}

However take a look at the changes to see why it now works.

You can use a structure to create the number of elements (switch led combinations) you need, then use your function to handle the led control.
You can even put your function into this structure.

.

Take a look at this BWD 'structure example' to see how a structure can be set up.

Then modify your sketch to use a structure to define the 'switch and led' combination of elements.
You can incorporate your led control function into the new structure or leave it after loop()

//Blink without Delay skeleton example using a structure.

//LED wiring options
//=============================================

//#define PlusEqualsON //uncomment to invert

#ifdef PlusEqualsON
//wired so +5V turns LED ON
#define ledON  HIGH
#define ledOFF LOW
//=========================
#else
//wired so +5V turns LED OFF
#define ledON  LOW
#define ledOFF HIGH

#endif

//======================================================================
struct timer
{
  //lastMillis = the time this "timer" was (re)started
  //waitMillis = delay time (mS) we are looking for
  //restart    = do we start "this timer" again and again  
  //enableFlag = is "this timer" enabled/allowed to be accessed
  //**********************
  //For each timer object you need: 
  //Example:
  //   timer myTimer = //give the timer a name "myTimer"
  //   {
  //     0, 200UL, true, true  //lastMillis, waitMillis, restart, enableFlag 
  //   };
  // You have access to: 
  // myTimer.lastMillis, myTimer.waitMillis, myTimer.restart, myTimer.enableFlag, myTimer.CheckTime() 
  //**********************

  unsigned long lastMillis; 
  unsigned long waitMillis; 
  bool          restart; 
  bool          enableFlag;
  bool CheckTime() //Delay time expired function "CheckTime()"
  {
    //is the time up for this task?
    if (enableFlag && millis() - lastMillis >= waitMillis) 
    //Note: if delays of < 2 millis are needed use micros() and adjust waitMillis as needed
    {
      //should this start again? 
      if(restart)
      {
        //get ready for the next iteration
        lastMillis += waitMillis;  
      }
      //time was reached
      return true;
    }
    //time was not reached
    return false;
    
  } //END of CheckTime()

}; //END of structure timer
//======================================================================


//**********************************************************************
//Let's create 6 timer objects and initialize them in this sketch
//**********************************************************************
timer pin13 = //create timer pin13
{
  0, 200UL, true, true //lastMillis, waitMillis, restart, enableFlag
};
//***************************
timer pin12 = //create timer pin12
{
  0, 3000UL, true, true //lastMillis, waitMillis, restart, enableFlag
};
//***************************
timer pin11 = //create timer pin11
{
  0, 10000UL, true, true //lastMillis, waitMillis, restart, enableFlag
};
//***************************
timer pin10 = //create timer pin10
{
  0, 6000UL, true, true //lastMillis, waitMillis, restart, enableFlag
};
//***************************
timer Toggle10 = //create timer Toggle10
{
  0, 50UL, true, true //lastMillis, waitMillis, restart, enableFlag
};
//***************************
timer checkSwitches = //create timer checkSwitches
{
  0, 50UL, true, true //lastMillis, waitMillis, restart, enableFlag
};
//***************************

byte lastMySwitchState = 1; //for mySwitch on Pin 2
byte counter           = 0; 

const byte Pin13 = 13;
const byte Pin12 = 12;
const byte Pin11 = 11;
const byte Pin10 = 10;
const byte Pin9  =  9;

const byte mySwitch = 2;

//**********************************************************************

void setup()
{
  Serial.begin(9600);

  pinMode(Pin13,OUTPUT);
  pinMode(Pin12,OUTPUT);
  pinMode(Pin11,OUTPUT);
  pinMode(Pin10,OUTPUT);
  pinMode(Pin9, OUTPUT);

  digitalWrite(Pin13,LOW);
  digitalWrite(Pin12,LOW);
  digitalWrite(Pin11,LOW);
  digitalWrite(Pin10,LOW);
  digitalWrite(Pin9, LOW);

  pinMode(mySwitch,INPUT_PULLUP);


} //  >>>>>>>>>>>>>> E N D  O F  s e t u p ( ) <<<<<<<<<<<<<<<<<


void loop()
{
  //Below are examples demonstrating different timing situations 

  //***************************
  //example 1    Toggle Pin13 every 200ms
  if (pin13.CheckTime())
  {
    //Toggle Pin13
    digitalWrite(Pin13,!digitalRead(Pin13));

    //if you only want this section of code to happen once
    //uncomment the next line 
    //pin13.enableFlag = false;
  }

  //***************************
  //example 2    After 3 seconds, Pin12 goes and stays HIGH 
  if (pin12.CheckTime())
  {
    //Pin12 HIGH now
    digitalWrite(Pin12,HIGH);
    //disable timing section of code 
    pin12.enableFlag = false;
  }

  //***************************
  //example 3    Pin11 is HIGH for 10 seconds, then goes and stays LOW
  //if (pin11.enableFlag && !pin11.CheckTime())
  if (!pin11.CheckTime())
  {
    digitalWrite(Pin11,HIGH);
  }
  //10 seconds is now up now, leave the Pin11 LOW
  else
  {
    digitalWrite(Pin11,LOW);
    //disable timing section of code 
    pin11.enableFlag = false;
  }

  //***************************
  //example 4    For 6 seconds, toggle Pin10
  //if (pin10.enableFlag && !pin10.CheckTime())
  if (!pin10.CheckTime())
  {
    //example 5  Toggling Pin10 every 50mS
    if(Toggle10.CheckTime())
    {  
      //toggle Pin10
      digitalWrite(Pin10,!digitalRead(Pin10));    
    }
  }
  //6 seconds is now up, stop toggling, toggle at 100ms
  else
  {
    digitalWrite(Pin10,LOW);
    //disable timing section of code 
    pin10.enableFlag = false;
  }

  //***************************
  //example 6    Is it time to check the switches?
  if (checkSwitches.CheckTime())
  {
    //time to read the switches
    Switches();      
  } 

  //**********************************
  //Put other non-blocking stuff here
  //**********************************

} //  >>>>>>>>>>>>>> E N D  O F  l o o p ( ) <<<<<<<<<<<<<<<<<


//======================================================================
//                      F U N C T I O N S
//======================================================================


//**********************************************************************
//switches are checked every checkSwitches.waitMillis milli seconds 
//no minimum switch press time is validated with this code (i.e. No glitch filter)
void Switches()  
{
  boolean thisState; //re-usable for all the switches      

  //****************************************** mySwitch Pin 2 code  
  //check if this switch has changed state
  thisState = digitalRead(mySwitch); 
  if (thisState != lastMySwitchState)
  {  
    //update the switch state
    lastMySwitchState = thisState;  

    //This switch position has changed, let's do some stuff

    //"HIGH condition code"
    //switch goes from LOW to HIGH
    if(thisState == HIGH)        
    {
      //example: LED on Pin9 is Push ON, Push OFF
      digitalWrite(Pin9,!digitalRead(Pin9)); 
    }

    //"LOW condition code"
    //switch goes from HIGH to LOW
    else                          
    {
      //example: display the current switch push count 
      Serial.println(++counter);      
    }
  } //END of mySwitch Pin 2 code

  //******************************************  
  //similar code for other switches goes here 
  //******************************************  

} //END of Switches()


//======================================================================
//                        E N D  O F  C O D E
//======================================================================

To modify global variables from within a function, you need to pass the addresses of the variables to the function.

That is nonsense. The whole reason for using global variables is so they can be modified from anywhere without having to pass anything anywhere.

PaulS:
That is nonsense. The whole reason for using global variables is so they can be modified from anywhere without having to pass anything anywhere.

But, with call by reference, you can choose which global variables are modified when you call the function. If they are referenced directly inside the function, you lose that ability to choose.

PaulS:
That is nonsense. The whole reason for using global variables is so they can be modified from anywhere without having to pass anything anywhere.

Quite right. I was distracted and rushed. What I meant to say was that variables that are passed to and modified by a function, that will later be used outside that function need to be passed by reference, not by value. A silly moment on my part. I don't know why I said 'global'.

I also realise that I didn't need to pass all of the parameters by reference, only any that would be modified inside the function, but my dinner was on the table and I didn't have time to look further to see exactly which were modified.

Sorry for the confusion, addotson.

aarg:
But, with call by reference, you can choose which global variables are modified when you call the function. If they are referenced directly inside the function, you lose that ability to choose.

Yep, I assumed that by the number 5 in the names, there would be more switches etc in the future and that addotson wanted to pass one of them to the function when it's called.int Actuator5Switch = 13;and "Actuator4Switch", "Actuator3Switch" etc

@YoungSteve
I too thought there would be more switch to come.

Structure example to accommodate multiple switch/LED combinations:

struct myStructure   //give it a better name ;-)
{
  byte LedPin;       //LED connects here, HIGH makes LED light
  byte ButtonPin;    //Switch connects here
  byte ButtonState;  //Last state of ButtonPin
  byte LedState;     //Current state of LED

  //Structure Functions
  void SwitchToggle()
  {
    byte val  = digitalRead(ButtonPin);
    delay(10);
    byte val2 = digitalRead(ButtonPin);

    if (val == val2)
    {
      if (val != ButtonState)
      {
        ButtonState = val;
        if (val == LOW)
        {
          if (LedState == LOW)
          {
            LedState = HIGH;
            digitalWrite(LedPin, HIGH);
          }
          else
          {
            LedState = LOW;
            digitalWrite(LedPin, LOW);
          }
        }
      }
    }
  } //END of switchtoggle()

  void begin()
  {
    pinMode(LedPin, OUTPUT);
    digitalWrite(LedPin, LOW);
    pinMode(ButtonPin, INPUT_PULLUP);
    ButtonState = digitalRead(ButtonPin);
  } //END of begin()

};  //END of myStructure

//Create objects:
myStructure FirstLED =
{
  13, 5  //LED pin , Switch pin
};

myStructure SecondLED =
{
  12, 6  //LED pin , Switch pin
};

// etc.

//**********************************************************************

void setup()
{
  FirstLED.begin();
  SecondLED.begin();

} //End of setup()


void loop()
{
  FirstLED.SwitchToggle();
  SecondLED.SwitchToggle();

} //End of loop()



//======================================================================
//                      F U N C T I O N S
//======================================================================

//**********************************************************************


//======================================================================
//                      E N D  O F  C O D E
//======================================================================

And yes you could have a 'begin' function to remove that crap from setup. Done.
And yes you could make a Class.
.

LarryD:
And yes you could have a 'begin' function to remove that crap from setup.
And yes you could make a Class.

The's more than one way to skin a cat.
(39, according to the robot in 'Lost In Space'. :smiley: )

39 :o

.

OldSteve:
To modify global variables from within a function, you need to pass the addresses of the variables to the function.

I think you mean local variables.

aarg:
But, with call by reference, you can choose which global variables are modified when you call the function. If they are referenced directly inside the function, you lose that ability to choose.

I'm trying to think of an example where I'd want to do that since I defined them as globals.

econjack:
I'm trying to think of an example where I'd want to do that since I defined them as globals.

What about if you want to modify different global variables in a function each time it's called, like 'Actuator5ButtonState' in one call, 'Actuator4ButtonState' in another call to that function, etc?

LarryD that code worked perfect. Now I have to review and learn something.

You were correct. I will have 10 valves that need a button to actuate. I am simulating this with an LED for now as the LED operation to turn on is the same as what is needed in my physical circuit to trigger the switch on the valve actuator.

The valves have limit switches in them also so I am going to work on the code to determine if the valve is open or closed... they are slow valves (10 sec to change position).

Thank you all for all your help and a great discussion on variables. It truly illustrates how helpful the community is.

If anyone is curious this is part of the code that I am working on for the Alaska Water Sewer Challenge (found at our website www.reusewaterAK.com). All products of this effort will live in the public domain and I am very excited to get it working. A picture of the system is attached also.

Thanks again!

Alaska!
I don't see any insulation in the walls :fearful:

Better put some water leakage sensors a strategic locations on the floor in case of plastic pipe problems. :wink:

Note: The structure example was offered to show how 'your code' could be made to work.
There are many other ways of doing the same thing, which may or may not be a better.
At some point, however, if things work you go with what you got.

.

OldSteve:
What about if you want to modify different global variables in a function each time it's called, like 'Actuator5ButtonState' in one call, 'Actuator4ButtonState' in another call to that function, etc?

I can see what you're saying. I guess it's just a difference of style where I prefer to encapsulate the data as much as possible. Since they are likely the same data type going in, I'd probably just pass in an index to an array that holds the globals.

econjack:
I can see what you're saying. I guess it's just a difference of style where I prefer to encapsulate the data as much as possible. Since they are likely the same data type going in, I'd probably just pass in an index to an array that holds the globals.

I'd probably do the same.