Led cycling with level

Hi everyone.

I am currently working on a project, where I want to make a nightlight for my daughter using my Uno R3.

The idea is to have two buttons, one cycles states = off, 1 led, 2 leds, 3 leds etc, and the other cycles brightness of all currently active leds.

I have tried several different solutions found here and elsewhere on the interwebs, but I feel I'm turning blind to the holes in my code.

I found a code on here that cycles between the leds one by one, so I know the hardware is working, but I couldn't find a way to modify it for my purpose, so now I've tried this, but all that happens now is that all leds turn on and nothing else.

//Variables for LED selection
int led = 1;            //Set variable led to 1
int prevUp = HIGH;      //Set variable prevUp to High

//Variables for brightness setting
int prevUp2 = HIGH;  //set variable prevUp2 to High
int level = 1; //set variable level to 1
int brightness = 64; //set variable brightness to 64


const int buttonPin = 8;
const int modePin = 2;

const int led1pin = 3;
const int led2pin = 5;
const int led3pin = 6;
const int led4pin = 9;
const int led5pin = 10;
const int led6pin = 11;

void setup() {
 //set buttons to input
 pinMode (buttonPin, INPUT);
 pinMode (modePin, INPUT);
 //turn leds off at boot
 analogWrite(led1pin, 0);
 analogWrite(led2pin, 0);
 analogWrite(led3pin, 0);
 analogWrite(led4pin, 0);
 analogWrite(led5pin, 0);
 analogWrite(led6pin, 0);
}

void loop() {

 int currUp = digitalRead(buttonPin);
 if (currUp != prevUp)
 {
   if (currUp == LOW)
     led++;
   if (led > 7) led = 1;
 }
 prevUp = currUp;

 int currUp2 = digitalRead(modePin);
 if (currUp2 != prevUp2)
 {
   if (currUp2 == LOW)
     level++;
   if (level > 7) level = 1;
 }
 prevUp2 = currUp2;

 switch (led) {
   case 1:
     {
       analogWrite(led1pin, brightness);

     }
   case 2:
     {
       analogWrite(led1pin, brightness);
       analogWrite(led2pin, brightness);
     }
   case 3:
     {
       analogWrite(led1pin, brightness);
       analogWrite(led2pin, brightness);
       analogWrite(led2pin, brightness);
     }
   case 4:
     {
       analogWrite(led1pin, brightness);
       analogWrite(led2pin, brightness);
       analogWrite(led3pin, brightness);
       analogWrite(led4pin, brightness);
     }
   case 5:
     {
       analogWrite(led1pin, brightness);
       analogWrite(led2pin, brightness);
       analogWrite(led3pin, brightness);
       analogWrite(led4pin, brightness);
       analogWrite(led5pin, brightness);
     }
   case 6:
     {
       analogWrite(led1pin, brightness);
       analogWrite(led2pin, brightness);
       analogWrite(led3pin, brightness);
       analogWrite(led4pin, brightness);
       analogWrite(led5pin, brightness);
       analogWrite(led6pin, brightness);
     }
     break;
   default:
     analogWrite(led1pin, brightness);
     break;
 }

 switch (level) {
   case 1:
     {
       int brightness = 0;
     }
   case 2:
     {
       int brightness = 16;
     }
   case 3:
     {
       int brightness = 32;
     }
   case 4:
     {
       int brightness = 64;
     }
   case 5:
     {
       int brightness = 128;
     }
   case 6:
     {
       int brightness = 255;
     }
     break;
   default:
     int brightness = 32;
     break;
 }
}
  switch (level) {
    case 1:
      {
        int brightness = 0;
      }
    case 2:
      {
        int brightness = 16;
      }
    case 3:
      {
        int brightness = 32;
      }
    case 4:
      {
        int brightness = 64;
      }
    case 5:
      {
        int brightness = 128;
      }
    case 6:
      {
        int brightness = 255;
      }
      break;
    default:
      int brightness = 32;
      break;
  }

What is the point of defining a local variable, assigning it a value, and letting it go out of scope immediately?

led is a lousy name for a counter. If you put the pin numbers in an array, you could use index as the name, and then the value as the index into the array, to determine which pin(s) to diddle with.

Well, the variable "brightness" is supposed to be between 0 and 255, as it is the brightness of the leds, so it is as far as I understand you, not really out of scope. The value I put in there was just to put something in it.

I agree it is a poor name for a counter, but that bit was copied from another program, so I just kept the values for ease of use for now.

