(new to arduino) why wont my Servos turn (code inside)

good evening and thx for taking time to look :slight_smile:

i made a bit of code that (i hope) can run my 18 servos :slight_smile: but even when the value changes the servo dont move

(small fakta so you dont need to ask :wink:
arduino mega 1280
windows 7
all motors er testet and working.
(700w powersupply)
can run the 18 servos at ones in another setup.

i made this code yesterday and as im a new in this feild im sry if not that pretty :frowning:

18 servo
Serial.print for all servo output to moitor. (if you have the mega plz try the code and see for your self)
all commands are sent over serial
all commands er 5 long ( 1 for open, servo number 10, angle for servo to turn 90 = 11090

im sure i dont understand how serial works properly and that what wrong but i need a little push to where i have gone wrong :slight_smile:

any help will be happly taking :wink:

have a max of 9500 char so heres a link to pastebin (sry)

You are constantly reading from serial, regardless of whether there is anything there to read. The vast majority of the time, there won't be anything there, and Serial.read() will return -1. Use Serial.available() (Serial.available() - Arduino Reference) to see if there is anything in the serial queue to read. If you need five characters for your protocol, don't do any reading until Serial.available() returns at least five.

  val = Serial.read()-48;
 
 
 
  if ( val == 1)
  {
   delay(5);
   data[0] = Serial.read()-48;
   delay(5);
   data[1] = Serial.read()-48;
   
   servoname = data[1] * 1 + data[0] * 10;
    if (servoname == 1)
       { delay(5);
       innerleg1angle[0] = Serial.read()-48;
       delay(5);
       innerleg1angle[1] = Serial.read()-48;

Not a check in sight that there is actually data to read. No wonder it doesn't work.

You need to check that there is stuff to read before reading it. Write your own block-until-there-is-data-to-read-then-read-it function if you need to.

The delay() is not only a waste of time, there is no guarantee that the delay is sufficient for another character to arrive.

hey and thx for the replay :slight_smile: my sript is picking up all the serial data i need, and only if the serial.read picks up a 1 first, it will fill the data[0-1] and this works like a chame from what i can see on the Serial.print i have at the last of my script.

  val = Serial.read()-48;
 
 
 
  if ( val == 1)
  {
   delay(5);
   data[0] = Serial.read()-48;
   delay(5);
   data[1] = Serial.read()-48;
   
   servoname = data[1] * 1 + data[0] * 10;
    if (servoname == 1)
       { delay(5);
       innerleg1angle[0] = Serial.read()-48;
       delay(5);
       innerleg1angle[1] = Serial.read()-48;
   
       servoangle[1] = innerleg1angle[0] * 10 + innerleg1angle[1] * 1;
       }

this bit dose what it have to and it works and if i send 10110 it will set the value of servoangle[1] to 10

but it will not turn this motor

innerleg1.write(innerleg1level + servoangle[1]);

( servo name innerleg1
innerleg1level is and int and the value is 90
servoangle[1] got it value from the first pice of code

PS
i know of the "while.Serial.available()" but this will make my scrip useless for any other code i want to run when there is no serial input (unless i got that worng, i am a neeb here ;))

servoangle[1] = innerleg1angle[0] * 10 + innerleg1angle[1] * 1;
int innerleg1angle[1];

Your innerleg1angle[] array only has one element.

dxw00d:

servoangle[1] = innerleg1angle[0] * 10 + innerleg1angle[1] * 1;
int innerleg1angle[1];

Your innerleg1angle[] array only has one element.

i dont understand ??:slight_smile: what element and what effect dose this have? :slight_smile:

The effect it has is that if you read element [1] you read garbage.

This

int innerleg1angle[1];

is an array declaration, (See http://arduino.cc/en/Reference/Array), but it only creates an array with one element in it, that is accessed as innerleg1angle[0].

This

if (servoname == 1)
       { delay(5);
       innerleg1angle[0] = Serial.read()-48;
       delay(5);
       innerleg1angle[1] = Serial.read()-48;
   
       servoangle[1] = innerleg1angle[0] * 10 + innerleg1angle[1] * 1;

is attempting to access two elements of a one element array. The compiler won't stop you doing that, but you are writing to memory that doesn't belong to the array, and most likely belongs to the next variable along. If you write to the variable that does own the memory, then when you try to access the array, it won't contain what you expect it too.

i have to thx all of you for the help and on the lession of arrays i ditten know that..:slight_smile:

but i got it working by simply connect the GND from the arduino to the GND on my powersupply (noob mistake :blush: :blush:)

but one last Q if you have the time..

is my way of commanding my servos using the 5 digt Serial command flawed ?? i mean it works now and i can send the 5digt and use a delay before i send the next 5 to move another servo.. and as long as i dont quie up too many commands for the arduino to remember ill be fine right??

is my way of commanding my servos using the 5 digt Serial command flawed ?? i mean it works now and i can send the 5digt and use a delay before i send the next 5 to move another servo.. and as long as i dont quie up too many commands for the arduino to remember ill be fine right??

Yes, and no. The 5 character serial command is fine. What is not is reading data that might not have arrived yet.

You could put in loop():

if(Serial.available() >= 5)
{
   // Read and use the data
}

Then, there will never be an attempt to read data that isn't there. Also, the delay()s between reading data are not needed with this method.

They are needed with your method, to give the sender time to send the next character and to give the Arduino time to receive the data. They are not needed to give the Arduino time to READ the data.

Properly written, there is no reason to use delay() in your code at all.

PaulS:
You could put in loop():

if(Serial.available() >= 5)

{
   // Read and use the data
}

When your serial port is receiving messages containing more than one byte, you also need a way to detect the start of a message to make sure the sender and receiver stay in sync. (If you assume that every five bytes contains a whole message, then any spurious or lost byte on the serial link or anything else that interrupts the sequence of well-formed messages will leave the receiver stuck out of sync with the sender.