Trouble with bluetooth connection to servo motors

I've been trying to get a code working for a project I am working on involving the control of 2 servo's and a relay. I have been working with a bluetooth module and an app built with the MIT app inventor 2 to control these devices from an app on my android. I actually had this code working before but i lost the code and now I can't for the life of me figure out what's wrong with the rewrite!

The app sends a 2 byte number to the bluetooth module. In order to differentiate the 2 servo's, I've made one servo be controlled by the numbers 1000-1180 and the other from 2000-2180.

The problem is that the void loop doesn't seem to be running properly. Every time the code runs, it runs from the beginning. Also, it doesn't seem to be accepting and processing the number that the bluetooth is receiving properly. When the number is sent from the app, say 1090, the code should go to the void loop and execute the if loop for the 1000-1180 servo and turn that servo to 90 degrees.

When i tried some testing myself, i put in initial conditions for the servo positions and tried to get the relay working along with it, but every time i tried to command a single servo, multiple things would happen. the relay might turn on, the servo would turn to the wrong angle, etc.

So if you can see anything apparently wrong with the code or would like any additional info on the issue, I would love some comments and can provide more if need be.

BeerCannon.ino (1.06 KB)

if(bluetooth.available()> 0 )
{
unsigned int servopos = bluetooth.read();
unsigned int servopos1 = bluetooth.read();
unsigned int realservo = servopos1*256+servopos;

If there is at least one byte to read, reading 2 bytes does not make sense.

The values read are NOT servo positions, so the names are crap.

The resulting value is no more real, or less real, than the bytes.

Your
indenting
leaves
a
lot
to
be
desired.

Learn to use Tools + Auto Format.

I got the idea for the code from another location - i understood that servopos would read the first byte and servopos1 would read the second byte, that is why the calculation takes place. How would I go about it if this way is not appropriate?

How would I go about it if this way is not appropriate?

The process is appropriate, if you do nothing until there is more than one (not more than 0) byte to read.

The names are not.

Okay but i'm only sending 2 byte numbers so that wouldn't matter. if it is >0 and I'm only sending 2 byte numbers it will work the same. I changed it anyways, but changing this and the names won't solve my problem. Do you have any feedback as to how to make the program actually work or are you here just to comment on the aesthetics of the code?

Derekrw:
if it is >0 and I'm only sending 2 byte numbers it will work the same.

A mistaken assumption. The 2 byte numbers come one byte at a time. If your reads happen between their arrivals then you'll get the wrong number.

Are you saying that your board is resetting and running setup again? How are your servos being powered?

It's true that you cannot assume two bytes are ready. If there is only one ready you get all out of sync. How about only reading one byte at a time and acting when you have read two bytes?

#include <Servo.h>
#include <SoftwareSerial.h>
#define RELAY1  5

Servo myservo1;
Servo myservo2;

int bluetoothTx = 10;
int bluetoothRx = 11;

unsigned int servopos;
unsigned int servopos1;
unsigned int realservo;
int byteCount = 0;

SoftwareSerial bluetooth(bluetoothTx, bluetoothRx);
void setup() {
  pinMode(RELAY1, OUTPUT);
  myservo1.attach(9);
  myservo2.attach(6);
  Serial.begin(9600);
  bluetooth.begin(9600); 
  myservo1.write(179);
  myservo2.write(179);
}

void loop() {
  if(bluetooth.available()> 0 )
  {
    // if no bytes read yet
    if( byteCount++ == 0 )
    {
      // read first byte
      servopos = bluetooth.read();

    } 
    else {

      // read second byte
      servopos1 = bluetooth.read();
      realservo = servopos1*256+servopos;

      Serial.println(realservo);

    } // else

  } // if


  // if 2 bytes read
  if( byteCount == 2 )
  {
    byteCount = 0;  // reset byte count

    if (realservo >= 1000 && realservo < 1180) {
      int servo1 = realservo-1000;
      //servo1 = map(servo1,1000,1180,0,180);
      myservo1.write(servo1);
      //delay(1000);
      //myservo1.write(servo1+90);
    }
    if (realservo >= 2000 && realservo < 2180) {
      int servo2 = realservo-2000;
      //servo2 = map(servo2,2000,2180,0,180);
      myservo2.write(servo2);
      delay(1000);
      myservo2.write(servo2+90);
      //if (realservo = 3000) {
      //  digitalWrite(RELAY1, LOW);
      //  delay(2000);
      //  digitalWrite(RELAY1, HIGH);
      //  delay(1000);
      //}
    }

  } // if

}

Wouldn't it be simpler to send the servo number in the first byte and the servo position in the second byte?