I tried to use an array, and the functioning program I mentioned does use that to cycle the leds, but I had problems turning multiple leds on or off using that. I also tried to use an array for the brightness,, but again same issue.

Scope, in this context, has nothing to do with the values that a variable can hold. Scope of a variable is the "lifetime" of the variable. A variable that is declared inside a function only "lives" in that function. When the function ends, the varible's life ends.

scope

So basically, he was telling me that it was useless of me (But not a cause of my problem) to add an actual value to the variable during the start of my code, I should have just said "int brightness;"

Which does not really help me at all to be honest..

And to be clear, I'm not looking for an easy way out and a finished solution here, I'm looking for good pointers in the right direction of actually building my code for this project.

6 LED's, activating in sequence as "OFF, 1 LED, 2 LED's, 3 LED's etc" by the press of button number one, while repeated presses of button number to cycles the brightness of these LED's.

Here's an attempt I made at using an array for changing the brightness at the press of a button (Using the Knight Rider code as base), but nothing happens.

/* Led Cycle 
   Based on Knight Rider array code by author: David Cuartielles
*/
const int buttonPin = 8;

int levelArray[] = {0, 2, 4, 8, 16, 32, 64};
int pinArray[] = {3, 5, 6, 9, 10, 11};
int count = 0;
int count2 = 0;
int timer = 15;
int level = 4;

void setup() {
  for (count = 0; count < 6; count++)
    for (count2 = 0; count2 < 7; count2++)
          {
      pinMode(pinArray[count], OUTPUT);
      pinMode (buttonPin, INPUT);
    }
}

void loop() {

    if (digitalRead(buttonPin), HIGH);
       { count2 + 1;
       }
    if (count2==7);
      {count2==0;
      }
  
  
  for (count = 0; count < 5; count++) {
    analogWrite(pinArray[count], levelArray[count2]);
    delay(timer);
    analogWrite(pinArray[count + 1], levelArray[count2]);
    delay(timer);
    digitalWrite(pinArray[count], LOW);
    delay(timer * 2);
  }
  for (count = 5; count > 0; count--) {
    analogWrite(pinArray[count], levelArray[count2]);
    delay(timer);
    analogWrite(pinArray[count - 1], levelArray[count2]);
    delay(timer);
    digitalWrite(pinArray[count], LOW);
    delay(timer * 2);
  }

}

I have some questions and comments

[pre][code]/* Led Cycle 
   Based on Knight Rider array code by author: David Cuartielles
*/
const int buttonPin = 8; // use byte, value is <= 255.  How is this wired? With pulldown resistor, and button press to +5?

int levelArray[] = {0, 2, 4, 8, 16, 32, 64}; // use byte, value is <= 255
int pinArray[] = {3, 5, 6, 9, 10, 11}; // use byte, value is <= 255
int count = 0;// use byte, value is and will be <= 255
int count2 = 0;// use byte, value is and will be <= 255
int timer = 15; // use byte, value is and will be <= 255
int level = 4;

void setup() {
  for (count = 0; count < 6; count++)
    for (count2 = 0; count2 < 7; count2++) // what is count2 doing here? It is not called
          {
      pinMode(pinArray[count], OUTPUT); 
      pinMode (buttonPin, INPUT); // doesn't need to be in these loops
    }
}

void loop() {

    if (digitalRead(buttonPin), HIGH);
       { count2 + 1; // syntax? count2 = count2 +1;
       }
    if (count2==7);
      {count2==0; // just 1 = here
      }
  
  
  for (count = 0; count < 5; count++) { 
    analogWrite(pinArray[count], levelArray[count2]);
    delay(timer); // 15mS delay
    analogWrite(pinArray[count + 1], levelArray[count2]);
    delay(timer); // 30mS delay
    digitalWrite(pinArray[count], LOW); // so first one called will only be on for a blip, barely noticable
    delay(timer * 2); // then the 2nd one about the same, just a blip
  }
  for (count = 5; count > 0; count--) {
    analogWrite(pinArray[count], levelArray[count2]);
    delay(timer);
    analogWrite(pinArray[count - 1], levelArray[count2]);
    delay(timer);
    digitalWrite(pinArray[count], LOW); // same again, barely on
    delay(timer * 2);
  }

}

[/code]

gbremset:
So basically, he was telling me that it was useless of me (But not a cause of my problem) to add an actual value to the variable during the start of my code, I should have just said "int brightness;"

Which does not really help me at all to be honest..

