LED red, green, off: one function to control them all?

[MaxG]

I am using LEDs that have two pins, and two colours, red and green, depending on polarity.

I have used these to indicate power on = red, connection to MQTT = green.

I now want to use 5 of these LEDs. The function I have for one LED is:

#define PIN_LED_GREEN      A0      // MQTT connected
#define PIN_LED_RED        A1      // Power ON

void led_power_mqtt (uint8_t led_state)
{
    switch (led_state)
    {
        case 1:
            // red on
            digitalWrite (PIN_LED_GREEN, LOW);
            digitalWrite (PIN_LED_RED, HIGH);
        break;

        case 2:
            // green on
            digitalWrite (PIN_LED_RED, LOW);
            digitalWrite (PIN_LED_GREEN, HIGH);
        break;

        case 0:
        default:
            // off
            digitalWrite (PIN_LED_RED, LOW);
            digitalWrite (PIN_LED_GREEN, LOW);
        break;
    }

}

I'd like to get to a function call like this:

led_red_green_off (led_name, [0..2])

(That is function name, pass in the LED identifier, and provide a state)

However, it seems there is no way of building variable names, hence, I seemingly need to have a function for each LED.

This could be a case for a multi-dimensional array, but I am not sure how to apply it.

Thinking out loud, I could write the 5 red/green in a single function and pass in the LED identifier and state... but it is still code smell.

Any pointers appreciated.

[UKHeliBob]

@MaxG

it seems there is no way of building variable names

As you have not posted your full sketch we cannot see what you have done but names such as PIN_LED_GREEN are either variables or they are #defined so it is not clear what you mean

You could put the LED pin numbers in a 2 dimensional array, one LED per row, one pin number per column and use an enum or const variable names as the row index and pass the pin values to a function with a parameter to indicate whether you want to show red, green or nothing on the LED

[anon46966594]

Wrapping the led in a class could be useful, then you can pass a reference to an LED and the state you want.

class Led {
  public:
    enum State {
      OFF = 0,
      RED_ON,
      GREEN_ON
    };

    static void powerMQTT(Led& led, State newState) {
      switch (newState) {
        case Led::RED_ON:
          led.redOn();
          break;

        case Led::GREEN_ON:
          led.greenOn();
          break;

        case Led::OFF:
        default:
          led.turnOff();
          break;
      }
    }

    Led(byte redPin, byte greenPin)
      : redPin(redPin), greenPin(greenPin) {
      pinMode (redPin, OUTPUT);
      pinMode (greenPin, OUTPUT);
    }

    void redOn() {
      digitalWrite (redPin, HIGH);
      digitalWrite (greenPin, LOW);
      Serial.println("RED");
    }
    void greenOn() {
      digitalWrite (redPin, LOW);
      digitalWrite (greenPin, HIGH);
       Serial.println("GREEN");
    }

    void turnOff() {
      digitalWrite (redPin, LOW);
      digitalWrite (greenPin, LOW);
       Serial.println("OFF");
    }

  private:
    const byte redPin;
    const byte greenPin;
};

Led testLed(5, 6);
Led* allLeds[3] = { &testLed, &testLed, &testLed };

void setup() {
  Serial.begin(115200);
  Led::powerMQTT(testLed, Led::RED_ON);
  Led::powerMQTT(testLed, Led::GREEN_ON);
  Led::powerMQTT(testLed, Led::OFF);

  // or... simpler (then you can delete the powerMQTT and the enum stuff)
  testLed.redOn();
  testLed.greenOn();
  testLed.turnOff(); 

  // or with an array of Leds
  for (Led*& led : allLeds)
    led->redOn();
}

void loop() {
  // put your main code here, to run repeatedly:

}

The above can be tidied up and stuff can be removed but I don't know exactly how you want to use it (see the comments in the setup function).

I'd also tuck the Led class into its own file(s) to keep it out of the way.

[Grumpy_Mike]

