LED lamp, preset change not working, possible loop problem

system · December 7, 2014, 1:03pm

First of all, i hope that the subject title isn't too vague. I'm new to programming, so it's hard for me to desribe the problem!

This program is designed to control 3 LED circuits via PWM. Using 4 buttons (this will later be changed to a 2-axis joystick), a user can switch between presets (left - right), and brightness of the LED's (up, down).
Currently, the hardware used for the project has a single button which controls the preset, and 2 buttons to control the brightness trim amount.
All buttons return LOW when pressed.

When i run the program, nothing lights up, and pressing a button doesn't change anything.
On the hardware side, i am positive that everything is wired up correctly, since previous, more simple code has been successfull. The problems started when i implemented the brightness trim code into each preset, using a while loop. Previously, i had the brightness trim section as a seperate if statement. This meant that a brightness change only would occur when a preset was changed - and the code is supposed to be able to change brightness while inside a preset.

I am using an Arduino Uno.

The code:

//Declare LED variables
int yled = 10;
int bled = 9;
int wled = 11;


int b_ch = 15; // amount of brightness change when brightness is trimmed

int p_up = 2; // preset up button
int b_up = 3; // Brightness up button
int b_down = 4; // brightness down button



// preset counter variables
int preset = 0; // counter for the number of the button presses
int presetUpState = 0; // current state of the button
int presetUpLBS = 1; // previous state of the button
int presetDownState = 0; // Current state of the button, currently NOT being used
int presetDownLBS = 0; // last state of the button, currently NOT being used


// brightness trim variables
int b_trim = 0; // holder for the brightness trim amount

//Brightness counter variables
int trimUpState = 0; // current state of button
int trimUpLBS = 1; // previous state of button
int trimDownState = 0; // Current state of button
int trimDownLBS = 1; // Previous state of button


// Declare I/O
void setup(){
    pinMode(yled, OUTPUT);
    pinMode(bled, OUTPUT);
    pinMode(wled, OUTPUT);
    pinMode(p_up, INPUT);
    pinMode(b_up, INPUT);
    pinMode(b_down, INPUT);

}

void loop() {
    presetUpState = digitalRead(p_up); //reads the state of the preset button
    if(presetUpState != presetUpLBS){ //if the preset button is not equal to the previous value (1), do function
      if(presetUpState == LOW){ //checks if the preset button is pressed
        preset++; //adds 1 to the preset counter integer
        if(preset == 1){ //if the preset counter equals 1
          while(digitalRead(p_up) == HIGH){ //While the preset button reads high (not pressed), do loop:
            if(digitalRead(p_up) == LOW){ //reads the preset button, if preset button returns low (pressed), breaks loop
             break;
            } 
            
            trimUpState = digitalRead(b_up); //reads the brightness trim up button
            if(trimUpState !=trimUpLBS){ //if the button doesn't equal the previous value (pressed), continue function
             if(trimUpState == LOW){ //if button is pressed, do said action
               b_trim = b_trim + b_ch;         
             }
             trimUpLBS = trimUpState; //Makes the last button state the current state, to check for another press.
            }
    
            trimDownState = digitalRead(b_down); //same as previous function
            if(trimDownState !=trimDownLBS){
             if(trimDownState == LOW){
              b_trim = b_trim - b_ch;
             }
            trimDownLBS = trimDownState;
            }
            analogWrite(yled, 0); //writes the values to the LED pins
            analogWrite(bled, 0);
            analogWrite(wled, 150 + b_trim);
            }
          }
        else if(preset == 2){ //same procedure as last preset change
            while(digitalRead(p_up) == HIGH){ //While the preset button reads high (not pressed), do loop:
            if(digitalRead(p_up) == LOW){ //reads the preset button, if preset button returns low (pressed), breaks loop
             break;
            } 
            
            trimUpState = digitalRead(b_up);
            if(trimUpState !=trimUpLBS){
             if(trimUpState == LOW){
             b_trim = b_trim + b_ch;         
             }
             trimUpLBS = trimUpState;
            }
    
          trimDownState = digitalRead(b_down);
          if(trimDownState !=trimDownLBS){
             if(trimDownState == LOW){
              b_trim = b_trim - b_ch;
             }
          trimDownLBS = trimDownState;
          }
          analogWrite(yled, 0);
          analogWrite(bled, 150 + b_trim);
          analogWrite(wled, 0);          
        }
        }
        else if(preset == 3){
          while(digitalRead(p_up) == HIGH){ //While the preset button reads high (not pressed), do loop:
            if(digitalRead(p_up) == LOW){ //reads the preset button, if preset button returns low (pressed), breaks loop
             break;
            }
            
            trimUpState = digitalRead(b_up);
            if(trimUpState !=trimUpLBS){
             if(trimUpState == LOW){
             b_trim = b_trim + b_ch;         
             }
             trimUpLBS = trimUpState;
            }
    
          trimDownState = digitalRead(b_down);
          if(trimDownState !=trimDownLBS){
             if(trimDownState == LOW){
              b_trim = b_trim - b_ch;
             }
          trimDownLBS = trimDownState;
          }          
          analogWrite(yled, 150 + b_trim);
          analogWrite(bled, 0);
          analogWrite(wled, 0);
          }  
        }
        else if(preset == 4){ //if the preset counter gets to 4, make the preset counter 0
            preset = 0;
        
        }
     }
    }
    presetUpLBS = presetUpState; //make LBS (last button state) of the preset button the current state, so function can check for a new press
}