No, what he said is BUG in your code. In the linked switch statement you create new local variable brightness which is destroyed as soon as you break out of the case. You must use
brightness = xxx;
instead of
int brightness = xxx;

Just to answer your questions below:

Yes, Pulldownresistor with Button press to +5

The original code that this is based on, is a Knight Rider cycle (Number 3 in the list here on arduino.cc), and I have attempted to amend it so I can change brightness of LED's by pressing button, cycling through levels of brightness from levelArray, while cycle is running. (To attempt to work out the code needed for the brightness adjustment for the project)
So this is in practice a dimmable knight rider LED cycle.

I've added the button, and "count2", and what I was thinking it would do, was to pick number "count2" from the "levelArray", and then set brightness to that level.

[pre][code]/* Led Cycle 
   Based on Knight Rider array code by author: David Cuartielles
*/
const int buttonPin = 8; // use byte, value is <= 255.  How is this wired? With pulldown resistor, and button press to +5? - Yes

int levelArray[] = {0, 2, 4, 8, 16, 32, 64}; // use byte, value is <= 255
int pinArray[] = {3, 5, 6, 9, 10, 11}; // use byte, value is <= 255
int count = 0;// use byte, value is and will be <= 255
int count2 = 0;// use byte, value is and will be <= 255
int timer = 15; // use byte, value is and will be <= 255
int level = 4;

void setup() {
  for (count = 0; count < 6; count++)
    for (count2 = 0; count2 < 7; count2++) // what is count2 doing here? It is not called
          {
      pinMode(pinArray[count], OUTPUT); 
      pinMode (buttonPin, INPUT); // doesn't need to be in these loops
    }
}

void loop() {

    if (digitalRead(buttonPin), HIGH);
       { count2 + 1; // syntax? count2 = count2 +1;
       }
    if (count2==7);
      {count2==0; // just 1 = here
      }
  
  
  for (count = 0; count < 5; count++) { 
    analogWrite(pinArray[count], levelArray[count2]);
    delay(timer); // 15mS delay
    analogWrite(pinArray[count + 1], levelArray[count2]);
    delay(timer); // 30mS delay
    digitalWrite(pinArray[count], LOW); // so first one called will only be on for a blip, barely noticable
    delay(timer * 2); // then the 2nd one about the same, just a blip
  }
  for (count = 5; count > 0; count--) {
    analogWrite(pinArray[count], levelArray[count2]);
    delay(timer);
    analogWrite(pinArray[count - 1], levelArray[count2]);
    delay(timer);
    digitalWrite(pinArray[count], LOW); // same again, barely on
    delay(timer * 2);
  }

}

No, what he said is BUG in your code. In the linked switch statement you create new local variable brightness which is destroyed as soon as you break out of the case. You must use
brightness = xxx;
instead of
int brightness = xxx;

Thank you for clarifying this, I'll look into it and try again.

I found a code on here that cycles between the leds one by one, so I know the hardware is working, but I couldn't find a way to modify it for my purpose, so now I've tried this, but all that happens now is that all leds turn on and nothing else.
[/quote]

Tell me more about your button circuit!

In your code you are setting the buttons to pinMode INPUT, not INPUT_PULLUP.

//set buttons to input
pinMode (buttonPin, INPUT);
pinMode (modePin, INPUT);

Therefore I'm assuming you are using an "external pull-down resistor" with each button.

Am I right? Or not?

If not, your hardware is not working, most likely, and your digitalRead() results are floating, maybe.

Why don't you just activate the internal pull-up resistor of the controller instead using external pull-down resistors? Cabling would be much easier due to less components in your circuit.

Besides of that: No matter whether your buttons use pinMode INPUT and external pull-down resistors or pinMode INPUT_PULLUP and activating internal pull-ups without extra resistor and extra cabling, this thing is easy to program.

I'd put the brightness steps into an array, maybe 8 steps of brightness:

const byte pwmtable[8] ={0, 4, 8, 16, 32, 64, 128, 255};

What do you think?

8-bit PWM CIE equal brightness steps commonly used.

prog_uint8_t CIEL8[] PROGMEM = {0, 1, 2, 3, 4, 5, 7, 9, 12, 15, 18, 22, 27, 32, 38, 44, 51, 58, 67, 76, 86, 96, 108, 120, 134, 148, 163, 180, 197, 216, 235, 255};

Leo..

Im a total noob compared to most on this thread.... however, I've written something similar recently for a bike light.

You need to track three states,

