Begginer trying to understand

Hello All,

I'm playing around with code to try and understand how arduino works.

I'm currently working on a simple toggle switch.
I've seen code example where this works but I am trying to write my own so that I better understand the system.

Why does the following code not let a push button behave as a toggle switch that either turns a led on to flash or stops it flashing.

int buttonPin = 2;
int buttonState = LOW;
int toggleOn = 0;

// the setup function runs once when you press reset or power the board
void setup() {
  // initialize digital pin 13 as an output.
  pinMode(13, OUTPUT);
  pinMode(buttonPin, INPUT);
}

// the loop function runs over and over again forever
void loop() {
  
  buttonState = digitalRead(buttonPin);
  if (buttonState == HIGH){ 
    if (toggleOn == 0){
      toggleOn = 1;
    }
    else {
      toggleOn = 0;
    }
  } 
  

  if(toggleOn == 1){
    digitalWrite(13, HIGH);   // turn the LED on (HIGH is the voltage level)
    delay(300);              // wait for a second
    digitalWrite(13, LOW);    // turn the LED off by making the voltage LOW
    delay(300);              // wait for a second
  }
}

Thanks fro you time in helping a beginner.

Because if you hold down the button it will constantly be HIGH and toggleOn will constantly switch between 1 and 0. Don’t update toggleOn unless buttonState is LOW again (signifying the button has been released, and can be pressed again.)

I made some comments for you in the following code. You want to look for a state change with your buttonPin… the moment when it goes from LOW to HIGH or HIGH to LOW.

int buttonPin = 2;
int lastButtonState;
int toggleOn = 0;

void setup() 
{
  pinMode(13, OUTPUT);
  pinMode(buttonPin, INPUT);
}

void loop() 
{
  int buttonState = digitalRead(buttonPin); // you only need to record the state for the following block of code, so a local variable is OK here
  if (buttonState == HIGH && lastButtonState == LOW) // you want to capture the moment that the buttonState chanes from LOW to HIGH  you need a GLOBAL variable (lastButtonState)
  { 
    if (toggleOn == 0)
    {
      toggleOn = 1;
    }
    else 
    {
      toggleOn = 0;
    }
  } 
  lastButtonState = buttonState;  // record the last reading of the last time the pin was checked on the first line of this block of code
  if(toggleOn == 1)
  {
    digitalWrite(13, HIGH);   // turn the LED on (HIGH is the voltage level)<<<// HIGH is the digital output equivalent to 1
    delay(300);              // wait for a second<<<// more like 300/1000ths of a second
    digitalWrite(13, LOW);    // turn the LED off by making the voltage LOW<<<//actually, you are making the voltage approximately equal to ground
    //delay(300);              // wait for a second<< why bother with this... off is off, so what are you waiting for?
  }
}

a couple other ways to do this block:

if (toggleOn == 0)
    {
      toggleOn = 1;
    }
    else 
    {
      toggleOn = 0;
    }

for example:

toggleOn = 1 - toggleOn;

or:

toggleOn = !toggleOn;

or the ternary operator:

toggleOn = toggleOn? 0 : 1;

Thanks Guys!

BulldogLowell, oddly enough the light would not flicker if I did not have the last delay you commented out?!?!
Weird when I saw what you did it made sense. This is certainly a slightly different way to think when programming hopefully I'll get used to it soon.

I also noticed the read of the button press does not work for quick presses. I assume that the sampling rate of the loop is not quick enough? Or is it something else? Maybe the delay? maybe not a god way to make an led flash, I assume the delay pauses the loop and stops it reading more input? IS that what PWM is better for?

Sorry lots of questions just trying to get my way of thinking right so that this programming comes more natural to me.

3DFun:
BulldogLowell, oddly enough the light would not flicker if I did not have the last delay you commented out?!?!
Weird when I saw what you did it made sense. This is certainly a slightly different way to think when programming hopefully I’ll get used to it soon.

I think your switch needs to be de-bounced.

you should learn about functions as in this modified example:

I cannot test it so take a look. practice changing the delay(50) to get your pushbutton under control

int buttonPin = 2;
int lastButtonState;
int toggleOn = 0;

void setup() 
{
  pinMode(13, OUTPUT);
  pinMode(buttonPin, INPUT);
}

void loop() 
{
  int buttonState = digitalRead(buttonPin);
  delay(50);// you can put this here to 'debounce' your switch.  search the forum for debouncing, it has to do with the electrical connecting bridging a closing physical gap... the signal can make/break many times in just a few milliseconds
  if (buttonState == HIGH && lastButtonState == LOW)
  { 
    toggleOn = 1 - toggleOn;//  I used this one because you declared toggleOn an int... math and numbers
  } 
  lastButtonState = buttonState;
  if(toggleOn)
  {
    blinkyBlinky(3); // three times in this example of a function
  }
}

void blinkyBlinky(int blinkCount)  //wrtten with 'blocking code'  this will 'block' the loop() function from detecting a button press... usually undesireable
{
  for (int i = 0; i < blinkCount; i++)
  {
    digitalWrite(13, HIGH);
    delay(300); 
    digitalWrite(13, LOW);
    delay(300);
  }
}

I also noticed the read of the button press does not work for quick presses.

Probably because it is not reading the input when it is held up doing the delay()

Thanks guys!
I’m very happy that I choose Arduino to play with, a big part of it was the online community and you guys have shown me how true that is!

Thank again!

Hopefully soon I’ll be good enough too to help out.

A question regarding the use of local variables...

I was taught in programming that its bad to have variables declared in loops due to this possible meaning that inefficient code is generated as the variable would need to be created and destroyed with each loop cycle.

This obviously depends on the environment and compiler.

Does this rule apply to Arduino and its compiler?

3DFun:
A question regarding the use of local variables...

I was taught in programming that its bad to have variables declared in loops due to this possible meaning that inefficient code is generated as the variable would need to be created and destroyed with each loop cycle.

This obviously depends on the environment and compiler.

Does this rule apply to Arduino and its compiler?

It depends on the complexity of the datatype of the variable.
It's inefficient if you have to call a complex constructor, but for integer datatypes, no problem.
You should always aim to keep the scope of any variable as small as possible.

Thanks Awol!

This is lots of fun.
I’ve now got it working without using the blocking delay and using a function call.
Could do with some cleaning, a thought about efficiency, maybe a parameter for the LED PIN and defiantly better code naming conventions…
but in case anyone is interested here is the function:

void pulseLED( long pDurationMS)
{
  static long lastMSValue = 0;
  long leaveOnDuration = pDurationMS;
  long currentMSValue = millis();

  if( lastMSValue + pDurationMS < currentMSValue){
    digitalWrite(13, HIGH);
    if(lastMSValue + pDurationMS + leaveOnDuration < currentMSValue) //let it stay on for some time.
      lastMSValue = currentMSValue;   
  }
  else
  {
    digitalWrite(13, LOW);
  }

}
  if( lastMSValue + pDurationMS < currentMSValue){

It may not cause you a problem in this program but it is better to subtract the start time of the event from the current time and compare it with the required interval.
if (currentMSValue - lastMSValue >= pDurationMS)Using unsigned long variables and doing it this way means that when millis() rolls over to zero after 49 and bit days the calculation will still produce the correct answer.