I appreciate any help or input you got!

Some info about the project:
Currently, i'm waiting for some High powered LEDs, 3x 10W PWM led drivers and other stuff to arrive, which i'm going to install in an old desk lamp. This, together with an arduino is used to control the brightness of the 3 different channels, so its possible to generate different light settings. bright white/bluish light - a "lab" mode, Yellowish light - "Reading" mode, Combined white/yellow light - "standard" mode, and so on.

system · December 7, 2014, 4:17pm

          while(digitalRead(p_up) == HIGH){ //While the preset button reads high (not pressed), do loop:
            if(digitalRead(p_up) == LOW){ //reads the preset button, if preset button returns low (pressed), breaks loop
             break;
            }

While the switch state is HIGH, if it is LOW, break out of the loop. Does that really make sense? There should be NOTHING inside the curly braces. The while loop will end when the p_up pin goes LOW.

            }
            analogWrite(yled, 0); //writes the values to the LED pins
            analogWrite(bled, 0);
            analogWrite(wled, 150 + b_trim);
            }

There is something wrong with your indenting. Sort it out, or use Tools + Auto Format.

            while(digitalRead(p_up) == HIGH){ //While the preset button reads high (not pressed), do loop:
            if(digitalRead(p_up) == LOW){ //reads the preset button, if preset button returns low (pressed), breaks loop
             break;
            }

More silliness.

What do your Serial.print() statements tell you is happening? Why don't you have any?

system · December 7, 2014, 7:12pm

Thanks for the answer! I've tried to take a different approach to the brightness trimming, which now works.
Here's the code so far. All variable declarations are the same as in the 1st post.

void setup(){
  pinMode(yled, OUTPUT);
  pinMode(bled, OUTPUT);
  pinMode(wled, OUTPUT);
  pinMode(p_up, INPUT);
  pinMode(b_up, INPUT);
  pinMode(b_down, INPUT);
  Serial.begin(9600);
  Serial.write("ready!");


}

void loop() {
  presetUpState = digitalRead(p_up); //reads the state of the preset button
  if(presetUpState != presetUpLBS){ //if the preset button is not equal to the previous value (1), do function
    preset = preset + 1; //adds 1 to the preset counter integer
    if(preset == 1){ //if the preset counter equals 1
      while(digitalRead(p_up) == HIGH){ //While the preset button reads high (not pressed), do loop:
        trimUpState = digitalRead(b_up); //reads the state of the brightness trim up button
        trimDownState = digitalRead(b_down); //reads the state of the brightness trim down button
        if(trimUpState == LOW){ //if button is pressed, do said action
          b_trim = b_trim + b_ch;
          delay(b_delay);         
        }
        if(trimDownState == LOW){
          b_trim = b_trim - b_ch;
          delay(b_delay);
        }
        analogWrite(yled, 0); //writes the values to the LED pins
        analogWrite(bled, 0);
        analogWrite(wled, 150 + b_trim);
        Serial.println(preset);
        Serial.println();
      }
    }
    else if(preset == 2){ //same procedure as last preset change
      while(digitalRead(p_up) == HIGH){ //While the preset button reads high (not pressed), do loop:
        trimUpState = digitalRead(b_up); //reads the brightness trim up button
        trimDownState = digitalRead(b_down);       
        if(trimUpState == LOW){ //if button is pressed, do said action
          b_trim = b_trim + b_ch;
          delay(b_delay);         
        }
        if(trimDownState == LOW){
          b_trim = b_trim - b_ch;
          delay(b_delay);
        }
        analogWrite(yled, 0); //writes the values to the LED pins
        analogWrite(bled, 0);
        analogWrite(wled, 150 + b_trim);
      }
    }
    else if(preset == 3){
      while(digitalRead(p_up) == HIGH){ //While the preset button reads high (not pressed), do loop:
        trimUpState = digitalRead(b_up); //reads the brightness trim up button
        trimDownState = digitalRead(b_down);       
        if(trimUpState == LOW){ //if button is pressed, do said action
          b_trim = b_trim + b_ch;
          delay(b_delay);         
        }
        if(trimDownState == LOW){
          b_trim = b_trim - b_ch;
          delay(b_delay);
        }
        analogWrite(yled, 0); //writes the values to the LED pins
        analogWrite(bled, 0);
        analogWrite(wled, 150 + b_trim);
      }
    }
    else{
      preset = 0;

    }
  }
  presetUpLBS = presetUpState; //make LBS (last button state) of the preset button the current state, so function can check for a new press
}

Currently, the program acts as following:

open serial monitor, string "ready!" is displayed.
press preset up button
White LED lights up
Brightness up and down button works as expected, fading the brightness of the LED nicely.
I expect that the preset value is printed in the serial monitor each time the program goes through the loop, but nothing is displayed. This is strange, since the brightness trimming still works.
When pressing the preset button the 2nd time, the white LED stays lit, but the trim buttons does not work.
when pressing the preset button the 3rd time, the white LED goes into its loop, and brightness trim buttons works, trimming the brightness of the white LED.

And thanks for the tip about auto formatting - useful!

system · December 7, 2014, 10:12pm

    if(preset == 1){ //if the preset counter equals 1

Does every line of code need a comment that states the obvious? NO! Get rid of the chaff, so the wheat stands out.

Put every { on a new line. Jammedupagainstthecode,theyarehardtoseeandtheydonotlineupwiththeclosingbrace.

I expect that the preset value is printed in the serial monitor each time the program goes through the loop, but nothing is displayed.

The time to print preset is when you change it. Not 47 lines later, embedded in an if block.

When pressing the preset button the 2nd time, the white LED stays lit, but the trim buttons does not work.

How do you know? There is no print statement.

I'd suggest dumping all that code. Start over, after reading and UNDERSTANDING the state change detection example. Right now, you are taking action whenever the state changes. That happens when the switch BECOMES pressed AND when the switch BECOMES released. I can't see you wanting to do that.

Write a sketch that does NOTHING except detect when each switch, INDEPENDENTLY, becomes pressed and released. ONLY when that correctly reports the change in state of each switch should you move forward.

Anonymous print statements are just about useless. What does 3 mean? What was printed to show that? What does "preset = 3" mean? What was printed to show that? The second string is MUCH more obvious, isn't it?

system · December 13, 2014, 9:16pm

Took your advice, and rewrote the code from scratch, starting on paper with what i wanted to achieve. I ended up ditching the state reading, and using if-statements instead.
The code starts with a "boot up" mode, where the led's fades in. The trimming of the LED's works perfectly and carries over when switching patch. Patch switching works smoothly both up and down.
The only thing that could be considered a bug is, if the preset button is pressed for more than 250ms, it will go up 2 presets - but thats the condition if i don't want to use state switching.

void setup(){
  pinMode(yled, OUTPUT);
  pinMode(bled, OUTPUT);
  pinMode(wled, OUTPUT);
  pinMode(p_up, INPUT);
  pinMode(p_down, INPUT);
  pinMode(b_up, INPUT);
  pinMode(b_down, INPUT);
  Serial.begin(9600);
  Serial.write("ready!");
  Serial.println();
  for(int i=2; i <= pre1_y; i++){
    Serial.write(i);
    Serial.println();
    analogWrite(yled, i);
    analogWrite(bled, i);
    analogWrite(wled, i);
    delay(30);
  }
}

void loop() {
  delay(250);
  Serial.write("preset counter set at: ");
  Serial.print(preset);
  if(preset == 1){ 
    Serial.print("entering preset: 1");
    Serial.println();
    while(digitalRead(p_up) == HIGH){ //While the preset button reads high (not pressed), do loop:
      Serial.write("inside preset 1");
      Serial.println();
      presetUpState = digitalRead(p_up);
      presetDownState = digitalRead(p_down);
      if(presetUpState == LOW){
        preset = preset + 1;
        break;
      }
      if(presetDownState == LOW){
        preset = preset - 1;
        break;
      }
      trimUpState = digitalRead(b_up); //reads the state of the brightness trim up button
      trimDownState = digitalRead(b_down); //reads the state of the brightness trim down button
      if(trimUpState == LOW){
        if(pre1_y + b_trim < 250){ 
          b_trim = b_trim + b_ch;
          delay(b_delay);
        }        
      }
      if(trimDownState == LOW){
        if(pre1_y + b_trim > 5){ 
          b_trim = b_trim - b_ch;
          delay(b_delay);
        }
      }

      //Serial.print("Pre 1 Y: ");
      //Serial.print(pre1_y);
      //Serial.println();
      analogWrite(yled, pre1_y + b_trim); //writes the values to the LED pins
      analogWrite(bled, pre1_b);
      analogWrite(wled, pre1_w + b_trim);
    }
  }
  else if(preset == 2){ //same procedure as last preset change
    Serial.write("entering preset: 2");
    Serial.println();
    while(digitalRead(p_up) == HIGH){ //While the preset button reads high (not pressed), do loop:
      Serial.write("inside preset 2");
      Serial.println();
      presetUpState = digitalRead(p_up);
      presetDownState = digitalRead(p_down);
      if(presetUpState == LOW){
        preset = preset + 1;
        break;
      }
      if(presetDownState == LOW){
        preset = preset - 1;
        break;
      }
      trimUpState = digitalRead(b_up); //reads the state of the brightness trim up button
      trimDownState = digitalRead(b_down); //reads the state of the brightness trim down button
      if(trimUpState == LOW){
        if(pre1_y + b_trim < 250){ 
          b_trim = b_trim + b_ch;
          delay(b_delay);
        }        
      }
      if(trimDownState == LOW){
        if(pre1_y + b_trim > 5){ 
          b_trim = b_trim - b_ch;
          delay(b_delay);
        }
      }
      analogWrite(yled, pre2_y); //writes the values to the LED pins
      analogWrite(bled, pre2_b + b_trim);
      analogWrite(wled, pre2_w + b_trim);
    }
  }
  else if(preset == 3){
    Serial.write("entering preset: 3");
    Serial.println();
    while(digitalRead(p_up) == HIGH){ //While the preset button reads high (not pressed), do loop:
      Serial.write("inside preset 3");
      Serial.println();
      presetUpState = digitalRead(p_up);
      presetDownState = digitalRead(p_down);
      if(presetUpState == LOW){
        preset = preset + 1;
        break;
      }
      if(presetDownState == LOW){
        preset = preset - 1;
        break;
      }
      trimUpState = digitalRead(b_up); //reads the state of the brightness trim up button
      trimDownState = digitalRead(b_down); //reads the state of the brightness trim down button
      if(trimUpState == LOW){
        if(pre1_y + b_trim < 250){ 
          b_trim = b_trim + b_ch;
          delay(b_delay);
        }        
      }
      if(trimDownState == LOW){
        if(pre1_y + b_trim > 5){ 
          b_trim = b_trim - b_ch;
          delay(b_delay);
        }
      }
      analogWrite(yled, pre3_y + b_trim); //writes the values to the LED pins
      analogWrite(bled, pre3_w + b_trim);
      analogWrite(wled, pre3_b);
    }
  }
  else if(preset == 4){
    preset = 1;
  }
  else if(preset == 0){
    preset = 3;
  }
}

dlloyd · December 13, 2014, 9:37pm

If you're not using external resistors, you should use INPUT_PULLUP when setting up the push button pins. Otherwise, the signals will just float to random level(s) while the buttons are released.

system · December 13, 2014, 10:07pm

dlloyd:
If you're not using external resistors, you should use INPUT_PULLUP when setting up the push button pins. Otherwise, the signals will just float to random level(s) while the buttons are released.

Clever! i wasn't aware of the internal pullup resistors on the arduino.
Currently, i am using external resistors for the buttons.
Thank you for the tip.

system · December 14, 2014, 3:00am

The only thing that could be considered a bug is, if the preset button is pressed for more than 250ms, it will go up 2 presets - but thats the condition if i don't want to use state switching.

If the current behavior is not what you want, you need to look at the state change detection example.

Topic		Replies	Views
Brightness Control General Guidance	5	460	May 5, 2021
Problem with multiple button brightness control Programming	14	4202	May 5, 2021
LED brightness control with two push buttons Syntax & Programs	10	11578	May 6, 2021
Small problem lighting an LED Programming	10	987	May 5, 2021
Strange behavior with the Arduino Syntax & Programs	6	1913	May 6, 2021

LED lamp, preset change not working, possible loop problem

Related topics