communication between galaxy s2 and arduino

hi i need help, i am setting up a control for the central locking in my car, my desire is to us my mobile to control it via bluetooth, i am trying to build in a password type security setup, ie phone transmits password that arduino recieves then if correct either locks or unlocks the car, however the problem i am having is that arduino only seems to recieve gibberish from the phone, (among others but the most pressing is the gibberish), i have had the phone and arduino communication via the amarino test code as well as other examples available out there

this is the bluetooth unit,
http://www.ebay.com/itm/160879748403?ssPageName=STRK:MEWNX:IT&_trksid=p3984.m1439.l2649
and i am using the mega2680 board, the phone is a galaxy s2, using the amarino app

i also intend (in the future) to sort out a custom app as the remote locking is only 1 feature i wish to use the arduino for in the car

i would appreciate help or guidance as to where to find out more info on doing this, or if its my code or the application from the phone that is causing my problem

the code is as follows

#define BUFFERSIZE 127
uint8_t inBuffer[BUFFERSIZE];

int ledpin = 13;
int inLength; // length of data in the buffer
int unlockpin = 22;//unlock relay pin
int lockpin = 23;// locking relay pin
int numLoop = 0; // number of times we looped
char unlocker = 'Apassword1'; //unlock code
char locker = 'Apassword2'; //lock code

void setup() { 
  Serial.begin(9600);
  pinMode(ledpin, OUTPUT);
  
} 

void unlock(){
  digitalWrite(unlockpin, HIGH);
        delay(1000);
        digitalWrite(unlockpin, LOW);
        delay(1000);
      Serial.print("welcome");}

void lock(){
  digitalWrite(lockpin, HIGH);
        delay(1000);
        digitalWrite(lockpin, LOW);
        delay(1000);
       Serial.print("goodbye");
     }

void loop () {
  
 
  // read string if available
  if (Serial.available()) {
    inLength =  0;
       while (Serial.available()) {
      inBuffer[ inLength] = Serial.read();
        if (inBuffer[ inLength] == unlocker){     //compares data recieved and if correct unlocks car
          unlock();
           }
        if (inBuffer[ inLength] == locker){       //compares data recieved and if correct locks car
           lock();
           }
        
      inLength++;
      if (inLength >= BUFFERSIZE)
         break;
    }
  
    Serial.print("Arduino received: ");
    Serial.write(inBuffer ,inLength);
    Serial.println();
  }
 
 
 // blink the led and send a number
  //digitalWrite(ledPin, HIGH); // set the LED on
  delay(1000); // wait a bit
  Serial.println(numLoop);
  //digitalWrite(ledPin, LOW); // set the LED off
  //delay(1000);
  numLoop++;
  

}

and i am using the mega2680 board, the phone is a galaxy s2, using the amarino app

Can you provide a link to your board, I never heard of a mega2680 board.

And edit your post to include code tags. Code is modified by the forum software if you don't use code tags, so they are a MUST!

The baud rate is not on the default value. Have you reconfigured your bluetooth module?

apologies, the board is a mega 2560 r3 rev 3

the baud rate was set through trial and error using the amarino tool kit, it seems that the bluetooth module will only work at this speed, i have made no attempt to set the baud rate

i hope this is what you mean by code tags?

a point of note, the intent is to remove the blinking LED and the counter, as you can most likely tell, I am attempting to modify an existing project that i found posted elsewhere

#define BUFFERSIZE 127
uint8_t inBuffer[BUFFERSIZE];

int ledpin = 13;
int inLength; // length of data in the buffer
int unlockpin = 22;//unlock relay pin
int lockpin = 23;// locking relay pin
int numLoop = 0; // number of times we looped
char unlocker = 'Apassword1'; //unlock code
char locker = 'Apassword2'; //lock code

void setup() { 
  Serial.begin(9600);
  pinMode(ledpin, OUTPUT);
  
} 

void unlock(){
  digitalWrite(unlockpin, HIGH);
        delay(1000);
        digitalWrite(unlockpin, LOW);
        delay(1000);
      Serial.print("welcome");}

void lock(){
  digitalWrite(lockpin, HIGH);
        delay(1000);
        digitalWrite(lockpin, LOW);
        delay(1000);
       Serial.print("goodbye");
     }

