making arrays for buttons and leds

Smitshac · November 14, 2018, 10:43pm

Dear forum members,

I need your help for a small project in my house. I try to get three LEDs working with a fade possibility

Hardware:

1x Arduino uno
3x LEDs controlled by ldd drivers with pwm signal
3x buttons with resistors

Software:

I have found a sketch that works for one LED and button, but I want to rewrite it to the possibility of controlling three LEDs or more in the future. So I have made an attempt, but I get errors.
I have used a libary called clickbutton, that libary works good for what I wanted.

#include "ClickButton.h"

// Nr. of buttons in the array
const int buttons = 3;

// Nr. of leds in the array
const int ledPin[buttons] = { 3, 5, 6 }; // Arduino pins to the leds
int ledState[buttons]     = { 0, 0, 0 };
int i = 0;

// Arduino input pins from the buttons (these are not in an array for simplicity just now)
const int buttonPin1 = 2;
const int buttonPin2 = 4;
const int buttonPin3 = 7;

// Instantiate ClickButton objects in an array
ClickButton button[3] = {
  ClickButton (buttonPin1, LOW, CLICKBTN_PULLUP),
  ClickButton (buttonPin2, LOW, CLICKBTN_PULLUP),
  ClickButton (buttonPin3, LOW, CLICKBTN_PULLUP),
};

// Fade variables
int fadeValue = 64;
boolean fadeUp = false;    // false means fade down
boolean oldFadeUp = fadeUp;
const long fadeDelay = 10; // Time in milliseconds between fade steps
long adjustFaderTime = 0;  // Time to adjust the fader

// other
long currentTime;
int function = 0;

void setup()
{
  for (int i=0; i<buttons; i++)
  {
    pinMode(ledPin[i],OUTPUT);  
  }
}

void loop()
{
  currentTime = (long)millis();
 
  // Go through each button and set the corresponding LED function
  for (int i=0; i<buttons; i++)
     
    // Update state of all buitton
    button[i].Update();

  if (button[i].clicks != 0) function = button[i].clicks;
  
  // Toggle LED on single clicks
  if(button[i].clicks == 1) ledState = !ledState;

  // fade if button is held down during single-click
  if(function == -1 && button[i].depressed == true)
  {
    ledState = true;  // force lights on, since we want to fade it up or down
    
    if (oldFadeUp == fadeUp) fadeUp = !fadeUp; // Switch direction
    
    if ( currentTime - adjustFaderTime > fadeDelay)
    {
      adjustFaderTime = currentTime + fadeDelay;
      if (fadeUp) fadeValue++; else fadeValue--;

      // Some boundary checking
      // Using signed ints, we can check for below 0 and above 255 (byte limit)
      if (fadeValue > 255) fadeValue = 255;
      if (fadeValue < 0)   fadeValue = 0;
      
    }

  } else {
    // Save old fade direction for next time
    oldFadeUp = fadeUp;
    // Reset function
    function = 0;
  }
  

  // update the LED
    for (int i=0; i<buttons; i++)
  {
    digitalWrite(ledPin[i],ledState[i]);
  }
}

Arduino:1.8.7 (Windows 8.1), Board:"Arduino/Genuino Uno"

C:\Users\H.A.C\Documents\Arduino\sketch_nov14a\sketch_nov14a.ino: In function 'void loop()':

sketch_nov14a:55:38: error: assignment of read-only variable 'ledState'

   if(button[i].clicks == 1) ledState = !ledState;

                                      ^

sketch_nov14a:55:38: error: incompatible types in assignment of 'bool' to 'const int [3]'

sketch_nov14a:60:14: error: assignment of read-only variable 'ledState'

     ledState = true;  // force lights on, since we want to fade it up or down

              ^

sketch_nov14a:60:14: error: incompatible types in assignment of 'bool' to 'const int [3]'

exit status 1
assignment of read-only variable 'ledState'

I want to achieve the following with the sketch:

1 button controls 1 led with the next fading possibilities

Short press led on and led off
Long press fading led to brightness high and brightness low
Remember the brightness of the led when the led pressed off.

And that for three leds.

Can you help me, I have just started arduino and the simple things are working fine and I understand that. But now it becomes a bit complicated for me.

thanks in advance

vaj4088 · November 14, 2018, 11:00pm

Early in the program, ledstate is defined as the name of an array, but then ledstate gets used as an int or a boolean variable. Which is it?