@MaxG

connection to MQTT = green.

What is this MQTT?
Do you have a resistor connected to the LED?

@MaxG

However, it seems there is no way of building variable names,

Correct when C compiles all variable names are lost.

@MaxG

hence, I seemingly need to have a function for each LED.

No don't be silly. There are lots of ways to do this. But first we need to know how these LEDs are wired up. You would need two digital outputs for each LED you have so it is a very wasteful way of getting LEDs to light up.

[MaxG]

@Grumpy_Mike

There are lots of ways to do this. But first we need to know how these LEDs are wired up. You would need two digital outputs for each LED you have so it is a very wasteful way of getting LEDs to light up.

Well, I need only one

The LED has two pins like any normal LED.
However, depending on which pin is high, the LED lights up red or green.
Why is this wasteful?
If I have a dedicated LED for red and one for green, I also need two pins.
In my case I save one connection: ground.

And yes, one of the pins has a resistor.

@anon46966594

a class could be useful

Black magic for me; will spend time to understand this... always happy to learn new things. (Never worked with classes; or at least have not coded one.)

[anon46966594]

Yeah, they're very useful at representing 'real life' behaviour in code. Well worth spending the time to learn classes.

So I think then, that you can simply remove most of the stuff in my last example and do this:

class Led {
  public:
    Led(byte redPin, byte greenPin)
      : redPin(redPin), greenPin(greenPin) {
      pinMode (redPin, OUTPUT);
      pinMode (greenPin, OUTPUT);
    }

    void redOn() {
      digitalWrite (redPin, HIGH);
      digitalWrite (greenPin, LOW);
    }
    void greenOn() {
      digitalWrite (redPin, LOW);
      digitalWrite (greenPin, HIGH);
    }

    void turnOff() {
      digitalWrite (redPin, LOW);
      digitalWrite (greenPin, LOW);
    }

  private:
    const byte redPin;
    const byte greenPin;
};

Led testLedOne(5, 6); // change to whatever your pins are
Led testLedTwo(6, 8); // change to whatever your pins are

void setup() {
  Serial.begin(115200);

  testLedOne.redOn();
  testLedTwo.redOn();
}

void loop() { }

[alto777]

@MaxG

This could be a case for a multi-dimensional array, but I am not sure how to apply it.

I like the advanced solution(s) on offer, but this is easily done with a one dimensional array and some simple math.

The simple math would be doing what the two dimensional array might do behind the scenes.

You shou,d def learn about arrays, no matter the number of dimensions. Most ppl start with one. Dimension.

I can’t tell where you are at with this stuff, just hate to see you thrown in the deep end before you learnt to swim.

I’m on my phone. After you post your code it will be easier to say what I mean.

a7

[alto777]

Like this. There is no error checking, but you might get the idea (not tested)

// either global or put them right in the function (need to pinMode ALL pins involved somewhere!)
const byte redPins[5] = {2, 4, 6, 8, 10};  // needn't be related at all

const byte RED = 0;
const byte GREEN = 1;   // green on next pin + 1

const byte OFF = HIGH;
const byte ON = LOW;    // swap if you wired the LEDs differently


// later... 

void redGreenOnOff(byte theLED, byte theColor, byte onOff)
{
	digitalWrite(redPins[theLED + theColor], onOff);
}

// and use it like

    redGreenOnOff(3, RED, OFF);

If the read and green leads to one LED are not x and x + 1, we start looking at a parallel array for the green pins, which is time to just use a two dimensional array.

a7

[kolaha]

I would take addressable strip of 5 LEDs and control it with 1 wire

[Grumpy_Mike]

@MaxG

depending on which pin is high, the LED lights up red or green.

Yes I know and have used this type of LED.

@MaxG

In my case I save one connection: ground.

I think save is pushing it don't you? A pin that can be duplicated at will, is not considered a pin that can be used, hence it is not a pin that is saved.

@MaxG