void loop () {
  
 
  // read string if available
  if (Serial.available()) {
    inLength =  0;
       while (Serial.available()) {
      inBuffer[ inLength] = Serial.read();
        if (inBuffer[ inLength] == unlocker){     //compares data recieved and if correct unlocks car
          unlock();
           }
        if (inBuffer[ inLength] == locker){       //compares data recieved and if correct locks car
           lock();
           }
        
      inLength++;
      if (inLength >= BUFFERSIZE)
         break;
    }
  
    Serial.print("Arduino received: ");
    Serial.write(inBuffer ,inLength);
    Serial.println();
  }
 
 }

i hope this is what you mean by code tags?

No, these are the quote tags. The code tags are behind the "#"-Button in the editor. Please EDIT (you don't have to repost!) your post and change the tags from "quote" to "code".

the baud rate was set through trial and error using the amarino tool kit, it seems that the bluetooth module will only work at this speed, i have made no attempt to set the baud rate

That means, the link you provided is not describing the device you got (because they explicitly mention the default baud rate of 38400).

        if (inBuffer[ inLength] == unlocker){     //compares data recieved and if correct unlocks car

This won't work. It doesn't compare strings but only a single character and even that not in the intended way. Use strncmp for this (Google gives you the description of the function).

  delay(1000); // wait a bit

Remove this delay completely. It's not waiting a bit, it's waiting almost an eternity (one second is a really long time for the Arduino) and doing nothing in this time. It's completely unnecessary.

You haven't described your actual problem yet. What does not work?

Ok the delay bit has been remove, that part of the code is a left over from where i started, and hopefully the code is in the correct code tags, again i apologize for that, first time using the forum,

the problem i am having; you seem to have identified

        if (inBuffer[ inLength] == unlocker){     //compares data recieved and if correct unlocks car

This won't work. It doesn't compare strings but only a single character and even that not in the intended way. Use strncmp for this (Google gives you the description of the function).

this is exactly what is happening, when i was attempting to send the password to arduino the serial port monitor was only showing it recieving 'a' and therefore nothing was happening, i will now look more into the strncmp as you suggested and hopefully be able to get this to work as intended, then to sort the bluetooth module out, (i believe that the problems there are more my doing than the units)

A problem with your program is that if character have a short delay - which can happen due to the asynchronuous nature of serial - the while loop breaks and the first half of the password is erased. And only the second half - which is the build up in the string - is not enough to open the door.

You should add a start and stop character around the string send so the password will not be split by teh code

read until startchar found // e.g. <
read the password // e.g. >
checkPassword();

You should add a start and stop character around the string send so the password will not be split by teh code

read until startchar found // e.g. <
read the password // e.g. >
checkPassword();

Some code that uses these start and stop markers:

#define SOP '<'
#define EOP '>'

bool started = false;
bool ended = false;

char inData[80];
byte index;

void setup()
{
   Serial.begin(57600);
   // Other stuff...
}

void loop()
{
  // Read all serial data available, as fast as possible
  while(Serial.available() > 0)
  {
    char inChar = Serial.read();
    if(inChar == SOP)
    {
       index = 0;
       inData[index] = '\0';
       started = true;
       ended = false;
    }
    else if(inChar == EOP)
    {
       ended = true;
       break;
    }
    else
    {
      if(index < 79)
      {
        inData[index] = inChar;
        index++;
        inData[index] = '\0';
      }
    }
  }

  // We are here either because all pending serial
  // data has been read OR because an end of
  // packet marker arrived. Which is it?
  if(started && ended)
  {
    // The end of packet marker arrived. Process the packet

    // Reset for the next packet
    started = false;
    ended = false;
    index = 0;
    inData[index] = '\0';
  }
}

Where is says "Process the packet" is where the call to checkPassword() goes.

PaulS

thanks for that, now please bear with me as i am not that familiar with the code aspect of arduino, I am the first to admit that i know just enough to be dangerous, so i would like to confirm what is happening in the code you put up

as i understand what i am reading is that the code is reading each character of the packet individually using the variable 'inchar', however it does nothing until it recieves the '<' to indicate that this is the data it is waiting for then it reads the data until '>' is recieved,

now the part i am not getting is what does it then do with the packet it has recieved, it appears that it is storing it in the array(??) indata, and as a result of the packet being stored it can't be split?

after the packet 'indata' is recieved it is compared to what i set as allowable passwords, and processed from there, and then we reset for the next packet

and does this mean that the password i send to the arduino will need '<' and '>' to indicate that it is the packet we're looking for?
and from this code it appears that the maximum packet length would be 80 characters?? and it would make no difference if they characters are letters or numbers?

i hope asking this doesn't offend, but i do like to understand what is happening at the more basic level,

also if i understand this correctly then the #define BUFFERSIZE 127 and uint_8 inbuffer[BUFFERSIZE] aspects of my code can be removed altogether as it is just basically wrong?

as i understand what i am reading is that the code is reading each character of the packet individually using the variable 'inchar', however it does nothing until it recieves the '<' to indicate that this is the data it is waiting for then it reads the data until '>' is recieved,

Reads, and stores, the data, yes.

now the part i am not getting is what does it then do with the packet it has recieved, it appears that it is storing it in the array(??) indata, and as a result of the packet being stored it can't be split?

There is a block, following the if(started && ended) statement, where YOU put the code to deal with the whole packet that has been stored in inData. That packet can be parsed any way you want. The idea with this code is that you don't attempt to parse a packet until you have a whole packet. In the if(started && ended) block, you DO have a whole packet.

after the packet 'indata' is recieved it is compared to what i set as allowable passwords, and processed from there, and then we reset for the next packet

That is what you were supposed to implement. You didn't share your code, so I can't tell if you did it right, but that is what is supposed to happen.

and does this mean that the password i send to the arduino will need '<' and '>' to indicate that it is the packet we're looking for?

Yes. Well, to indicate that it is a packet. Any packets sent will need the start and end of packet markers.

and from this code it appears that the maximum packet length would be 80 characters?? and it would make no difference if they characters are letters or numbers?

True, though you can change the size, and true. Numbers are letters, too.

i hope asking this doesn't offend, but i do like to understand what is happening at the more basic level,

Actually, I approve of you asking. It means that you are trying.

also if i understand this correctly then the #define BUFFERSIZE 127 and uint_8 inbuffer[BUFFERSIZE] aspects of my code can be removed altogether as it is just basically wrong?

Not wrong as much as incomplete. There were assumptions about when it was OK to use the packet received that were not true. The code I posted, following robtillaart's suggestion, has different assumptions about when it is OK to use the packet, and those assumptions will be true when the packet is well formed (i.e. properly delimited).

Pauls sorry for the delay but i had a few days away with the family, and now have had time to look at and try to understand further what you surely see clearly

now the part i am not getting is what does it then do with the packet it has recieved, it appears that it is storing it in the array(??) indata, and as a result of the packet being stored it can't be split?

There is a block, following the if(started && ended) statement, where YOU put the code to deal with the whole packet that has been stored in inData. That packet can be parsed any way you want. The idea with this code is that you don't attempt to parse a packet until you have a whole packet. In the if(started && ended) block, you DO have a whole packet.

this part i understand, however what is the packet been called, do i call it up as indata, or indata[ index] or do i declare it to be something else entirely? - i have tried several approaches to this and it doesn't seem to be doing what i want it to

the code i am using now at leasts processes the packets, however it executes the both functions i have written, as lock and unlock, regardless of what packet it recieves, as long as the packet has the beginning and end markers

it is to be expected that i have screwed this up as well, at least at this point the lock and unlock functions are actually being called which is a step in the right direction.

also in my code i have it sending back to me what it recieved, and it also doesn't do that, which leads me to believe that i am comparing the wrong data to my set passwords

my code is as follows

#define SOP '<'
#define EOP '>'

bool started = false;
bool ended = false;


int unlockpin = 22;//unlock relay pin
int lockpin = 23;// locking relay pin
char unlocker = 'password1'; //unlock code
char locker = 'password2'; //lock code
char inData[80];//recieved packet
byte index;//recieved characters to put into indata



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

void unlock(){
  digitalWrite(unlockpin, HIGH);
        delay(1000);
        digitalWrite(unlockpin, LOW);
        
      Serial.print("welcome");}

void lock(){
  digitalWrite(lockpin, HIGH);
        delay(1000);
        digitalWrite(lockpin, LOW);
        
       Serial.print("goodbye");
     }

void loop () {
  
 while(Serial.available() > 0)
  {
    char inChar = Serial.read();
    if(inChar == SOP)
    {
       index = 0;
       inData[index] = '\0';
       started = true;
       ended = false;
    }
    else if(inChar == EOP)
    {
       ended = true;
       break;
    }
    else
    {
      if(index < 79)
      {
        inData[index] = inChar;
        index++;
        inData[index] = '\0';
      }
    }
  } 
  
    // We are here either because all pending serial
  // data has been read OR because an end of
  // packet marker arrived. Which is it?
 if(started && ended)
  {
    // The end of packet marker arrived. Process the packet
       if (inData[ index] == locker);
           {lock();}

       if (inData[ index] == unlocker);
           {unlock();}
           
        Serial.print("Arduino received: ");
        Serial.write(inData[ index]);
        Serial.println();
   
  
    // Reset for the next packet
    started = false;
    ended = false;
    index = 0;
    inData[index] = '\0';
  }
        
}

this part i understand, however what is the packet been called, do i call it up as indata, or indata[ index] or do i declare it to be something else entirely?

The inData variable is an array. inData[index] is one element of that array. Suppose that inData[0] hold 'c', inData[1] holds 'a', inData[2] hold 't', and inData[3] holds a NULL. Then, inData holds the name of an animal that claws the furniture, sheds all over the place, and sleeps all day (when not clawing the furniture).

  • i have tried several approaches to this and it doesn't seem to be doing what i want it to

I'm sorry. Can you over closer to the camera? All I can see is your arm waving.

the code i am using now at leasts processes the packets, however it executes the both functions i have written, as lock and unlock, regardless of what packet it recieves, as long as the packet has the beginning and end markers

This part?

       if (inData[ index] == locker);
           {lock();}

       if (inData[ index] == unlocker);
           {unlock();}

There are at least four problems here. First, index points to the NULL character that terminates the array. You want to be comparing inData to locker and/or unlocker, not one character of the array (and the wrong one at that).

Second, the ; at the end forms the body of the statement, to be executed if the statement is true. Then, unconditionally, you call the lock() and unlock() functions.

Finally, inData (without the []) is an address. locker is an address. Those two addresses are clearly not the same. To compare strings, you need to use a string compare function, like maybe strcmp().

And last, but not least,

char unlocker = 'password1'; //unlock code
char locker = 'password2'; //lock code

The Arduino is not capable of dealing well with multi-byte characters. You want locker and unlocker to be strings:

char *locker = "someStuff";
char *unlocker = "otherStuff";

also in my code i have it sending back to me what it recieved, and it also doesn't do that, which leads me to believe that i am comparing the wrong data to my set passwords

No, you don't:

        Serial.print("Arduino received: ");
        Serial.write(inData[ index]);
        Serial.println();

As I mentioned earlier, inData[index] is the NULL that terminates the array. Hardly a printable character. Just print inData.

Pauls

thanks for your help and after only an hour of reading and research and your help i have the project more or less doing what i want it to now, the 4 problems that you pointed out have been corrected, and i now understand a great deal more about using strings and comparing them, if you are interested the mostly finished code is as follows

#define SOP '<'
#define EOP '>'

bool started = false;
bool ended = false;


int unlockpin = 22;//unlock relay pin
int lockpin = 23;// locking relay pin
char *unlocker = "password1"; //unlock code
char *locker = "password2"; //lock code
char inData[80];//recieved packet
byte index;//recieved characters to put into indata



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

void unlock(){
  digitalWrite(unlockpin, HIGH);
        delay(1000);
        digitalWrite(unlockpin, LOW);
        
      Serial.println("welcome");}

void lock(){
  digitalWrite(lockpin, HIGH);
        delay(1000);
        digitalWrite(lockpin, LOW);
        
       Serial.println("goodbye");
     }

void loop () {
  
 while(Serial.available() > 0)
  {
    char inChar = Serial.read();
    if(inChar == SOP)
    {
       index = 0;
       inData[index] = '\0';
       started = true;
       ended = false;
    }
    else if(inChar == EOP)
    {
       ended = true;
       break;
    }
    else
    {
      if(index < 79)
      {
        inData[index] = inChar;
        index++;
        inData[index] = '\0';
      }
    }
  } 
  
    // We are here either because all pending serial
  // data has been read OR because an end of
  // packet marker arrived. Which is it?
 if(started && ended)
  {
    // The end of packet marker arrived. Process the packet
       if (strcmp(inData, locker) == 0)
           {lock();}

       if (strcmp(inData, unlocker) == 0)
           {unlock();}
           
        Serial.print("Arduino received: ");
        Serial.write(inData);
        Serial.println();
   
  
    // Reset for the next packet
    started = false;
    ended = false;
    index = 0;
    inData[index] = '\0';
  }
        
}

the only niggly detail is that no details are being sent to my phone, which has me puzzled, as during my frustration over the last few weeks the phone would intermittently recieve data from the arduino, the serial monitor does show that the information is being sent,

again many thanks