0

I'm working on a project that has 3 RGB LEDs. I want to make a simple method that I can call to control the LEDs for example:

showColor(left, 255, 0, 50);

Which would light up the left LED at 100% red, 0% green, and 20% blue. The code I have running for the method is:

 void showColor(int ledPosition, int redIntensity, int blueIntensity, int greenIntensity)
    {
      if (ledPosition == left)
      {
        analogWrite (redPin1, redIntesity);
        analogWrite (greenPin1, greenIntesity);
        analogWrite (bluePin1, blueIntesity);
      }
      else if (ledPosition == middle)
      {
        analogWrite (redPin2, redIntesity);
        analogWrite (greenPin2, greenIntesity);
        analogWrite (bluePin2, blueIntesity);
      }
      else if (ledPosition == right)
      {
        analogWrite (redPin3, redIntesity);
        analogWrite (greenPin3, greenIntesity);
        analogWrite (bluePin3, blueIntesity);
      }
      else if (ledPosition == all)
      {
        for (int index=0, index < 3 ; index++)
        {
          analogWrite (redPins[index], redIntensity);
          analogWrite (greenPins[index], greenIntensity);
          analogWrite (bluePins[index], blueIntensity);
        }
      }
    }

I'm getting tons of errors, like that all of the Intensity variables are not declared and that left, right, all, and middle are not declared. I don't know if this is possible, or what I'm doing wrong. I think I've seen something like this before somewhere else, and I don't want to just use 1 variable to control everything. An answer would be greatly appreciated!

3
  • 3
    The answer is yes. Beyond that it will be impossible for us to help you with your problem unless you provide a minimal, complete, and verifiable example and the full error messages.
    – per1234
    Commented Jan 4, 2018 at 0:38
  • 2
    the compiler is telling you to declare variables, so instead of declaring those variables, you post a question here? ... why?
    – jsotola
    Commented Jan 4, 2018 at 6:42
  • it is a function, not a method. Classes have methods.
    – Juraj
    Commented Jan 4, 2018 at 7:20

3 Answers 3

3

Check your spelling Intensity instead of Intesity.

Consider using switch instead of if-then-else.

Define left, right and middle as [integer] constants somewhere (and probably something more descriptive like rightLED, leftLED and middleLED) because your code is looking for an int, instead you give it something undefined like left, right and middle (and all). I would consider using bitwise operators for identifying the LEDS, instead of a simple integer flag as this gives future flexibility in lighting up combinations without changing your code (the current function would be rewritten to accommodate this).

0
0

There are two errors in your code:

  1. The word “Intensity” should always be spelled like this, rather than “Intesity” (missing “n”)
  2. In for (int index=0; index < 3; index++), the separators should be semicolons, not commas.

Other than that, and provided proper declarations, your function compiles just fine.

0

This answer is slightly off track, but after reading the title my first though was this.

You can have a function that takes multiple integer parameters and there is nothing wrong with that, but there comes a point in your code where you should be looking at better data management. In this case you have three intensity values that are always going to be related so you could look at a class or struct to hold the data as a single object.

struct Colour
{
    int Red;
    int Green;
    int Blue;
    Colour (const int& r = 0, const int& g = 0, const int& b = 0)
        : Red(r)
        , Green(g)
        , Blue(b)
    {}
}

This would allow you to define an object (called LEDLeft) that held the value for that LED, i.e.

Colour LEDLeft(0xFF, 0xFF, 0x00);    // Define a yellow left LED.

Extending this further you could define a second structure that defined the pins to control the LED, and a third that paired the current colour with the LED.

struct LED
{
    LEDPinout Pins;
    Colour    Colour;
   // Constructor ...
}

Its about this point that using a class rather than a struct would be a better (more conventional*) idea. i.e.

// THIS IS NOT COMPLETE CODE - IT HAS NOT BEEN TESTED.
class LED 
{
    LED (const int& pinRed, const int& pinGreen, const int& pinBlue);
    LED (const LED& other);  // Copy constructor

    void SetColour(const Colour& colour);
    Colour GetColour(void)const;
}

static const Colour RED(0xFF, 0x00, 0x00);
static const Colour BLUE(0x00, 0x00, 0xFF);
enum Positions {Left = 0, Middle, Right, All};
LED TheLeds[] = {LED(2, 3, 4), LED(5, 6, 7), LED(8, 9, 10)};
TheLeds[Left].SetColour (RED);
TheLeds[Right].SetColour (RED);
TheLeds[Middle].SetColour (RED);
delay(1000);
for (int loop = 0; loop < All; ++loop)
    TheLeds[loop].SetColour(BLUE);
delay(1000);

Not the answer you're looking for? Browse other questions tagged or ask your own question.