Smitshac · November 14, 2018, 11:13pm

vaj4088:
Early in the program, ledstate is defined as the name of an array, but then ledstate gets used as an int or a boolean variable. Which is it?

An boolean, but an boolean can't work with an int I guess. I mixed those two variables in the wrong way. How could a boolean becomes an array? is that possible what I want?

vaj4088 · November 14, 2018, 11:56pm

You probably want:

if(button[i].clicks == 1) ledState[i] = !ledState[i] ;

Notice the addition of an index in two places.

You can have an array of booleans or an array of int, but you would have to be careful about mixing them. Mixing them is usually a bad idea. A boolean cannot BECOME an array and an int cannot BECOME an array.

Smitshac · November 15, 2018, 10:20am

vaj4088:
You probably want:
if(button[i].clicks == 1) ledState[i] = !ledState[i] ;
Notice the addition of an index in two places.

You can have an array of booleans or an array of int, but you would have to be careful about mixing them. Mixing them is usually a bad idea. A boolean cannot BECOME an array and an int cannot BECOME an array.

It solved the multiple errors and it compiles correctly. But the program doesn't work properly. The first led is on, and stays on. But the buttons and the other two leds are not working. I think the mixing is indeed the wrong way. I am going to try to change the arrays, also I am not sure where to begin.

Smitshac · November 15, 2018, 8:00pm

Thanks for your help, I understand more and more. I have removed many error messages and unnecessary code.

This shows that one button and one LED works in a good way. The LED works as it should with dim capabilities.

The rest of the array with LEDs and buttons do not work yet. Can you see if I need to further optimize the code? I have tried to rewrite the boolean and int arrays but that results in error messages. What can I do more?

Many thanks in advance,

#include "ClickButton.h"

// Nr. of buttons in the array
const int buttons = 3;

// Nr. of leds in the array
const int ledPin[buttons] = { 3, 5, 6 }; // Arduino pins to the leds
int ledState[buttons]     = { 0, 0, 0 };
int i = 0; 

// Arduino input pins from the buttons (these are not in an array for simplicity just now)
const int buttonPin1 = 2;
const int buttonPin2 = 4;
const int buttonPin3 = 7;

// Instantiate ClickButton objects in an array
  ClickButton button[3] = {
  ClickButton (buttonPin1, LOW, CLICKBTN_PULLUP),
  ClickButton (buttonPin2, LOW, CLICKBTN_PULLUP),
  ClickButton (buttonPin3, LOW, CLICKBTN_PULLUP),
};

// Fade variables
int fadeValue = 64;
boolean fadeUp = false;    // false means fade down
boolean oldFadeUp = fadeUp;
const long fadeDelay = 10; // Time in milliseconds between fade steps
long adjustFaderTime = 0;  // Time to adjust the fader

// other
long currentTime;
int function = 0;

void setup()

  {
    pinMode(ledPin[i],OUTPUT);  
  }

void loop()
{
  currentTime = (long)millis();
 
  // Update state of all buitton
    button[i].Update();

  if (button[i].clicks != 0) function = button[i].clicks;
  
  // Toggle LED on single clicks
  if(button[i].clicks == 1) ledState[i] = !ledState[i] ;

  // fade if button is held down during single-click
  if(function == -1 && button[i].depressed == true)
  {
    ledState[i] = true;  // force lights on, since we want to fade it up or down
    
    if (oldFadeUp == fadeUp) fadeUp = !fadeUp; // Switch direction
    
    if ( currentTime - adjustFaderTime > fadeDelay)
    {
      adjustFaderTime = currentTime + fadeDelay;
      if (fadeUp) fadeValue++; else fadeValue--;

      // Some boundary checking
      // Using signed ints, we can check for below 0 and above 255 (byte limit)
      if (fadeValue > 255) fadeValue = 255;
      if (fadeValue < 0)   fadeValue = 0;
      
    }

  } else {
    // Save old fade direction for next time
    oldFadeUp = fadeUp;
    // Reset function
    function = 0;
  }
  

  // update the LED

  {
      if (ledState[i]) analogWrite(ledPin[i],fadeValue); else analogWrite(ledPin[i], 0);
  }
}

vaj4088 · November 16, 2018, 2:07pm

Friday is a busy day for me so any help from me must wait a bit, but if you remove or comment out this one line

