please help me with my Arduino code!

[lolimpol]

Can anyone help me with some Arduino code? i'm completely stuck.
The basic idea is that I will be able to control the RGB led strip under my desk and in my pc with my pc. i made a quick image of what i had in mind for the visual studio program (which my friend could do hopefully ) it's attached.
 
This is the Arduino code, and the basic idea is that through serial communication the visual program will send a value of:
- 1 for off
- 2 for on
- 3 for color in the format of: 3,255,255,255, 3/255/255/255 or some other divider.
the original Instructable code didn't work either. (http://www.instructables.com/id/Computer-Controlled-RGB-Led/)
 
I have a green led hooked up to pwm pin 5,
a red led hooked up to pwm pin 3
and finally a yellow (dammit where's my blue led? ) led hooked up to pwm pin 6.
 
I tried to bodge together the readASCIIString built in code, but that didn't work either.
here are both:
1st My code:

Code: [Select]

int redpin = 3; //intialize all the pins
int greenpin = 5;
int bluepin = 6;
int r = 255; //initialize the rgb values
int g = 255;
int b = 255;
int serial; //initialize the variable to store the serial input

void setup() {
  pinMode(redpin, OUTPUT); //set the pins as output
  pinMode(greenpin, OUTPUT);
  pinMode(bluepin, OUTPUT);
  analogWrite(redpin, 0); //turn the pins off
  analogWrite(greenpin, 0);
  analogWrite(bluepin, 0);

  Serial.begin(9600); //begin the serial monitor at 9600 baud
  Serial.print("Hello"); //say Hello, be polite!
}

void loop() {
    int serial = Serial.read();
    if (Serial.available() > 0)
    if (serial == 0) //if it's 0 the pins turn off
    {
      analogWrite(redpin, 0); //turn the pins off
      analogWrite(greenpin, 0);
      analogWrite(bluepin, 0);
      }
    else if (serial == 1)//if it's 1 the pins turn on
    {
      analogWrite(redpin, r); //turn the pins on
      analogWrite(greenpin, g);
      analogWrite(bluepin, b);
      }
    else if (serial == 3)//if it's 3 change the colours
    {
      int r = Serial.parseInt();//look for the next valid integer in the incoming serial stream
      int g = Serial.parseInt();//do it again
      int b = Serial.parseInt();//and again
      }
    }
and the Instructables code:

Code: [Select]

#define RedLED 3
#define GreenLED 6
#define BlueLED 5

boolean powerOn = false;

int r = 0;
int g = 0;
int b = 0;

boolean blinking = false; //Whether or not we should be blinking the light
boolean blinkState = false; //When we are blinking, is the light supposed to be on (true) or off (false)
unsigned long blinkInterval = 1000; //How long we want between blinks.
long lastBlink = 0;

char inData;

void setup()
{
  pinMode(RedLED, OUTPUT);
  pinMode(GreenLED, OUTPUT);
  pinMode(BlueLED, OUTPUT);
  analogWrite(RedLED, 255); //Start out with the light off
  analogWrite(GreenLED, 255);
  analogWrite(BlueLED, 255);
 
  Serial.begin(9600);
}

void loop()
{
  if (Serial.available() > 0) //If there is data coming from the pc
  {
     inData = Serial.read() - '10';
     
     if (inData == 0) // 0 means turn light off
     {
         togglePower(false);
     }
     else if (inData == 1) // 1 means turn light on
     {
        togglePower(true);
     }
     else if (inData == 3) // 3 means change colors
     {
       r = 255 - getIncomingData();
       g = 255 - getIncomingData();
       b = 255 - getIncomingData();
       
       //Serial.println(r); //For testing purposes
       //Serial.println(g);
       //Serial.println(b);
       
       if (powerOn)
       {
         updateColors();
       }
     }
     else if (inData == 4) // 4 means turn off blink mode
     {
       blinking = false;
       updateColors();
       //Serial.println("Blink mode off"); //For testing purposes
     }
     else if (inData == 5) // 5 means turn on blink mode
     {
       blinkInterval = getIncomingData();
       blinking = true;
       //Serial.println("Blink mode on "); //For testing purposes
       //Serial.println(blinkInterval); //For testing purposes
     }

  }
 
  //This section controls the blinking
  if (blinking && powerOn)
  {
    if (millis() - lastBlink >= blinkInterval)
    {
      lastBlink = millis();
      if (blinkState)
      {
        updateColors();
      }
      else if (!blinkState)
      {
        turnLEDoff();
      }
     
      blinkState = !blinkState;
    }
  }
}

void togglePower(boolean on)
{
  if (!on)
  {
    powerOn = false;
   
    turnLEDoff();
   
    //Serial.println("Power is off"); //For testing purposes
  }
  else if (on)
  {
    powerOn = true;
    updateColors(); //This turns on the rgb LED
    //Serial.println("Power is on"); //For testing purposes
  }
}

void turnLEDoff()
{
    //For thie RGB led, 255 is what we think of as 0.
    digitalWrite(RedLED, 255);
    digitalWrite(GreenLED, 255);
    digitalWrite(BlueLED, 255);
}

// Credit for this section goes to ROBOTMAN at Trossen Robotics Community forums.
int getIncomingData()
{
  inData = 0;
  int newNumber = 0;
 
  while (inData != '/')
  {
    inData = Serial.read();
    if (inData > 0 && inData != '/')
    {
       newNumber = newNumber * 10 + inData - '0';
    }
  }
 
  return newNumber;
}

void updateColors()
{
 analogWrite(RedLED, r);
 analogWrite(GreenLED, g);
 analogWrite(BlueLED, b);
}


[lolimpol]

as you can probably tell from my code, i am a complete noob. I understand that serial.read or .print is read and write, but i have no other experience with arduino. could you help me?

[Avacee]

Your text says 1 = off but your if statement uses ==0

You are calling Serial.available() after you just read the data with Serial.read() - unless there is data waiting it will return false.

Serial.read() returns a byte .. so your check for ==0 means you are checking for null
The byte value for "0" is 48 so you need

Code: [Select]

if (serial == 48)
or

Code: [Select]

if (serial = '0')

so your loop should start

Code: [Select]

void loop()
{
    if (Serial.available() > 0)
    {
         int serial = Serial.read();
         if (serial == '0') //if it's 0 the pins turn off
         {......}
    }
}

You are also re-declaring the variables r,g and b within the if statement for ==3 this means the values are stored in the local variables and not the global variables.
So when you turn on the LEDs within the ==1 statement the global values are all still 255

[brucehoult]

You're fairly close. Two main things wrong.

Code: [Select]

    int serial = Serial.read();
    if (Serial.available() > 0)

Serial.read() will read a character if one is there and return its ASCII value, otherwise returns -1.
Serial.available() will tell if anything if there after you tried to read a character. Which there won't be if it's  a 1 or 2 command.

If you check for serial == -1 then you don't need the Serial.available() (and SHOULD NOT have it).

You can check Serial.available() first if you want, but then everything else after it should be in { } so it's only done if there are characters waiting.

Let's assume that the PC can only send correct commands, so we don't have to worry about errors.

The next thing is you're confusing numbers and characters.

If the PC sends a 1 then Serial.read() will return the character '1', which is the number 49. Not the number 1.

EITHER

make your tests like

Code: [Select]

if (serial == '1'){
OR

do

Code: [Select]

serial = serial - '0'; and then

Code: [Select]

if (serial == 1){.

The Serial.parseInt() stuff should work fine (each call will swallow the comma or / or whatever, and then read the number), but you forgot to actually do analogueWrite() for each of the LED pins.

[lolimpol]

thank you very much, and i know. i completely gave up after a certain point
but what is the second thing? i put 'number' on each if.

[Avacee]

Updated my reply as with second sight the syntax could be confusing

Is it working now? .. if not what does your code look like?

[lolimpol]

the one and two are working! but writing a value trough 3 does not work. i type: 4
I get: 255 255 255
I type: 3 4,5,6
I get: 4 5 6
I type: 4 again,
I get: 255 255 255

so 3 isn't writing to r g and b.

(BTW: I added 4 for debugging temporarily)

code:

Code: [Select]

int redpin = 3; //intialize all the pins
int greenpin = 5;
int bluepin = 6;
int r = 255; //initialize the rgb values
int g = 255;
int b = 255;
int serial; //initialize the variable to store the serial input

void setup() {
  pinMode(redpin, OUTPUT); //set the pins as output
  pinMode(greenpin, OUTPUT);
  pinMode(bluepin, OUTPUT);
  analogWrite(redpin, 0); //turn the pins off
  analogWrite(greenpin, 0);
  analogWrite(bluepin, 0);

  Serial.begin(9600); //begin the serial monitor at 9600 baud
  Serial.print("Hello"); //say Hello, be polite!
}

void loop() {
    int serial = Serial.read();

     
    if (serial == '1') //if it's 1 the pins turn off
    {
      analogWrite(redpin, 0); //turn the pins off
      analogWrite(greenpin, 0);
      analogWrite(bluepin, 0);
      Serial.print(" The led's are now off ");
      }

     
    else if (serial == '2')//if it's 2 the pins turn on
    {
      analogWrite(redpin, r); //turn the pins on
      analogWrite(greenpin, g);
      analogWrite(bluepin, b);
      Serial.print(" The led's are now on ");
      }

     
    else if (serial == '3')//if it's 3 change the colours
    {
      int r = Serial.parseInt();//look for the next valid integer in the incoming serial stream
      int g = Serial.parseInt();//do it again
      int b = Serial.parseInt();//and again

      //for debugging:
      Serial.print(" ");
      Serial.print(r);
      Serial.print(" ");
      Serial.print(g);
      Serial.print(" ");
      Serial.print(b);
      Serial.print(" ");
      }

     
      else if (serial == '4')//if it's 3 change the colours
    {
      Serial.print(" ");
      Serial.print(r);
      Serial.print(" ");
      Serial.print(g);
      Serial.print(" ");
      Serial.print(b);
      Serial.print(" ");
      }
}


[Avacee]

You are declaring new variables r,g and b which means the values are stored in these new local variables and not the global variables.
That's why when you press '4' you get the 255 again.

[obiwanjacobi]

You could simplify the protocol. Simply send 3 rgb values or nothing. 3 values of zero is off.

Code: [Select]

int redpin = 3;
int greenpin = 5;
int bluepin = 6;

void setLeds(int r, int g, int b)
{
     analogWrite(redpin, r);
     analogWrite(greenpin, g);
     analogWrite(bluepin, b);
}

void setup() {
  pinMode(redpin, OUTPUT); //set the pins as output
  pinMode(greenpin, OUTPUT);
  pinMode(bluepin, OUTPUT);

  setLeds(0, 0, 0); //turn the pins off

  Serial.begin(9600); //begin the serial monitor at 9600 baud
  Serial.print("Hello"); //say Hello, be polite!
}

void dumpValues(int r, int g, int b)
{
      //for debugging:
      Serial.print(" ");
      Serial.print(r);
      Serial.print(" ");
      Serial.print(g);
      Serial.print(" ");
      Serial.print(b);
      Serial.print(" ");
}

int readSerial()
{
    while(Serial.available() == 0);
    return Serial.parseInt();
}

void loop() {

    int r = readSerial();
    int g = readSerial();
    int b = readSerial();

    setLeds(r, g, b);
    dumpValues(r, g, b);
}


[lolimpol]

i can use the void setleds, but the rest doesn't work, the led's are acting strange:
sometimes the red one doesn't react, other times there are only two lit up 

[lolimpol]

well how do i change the global variables than?

[obiwanjacobi]

Change

Code: [Select]

else if (serial == '3')//if it's 3 change the colours
    {
      int r = Serial.parseInt();//look for the next valid integer in the incoming serial stream
      int g = Serial.parseInt();//do it again
      int b = Serial.parseInt();//and again

into

Code: [Select]

else if (serial == '3')//if it's 3 change the colours
    {
      r = Serial.parseInt();//look for the next valid integer in the incoming serial stream
      g = Serial.parseInt();//do it again
      b = Serial.parseInt();//and again


[Avacee]

well how do i change the global variables than?

Read this:  https://www.arduino.cc/en/reference/scope

Code: [Select]

     else if (serial == '3') //3 = change the colours
      {
        r = Serial.parseInt();        // Your old code had "int r = ....." whuch declared a new variable called r but it only existed within the scope of this if statement.
        g = Serial.parseInt();       // You therefore saved the parseInt() result into the new variable and not into the global variable
        b = Serial.parseInt();       // Without the "int b = ..." the parseInt() is saved into global variable b which was declared at the top of your file.
    }


i can use the void setleds, but the rest doesn't work, the led's are acting strange:
sometimes the red one doesn't react, other times there are only two lit up 

obiwanjacobi's simpler protocol is 

[lolimpol]

YESSSSS!  IT WORKS!!!! woohoo
thank you very very much.

now i don't want to be some spoiled piec 'a crap, but can i store the variable somehow even when restarting the arduino or will i have to do that in the visual program somehow?

[Avacee]

now i don't want to be some spoiled piec 'a crap, but can i store the variable somehow even when restarting the arduino or will i have to do that in the visual program somehow?

read this:  https://www.arduino.cc/en/Reference/EEPROM

You can save values into the EEPROM memory between reboots
note: each bit of the EEPROM can only be written to about 100,000 times before it goes wonky

[lolimpol]

about 100,000 per reboot? or in total?

[Avacee]

Total over the life of the arduino - so 100,000 number 3's if you save each time - which is a LOT of RGB updates..
You don't need to worry about this unless you start saving data to it every second.

With hindsight I shouldn't have mentioned it and caused any alarms. I apologise.

[obiwanjacobi]

Just don't write the rgb values each time they change.
Have a command to save the explicitly and send that.

[lolimpol]

no, i don't like maximum values i'll try to do it in visual, thank everyone for your help!