State 1: Number of LED's active
State 2: brightness
State 3: button

You can either track them using enums, or even a single byte value for each state would work. The relevant button activity can then be used to either increment the byte value, or switch modes (switch/case statement) for the enums.

Brightness values need to be in an array and are NOT linear (see the table Wawa posted above). If you want to generate your own values, take a peak here: Gamma Calc

I used 5% increments in an array since I was generating pulse effects etc;

// gamma corrected value lookup table: more memory efficient and faster than float maths. external calc of gamma using GF 2.2. power in 5% increments.
const byte gamma [] = {0, 1, 2, 4, 7, 12, 18, 25, 34, 44, 56, 68, 83, 99, 117, 135, 156, 179, 203, 227, 255} ;

Does that help?

PS. Red nightlights are generally seen as better for preservation of night vision and reduced stimulus.

Yes, I am using a 10k pullup resistor, and the button is working, as I use the same setup for several different codes, working fine, but when I try to use it for the code I am trying to figure out through this post, something is wrong (But not with the button circuit)

Here is a link to the post where I got the first code I used, post number 5 and from which the circuit is set up, tested and working (Obviously not the mode button, as it does nothing in this code)

http://forum.arduino.cc/index.php?topic=128770.0

I will most definitely use the pullup function instead, but for now I have the setup on my breadboard as it is, so I'll just leave it for now.

The array for brightness I have tried to use in my last posted code, (From the knight rider example) is from testing the actual visual levels of the LED's I am using, as I found this better than just using a table of known levels.

As for the colors, I will see how I do it, but I will use LED's to form the most comfortable light.

Version 2.0 will also have a RTC module, and so will have time added into the mix, so if it's bedtime, the light will turn off automatically after X minutes, and only lowest mode will be available.

jurs:
Tell me more about your button circuit!

In your code you are setting the buttons to pinMode INPUT, not INPUT_PULLUP.

//set buttons to input
pinMode (buttonPin, INPUT);
pinMode (modePin, INPUT);

Therefore I'm assuming you are using an "external pull-down resistor" with each button.

Am I right? Or not?

If not, your hardware is not working, most likely, and your digitalRead() results are floating, maybe.

Why don't you just activate the internal pull-up resistor of the controller instead using external pull-down resistors? Cabling would be much easier due to less components in your circuit.

Besides of that: No matter whether your buttons use pinMode INPUT and external pull-down resistors or pinMode INPUT_PULLUP and activating internal pull-ups without extra resistor and extra cabling, this thing is easy to program.

I'd put the brightness steps into an array, maybe 8 steps of brightness:

const byte pwmtable[8] ={0, 4, 8, 16, 32, 64, 128, 255};

What do you think?

This is the code I'm working with at the moment, and it works perfectly as is, meaning it cycles between the 6 LED's one by one when button is pressed. (I have tested both buttons by changing the buttonpins, so I know the hardware is working)

The main functionality (Cycle something when button is pressed) is what I'm looking for in my final project.

