dimming and running LEDs

Hi all,

I am still new to Arduino, I have a project to do.

I wanted to run LEDs in a running pattern, once I pressed and hold down the buttons, the LEDs keep running.

At the same time I have a 4-position rotary switch to select the brightness of LEDs

Here is the code

byte button = 7;
//byte led = 9;
byte led[4] = {2,3,4,5}; // 4 leds.
byte rSwitch[4] = {8,9,10,11}; // 4-position rotary switch
byte brightness;

void setup()
{
pinMode(button, INPUT);
for(int i = 0; i<4; i++) 
{
pinMode(led[i], OUTPUT); 
digitalWrite(led[i], LOW);
} 

for(int a = 0; a<4; a++) 
{
pinMode(rSwitch[a], INPUT);
} 



void loop()
{
if(digitalRead(button) == HIGH) {// if button is pressed

  if(digitalRead(rSwitch[1]) == HIGH {  //select 1st brightness
    brightness = 10; // set brightness
    blink_led_task();
  } // blink button
    
  if(digitalRead(rSwitch[2]) == HIGH {  //select 2nd brightness
    brightness = 50; // set brightness
    blink_led_task(); // blink button
  } 
    
  if(digitalRead(rSwitch[3]) == HIGH {  //select 3rd brightness
    brightness = 100; // set brightness
    blink_led_task(); // blink button
  }
  
  if(digitalRead(rSwitch[4]) == HIGH {  //select 2nd brightness
    brightness = 200; // set brightness
    blink_led_task(); // blink button
  } 
}
else
{
for(int i = 0; i<4; i++)
digitalWrite(led[i], LOW); // turn off all led.
}
}
unsigned long blink_led_time;
void blink_led_task()
{
if(millis() - blink_led_time > 500) // 200 ms.
blink_led_time = millis();
else
return;
static byte number = 0;
for(int i = 0; i<4; i++)
digitalWrite(led[i], LOW); // turn off all leds first.
analogWrite(led[number], brightness);
number++;
if(number == 4)
number = 0; // reset back to zero.
// digitalWrite(led, digitalRead(led) ^ 1); // toggle led.
}

It does not compile, says there is something wrong about the bracket.

I did check but I can't see the problem.

Can anyone give me some hints or suggestion ?

Cheers

I can't see the problem

Hints:

Look carefully at setup() for the missing brace

if(digitalRead(rSwitch[1]) == HIGH <<<<<< is this ok? Maybe there is more

for(int i = 0; i<4; i++)
digitalWrite(led[ i ], LOW); // turn off all leds first.<<<<< Do you need to
analogWrite(led[number], brightness); <<<<< do something here?
number++;

In order to make your code more readable, always go to tools/Auto Format in order to make your code more readable

Edit:
Also, you may find it beneficial to line up your braces

if(digitalRead(rSwitch[1]) == HIGH { <<<<move to the next line
{ // Line the braces up
brightness = 10; // set brightness
blink_led_task();
} // blink button

Thanks for your reminding about the Tools > Auto Edit

I reviewed my code again and found out what brackets are missing.

byte button = 7;
byte led[4] = 
{
  2,3,4,5}; // 4 leds.
byte rSwitch[4] = 
{
  8,9,10,11}; // 4-position rotary switch
unsigned long int brightness;

void setup()
{
  pinMode(button, INPUT);
  for(int i = 0; i<4; i++) 
  {
    pinMode(led[i], OUTPUT); 
    digitalWrite(led[i], LOW);
  } 

  for(int a = 0; a<4; a++) 
  {
    pinMode(rSwitch[a], INPUT);
  }
}



void loop()
{
  if(digitalRead(button) == HIGH) // if button is pressed
  {
    if(digitalRead(rSwitch[1]) == HIGH)  //select 1st brightness
    {
      brightness = 10; // set brightness
      blink_led_task(); // blink LED function
    }

    else if(digitalRead(rSwitch[2]) == HIGH)  //select 2nd brightness
    {  
      brightness = 50; // set brightness
      blink_led_task(); // blink button
    }

    else if(digitalRead(rSwitch[3]) == HIGH)  //select 3rd brightness
    {  
      brightness = 100; // set brightness
      blink_led_task(); // blink LED function
    }

    else if (digitalRead(rSwitch[4]) == HIGH) //select 2nd brightness
    {
      brightness = 200; // set brightness
      blink_led_task(); // blink LED function
    }
  }
  else
  {
    for(int i = 0; i<4; i++)
      digitalWrite(led[i], LOW); // turn off all led.
  }
}

unsigned long blink_led_time;

void blink_led_task()
{
  if(millis() - blink_led_time > 500) // 500 ms.
    blink_led_time = millis();
  else
    return;
  static byte number = 0;
  for(int i = 0; i<4; i++)
    digitalWrite(led[i], LOW); // turn off all leds first.
  analogWrite(led[number], brightness);
  number++;
  if(number == 4)
    number = 0; // reset back to zero.
  // digitalWrite(led, digitalRead(led) ^ 1); // toggle led.
}

it compiles.

I uploaded it to my Mega 2560 to see it works as expected. Unfortunately not.

LarryD:

I can't see the problem

Hints:

Look carefully at setup() for the missing brace

if(digitalRead(rSwitch[1]) == HIGH <<<<<< is this ok? Maybe there is more

for(int i = 0; i<4; i++)
digitalWrite(led[ i ], LOW); // turn off all leds first.<<<<< Do you need to
analogWrite(led[number], brightness); <<<<< do something here?
number++;

In order to make your code more readable, always go to tools/Auto Format in order to make your code more readable

Edit:
Also, you may find it beneficial to line up your braces

if(digitalRead(rSwitch[1]) == HIGH { <<<<move to the next line
{ // Line the braces up
brightness = 10; // set brightness
blink_led_task();
} // blink button

I turn the LEDs off just want to make sure they would not be turned on before the sequence starts. I know it may be not necessary in this case.

Originally, I developed a code which only contains the ON button and LEDs, no brightness/dimming control.
That worked fine. But it goes wrong after I added the rotary switch for dimming.

Can you confirm ALL switches have pull resistors (you could use internal ones) and when they close they go to 0 volts (GND)?

Arrays start at 0
http://www.thebox.myzen.co.uk/Tutorial/Arrays.html
if(digitalRead(rSwitch[1]) == HIGH { //select 1st brightness
if(digitalRead(rSwitch[0]) == HIGH { //select 1st brightness

FYI: some sketch applications have to have switches de-bounced.

Change this to

pinMode(rSwitch[a], INPUT_PULLUP); // enable internal pullups

and then read if they got pulled Low & act on that:

if(digitalRead(rSwitch[0]) == LOW) {

Or be clever about the rotary switch:

rSwitch = PINB & B00001111;  // read PortB, mask off the upper 4 bits
switch(rSwitch){  // faster than a bunch of if statements
case 0x01:
      brightness = 10; // set brightness
      blink_led_task(); // blink LED function
break;
case 0x02:
      brightness = 50; // set brightness
      blink_led_task(); // blink button
break;
case 0x04:
      brightness = 100; // set brightness
      blink_led_task(); // blink button
break;
case 0x08: 
      brightness = 200; // set brightness
      blink_led_task(); // blink button
break; 
// no valid cases selected
PORTB = PORTB & B11110000; // clear lower 4 bits, leave upper bits as they were
} // end switch:case

PINB

Did not know about Port Manipulation, thank you very much, just goes to show you that you might learn something by reading a Post!

You might indeed 8)
I still use pinMode to set the pins as INPUT & OUTPUT in setup, makes it easier for me to keep track of.

CrossRoads:
Change this to

pinMode(rSwitch[a], INPUT_PULLUP); // enable internal pullups

and then read if they got pulled Low & act on that:

if(digitalRead(rSwitch[0]) == LOW) {

I dun understand this , does the INPUT_PULLUP make a difference ?
If I change the rSwitch into pullup, do I have to change the ON/OFF button as well ??

LarryD:
Can you confirm ALL switches have pull resistors (you could use internal ones) and when they close they go to 0 volts (GND)?

Arrays start at 0
Arrays
if(digitalRead(rSwitch[1]) == HIGH { //select 1st brightness
if(digitalRead(rSwitch[0]) == HIGH { //select 1st brightness

FYI: some sketch applications have to have switches de-bounced.

I think I do not have anything to do with pull up resistor in this project. (I think)

I connect the ON/FF button following this basic method:
http://arduino.cc/en/tutorial/button

The 4 pins of rotary switch is also connected like that.

It does work without the rotary switch.

This is my old code:

byte button = 8;
byte led[4] = {
  2,3,4,5}; // 4 leds.
void setup()
{
  pinMode(button, INPUT);
  for(int i = 0; i<4; i++) 
  {
    pinMode(led[i], OUTPUT); 
    digitalWrite(led[i], LOW);
  }
}
void loop()
{
  if(digitalRead(button) == HIGH) // if button is pressed
  {
    blink_led_task(); // blink button
  }
  else
  {
    for(int i = 0; i<4; i++)
      digitalWrite(led[i], LOW); // turn off all led.
  }
}
unsigned long blink_led_time;
void blink_led_task()
{
  if(millis() - blink_led_time > 500) // 500 ms.
    blink_led_time = millis();
  else
    return;
  static byte number = 0;
  for(int i = 0; i<4; i++)
    digitalWrite(led[i], LOW); // turn off all leds first.
  digitalWrite(led[number], HIGH);
  number++;
  if(number == 4)
    number = 0; // reset back to zero.
  // digitalWrite(led, digitalRead(led) ^ 1); // toggle led.
}

It works fine.

You said the dimming didn't work with the rotary switch, I was offering suggestions to fix that.
The PULLUP makes sure the pins do not float. They are either HIGH from the pullup, or LOW when connected to Gnd with your switches. Wire your switch so that it connects the selected pin to Gnd. I imagine you have it connected to +5 right now?

CrossRoads:
You said the dimming didn't work with the rotary switch, I was offering suggestions to fix that.
The PULLUP makes sure the pins do not float. They are either HIGH from the pullup, or LOW when connected to Gnd with your switches. Wire your switch so that it connects the selected pin to Gnd. I imagine you have it connected to +5 right now?

yes, both ON/OFF button and rotary switch connected to the 5V pin of mega 2560.

5V ------ Button ------ [resistor] ----- GND
|
|
|
Input Pin

This is how I connect them.

Lose the resistor, put the button where the resistor was, no external connection to 5V. Only the internal pullup resistor.
Change the software logic to look for a Low when the button is pressed.

if (digitalRead (inputPin) == LOW){
//then button was pressed, so do something
}
else {
// button was not pressed, do something else, or not
}

CrossRoads:
Lose the resistor, put the button where the resistor was, no external connection to 5V. Only the internal pullup resistor.
Change the software logic to look for a Low when the button is pressed.

if (digitalRead (inputPin) == LOW){
//then button was pressed, so do something
}
else {
// button was not pressed, do something else, or not
}

Just changed the wiring and the code as you suggested.

Here is the code

byte button = 7;
byte led[4] = 
{
  2,3,4,5}; // 4 leds.
byte rSwitch[4] = 
{
  8,9,10,11}; // 4-position rotary switch
unsigned long int brightness;

void setup()
{
  pinMode(button, INPUT_PULLUP);
  for(int i = 0; i<4; i++) 
  {
    pinMode(led[i], OUTPUT); 
  } 

  for(int a = 0; a<4; a++) 
  {
    pinMode(rSwitch[a], INPUT_PULLUP);
  }
}



void loop()
{
  if(digitalRead(button) == LOW) // if button is pressed
  {
    if(digitalRead(rSwitch[0]) == LOW)  //select 1st brightness
    {
      brightness = 10; // set brightness
      blink_led_task(); // blink LED function
    }

    else if(digitalRead(rSwitch[1]) == LOW)  //select 2nd brightness
    {  
      brightness = 60; // set brightness
      blink_led_task(); // blink button
    }

    else if(digitalRead(rSwitch[2]) == LOW)  //select 3rd brightness
    {  
      brightness = 120; // set brightness
      blink_led_task(); // blink LED function
    }

    else if (digitalRead(rSwitch[3]) == LOW) //select 2nd brightness
    {
      brightness = 200; // set brightness
      blink_led_task(); // blink LED function
    }
  }
  else
  {
    for(int i = 0; i<4; i++)
      digitalWrite(led[i], LOW); // turn off all led.
  }
}

unsigned long blink_led_time;

void blink_led_task()
{
  if(millis() - blink_led_time > 500) // 500 ms.
    blink_led_time = millis();
  else
    return;
  static byte number = 0;
  for(int i = 0; i<4; i++)
    digitalWrite(led[i], LOW); // turn off all leds first.
  analogWrite(led[number], brightness);
  number++;
  if(number == 4)
    number = 0; // reset back to zero.
  // digitalWrite(led, digitalRead(led) ^ 1); // toggle led.
}

It works now, thank you so much for your advise.

I tried to use the switch case as you mentioned previously, it doesn't compile.

There might be some minor typo in there, I didn't try compiling it before I posted it.