Why is this wasteful?

Because one pin is needed for one LED. There are much better ways these days to control 10 LEDs. Have you 10 pins to burn?

@MaxG

And yes, one of the pins has a resistor.

Good.

@MaxG

Well, I need only one

So then, put each pin number for one type of pin in an array. Have another array holding the corresponding low pin number. Then simply use a for loop to iterate through the array entries setting one pin HIGH and the other LOW. So you turn off all the LEDs you have. Simples.

If you want to associate names with each LED pair then you can use the "enum" structure to allow you to use that name in your code, but at the end of the day it is just referencing an array entry.

If you want to use these LEDs more generally then I suggest a other function to control individual LEDs that you pass the array index ( or enum name ) and a boolean variable that decides if the individual LED is turned on or off.

[MaxG]

I am 'electronifying' a mechanical engine control: throttle, choke, brake, etc. ...

The LEDs are for individual engine functions; e.g.:

LED1 engine:
... green = ready to start
... red = engine off

LED2 brake:
... green OFF; red = ON

LED3 choke:
... green = choke is reset; red = choke is set

LED4, LEDx ... for other functions, indicating ON|OFF

Some of these LEDs are dedicated 5mm or embedded in a push button; hence, the separation, rendering the LED strip idea less suitable.

@anon46966594

    Led(byte redPin, byte greenPin)
      : redPin(redPin), greenPin(greenPin) {

Thank you for the class... I like it...
One question, what is the colon for?

[GolamMostafa]

@MaxG
Check if the following skecth helpful!
For test purpose:
When you send A0 command from the InputBox of the Serial Monitor, all LEDs of Group-A are OFF.

When you send A1 command from the InputBox of the Serial Monitor,RED LED of Group-A is ON.

And so on....

//{A0, A1, A2}; {B0, B1, B2}; {C0, C1, C2};
//{D0, D1, D2}; {E0, E1, E2};

void setup()
{
  Serial.begin(9600);
  for (int i = 2; i < 12; i++)
  {
    pinMode(i, OUTPUT); //Dpin-2, 3, 4, 5, 6, 7, 8, 9, 10, 11
  }
}

void loop()
{
  byte n = Serial.available();
  if (n == 2)
  {
    char ch = Serial.read();
    byte x = (Serial.read() - '0');
    switch (ch)
    {
      case 'A':
        led_power_mqtt(2, 3, x);
        break;
      case 'B':
        led_power_mqtt(4, 5, x);
        break;
      case 'C':
        led_power_mqtt(6, 7, x);
        break;
      case 'D':
        led_power_mqtt(8, 9, x);
        break;
      case 'E':
        led_power_mqtt(10, 13, x);
        break;
    }
  }
}

void led_power_mqtt (int PIN_LED_GREEN, int PIN_LED_RED, byte led_state)
{
  switch (led_state)
  {
    case 1:
      // red on
      digitalWrite (PIN_LED_GREEN, LOW);
      digitalWrite (PIN_LED_RED, HIGH);
      break;

    case 2:
      // green on
      digitalWrite (PIN_LED_GREEN, HIGH);
      digitalWrite (PIN_LED_RED, LOW);
      break;

    case 0:
    default:
      // off
      digitalWrite (PIN_LED_GREEN, LOW);
      digitalWrite (PIN_LED_RED, LOW);
      break;
  }
}

[gfvalvo]

@MaxG

One question, what is the colon for?

The code between the ':' and the following '{' is an Initializer List.

[GolamMostafa]

@MaxG

Black magic for me; will spend time to understand this... always happy to learn new things. (Never worked with classes; or at least have not coded one.)

1. For example: To blink L (the built-in LED of UNO), we execute the following the codes.

void setup()
{
    pinMode(13, OUTPUT);
}

void loop()
{
   digitalWrite(13, HIGH);
   delay(1000);
   digitalWrite(13, LOW);
   delay(1000);
}

2. The following is a Class-based skecth for the sketch of Step-1. This simple program might help you to understand the meanings of various features of Class-based program.

class DigitalIO  //Class Name (DigitalIO) is declared/created using class keyword
{
  private:
    int ledPin;  //variable can only be accessed by the functions under public: specifier

  public:
    DigitalIO(int powerpin): ledPin(powerpin) {} //inline constructor to initilize ledPin
    void ioDir();  //member function
    void ledON();
    void ledOFF();
};

DigitalIO led(13);  //led is called object of type DigitalIO

void setup()
{
  Serial.begin(9600);
  led.ioDir();                  //setting up the direction of IO line(13) as output
}

void loop()
{
  led.ledON();
  delay(1000);
  led.ledOFF();
  delay(1000);
}

void DigitalIO::ioDir()   //member function definition; :: (double colon) is calld scope resolution operator
{
  pinMode(ledPin, OUTPUT);
}

void DigitalIO::ledON()
{
  digitalWrite(ledPin, HIGH);
}

void DigitalIO::ledOFF()
{
  digitalWrite(ledPin, LOW);
}

3. Another example of a Class-based sketch for calculating area of a rectangle.

class Sample
{
  private:
    int length, breadth;

  public:
    Sample(int l, int b);// Constructor declarartion; functional form
    friend int calcArea(Sample s); //friend function declaration
};

Sample s(10, 15);

void setup()
{
  Serial.begin(9600);
  int area =  calcArea(s);
  Serial.println(area, DEC);
}

void loop()
{
  
}

int calcArea(Sample s)
{
  int a = (s.length * s.breadth);
  return(a);
}

Sample::Sample(int x, int y)//Constructor definition (functional form)
{
  length = x;
  breadth = y;
}

4. Another example of Class-based sketch for adding two numbers of two different classes using friend keyword for friend function.

// Add members of two different classes using friend functions
class ClassA; //forward declaration of a Class
class ClassB;  //forward declaration

class ClassA
{
  private:
    int numA;

  public:
    ClassA(int x);// : numA(12) {}// inline constructor to initialize numA to 12
    friend int add(ClassA, ClassB);// friend function declaration
};

class ClassB
{
  private:
    int numB;

  public:
    ClassB(int x);//normal constructor definition
    friend int add(ClassA, ClassB); // friend function declaration
};

ClassA objectA(13);       //object declaration
ClassB objectB(14);       //numB of ClassB is intialized t0 14

void setup()
{
  Serial.begin(9600);
  int x = add(objectA, objectB);//actual parameters are objects
  Serial.println(x, DEC);
}

void loop()
{

}

int add(ClassA objectA, ClassB objectB)// access members of both classes
{
  return (objectA.numA + objectB.numB);
}

ClassA::ClassA(int x)
{
  numA = x; 
}

ClassB::ClassB(int x)
{
  numB = x; 
}

[paulpaulson]

Hallo MaxG
This is a simple sketch for demonstration to be modified to your need.
You add new devices to

enum {Engine, Brake};

and the related pin numbers in

DUOLED duoLed[] {
  {9, 10},                  // redPin, greenPin
  {11, 12},
};

that´s all.

/* BLOCK COMMENT
  ATTENTION: This Sketch contains elements of C++.
  https://www.learncpp.com/cpp-tutorial/
  Many thanks to LarryD
  https://europe1.discourse-cdn.com/arduino/original/4X/7/e/0/7e0ee1e51f1df32e30893550c85f0dd33244fb0e.jpeg
  https://forum.arduino.cc/t/led-red-green-off-one-function-to-control-them-all/970658
  Tested with Arduino: Mega[x] - UNO [ ] - Nano [ ]
*/
#define ProjectName "LED red, green, off: one function to control them all?"
// HARDWARE AND TIMER SETTINGS
// YOU MAY NEED TO CHANGE THESE CONSTANTS TO YOUR HARDWARE AND NEEDS
// CONSTANT DEFINITION
enum {Engine, Brake};
enum {Off, Green, Red};
// VARIABLE DECLARATION AND DEFINITION
struct DUOLED {
  byte redPin;              // portPin o---|220|---|LED|---GND
  byte greenPin;            // portPin o---|220|---|LED|---GND
};
DUOLED duoLed[] {
  {9, 10},                  // redPin, greenPin
  {11, 12},
};
void makeLed(DUOLED &part, int stati ) {
  switch (stati) {
    case Off:
      digitalWrite(part.redPin, LOW);
      digitalWrite(part.greenPin, LOW);
      break;
    case Red:
      digitalWrite(part.redPin, HIGH);
      digitalWrite(part.greenPin, LOW);
      break;
    case Green:
      digitalWrite(part.redPin, LOW);
      digitalWrite(part.greenPin, HIGH);
      break;
  }
}
// -------------------------------------------------------------------
void setup() {
  Serial.begin(9600);
  Serial.println(F("."));
  Serial.print(F("File   : ")), Serial.println(__FILE__);
  Serial.print(F("Date   : ")), Serial.println(__DATE__);
  Serial.print(F("Project: ")), Serial.println(ProjectName);
  for (auto &duo : duoLed) pinMode(duo.redPin, OUTPUT),pinMode(duo.greenPin, OUTPUT);
}
void loop () {
  makeLed(duoLed[Engine], Off);
  delay(1000);
  makeLed(duoLed[Engine], Red);
  delay(1000);
  makeLed(duoLed[Engine], Green);
  delay(1000);
   makeLed(duoLed[Brake], Off);
  delay(1000);
  makeLed(duoLed[Brake], Red);
  delay(1000);
  makeLed(duoLed[Brake], Green);
  delay(1000);
}

Have a nice day and enjoy coding in C++.

[Grumpy_Mike]

@MaxG

hence, the separation, rendering the LED strip idea less suitable.

Who mentioned a strip? Not me.

[GolamMostafa]

@anon46966594

So I think then, that you can simply remove most of the stuff in my last example and do this:

Can we avoid the inline definitions of the functions and organize your sketch as follows?

class Led
{
  private:
    const byte redPin;
    const byte greenPin;

  public:
    //Led(byte x, byte y); //constructor ; functional form 
    Led(byte x, byte y):redPin(x), greenPin(y){}   //edit
    void ioDir();  //edit
    void redOn();
    void greenOn();
    void turnOff();
};

Led testLedOne(5, 6); // change to whatever your pins are
Led testLedTwo(6, 8); // change to whatever your pins are

void setup()
{
  Serial.begin(115200);

  testLedOne.redOn();
  testLedTwo.redOn();
}

void loop()
{

}

Led::ioDir()//Edit: Led(byte redPin, byte greenPin)
{
  pinMode (redPin, OUTPUT);
  pinMode (greenPin, OUTPUT);
}

void Led::redOn()
{
  digitalWrite (redPin, HIGH);
  digitalWrite (greenPin, LOW);
}

void Led::greenOn()
{
  digitalWrite (redPin, LOW);
  digitalWrite (greenPin, HIGH);
}

void Led::turnOff()
{
  digitalWrite (redPin, LOW);
  digitalWrite (greenPin, LOW);
}

[MaxG]

What's the reason for taking the functions out of the class?
To me, as the nOOb, it seems 'contained'.

[MaxG]

Sorry, I thought I had replied to the post; not you specifically.

[GolamMostafa]

@MaxG

What's the reason for taking the functions out of the class?
To me, as the nOOb, it seems 'contained'.

This kind of style I (many others) like for the following reasons:
1. I can quickly see the declared member variables and member functions in the Class in a not-much-populated geography.

2. If the function definitios are seperated, then two files could be easily created --
(1) Header File that contain only the declarations.
(2) cpp File that contains ony the function definitions.