Code changing a variable in unexplained way

Hi Folks,
I'm very new to C++, but have experience of BASIC.
I have started to run through some examples of sketches and then attempt to 'tweak' them, in order to modify their function.

I loaded an example sketch, that turns on the pin 13 led when an external push button is pressed, and turns off the led when button released. I initially modified the sketch to remove the 'turn led off' part of the code, (which worked) and then tried to add code which would turn the led off after a delay of 50 seconds. (which didn't)

What happens now, is that the led comes on immediately the sketch is run i.e button not pressed, and stays on after the count
reaches 5000.

Any help would be greatly appreciated.

/*
  Button
 
 Turns on and off a light emitting diode(LED) connected to digital  
 pin 13, when pressing a pushbutton attached to pin 2. 
 
 
 The circuit:
 * LED attached from pin 13 to ground 
 * pushbutton attached to pin 2 from +5V
 * 10K resistor attached to pin 2 from ground
 
 * Note: on most Arduinos there is already an LED on the board
 attached to pin 13.
 
 
 created 2005
 by DojoDave <http://www.0j0.org>
 modified 30 Aug 2011
 by Tom Igoe
 
 This example code is in the public domain.
 
 http://www.arduino.cc/en/Tutorial/Button
 */

// constants won't change. They're used here to 
// set pin numbers:

const int buttonPin = 2;     // the number of the pushbutton pin
const int ledPin =  13;      // the number of the LED pin
             
// variables will change:
int buttonState = 0;         // variable for reading the pushbutton status
int offski = 0;              // number of times through loop
void setup() {
  Serial.begin(9600);
  // initialize the LED pin as an output:
  pinMode(ledPin, OUTPUT);      
  // initialize the pushbutton pin as an input:
  pinMode(buttonPin, INPUT);     
}

void loop(){
  // read the state of the pushbutton value:
  buttonState = digitalRead(buttonPin);

  // check if the pushbutton is pressed.
  // if it is, the buttonState is HIGH: 
  if (buttonState == HIGH);      
    // turn LED on:    
    digitalWrite(ledPin, HIGH);
  
  /* start a loop counter delayed 10mS per count. Increment 
  variable 'offski' by one for every iteration of the loop,
  when 5000 loops have been counted, turn off the led*/
  
  delay (10); offski ++; 
  if (offski == 5000);
   digitalWrite(ledPin, LOW);
  Serial.println(offski);

}

Get rid of any resistor and wires on Pin 2.
Wire the button so that when pressed, pin 2 is connected to Gnd.

Change your code:

pinMode(buttonPin, INPUT_PULLUP); // enables internal pullup resistor

// check if the pushbutton is pressed.
// if it is, the buttonState is LOW:
if (buttonState == LOW); // Look to see if the button was pressed

Is your pin 2 wired just as described in the comments?

Yes Mark, 10K holding pin 2 low, push button from pin 2 to +5V.

Have you checked the voltage on pin 2 with a multimeter - it seems to be reading HIGH.

Thanks CrossRoads. will try that in the morning, as it's getting close to 'beer O' clock' on this side of the pond.

Would still like to understand the reason my version doesn't work though.

  if (buttonState == HIGH);

There's a semi-colon that should not be there in that fragment. Same thing further down too.

When using if statements...

ALWAYS use the {}

bad:

if (something);
  code; //this will always execute regardless of the if() because of the ;
  code; //this will always execute - could cause confusion

bad:

if (something)
  code; //the if controls this line (no ;)
  code; //this will always execute - could cause confusion

good :slight_smile: :

if (something){
  code; //the if controls this line
  code; //the if controls this line 
}

Many, many thanks Tom. The sketch now works as expected, with your correction.

Coming from a background in BASIC programming, I find the 'curly braces' very confusing.
I have searched online, but cannot seem to find a definitive guide to their usage.

{...} specifies a 'scope' which is basically a group of code. So for example:

void someFunction() {
  //This is in the scope of someFunction
  if (...) {
    //This is in the scope of the if() statement
  }
  {
    //This creates a new scope which will always be entered
  }
}
//This is not in the scope of someFunction

Now more importantly, an example of why 'scopes' are used:

int a;
void someFunction() {
  int b;
  ...
  if (...) {
    int c;
    //variables 'a', 'b' and 'c' can be seen here
  }
  {
    int d;
    //variables 'a', 'b' and 'd' can be seen here (not 'c').
  }
  //only variable 'a' and variable 'b' can be seen here
}
//only variable 'a' can be seen here

There are different ways to enter a scope, calling a function is one, a while loop is another, an if statement a third and so on. The statement which comes before the open brace ( { ) is the statement which controls entry into the scope - although this is optional, as shown in my example above. So for an if(){} statement, the scope is entered if the if statement is true.

Hopefully that makes sense.

Thanks for taking the time to explain that, Tom. It was the clearest explanation I have seen.
I think I've got my head around it now.