Variable "A" control the state of 2 LEDs...how to ?

Projects Programming
1

I'm sure this is an elementary question but how do I control the state of 2(or more) leds. something similar to...

const int led1 = 8;
const int led2 = 9;
const int pot = A0;

int potval = 0;
int state1 = digitalWrite(led1, HIGH) && digitalWrite(led2, HIGH);
int state2 = digitalWrite(led1, LOW) && digitalWrite(led2, LOW);

void setup()
{
pinMode(led1, OUTPUT);
pinMode(led2, OUTPUT);
}

void loop()
{
potval = map(analogRead(pot)0,1023,0,255)

if (potval >= 128)
{
"do state1"
else
"do state2"
}

Ultimate goal is to reduce code size and the amount of if statements. tia

Bill

2

i have quite a few variable's and state changes which need to take place and I'm trying to avoid having to write a ton of "if's" like this...

if ( potval >= 250 )
{
digitalWrite(led1, HIGH);
digitalWrite(led2, LOW);
}
if (potval >= 245 && < 250)
{
digitalWrite(led1, LOW);
digitalWrite(led2, HIGH);
}
...etc
3

Billdefish:
i have quite a few variable's and state changes which need to take place and I'm trying to avoid having to write a ton of "if's"

Have a look at the 'Switch case' construct in the reference page.

4 
int state1 = digitalWrite(led1, HIGH) && digitalWrite(led2, HIGH);
int state2 = digitalWrite(led1, LOW) && digitalWrite(led2, LOW);

These lines don't make sense on a few different levels.

First, I would expect a variable to be set from a digitalRead() not a -Write().

Second, I don't think digitalWrite() or -Read will work where these lines are positioned.

Even if they did work they will only work once for the entire code - which may not be what is required.

...R

5

and I'm trying to avoid having to write a ton of "if's" like this...

if ( potval >= 250 )
{
digitalWrite(led1, HIGH);
digitalWrite(led2, LOW);
}

could be:

if ( potval >= 250 )
{
   setPinStates(led1, HIGH, led2, LOW);
}

where YOU write the setPinStates() function.

6

After searching the forum I found a similar post using the code below via econjack.

void (*funcPtr[])() = {func1, func2, func3};   // Define an array of pointers to function all of which return void

void func1() {
  Serial.println("Function 1");
}

void func2() {
  Serial.println("Function 2");
}
void func3() {
  Serial.println("Function 3");
}

void setup() {
  Serial.begin(115200);
  for (int i = 0; i < 3; i++) 
    (*funcPtr[i])();
}

void loop() {
}

My problem is that I need multiple conditions to be true in order for the change in led state. Can I use multiple arguments, something like...

for(int i =0;i <= 128 && j <= pulse[3];1++)

Bill

7

Can I use multiple arguments, something like...

One of the things that I like most about the Arduino is that it is so easy to try ideas out .....

8

"One of the things that I like most about the Arduino is that it is so easy to try ideas out .

Bob, this is true lol. just trying to conceptualize before the pen hits the paper.

Robin,

Those two lines aren't meant to make sense, so to speak. That's the issue is I guess I need to write a function containing the 2 leds and call them from the array in the sketch from econjack. However, I tried and it didn't return the desired results, neither led lit up.

Back to the drawing board...

Bill

9

so if you have a state engine as the backbone of your sketch, you could have an LED function call like this:

int state = 0;


void setup() 
{
  Serial.begin(9600);
}

void loop() 
{
  if (state == 1)
  {
    //my state == 0 stuff
  }
  else if (state == 1)
  {
    //my state == 1 stuff
  }
  else if (state == 2)
  {
    //my state == 2 stuff
  }
  else if (state == 3)
  {
    //my state == 3 stuff
  }
  else if (state == 4)
  {
    //my state == 1 stuff
  }
  
  ledLite(state);
}

void ledLite(int theState)
{
  if (theState == 0)
  {
    //blink like this
  }
  if (theState == 1)
  {
    //blink this way
  }
  if (theState == 2 || theState == 3 || theState == 4)
  {
    //blink like that
  }
  // well, you get it...
}
10

Billdefish:
Back to the drawing board...

Perhaps this Thread about planning and implementing a program will provide some ideas.

...R

11

BulldogLowell,

Don't know why I addressed you as Bob. Sorry. Anywho, I'm still a little unclear on the state machine. You may have missed my point though of reducing the amount of if statements and related components.

I want a function like...

const int led1 = 8;
const int led2 =9;
const int pot = A0;
int potval = 0;
int outstate[] = {out1,out2,out3};    //array of function pointers


void setup()
{
pinMode(led1, OUTPUT);
pinMode(led2, OUTPUT);
}
void loop()
{
potval = analogRead(pot);

if (potval >= 250)
{
outstate[0];      //this expression would induce output state 1, being led1, HIGH and led2, HIGH
}

Does that make more sense?

Robin2,

Read through the link you provided, thank you.

The only option I can make work correctly(outputs the correct states and data) consists of a ton of if statements and very little interchangeability between boards.

Bill

12

Billdefish:

void loop()

{
potval = analogRead(pot);

if (potval >= 250)
{
outstate[0];      //this expression would induce output state 1, being led1, HIGH and led2, HIGH
}

If you want to take in a value and then do several different things in response to that value you probably do need a series of IF or CASE statements. The IF statements will be easier to write if you start at the top and work down i.e. >=250, > 100, > 75 etc.

The only other situation I can conceive of is where the value (or a transformation of it) can be used directly in a function such as

void myFunction() {
     analogWrite(pin, potval/4);
}

You could reduce the amount of code by using the IF statements to set the values of the LEDs and at the bottom of the IF sequence do the digitalWrite() stuff. Depending on what you want to do with the LEDs it might be simpler to set all the values to LOW before the start of the IF sequence so you only need to deal with the ones that need to be HIGH.

DON'T choose a solution that makes the code hard to understand unless it is unavoidable.

...R

13 
int outstate[] = {out1,out2,out3};    //array of function pointers

That is NOT how to create an array of function pointers (which ARE the answer). There are at least three active threads now involving function pointers. Spend some time looking for them.

14

Have you considered making this data driven? Define a struct that has elements that specify the things you need to control the LEDS and make an array of them. Then you can just iterate through them and have a single if statement in the loop. The struct might look like:

typedef struct
  {
  int lowerLimit;              // low end of analog range that specifies this state
  int UpperLimit;             // High end..........
  byte pinNumber;         // Pin that this state's LED is on
  boolean setLEDOn;     // Do we turn it on in this state?
  } PinData;
15

Billdefish:
BulldogLowell,

Don't know why I addressed you as Bob. Sorry. Anywho, I'm still a little unclear on the state machine. You may have missed my point though of reducing the amount of if statements and related components.

I want a function like...

Does that make more sense?

Robin2,

Read through the link you provided, thank you.

The only option I can make work correctly(outputs the correct states and data) consists of a ton of if statements and very little interchangeability between boards.

Bill

below is an example of function pointers, in the case that you don't yet understand how to put that together...

if that method works, let us know.

If not, we can always 'point' you in the correct direction :blush:

focus on the loop() function and how simple it is... most of it is to process a button press or entering a char in the serial monitor (which you can do to see how it works).

#include <math.h>

#define NUMBER_OF_ROUTINES 10
#define DEBUG

#ifdef DEBUG
#define DEBUG_PRINTLN(x) Serial.println(x)
#define DEBUG_PRINT(x)   Serial.print(x)
#else 
#define DEBUG_PRINTLN(x) 
#define DEBUG_PRINT(x) 
#endif

void (*ledFunction[NUMBER_OF_ROUTINES])(void)={ ledFunction0, ledFunction1, ledFunction2, ledFunction3, ledFunction4, ledFunction5, ledFunction6, ledFunction7, ledFunction8, ledFunction9 };

byte ledPin = 13;
byte buttonPin = 2;
byte lastPressed;
byte state = 0;
byte increment = 5;
byte briteness;
unsigned long startTime;
boolean printed = false;
//
void setup() 
{  
  Serial.begin(115200);
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP); 
}
//
void loop()
{
  int pressed = digitalRead(buttonPin); 
  if (pressed == LOW)
  {
    if (pressed != lastPressed)
    {
      state++;
      printed = false;
    }
  }
  lastPressed = pressed;
  if (Serial.available())
  {
    char myChar = Serial.read();
    {
      state++;
      printed = false;
    }
  }
  if (state >= NUMBER_OF_ROUTINES) state = 0;
  ledFunction[state]();
}

void ledFunction0()
{
  if (!printed)
  {
    DEBUG_PRINTLN(F("Math Fade"));
    printed = true;
  }
  float val = (exp(sin(millis()/2000.0*PI)) - 0.36787944)*108.0;
  analogWrite(ledPin, val);
}
//
void ledFunction1()
{
  if (!printed)
  {
    DEBUG_PRINTLN(F("Fast Flash"));
    printed = true;
  }
  if (millis() - startTime >= 100UL)
  {
    digitalWrite(ledPin, !digitalRead(ledPin));
    startTime += 100UL;
  }
}
//
void ledFunction2()
{
  if (!printed)
  {
    DEBUG_PRINTLN(F("Slow Flash"));
    printed = true;
  }
  if (millis() - startTime >= 500UL)
  {
    digitalWrite(ledPin, !digitalRead(ledPin));
    startTime += 500UL;
  }
}
//
void ledFunction3()
{
  if (!printed)
  {
    DEBUG_PRINTLN(F("2.5% Duty Cycle Flash"));
    printed = true;
  }
  unsigned long nowMillis = millis();
  if (nowMillis - startTime < 25UL)
  {
    digitalWrite(ledPin, HIGH);
  }
  if (nowMillis - startTime < 1000UL)
  {
    digitalWrite(ledPin, LOW);
  }
  else
  {
    startTime += 1000UL;
  }
}
//
void ledFunction4()
{
  if (!printed)
  {
    DEBUG_PRINTLN(F("Quick Fade"));
    printed = true;
  }
  if (millis() - startTime >= 10UL)
  {
    briteness += increment;
    if (briteness >= 255 || briteness <= 0) increment = - increment;
    analogWrite(ledPin, briteness);
    startTime += 10UL;
  }
}
//
void ledFunction5()
{
  if (!printed)
  {
    DEBUG_PRINTLN(F("Slow Fade"));
    printed = true;
  }
  if (millis() - startTime >= 35UL)
  {
    briteness += increment;
    if (briteness >= 255 || briteness <= 0) increment = - increment;
    analogWrite(ledPin, briteness);
    startTime += 35UL;
  }
}
//
void ledFunction6()
{
  static boolean fadeNow;
  if (!printed)
  {
    DEBUG_PRINTLN(F("Fade Down and Up with Pause"));
    printed = true;
  }
  if (millis() - startTime > 1000UL)
  {
    fadeNow = true;
    briteness = 255;
    increment = -5;
  }
  if (fadeNow)
  {
    for (int i = 0; i < 2; i++)
    {
      for (int j = 0; j <= 255; j += 5)
      {
        analogWrite(ledPin, briteness);
        briteness += increment;
        delay(25);
      }
      increment = - increment;
    }
    fadeNow = false;
  }
}
//
void ledFunction7()
{
  static boolean fadeNow;
  if (!printed)
  {
    DEBUG_PRINTLN(F("Fade Up then Down with Pause"));
    printed = true;
  }
  if (millis() - startTime > 1000UL)
  {
    fadeNow = true;
    briteness = 0;
    increment = 5;
  }
  if (fadeNow)
  {
    for (int i = 0; i < 2; i++)
    {
      for (int j = 0; j <= 255; j += 5)
      {
        analogWrite(ledPin, briteness);
        briteness += increment;
        delay(25);
      }
      increment = - increment;
    }
    fadeNow = false;
  }
}
//
void ledFunction8()
{
  if (!printed)
  {
    DEBUG_PRINTLN(F("Led On"));
    printed = true;
  }
  digitalWrite(ledPin, HIGH);
}
//
void ledFunction9()
{
  if (!printed)
  {
    DEBUG_PRINTLN(F("Led Off"));
    printed = true;
  }
  digitalWrite(ledPin, LOW);
}
16

PaulS,

Thanks for pointing out that the way I "created an array of pointers" is incorrect. I have read most the recent threads concerning function pointers but it's obviously not making sense to me which is why I am here, to learn not to be chastised. You should try constructive criticism as opposed to your regular rants.

Robin2, wildbill, & BulldogLowell,

I will try all of your suggestions a report back with my results. Thank you all for taking the time to try and help me.

Bill

17

Billdefish:
Thanks for pointing out that the way I "created an array of pointers" is incorrect. I have read most the recent threads concerning function pointers but it's obviously not making sense to me which is why I am here, to learn not to be chastised. You should try constructive criticism as opposed to your regular rants.

Life would be very boring if we didn't occasionally get some salt to help us appreciate the taste of sugar :slight_smile:

I am not in favour of function pointers - and I have used them successfully when I had no choice.

In this case all they would seem to do is reduce the amount of code that you need to write to call the functions. In doing so they would almost certainly make the code much less easy to understand and to debug - even if you were perfectly familiar with how to use them.

I doubt if the use of function pointers will make any significant difference to the execution speed or the size of the compiled code.

...R

18

Yes, Robin2, that is true, but he could be a bit more constructive in my opinion. :smiley:

Im not very concerned with compiled code size as my sketch will fit nicely on anything from a pro Mini to the Mega2560. Simplifying the code structure is what I am after. I already know without counting I have way more IF statements than I'd like and even if I only remove 10-20 of them in the body of the code then I will be happy. For now I will continue to use my cascading IF statements since it works.

Thanks,,

Bill