int i = 0;

you will be shown the many places that a for-loop was needed but omitted.

Smitshac · November 16, 2018, 10:34pm

Thank you for responding, I am happy with all the help. I understand the int issue, wherever necessary I have generated a for loop. I have not been able to test it with hardware today. But it compiles without error messages and without define int i = 0

Tomorrow I will test again, below the updated sketch. If you have time please check the code.

#include "ClickButton.h"

// Nr. of buttons in the array
const int buttons = 3;

// Nr. of leds in the array
const int ledPin[buttons] = { 3, 5, 6 }; // Arduino pins to the leds
int ledState[buttons]     = { 0, 0, 0 };

// Arduino input pins from the buttons (these are not in an array for simplicity just now)
const int buttonPin1 = 2;
const int buttonPin2 = 4;
const int buttonPin3 = 7;

// Instantiate ClickButton objects in an array
ClickButton button[3] = {
  ClickButton (buttonPin1, LOW, CLICKBTN_PULLUP),
  ClickButton (buttonPin2, LOW, CLICKBTN_PULLUP),
  ClickButton (buttonPin3, LOW, CLICKBTN_PULLUP),
};

// Fade variables
int fadeValue = 64;
boolean fadeUp = false;    // false means fade down
boolean oldFadeUp = fadeUp;
const long fadeDelay = 10; // Time in milliseconds between fade steps
long adjustFaderTime = 0;  // Time to adjust the fader

// other
long currentTime;
int function = 0;

void setup()

{ for (int i=0; i<buttons; i++)

  {
    pinMode(ledPin[i],OUTPUT);  
  }
}

void loop()
{
  currentTime = (long)millis();
  
  // Go through each button and set the corresponding LED function
 
 for (int i=0; i<buttons; i++)
  // Update state of all button
    button[i].Update();

 for (int i=0; i<buttons; i++)

  if (button[i].clicks != 0) function = button[i].clicks;

 for (int i=0; i<buttons; i++)
  // Toggle LED on single clicks  
  if(button[i].clicks == 1) ledState[i] = !ledState[i] ;

 for (int i=0; i<buttons; i++)
  // fade if button is held down during single-click  
  if(function == -1 && button[i].depressed == true)

  {
    ledState[i] = true;  // force lights on, since we want to fade it up or down
    
    if (oldFadeUp == fadeUp) fadeUp = !fadeUp; // Switch direction
    
    if ( currentTime - adjustFaderTime > fadeDelay)
    {
      adjustFaderTime = currentTime + fadeDelay;
      if (fadeUp) fadeValue++; else fadeValue--;

      // Some boundary checking
      // Using signed ints, we can check for below 0 and above 255 (byte limit)
      if (fadeValue > 255) fadeValue = 255;
      if (fadeValue < 0)   fadeValue = 0;
      
    }

  } else {
    // Save old fade direction for next time
    oldFadeUp = fadeUp;
    // Reset function
    function = 0;

  }
  

  // update the LED
 for (int i=0; i<buttons; i++)
  {
      if (ledState[i]) analogWrite(ledPin[i],fadeValue); else analogWrite(ledPin[i], 0);
  }
}

Thank you in advance for the effort you have taken

PaulRB · November 17, 2018, 6:48am

const long fadeDelay = 10; // Time in milliseconds between fade steps
long adjustFaderTime = 0;  // Time to adjust the fader

// other
long currentTime;

...
  currentTime = (long)millis();

Variables for timing should be "unsigned long" like this:

const unsigned long fadeDelay = 10; // Time in milliseconds between fade steps
unsigned long adjustFaderTime = 0;  // Time to adjust the fader

// other
unsigned long currentTime;

...
  currentTime = millis();

This line of code is repeated 5 times in loop()

for (int i=0; i<buttons; i++)

You can reduce this to once, making your code shorter and easier to read.

Smitshac · November 17, 2018, 10:15am

PaulRB:

const long fadeDelay = 10; // Time in milliseconds between fade steps

long adjustFaderTime = 0; // Time to adjust the fader

// other
long currentTime;

...
currentTime = (long)millis();

Variables for timing should be "unsigned long" like this:
const unsigned long fadeDelay = 10; // Time in milliseconds between fade steps
unsigned long adjustFaderTime = 0; // Time to adjust the fader

// other
unsigned long currentTime;

...
currentTime = millis();