But I'm wondering if there's a way to change this around to not cycle the LED's but turn them on one by one instead? I've been trying different solutions, as shown in this thread, but I seem to get lost in this bit of code: (Can't figure out really what this does)

void turnOnLED (const byte which, const byte brightness)
{
for (byte i = 0; i < pinCount; i++)
analogWrite (ledPins , 0);

  • if (which > 1)*
  • analogWrite (ledPins [which - 2], brightness); // make "which" zero-relative*
  • } // end of turnOnLED*
    ```
    *const byte pinCount = 6;
    const byte ledPins [pinCount] = { 3, 5, 6, 9, 10, 11 };

const int buttonPin = 8;
const int modePin = 2;

int led = 1;

int prevUp = HIGH;

void setup()
  {
  pinMode (buttonPin, INPUT);
  pinMode (modePin, INPUT);
  }  // end of setup

void turnOnLED (const byte which, const byte brightness)
  {
  for (byte i = 0; i < pinCount; i++)
    analogWrite (ledPins [i], 0);
 
  if (which > 1)
    analogWrite (ledPins [which - 2], brightness);  // make "which" zero-relative
  }  // end of turnOnLED
 
void loop()
  {
  if( digitalRead(modePin == HIGH)){

int currUp = digitalRead(buttonPin);
    if(currUp != prevUp)
    {
      if(currUp == LOW)
        led++;
      if (led >7) led = 1;
    }
    prevUp = currUp;
  }

else{
    int currUp = digitalRead(buttonPin);  //set currUp to status of buttonpin
    if(currUp != prevUp)  //check if currUp is absolutely equal to prevUp to continue
    {
      if(currUp == LOW)  //check if currUp is LOW to continue
        //Statement that will be executed in above case
        if (led > 7) led = 1;
    }
    prevUp = currUp;
  }

turnOnLED (led, 255);
 
}  // end of loop*
```

After a few pointers, I now got a much more functioning code for this.

It's still not running stable, but at least it behaves more like I expected.

//Variables for LED selection
int led = 1;            //Set variable led to 1
int prevUp = HIGH;      //Set variable prevUp to High

//Variables for brightness setting
int prevUp2 = HIGH;  //set variable prevUp2 to High
int level = 1; //set variable level to 1
int brightness = 64; //set variable brightness to 64


const int buttonPin = 8;
const int modePin = 2;

const int led1pin = 3;
const int led2pin = 5;
const int led3pin = 6;
const int led4pin = 9;
const int led5pin = 10;
const int led6pin = 11;

void setup() {
  //set buttons to input
  pinMode (buttonPin, INPUT);
  pinMode (modePin, INPUT);
  
  //turn leds off at boot
  analogWrite(led1pin, 0);
  analogWrite(led2pin, 0);
  analogWrite(led3pin, 0);
  analogWrite(led4pin, 0);
  analogWrite(led5pin, 0);
  analogWrite(led6pin, 0);
  
}

void loop() {

  int currUp = digitalRead(buttonPin);
  if (currUp != prevUp)
  {
    if (currUp == LOW)
      led++;
    if (led > 7) led = 1;
  }
  prevUp = currUp;

  int currUp2 = digitalRead(modePin);
  if (currUp2 != prevUp2)
  {
    if (currUp2 == LOW)
      level++;
    if (level > 7) level = 1;
  }
  prevUp2 = currUp2;

  switch (led) {
    case 1:
      {
        analogWrite(led1pin, brightness);

      }
      break;
    case 2:
      {
        analogWrite(led1pin, brightness);
        analogWrite(led2pin, brightness);
      }
      break;
    case 3:
      {
        analogWrite(led1pin, brightness);
        analogWrite(led2pin, brightness);
        analogWrite(led2pin, brightness);
      }
      break;
    case 4:
      {
        analogWrite(led1pin, brightness);
        analogWrite(led2pin, brightness);
        analogWrite(led3pin, brightness);
        analogWrite(led4pin, brightness);
      }
      break;
    case 5:
      {
        analogWrite(led1pin, brightness);
        analogWrite(led2pin, brightness);
        analogWrite(led3pin, brightness);
        analogWrite(led4pin, brightness);
        analogWrite(led5pin, brightness);
      }
      break;
    case 6:
      {
        analogWrite(led1pin, brightness);
        analogWrite(led2pin, brightness);
        analogWrite(led3pin, brightness);
        analogWrite(led4pin, brightness);
        analogWrite(led5pin, brightness);
        analogWrite(led6pin, brightness);
      }
      break;

  }

  switch (level) {
    case 1:
      {
        brightness = 0;
      }
      break;
    case 2:
      {
        brightness = 16;
      }
      break;
    case 3:
      {
        brightness = 32;
      }
      break;
    case 4:
      {
        brightness = 64;
      }
      break;
    case 5:
      {
        brightness = 128;
      }
      break;
    case 6:
      {
        brightness = 255;
      }
      break;

  }
}

Problem solved, cycling and brightness with buttons is now working as expected.

Here's the finished code

//Variables for LED selection
int led = 1;            //Set variable led to 1
int prevUp = HIGH;      //Set variable prevUp to High

//Variables for brightness setting
int prevUp2 = HIGH;  //set variable prevUp2 to High
int level = 1; //set variable level to 1
int brightness = 64; //set variable brightness to 64


const int buttonPin = 8;
const int modePin = 2;

const int led1pin = 3;
const int led2pin = 5;
const int led3pin = 6;
const int led4pin = 9;
const int led5pin = 10;
const int led6pin = 11;

void setup() {
  
  //set buttons to input
  pinMode (buttonPin, INPUT);
  pinMode (modePin, INPUT);
  
  //set LED's to output
  pinMode (led1pin, OUTPUT);
  pinMode (led2pin, OUTPUT);
  pinMode (led3pin, OUTPUT);
  pinMode (led4pin, OUTPUT);
  pinMode (led5pin, OUTPUT);
  pinMode (led6pin, OUTPUT);

  //turn leds off at boot
  analogWrite(led1pin, 0);
  analogWrite(led2pin, 0);
  analogWrite(led3pin, 0);
  analogWrite(led4pin, 0);
  analogWrite(led5pin, 0);
  analogWrite(led6pin, 0);

  // start serial port at 9600 bps:
  Serial.begin(9600);

}

void loop() {

  int currUp = digitalRead(buttonPin);
  if (currUp != prevUp)
  {
    if (currUp == LOW)
      led++;
    if (led > 7) led = 1;
  }
  prevUp = currUp;

  int currUp2 = digitalRead(modePin);
  if (currUp2 != prevUp2)
  {
    if (currUp2 == LOW)
      level++;
    if (level > 6) level = 1;
  }
  prevUp2 = currUp2;

  switch (led) {
    case 1:
      {
        analogWrite(led1pin, brightness);

      }
      break;
    case 2:
      {
        analogWrite(led1pin, brightness);
        analogWrite(led2pin, brightness);
      }
      break;
    case 3:
      {
        analogWrite(led1pin, brightness);
        analogWrite(led2pin, brightness);
        analogWrite(led3pin, brightness);
      }
      break;
    case 4:
      {
        analogWrite(led1pin, brightness);
        analogWrite(led2pin, brightness);
        analogWrite(led3pin, brightness);
        analogWrite(led4pin, brightness);
      }
      break;
    case 5:
      {
        analogWrite(led1pin, brightness);
        analogWrite(led2pin, brightness);
        analogWrite(led3pin, brightness);
        analogWrite(led4pin, brightness);
        analogWrite(led5pin, brightness);
      }
      break;
    case 6:
      {
        analogWrite(led1pin, brightness);
        analogWrite(led2pin, brightness);
        analogWrite(led3pin, brightness);
        analogWrite(led4pin, brightness);
        analogWrite(led5pin, brightness);
        analogWrite(led6pin, brightness);
      }
      break;
    case 7:
      {
        analogWrite(led1pin, 0);
        analogWrite(led2pin, 0);
        analogWrite(led3pin, 0);
        analogWrite(led4pin, 0);
        analogWrite(led5pin, 0);
        analogWrite(led6pin, 0);
      }
      break;
  }

  switch (level) {
    case 1:
      {
        brightness = 16;
      }
      break;
    case 2:
      {
        brightness = 32;
      }
      break;
    case 3:
      {
        brightness = 64;
      }
      break;
    case 4:
      {
        brightness = 128;
      }
      break;
    case 5:
      {
        brightness = 255;
      }
      break;
    case 6:
      {
        brightness = 0;
      }
      break;
    
  }
  Serial.println(led);
  Serial.println(level);
}

It took me a while to figure out why you don't get problems from button bounce but while modifying the code to fit my breadboard common-power row of leds (used in a previous example code) I finally found what's likely slowing your loop down to where it can't read the bounces.

1st part, the low speed serial in setup()

Serial.begin(9600);

2nd part, overfilling the serial output buffer in loop() by printing every time loop() executes

Serial.println(led);
Serial.println(level);


This make a kind of button governor that allows you press and release a button to get only one press noticed. Kudos because it works but I'm guessing that it's not by design though you can prove me wrong!

Once I saw that I quit setting up a test though maybe I should.... hang on, be back in a few....

Edit: back!

Note all this is assuming that you're not using buttons debounced in hardware.

What I did was to add code around those println statements, and a couple of global variables.

my globals:
int prevLed, prevLevel;

code change:

  if ( led != prevLed )
  {
    Serial.print("led ");
    Serial.println(led);
    prevLed = led;
  }

  if ( level != prevLevel )
  {
    Serial.print("level ");
    Serial.println(level);
    prevLevel = level;
  }

With those it only prints what has changed and sure enough one press can read as many.

A quick way to see the bounce would be to turn your print lines into comments and run the project. With the above changes you get a log on serial monitor.

It was pure luck on my part, I would probably have been whining here, or scratching my head to a bloody mess figuring this out.

Any suggestions to solve this issue so I can remove the serial, which was just there in the beginning to help me sort out the sequence.

GoForSmoke:
It took me a while to figure out why you don't get problems from button bounce but while modifying the code to fit my breadboard common-power row of leds (used in a previous example code) I finally found what's likely slowing your loop down to where it can't read the bounces.

1st part, the low speed serial in setup()

Serial.begin(9600);

2nd part, overfilling the serial output buffer in loop() by printing every time loop() executes

Serial.println(led);
Serial.println(level);


This make a kind of button governor that allows you press and release a button to get only one press noticed. Kudos because it works but I'm guessing that it's not by design though you can prove me wrong!

Once I saw that I quit setting up a test though maybe I should.... hang on, be back in a few....

Edit: back!

Note all this is assuming that you're not using buttons debounced in hardware.

What I did was to add code around those println statements, and a couple of global variables.

my globals:
int prevLed, prevLevel;

code change:

  if ( led != prevLed )

{
    Serial.print("led ");
    Serial.println(led);
    prevLed = led;
  }

if ( level != prevLevel )
  {
    Serial.print("level ");
    Serial.println(level);
    prevLevel = level;
  }




With those it only prints what has changed and sure enough one press can read as many.

A quick way to see the bounce would be to turn your print lines into comments and run the project. With the above changes you get a log on serial monitor.

Contact switch (including most buttons) bouncing is a regular topic here.

Step one is know what's going on. As metal contacts come very close, sparks cross the gap. An Arduino that is not being slowed down (serial buffer overflow makes your code wait until it's only full, more often the execution block is caused by use of the delay command) can see spikes as short as a few microseconds. They can appear as button press and release events, as you see now.

