Need help with my code: fading LED with button

I’m reading “Make: Getting started with Arduino” and am trying to challenge myself to apply and innovate on what I’ve learned so far.

My code is below. When I press the button, the LED doesn’t turn off.
Where have I gone wrong in the code?

const int LED = 9;
const int BUTTON = 12;

int val = 0;
int old_val = 0;
int state = 1;
int i = 0;

void setup() {
  pinMode(LED,OUTPUT);
  pinMode(BUTTON, INPUT);

}

void loop() {
  val = digitalRead(BUTTON);

//Set a system to prevent random responses
  if ((val == HIGH) && (old_val == LOW)) {
    state = 1 - state;
  }

  old_val = val;
//If the button is clicked and was previously off, start the pulsing the LED
  if (state == 1) {
//slowly turning on the LED
    for (i=0; i<255; i++) {
      digitalWrite(LED, i);
      delay(300);  
    }    
//Slowly turning off the LED
    for (i=255; i>0; i--) {
      digitalWrite(LED, i);
      delay(300);
    }
  } 
//If the button is pressed and the previous state was on, turn the LED off 
 else {
    digitalWrite(LED, 0);
  }

}

What do you expect to happen in the for loops?

   for (i = 0; i < 255; i++)
    {
      digitalWrite(LED, i);
      delay(300);
    }

I think you meant to use analogWrite() here and in the other for loop. Also, it is going to take a long while (255 * 300 milliseconds) for each loop to execute. Is that what you want ?

Put comments in your code so we can try to follow your thoughts of what’s happening.

I've updated my code with comments, sorry about that. I was taught to always use comments.

I want to make a program where when I click the button, the LED starts to pulse. When I click it again, it turns off.

Did you see ‘UKHeliBob’ comment about analogWrite() ?

Did you want your code to be locked up until things are finished looping or did you want it responsive to changes in the switch position?

Thanks for the comments.

this is not a comment

//this is a comment

/this is a comment/

Sorry I'm learning Python at the same time, mixed them up.

UKHeliBob was correct. It now pulses like intended.

I want it responsive to changes in the switch position. Right now it doesn't turn off when I push the button, even after the pulse finishes. It seems it's not reaching the off state part of the code

It won’t be responsive until you get rid of the for loops and the delays

//Version 1.02

const byte LED          = 9;     //HIGH turns on the LED
const byte BUTTON       = 12;    //our pin has a pulldown resistor i.e. a switch push/closed gives a HIGH on the pin
const byte heartBeatLED = 13;    //a LED that toggles to show if the sketch is blocking

byte val                = 0;
byte old_val            = 0;
byte state              = 1;

int i                   = 0;
int changeAmount        = 1;     //1 = increment, -1 = decrement

boolean pulseFlag       = false; //false = pulsing disabled

//timing stuff
unsigned long heartBeatMillis;
unsigned long switchMillis;
unsigned long LEDmillis;

const unsigned long heartBeatInterval = 500ul;  //1/2 second
const unsigned long sampleInterval    = 50ul;   //50 milliseconds
//const unsigned long LEDinteval      = 300ul;
const unsigned long LEDinteval        = 3ul;    //3 milliseconds for testing

//*****************************************************************************************************
void setup()
{
  pinMode(heartBeatLED, OUTPUT); //used to monitor sketch for blocking code

  pinMode(LED, OUTPUT);          //LOW = LED ON
  pinMode(BUTTON, INPUT);        //pushed/closed = HIGH

} //END of setup()


//*****************************************************************************************************
void loop()
{
  //*********************************
  //is it time to toggle the 'HeartBeatLED'?
  if (millis() - heartBeatMillis >= heartBeatInterval)
  {
    //restart the timer
    heartBeatMillis = millis();

    //Toggle the LED
    digitalWrite(heartBeatLED, !digitalRead(heartBeatLED));
  }

  //*********************************
  //is it time to scan the switches?
  if (millis() - switchMillis >= sampleInterval)
  {
    //restart the timer
    switchMillis = millis();

    //go and scan for switch state changes
    checkSwitches();
  }

  //*********************************
  //should we adjust the LED brightness?
  if (pulseFlag == true && millis() - LEDmillis >= LEDinteval)
  {
    //restart the timer
    LEDmillis = millis();

    //send the brightness value to the LED
    analogWrite(LED, i);

    //next brightness level
    i = i + changeAmount;

    //are we at the top brightness
    if (i > 255)
    {
      //other direction
      changeAmount = -changeAmount;
      
      i = 254;
    }

    //are we at the bottom brightness?
    else if (i < 0)
    {
      //other direction
      changeAmount = -changeAmount;
      
      i = 1;
    }

  } //END of    if (pulseFlag == true && millis() - LEDmillis >= LEDinteval)

  //*********************************
  //other non blocking code
  //*********************************

} //END of loop()

//*****************************************************************************************************
void checkSwitches()
{
  //*********************************
  //retrieve the current state of the switch
  val = digitalRead(BUTTON);

  //has this switch changed state?
  if (old_val != val)
  {
    //update to the new switch state
    old_val = val;

    //are we 'not pulsing' and has the switch gone HIGH, pushed/closed?
    if (pulseFlag == false && val == HIGH)
    {
      //enable LED pulsing
      pulseFlag = true;
      
      //timer reset to zero
      LEDmillis = millis();
    }
    
    else if (pulseFlag == true && val == HIGH)
    {
      //disable pulsing on the LED
      pulseFlag = false;

      //turn OFF the LED
      digitalWrite(LED, LOW);

      //reset to initial conditions
      i = 0;
      changeAmount = 1;
    }

    //must have gone from HIGH to LOW then
    else
    {
      //do nothing
    }

  } //END of     if (lastSW1State != currentState)


  //*********************************
  //future switches go here
  //*********************************

} //END of   checkSwitches()



//*****************************************************************************************************
//                           E N D   o f   s k e t c h   c o d e
//*****************************************************************************************************

Wow. That is way beyond my current knowledge haha.
I’ll learn from going through it.

Is it really that code intensive to accomplish a simple task through the arduino?

“ Is it really that code intensive to accomplish a simple task through the arduino?”

Most of the code is reusable in others sketches. copy and paste ;)

When you want a sketch to do ‘multiple things at once’, i.e. make it ‘non blocking’, and you want switches to be ‘debounced’, you do have to follow certain techniques.

These techniques can be placed into reusable ‘functions’ and/or libraries so once proven to work, you do not have to reinvent the wheel.

If there are things you do not understand, it is up to you to ask for an explanation, lots of volunteers here ready to help.

"Is it really that code intensive to accomplish a simple task through the arduino?"

Use the google forum search box in the upper right and search for key project words like "dim led" and "button". also study the examples in the IDE examples for similar code to develop from>