bluetooth relays, error compiling!!

so… this is the sketch that i am trying to use to switch a relay on or off with bluetooth via ArduDroid but i keep getting an error compiling
expected } at the end of input
now it is probably because i am incredibly tired but i just cant see it.
anyone shed some light?
Thanks!!

heres the sketch

char val; // variable to receive data from the serial port
int ledpin = 2; // LED connected to pin 2 (on-board LED)

void setup()
{
pinMode(ledpin = 2, OUTPUT); // pin 2 (on-board LED) as OUTPUT
pinMode(ledpin = 3, OUTPUT); // pin 3 (on-board LED) as OUTPUT

Serial.begin(9600); // start serial communication at 115200bps

}

void loop()

{
if ( Serial.available() ) // if data is available to read

{
;
}

val = Serial.read(); // read it and store it in ‘val’

if ( val == ‘a’ ) // if ‘a’ was received led 2 is switched off
{
digitalWrite(ledpin = 2, HIGH); // turn Off pin 2
}

if ( val == ‘A’ ) // if ‘A’ was received led 2 on
{
digitalWrite(ledpin = 2, LOW); // turn ON pin 2
}

if ( val == ‘b’ ) // if ‘b’ was received led 3 is switched off
{
digitalWrite(ledpin = 3, HIGH); // turn Off pin 3
}

if ( val == ‘B’ ) // if ‘B’ was received led 3 on
{
digitalWrite(ledpin = 3, LOW); // turn ON pin 3
} //else (ledpin = 3, LOW) //set led pin 3 to low state

if ( val == ‘C’ ) // if ‘C’ was received led 2 on for 1 second
{
digitalWrite(ledpin = 2, LOW); // turn ON pin 2
delay(1000); // wait 1 second
digitalWrite(ledpin, HIGH); // turn Off pin 2
}

if ( val == ‘D’ ) // if ‘D’ was received led 3 on for 1 second
{
digitalWrite(ledpin = 3, LOW); // turn ON pin 3
delay(1000); // wait 1 second
digitalWrite(ledpin, HIGH); // turn Off pin 3
}

if ( val == ‘E’ ) // if ‘E’ was received led 2 on for 5 seconds
{
digitalWrite(ledpin = 2, LOW); // turn ON pin 2
delay(5000); // wait 500 milli seconds
digitalWrite(ledpin, HIGH); // turn Off pin 2
}

if ( val == ‘F’ ) // if ‘F’ was received led 3 on for 5 seconds
{
digitalWrite(ledpin = 3, LOW); // turn ON pin 3
delay(5000); // wait 500 milli seconds
digitalWrite(ledpin, HIGH); // turn Off pin 3
}

if ( val == ‘G’ ) // if ‘G’ was received turn led pin 2 on for 500ms then switch off and turn on pin 3 for 500 mili seconds then off
{
digitalWrite(ledpin = 2, LOW); // turn ON pin 2
delay(500); // wait 500mili second
digitalWrite(ledpin, HIGH); // turn Off pin 2
digitalWrite(ledpin = 3, LOW); // turn ON pin 2
delay(500); // wait 500 mili second
digitalWrite(ledpin, HIGH); // turn Off pin 2
}

if ( val == ‘h’ ) // if ‘h’ was received switch off all pins
{
digitalWrite(ledpin = 13, LOW); // turn Off pin 13
digitalWrite(ledpin = 2, HIGH); // turn Off pin 2
digitalWrite(ledpin = 3, HIGH); // turn Off pin 3
}

if ( val == ‘H’ ) // if ‘H’ was received switch pin 2 on and off 1000 times

for (int i = 0; i < 1000; i++)
{
digitalWrite(ledpin = 2, HIGH); // turn ON pin 2
delay (1000); //wait 1000 mili seconds
digitalWrite(ledpin = 2, LOW); // turn Off pin 2
delay (1000); //wait 1000 mili seconds

}

Hi, you need one last closing } and that should fix it.
The last one at the moment is part of the last IF statement.
You need one for the void loop().

Tom... :slight_smile:

http://forum.arduino.cc/index.php/topic,148850.0.html

scroll down to #7

  pinMode(ledpin = 2, OUTPUT); // pin 2 (on-board LED) as OUTPUT
  pinMode(ledpin = 3, OUTPUT); // pin 3 (on-board LED) as OUTPUT

This method of re-defining 'ledpin' almost every time you use it is confusing. It would be easier to read if you used the normal convention of using named constants for pin numbers:

const int led0Pin = 13;
const int led1Pin = 2;
const int led2Pin = 3;

  pinMode(led0Pin, OUTPUT);
  pinMode(led1Pin, OUTPUT);
  pinMode(led2Pin, OUTPUT);

This construct is strange:

  if ( Serial.available() )      // if data is available to read
      { ;}  // Do nothing

  val = Serial.read();         // read regardless

It is more common to do this:

  if ( Serial.available() )  {    // if data is available to read
     val = Serial.read();         // read regardless
     // Process the input character
  }

A series of "if (val == constant)" statements can be simplified with a switch/case statement:

    switch (val) {
    case 'a': digitalWrite(led1Pin, HIGH); break;
    case 'A': digitalWrite(led1Pin, LOW); break;
    case 'b': digitalWrite(led2Pin, HIGH); break;
    case 'B': digitalWrite(led2Pin, LOW); break;
    case 'C': digitalWrite(led1Pin, LOW); 
                 delay(1000); 
                 digitalWrite(led1Pin, HIGH); 
                 break;
.
.
.
     }

Thanks all for your input! Don't know how i missed the last } but that did indeed fix it.
I've gone through, gotten rid of the crap i didn't need, shortened and simplified the sketch.

thanks heaps to johnwasser, I've made some changes and will be fixing the 'if' statements up with switch/case statements instead.

Now I've got a new question!

This sketch is now working perfectly, I've now got 4 relays running without a problem, although when i turn the board on, the relays instantly become active, anyway around this?

Also, it there a way to communicate wirelessly (is that a word?) Between the relay boards and the arduino board? So for instance, i can have the arduino set up in the loungeroom and have my relay board in the switch box?
I currently don't have a wifi connection or even phone lines in the house so i can't use a power over ethernet system.

Thanks all!!

Aiden-is-Aiden:
This sketch is now working perfectly, I've now got 4 relays running without a problem, although when i turn the board on, the relays instantly become active, anyway around this?

According to your sketch you don't have any relays, you just have LEDs.

I have changed the sketch since i first posted, they are relays rather than led's

Aiden-is-Aiden:
I have changed the sketch since i first posted, they are relays rather than led's

But you're not going to post the new code? Good luck with it.

Currently on my phone so can't post the new code, will post in the next day or so.