The Nick gammon blog on switches and debouncing, a full treatment.

You can use a delay() in place of the prints. I can't recommend it as it cuts the capability of your code (makes reuse or adding more that needs speed or close timing buggy) badly but maybe no worse than those prints already do.

You could debounce the button with a capacitor across the leads (hardware debounce).

You could debounce it in software. I have a button library for that, there's dozens of routines that do the same more or less as it's a kind of hobby around here.

Whichever way you go, there's a few other things you can do to clean up your code that will save you time and ram in later projects.

Of course I like my library. It lets the user make button code-objects, even button arrays. The objects only use 4 bytes of memory each.

Example showing how to use the library to use multiple buttons is below. I have it set up so that the library files go into the project folder. That lets the user work on them in IDE tabs and save to the right file.

You don't need to know how the library works except for what's in the example.
The button object returns a value of 2 when pressed and 1 when released, you don't need to keep track of previous state because the library rolled that into the return value but you can track actual current up or down or bouncing state as 0 or 3 or > 3 as well (like for game use).

One thing. I code buttons to use the AVR on-chip pullup resistor. All the button has to do is ground the pin for press. The resistor is 20K to 50K, safe to ground, saves me a resistor and lets me use jumpers as buttons (I ground them on the USB port box). It does mean that LOW is pressed and HIGH is not pressed but your code never sees that. Your wiring does need to ground the button instead of pull it up (safe if you don't but it won't work) and you need to mode the pin(s) as INPUT_PULLUP instead of INPUT but again, it's safe if you don't. That uses a great AVR feature.

