Pre test review

Afternoon all,

I have been asking for help over the last months with the RS485 protocol, after lots of help from the forum I managed to get my head around the concept. Instead of using someone else’s code I wanted to write my own (silly I know just wanted a challenge) seems like I bit off more than I could chew, but none the less I have managed to write some sending and receiving code, I plan on setting up the hardware tonight/tomorrow for a full test, but I would like if possible for some member’s to review it and maybe advise me where I could make the code less repetitive or comber sum, many thanks in advance for any help.

Sending code

//Proof of concept for sending array of rs485 created 10.05.12 by Mark Netto

long A = A2;// sets analog pin two as A
long B = A3;// sets analog pin three as B
long newA = 0; //variable to store value of inputA
long newB = 0; //variable to store value of inputB
static long oldA = 0;//variable used to compare values of input A
static long oldB = 0;//variable used to compare values of input B
long packet[4];//packet array to store all data to transmit.
int TXpin = 8;//transmit enable pin = pin 8
long START = 11111;//start of transmission byte
long END   = 10101;//end of transmission byte
long CRC = (packet[0]+packet[1]+packet[2]+packet[3]);//crc check

void setup()
{
  Serial.begin(19200);
  pinMode(TX, OUTPUT);
}

void loop()
{
  newA = analogRead(A/4);
  newB = analogRead(B/4);
 
 if ((oldA != newA)||(oldB != newB))
 {
   oldA=newA;
   oldB=newB;
   create();
   send();
   delay(2500);
 }
}


void create()
{
 packet[0] = START;
 packet[1] = oldA ;
 packet[2] = oldB ;
 packet[3] = END ;
 packet[4] = CRC  ;
}

void send()
{
  digitalWrite(TXpin, HIGH);//enable transmission
  for(int i=0; i < 4; i++)
{
Serial.println(packet[i]);
}
 digitalWrite(TXpin, LOW);//disable transmission
}

Receiving code

//Proof of concept for recieving array over rs485 created 11.05.12 by M. Netto


#include <Servo.h>
Servo myservo; // Create servo object to control a servo.
int pos1 = 0; // Variable to store the servo position.
int pos2 = 0; // Variable to store the servo position.
long packet[4];//packet array to store incoming values.
long A;//variable A.
long B;//Variable B.
long ICRC;//variable for incoming CRC value.
int RXpin = 5;//sets pin 5 as RX enable pin.
long START = 11111;//start of transmission byte.
long END   = 10101;//end of transmission byte.
long CRC = (packet[0]+packet[1]+packet[2]+packet[3]);//crc check

void setup()
{
Serial.begin(19200);
pinMode(RXpin,OUTPUT);
myservo.attach(8); // Attaches the servo on pin 8 to the servo object.
myservo.attach(9); // Attaches the servo on pin 9 to the servo object.  
}

void loop()
{
  start:
  void recieve();
  ICRC = packet[4];
  if(ICRC == CRC)
  {
    A = packet[1];
    B = packet[2];
  }
  else
  {
   goto start; 
  }
  
pos1 = map(A, 0,1000, 180, 0);
myservo.write(pos1);
pos1 = map(B, 0,1000, 180, 0);
myservo.write(pos2);
delay(5);
  
}


void receive()
{
 if (Serial.available() > 0) // confirm data is available
{ 
 if (Serial.read() == START);
 {
  packet[0]=START;//asign start to packet [0]
  }
 if(Serial.available() >0);
 {
  packet[1] = Serial.read();//asign start to packet [1]
  }
 if(Serial.available() >0);
 {
  packet[2] = Serial.read();//asign start to packet [2]
  }
 if(Serial.available() == END);
 {
  packet[3] = END;
  }
  if(Serial.available() >0);
 {
  packet[4] = Serial.read();//asign start to packet [4]
  }
  
}
}

Regards,

Mark

This one leapt off the page at me.

long END   = 10101;//end of transmission byte.

Hint: That's not binary, and so it is more than a byte's worth.

Gosh, they're fairly flying off the page.

newA = analogRead(A/4);
long A = A2;

Why use only one byte of precious RAM when you could use four?

long CRC = (packet[0]+packet[1]+packet[2]+packet[3]);

Zero plus zero plus zero plus zero.

You never compiled this code, I would start by giving that a try. You don't have to upload anything to the Arduino, the leftmost button is just a compile run.

Agreed, but to be fair, with just one tiny change, the first will compile.
It won’t do what you want, but at least it’ll run.

The second compiles OK, and will run, but again, isn’t going to work correctly.

if(Serial.available() == END);

Remind me again how big the receiver buffer is?
And when you’ve corrected the function, lose the semicolon.

 packet[4] = CRC  ;

“packet” is declared with only four elements; writing to the fifth one is going bite you on the posterior.

 goto start;

“…INCOMING!..”

At some point, you’re going to wonder why you’re attaching the same servo object to two different pins

AWOL/Pylon many thanks for your comments, I guess I still have lots to learn! :cold_sweat: I will try again.