Serial communication doesnt work after adding code ? Help!

Using Arduino Programming Questions
#1

Hello there!

I am relatively new to programming and i have a small project for my school that is very simple but i have a little bug or code error but not quite sure what i am doing wrong.

Its a small robot that i control with wii joysticks via wireless serial communication.
I tried adding obstacle avoidance but after i added the code it seems if the serial communication is not working anymore.

So if someone could point out what im doing wrong it would be much appreciated!

Code with obstacle avoidance:

#include <Servo.h>

#define trigPin 12
#define echoPin 13
#define led 10

Servo myServo, motor; 
String pos;
String sped;
String readString;

void setup(){
  
 Serial.begin(19200);
 myServo.attach(8);
 motor.attach(9);
 pinMode(trigPin, OUTPUT);
 pinMode(echoPin, INPUT);
 pinMode(led, OUTPUT);
 
}

void loop(){
  
  int duration, distance;
  
  
 digitalWrite(trigPin, LOW);
  delayMicroseconds(5);
  digitalWrite(trigPin, HIGH);
  delayMicroseconds(10);
  digitalWrite(trigPin, LOW);

  duration = pulseIn(echoPin, HIGH);
  distance = duration/58.2;
  
 if(distance < 7 && distance > 0 ){
     digitalWrite(led, HIGH);
  }else{
    digitalWrite(led, LOW);
  }
  
  if (Serial.available())  {
    char c = Serial.read();  //gets one byte from serial buffer
    if (c == '*') {
      //do stuff



      int ind1 = readString.indexOf(',');  //finds location of first ,
      pos = readString.substring(0, ind1); 
      int ind2 = readString.indexOf(',', ind1+1);
      sped = readString.substring(ind1+1, ind2);
      
      int Pos = pos.toInt();
      int Sped = sped.toInt();
      
      
      
      if (Pos == 125){
      Pos = 90;
      }else if( Pos > 125){
      Pos = map(Pos, 126, 225, 90, 160);
      }else if ( Pos < 125){
      Pos = map(Pos, 124, 30, 90, 20);
      }else;
      Pos = Pos;
      
      myServo.write(Pos);
      
   if(distance < 7 && distance > 0 ){
     digitalWrite(led, HIGH);
  }else{
    
    digitalWrite(led, LOW);
    
      if (Sped == 100){
        motor.write(30);
      }else{
        motor.write(90);
      }
  }
      
      readString = "";
      pos = "";
      sped = "";
      
    }else{
      readString += c;
      
    }
  }
}

Original code (Communication works fine):

#include <Servo.h>



Servo myServo, motor; 
String pos;
String sped;
String readString;

void setup(){
  
 Serial.begin(19200);
 myServo.attach(8);
 motor.attach(9);
 pinMode(led, OUTPUT);
 
}

void loop(){
  
  
  if (Serial.available())  {
    char c = Serial.read();  //gets one byte from serial buffer
    if (c == '*') {
      //do stuff



      int ind1 = readString.indexOf(',');  //finds location of first ,
      pos = readString.substring(0, ind1); 
      int ind2 = readString.indexOf(',', ind1+1);
      sped = readString.substring(ind1+1, ind2);
      
      int Pos = pos.toInt();
      int Sped = sped.toInt();
      
      
      
      if (Pos == 125){
      Pos = 90;
      }else if( Pos > 125){
      Pos = map(Pos, 126, 225, 90, 160);
      }else if ( Pos < 125){
      Pos = map(Pos, 124, 30, 90, 20);
      }else;
      Pos = Pos;
      
      myServo.write(Pos);
      
   
    
    digitalWrite(led, LOW);
    
      if (Sped == 100){
        motor.write(30);
      }else{
        motor.write(90);
      }
  
      
      readString = "";
      pos = "";
      sped = "";
      
    }else{
      readString += c;
      
    }
  }
}

Thank you!

#2 
}else;

Now that is just plain silly. If there is nothing to do, DO NOT PUT AN ELSE STATEMENT IN.

And, get your enter key fixed. NOTHING follows the }.

#3

Oh... I thought if it just was one line after and if or else statement you did not need to use brackets.

Eg.

if(something > something);
do something;

instead of

if(something > something){
do something;
}

Is that wrong?

And is that the only problem?

#4

In this case
if(something > something);
do something;

"do something;" will run every time, no matter what happens in the if. The first ; closes the code out that occurs from the if.

This
if(something > something){
do something;
}

and this
if(something > something)
do something;

would "do something;"
when the if evaluated true. But only for simple cases. The braces { } make it clear what will happen for things that are beyond simple.

#5

Oh ok, thank you!

#6

But none the less it still does not work....

Is it because there is code before the Serial.available line?

Im sorry im just really frustrated i have spent so much time trying to fix it!
Thanks!

#7

But none the less it still does not work…

Your before and after code is poorly formatted. Use Tools + Auto Format AND put every { and every } on lines BY THEMSELVES.

It is not clear what you added to the first sketch to create the second sketch. However, the fact that you seem to think you have a couple of terabytes of memory, and can, therefore, use Strings, suggests that you have run out of memory.

The complete lack of debug statements isn’t helping your case.

#8 
  if (Serial.available())  
  {
    char c = Serial.read();  //gets one byte from serial buffer
    //Then does nothing with c
    blah
    blah
 }
else
 {
  readString += c;  //Even though c doesn't contain anything
 }

Edit: Actually, looking again, that's not quite what it does, so I've just proved the point made by PaulS and others. You really need to polish up the presentation some.

#9

What KenF?

It reads char c to check if its ever a '*' which indicates end of line and if it not that then it adds the incoming byte to the string 'readString' which is later used to find the commas so that the information can be seperated....

Im sorry i dont get what the problem is.

#10

And PaulS i will try to reformat it and make sure its all good... But i am for sure not out of memory as it only uses 7k out of 32k bytes (arduino uno).

And what sort of debugging statements are you reffering/suggesting to?

#11

pulseIn() is a blocking function and since it called every time through the loop right before you read a character to build a String, it may be causing problems in the serial communication.

Try reading the echo pulse length with an external interrupt or a pin change interrupt.

#12

Finally!

Thank you cattledog!

#13