This line of code is repeated 5 times in loop()

for (int i=0; i<buttons; i++)



You can reduce this to once, making your code shorter and easier to read.

Thank you for responding, indeed the millis was wrong. The funny thing is when I comment the function = 0; the fading or LEDs work with three buttons. Only the LEDs flicker and do not dim properly.

 } else {
   // Save old fade direction for next time
   oldFadeUp = fadeUp;
   // Reset function
   // function = 0;

 }

If I want to declare the for statement only once, do I need to wright the various if statements in cases?

At the moment the three LEDs and buttons are working with on and off. Only the fading of the LEDs are not working properly at the moment. Only if I comment the function = 0 then there is a fading of the LEDs but the leds are flickering.

vaj4088 · November 18, 2018, 1:22am

You are to be commended for putting in some effort of your own instead of having others do your work for you. THANKS!

I have some comments:

1a) There are many places where you are using int for numbers that are unlikely to ever be greater than 127. It would be better to learn to use byte and save the precious memory of your Uno. Not critical, just helpful in the long run.

1b) There are many places where you are using int or long for numbers that will never be negative. It would be better to make use of unsigned, as in unsigned int or unsigned long. Not critical, just clearer to someone who reads your code (like you), and simplifies some of the output of the compiler.

I don't know why the variable "function" is named this way. This is a poor choice of name.
The variable "function" is not an array and yet it is used in a for loop where it will store the last value it had in the loop. This will be for i == 2 . Instead of

int function = 0;

perhaps you should have

int function[buttons] ;

but you will have to use function[ i ] instead of function,
or even better

int clicks[buttons] ;

and use clicks[ i ] instead of function.

It would be more efficient to combine all of your for-loops that are in the loop() function. You would need to use curly brackets "{" and "}". Even better would be to make use of the loop() function to perform all of your looping. Then the variable "function" could be a scalar instead of an array. These suggestions are not critical, just simpler and a tad quicker.
I do not have the library, so I cannot test the code and I do not know why the variable "function" would ever be -1.
Similarly, I do not have your hardware, so I cannot test how anything works.
It would be helpful for you to provide a schematic. I notice, for example, that your buttons do not use INPUT_PULLUP, so I hope that you have wired in your own pullup resistors. Without INPUT_PULLUP and without pullup resistors, the functioning of the buttons is likely to be quite random.

8 ) The variable "function" always gets a value so I do not understand why these lines are needed:

    // Reset function
    function = 0;

I am inclined to clean up your code for you but the biggest issue that we face is that there is no schematic.

Best Wishes!

PaulRB · November 18, 2018, 7:16am

If I want to declare the for statement only once, do I need to wright the various if statements in cases?

If you mean a switch statement, then no. Just use { and } after your for statement and place all your code inside those braces.

Only if I comment the function = 0 then there is a fading of the LEDs but the leds are flickering.

Post your complete code again, set up to show the flickering. Could be that analogWrite() is called too often.

vaj4088 · November 18, 2018, 2:42pm

... and while you are at it, post a schematic or wiring diagram. A clear picture of a hand drawing will be satisfactory.

Note to PaulRB:
analogWrite(...) is called on every output pin every time through loop().
Could this be a problem? What is a satisfactory rate?

Grumpy_Mike · November 18, 2018, 2:51pm

What is a satisfactory rate?

Don’t update it faster than one cycle of the PWM. So it depends on what pin it is on.

You are best only changing the PWM value when it changes.

Paul_B · November 18, 2018, 11:18pm

Grumpy_Mike:
You are best only changing the PWM value when it changes.

In other words, only ever perform a new analogWrite when you actually need to change the value. The structure of your code should automatically reflect this, if it does not, then you have designed it wrongly.

The same applies, perhaps with more critical importance, to updating an LCD - getting this wrong is a rather common newbie blunder resulting in flickering or unreadable displays.

Topic		Replies	Views
Multiple LED fade Programming Questions	23	1434	July 23, 2022
Fading multiple leds Programming Questions	9	410	May 5, 2021
Fading 3 LEDs to different values simultaneously LEDs and Multiplexing	5	2020	May 6, 2021
Multiple LED fade coding. LEDs and Multiplexing	2	3146	May 6, 2021
An Array of Leds controlled by a button LEDs and Multiplexing	4	1006	May 6, 2021

making arrays for buttons and leds

Related Topics