Any specific questions, just ask.

/* buttonarrayclass example -- by GoForSmoke 2015 for public domain use
   revised with better library Oct 2016

  This uses 2 buttons or switches or jumpers for pins 2 & 3 to GND.
  Depending on switch, debounce value may need tuning.

  Button class object output is a state value for the main sketch code.
   // bit 0 = current, bit 1 = previous, bit 2 = bounce
  0, button currently still pressed. bits 000; same current and previous
  1, button has just been released. bits 100; different current and previous
  2, button has just been pressed.  bits 010; different the other way
  3, button currently still released. bits 110; same current and previous
  4 or more means the button is being debounced but not finished.
*/

#include "Arduino.h"
#include "button.h" // check for newest

// these variables are here to ward off the WinXP bug!
// if you're not running Windoze XP you won't need them
// but in any case, the compiler will optimize them away.
int a, b, c;
byte d, e, f;

const byte howManyButtons = 4;
byte buttonIdx;
byte buttonPin[ howManyButtons ] = { 4,5,6,7 };
button user[ howManyButtons ]; // button array

const byte ledpin = 13;

void setup()
{
  Serial.begin( 250000 );
  Serial.println( F( "\n  Startup\n" ));

  pinMode( ledpin, OUTPUT ); // default is INPUT LOW, now is OUTPUT LOW
  
  for ( buttonIdx = 0; buttonIdx < howManyButtons; buttonIdx++ )
  {
    user[ buttonIdx ].setButton( buttonPin[ buttonIdx ], 5 ); // buttons on pins 4,5,6,7
  }
  buttonIdx = 0;
}

