Arduino UNO button LED brightness vary

Question

I am using an Arduino UNO and I am trying to vary the brightness by steps by 0.5 volts till it gets to a max of 5.0 V using buttonA and decrease the brightness of the LED by 0.5 volts with buttonB. The Problem I have is that when I push buttonA the LED steps it's voltage by 0.5V all the way to 5.0V and with buttonB its steps down to 0. I would like to have the LED step each time I press the button not all at once. Here is my code that I have so far can someone help me with telling the Arduino to Step each time a button is pressed. CODE IS DIFFERENT from another post I did

HERE is my code:

int ledPin = 9; // must be a PWM digital pin
int buttonApin = A2; //buttonA to analog 2
int buttonBpin = A1; // buttonB to analog 1
int maxBright = 255; // between 0 and 255
int ms = 1500;    // delay in milliseconds
int buttonPushCounterA = 0; //counter for the number of button presses
int buttonStateA = 0; // current state of the button
int lastButtonStateA = 0; // previous state of the button
int buttonPushCounterB = 0; //counter for the number of button presses
int buttonStateB = 0; // current state of the button
int lastButtonStateB = 0; // previous state of the button

void setup() 
{
  pinMode(ledPin, OUTPUT);
  pinMode(buttonApin, INPUT_PULLUP);
  pinMode(buttonBpin, INPUT_PULLUP);
}

void loop() 
{
  if (digitalRead(buttonApin) == LOW) //if buttonA is pushed
   {
      buttonStateA = digitalRead(buttonApin);

     if (buttonStateA != lastButtonStateA)
     {
        if(buttonStateA == LOW)
     {
        buttonPushCounterA++;
     }
    }

// fade from min to max in increments of 25.5 points: basically (0.5 volts)
for (float n = 0; n <= maxBright; n +=25.5) 
{
 // sets the value (range from 0 to 255):
 analogWrite(ledPin,n);
 delay(ms);
}
  }

  if (digitalRead(buttonBpin) == LOW) //if buttonB is pushed
  {
     buttonStateB = digitalRead(buttonBpin);

       if (buttonStateB != lastButtonStateB)
         {
            if(buttonStateB == LOW)
              {
                buttonPushCounterB--;
              }
         }


     // fade from max to min in increments of 25.5 points: basically (0.5 volts)
     for (float n = maxBright; n >= 0; n -= 25.5) 
    {
     // sets the value (range from 0 to 255):
     analogWrite(ledPin, n);
     delay(ms);
    }
  } 

  lastButtonStateA = buttonStateA;
  lastButtonStateB = buttonStateB;
 }

Answer 1

One problem in your original code is that you are only storing the current state of the buttons if they are pressed. The next problem is that you are looping through all your brightness values when a button is pressed giving you your "fades all the way up in one go" problem.

This would be my replacement for your loop() code:

loop()
{
    int currentButtonStateA = digitalRead(buttonApin);
    if (currentButtonStateA != lastButtonStateA && currentButtonStateA == LOW)
    {
        currentBrightness += BRIGHTNESS_INCREMENT;  // BRIGHTNESS_INCREMENT is defined at the top of the file with #define BRIGHTNESS_INCREMENT 25
        if (currentBrightness > MAX_BRIGHTNESS)     // MAX_BRIGHTNESS is defined at the top of the file with #define MAX_BRIGHTNESS 255
        {
            currentBrightness = MAX_BRIGHTNESS;
        }
    }
    lastButtonStateA = currentButtonStateA;

    int currentButtonStateB = digitalRead(buttonBpin);
    if (currentButtonStateB != lastButtonStateB && currentButtonStateB == LOW)
    {
        currentBrightness -= BRIGHTNESS_INCREMENT;
        if (currentBrightness < MIN_BRIGHTNESS)     // MIN_BRIGHTNESS is defined at the top of the file with #define MAX_BRIGHTNESS 0
        {
            currentBrightness = MIN_BRIGHTNESS;
        }
    }
    lastButtonStateB = currentButtonStateB;

    analogWrite(ledPin, currentBrightness);
}

Hopefully this is pretty self explanatory. The current button A state is read and stored in a local variable. If it is different to the previous state and it is low, the current brightness is incremented and capped at a maximum value. Next the current button state is stored as the previous state ready for the next run through the loop.

Similar actions are performed for the B button, but with a decrement rather than an increment. Finally the LED brightness is set regardless of whether any buttons have been pressed.

You do not need to store your current button states (buttonStateA) or your counts for the number of button presses (buttonPushCounterA - which you never referenced anyway) or your ms value so could get rid of those global variables. You need a new global variable at the top of your file for currentBrightness (int currentBrightness = 0;).

You have used variables to store various non-changing numeric values, such as "maxBright = 255;" which is good practise, but is better (particularly with Arduino development) to use #define macros instead, such as "#define MAX_BRIGHTNESS 255".

The reason for this is that variables take up memory when the code is running - something that an Arduino Uno doesn't have a lot of, whereas a #define takes up no memory - the compiler uses it as a "search and replace" before compiling your code.

Answer 2

You forgot to check to see if the button was actually pushed or if it was just held down. See the State Change Detection example to understand how to perform edge detection.