void loop()
{
  static byte buttonRead; // allocated once, used often

  buttonRead = user[ buttonIdx ].runButton(); // turns the crank once, make sure it runs often!

  if ( buttonRead == 2 ) // just pressed
  {
    digitalWrite( ledpin, HIGH );
    Serial.print( F( "button " ));
    Serial.print( buttonIdx );
    Serial.print( F( " pressed millis() == " ));
    Serial.println( millis());
  }
  else if ( buttonRead == 1 ) // just released
  {
    digitalWrite( ledpin, LOW );
    Serial.print( F( "button " ));
    Serial.print( buttonIdx );
    Serial.print( F( " released millis() == " ));
    Serial.println( millis());
    Serial.println( );
  }
  
  if ( ++buttonIdx >= howManyButtons ) buttonIdx = 0; // adds 1 then tests limit
}

The library .h file

/*
  button.h for public domain use by GoForSmoke May 29 2015
  Revised Sept 2016  
  To use:
  make a buttonclass object in your sketch
  run that object every time through loop(), it is quickly done
  when you want to know the status of the button, you read the object
  it returns state;  bit 0 = current, bit 1 = previous, bit 2 = bounce
*/

// button library, Sept 25th, 2016 by GoForSmoke

#ifndef button_h
#define button_h

#include "Arduino.h"

#define CURRENT_1 1
#define PREVIOUS_2 2
#define BOUNCE_4 4

typedef class button
{
private:
  byte arduPin;
  byte buttonState; // bit 0 = current, bit 1 = previous, bit 2 = bounce
  byte startMs;
  byte debounceMs; 

public:
  button(); // default constructor
  void setButton( byte, byte ); // pin, debounce millis
  button( byte, byte ); // pin, debounce millis
  void setUpButton( void );
  byte runButton( void ); // returns buttonState as below
  // buttonState: bit 0 = current, bit 1 = previous, bit 2 = bounce
  byte buttonRead();  // returns buttonState as above
};

#endif

The library .cpp file

// button library, Sept 25th, 2016 by GoForSmoke

#include "Arduino.h"
#include "button.h"

//    button ================================================

button::button() // default constructor for arrays, needs the full setup
{
  buttonState = CURRENT_1;
}

button::button( byte ap, byte dbm )
{
  arduPin = ap;
  debounceMs = dbm;
  buttonState = CURRENT_1;
};

void button::setButton( byte ap, byte dbm ) // pin, debounce millis
{
  arduPin = ap;
  debounceMs = dbm;
  pinMode( arduPin, INPUT_PULLUP );
};


void button::setUpButton( void )
{
  pinMode( arduPin, INPUT_PULLUP );
};

byte button::buttonRead()
{
  return buttonState;
};


byte button::runButton( void )
{
//  static byte msNow;

  buttonState &= BOUNCE_4 | CURRENT_1; // clears previous state bit
  buttonState |= ( buttonState & CURRENT_1 ) << 1; // copy current state to previous
  buttonState &= BOUNCE_4 | PREVIOUS_2; // clears current state bit
  buttonState += digitalRead( arduPin ); // current state loaded into bit 0

//  msNow = (byte) millis(); // gets the low byte of millis

  if ( buttonState & 3 == CURRENT_1 || buttonState & 3 == PREVIOUS_2 )  // state change detected
  {
    buttonState ^= BOUNCE_4;      // toggles debounce bit
    // on 1st and odd # changes since last stable state, debounce is on
    // on bounces back to original state, debounce is off 
    if ( buttonState & BOUNCE_4 )
    {
//      startMs = msNow;    // starts/restarts the bounce clock on any change.
      startMs = (byte) millis();    // starts/restarts the bounce clock on any change.
    }
  }
  else if ( buttonState & BOUNCE_4 ) // then wait to clear debounce bit
  {
//    if ( msNow - startMs >= debounceMs ) // then stable button state achieved
    if ( (byte)((byte)millis()) - startMs >= debounceMs ) // then stable button state achieved
    {
      //   understand that stable state means no change for debounceMs. When time
      //   is over the state bits are manipulated to show a state change.
      buttonState &= CURRENT_1; // clear all but the current state bit
      if ( buttonState == 0 )  buttonState = PREVIOUS_2;  // HIGH->LOW
      else                     buttonState = CURRENT_1;  // LOW->HIGH
      //   buttonState now appears as a debounced state change in bits 0 and 1
    }
  }

  return buttonState;
};

//    end button ============================================