Pick My Code Apart (Maybe answer a question)

So I am trying to set up a controller for my car that will eventually start the car, lock/unlock doors temperature control, battery control and much more using XBee (for active RFID) and Text messages through a SM5100b

I am starting by creating an active RFID key FOB that will unlock and lock the doors when I am approaching/leaving the car.

the current hardware:
keyfob:
Arduino Fio (blue one not V3) with XBee series 1 (AT Mode) and adxl345(stays off when motionless)
Base Station (in the car)
Arduino Mega with xbee shield

I am setting everything up as state machines so as the code gets more complex everything will run without blocking and states may be accessed by other functions if needed.(some already are)

The main question I have is that I need the delay(1); in the loop function or all my read serial data is garbage, I am assuming it is reading too fast, but can't understand why. (i have searched and my code seems to follow non blocking examples i have seen)

my guess would also be that if i had other tasks running in the loop, which eventually there will be, this would take the time of the delay and everything would work, but i somehow feel like this is cheating as the code should work as a stand alone.

I have come up with two possible solutions

  1. change if(Serial.available()) to if(Serial.available()>2), and simply add a character to the end of strings that are sent and code it to ignore that character when it gets pushed through by the next command. Seems simple, but maybe sloppy I don't know.

  2. increase the baud, maybe this will help the data keep up with the processor (I seriously doubt this will work, but not sure)

Tell me if there is anything about my coding that looks amateur as I am an armature and would really like to take this further.

FYI I am using atmel studio 6 with visual micro plugin (Any suggestions/opinions on ditching arduino environment for atmel now before i get too deep with arduino?)

The code on the fio:
Listens for "hello1"
responds "hello2"
listens for "hello3"
responds "hello4"
after this the FOB and Base Station are considered associated.

my code is:

/* variables for SendXBeeSerial*/
byte SendXBeeSerialCase = 0;  // state machine uses to send serial data while multitasking
byte XBeeCommandLength = 0;
String XBeeCommand = "";
const int XBeeSerialReady = 0;
const int XBeeCommandReceived = 1;
/*Variables for ReadXBeeSerial*/
byte ReadXBeeSerialCase = 0;
char XBeeInChar;
String XBeeInString = "";
const int XBeeWaitingCommand = 0;
const int XBeeReadingCommand = 1;
const int XBeeCommandReady = 2;
/* Variables for KeyFOB*/
byte FOBCase = 0;
const int CallingFOB = 0;
const int WaitFOBResponse = 1;
const int VerifyFOBResponse = 2;
const int CallingFOB2 = 3;
const int WaitFOBResponse2 = 4;
const int VerifyFOBResponse2 = 5;
const int FOBAssociated = 6;
unsigned long FOBTimer;





int XBeeSleepTime = 2500; // Should match sleep of xbee module
int XBeeSleepCorrect = 50;  // used to correct to eliminate buffer containing multiple commands
unsigned long FOBAssociateTimOut = XBeeSleepTime + XBeeSleepCorrect;

void setup(){
	pinMode (13,OUTPUT);
	Serial.begin(9600);
	Serial3.begin(9600);
	FOBTimer = millis();
}

void loop(){

	SendXBeeSerial(); // send commands to xbee (if available)
	ReadXBeeSerial();                   // read commands (from xbee if available)
	KeyFOB();
	delay(1);
	////  reading inputs and setting outputs should be done here /////
}

void SendXBeeSerial(){
	switch (SendXBeeSerialCase) {
		case XBeeSerialReady :

		break ;
		case XBeeCommandReceived :
			XBeeCommandLength = XBeeCommand.length();
			SendXBeeSerialCase ++;
		break;
		case 12 :  // case # defines the maximum string length of XBeeCommand - 2
			Serial3.print('\n');
			SendXBeeSerialCase = 0;
			XBeeCommand = "";
			Serial.print("xbee received string that is too long");/////////////////////////////////////////////////////////////////////////////////
		break;
		default:
			if (SendXBeeSerialCase <= XBeeCommandLength + 1 ){
				Serial3.print(XBeeCommand.charAt(SendXBeeSerialCase - 2));
				SendXBeeSerialCase ++;
			}
			else{
				Serial3.print('\n');
				SendXBeeSerialCase = 0;
				XBeeCommand = "";
			}
		break;
	}
}

void ReadXBeeSerial (){
	switch (ReadXBeeSerialCase){
		case XBeeWaitingCommand :
			if (Serial3.available()){
				ReadXBeeSerialCase ++;
			}
		break;
		case XBeeReadingCommand :
			XBeeInChar = Serial3.read();
		switch (XBeeInChar){
			case '\r':
				ReadXBeeSerialCase ++;
			break;
			case '\n':
				ReadXBeeSerialCase ++;
			break;
			default:
				XBeeInString += XBeeInChar;
			break;
		}
		break;
		case XBeeCommandReady :

		break;
	}
}

void KeyFOB(){
	switch (FOBCase){
		case CallingFOB :
			if (SendXBeeSerialCase == XBeeSerialReady){
				XBeeCommand = "hello1";
				SendXBeeSerialCase = 1;
				FOBCase ++;
				FOBTimer = millis();
			}
		break;
		case WaitFOBResponse :
			if (millis() - FOBTimer <= FOBAssociateTimOut ){
				if (ReadXBeeSerialCase == XBeeCommandReady){
					FOBCase = VerifyFOBResponse;
					Serial.println("command found");
				}
			}
			else {
				Serial.println("fob response timout");
				FOBCase = CallingFOB;
			}
		break;
		case VerifyFOBResponse :
				if (XBeeInString == "hello2"){
				FOBCase ++;
			}
			else {
				FOBCase = CallingFOB;
			}
			XBeeInString = "";
			ReadXBeeSerialCase = XBeeWaitingCommand;
		break;
		case CallingFOB2 :
			if (SendXBeeSerialCase == XBeeSerialReady){
				XBeeCommand = "hello3";
				SendXBeeSerialCase = 1;
				FOBCase ++;
				FOBTimer = millis();
				Serial.println("hello3 sent");
			}
		break;
		case WaitFOBResponse2 :
			if (millis() - FOBTimer <= FOBAssociateTimOut ){
				if (ReadXBeeSerialCase == XBeeCommandReady){
					FOBCase = VerifyFOBResponse2;
					Serial.println("command found");
				}
			}
			else {
				Serial.println("fob response timout");
				FOBCase = CallingFOB;
			}
		break;
		case VerifyFOBResponse2 :
			if (XBeeInString == "hello4"){
				FOBCase ++;
				FOBTimer = millis();
			}
			else {
				FOBCase = CallingFOB;
			}
			XBeeInString = "";
			ReadXBeeSerialCase = XBeeWaitingCommand;
		break;
		case FOBAssociated :
			if (millis() - FOBTimer <= FOBAssociateTimOut ){
				Serial.println("fob associated");
			}
			else {
				FOBCase = 0;
			}
		break;
		default:
			FOBCase = 0;
		break;
	}
}

Read this and fix your message so that the code is easier to read: How to post code properly

Pete

Fixed the post

You certainly should not need a delay() for the reading process. I wonder is the problem that your read function does not check whether there is a character to read - and return immediately if there is not. That is probably what your suggestion 1 is doing.

I think your approach is very similar to the examples in serial input basics which do not need delay() and do not block. It may be useful to look at them.

...R

You aren't reading the serial from XBEE properly.

    case XBeeReadingCommand :
        XBeeInChar = Serial3.read();

The first time through, you have checked that Serial3.available() is non-zero. But once you get into the XBeeReadingCommand state you assume that another character is available. The delay will usually make sure that you have received an entire string, but the processor can execute a lot of instructions in between receiving and without the delay you will try to read when nothing is there.
As a first crack at things, try changing the above snippet to this:

    case XBeeReadingCommand:
        if(Serial3.available() < 1)break;
        XBeeInChar = Serial3.read();

Pete

That worked!

I tried it that way and a few others and as long as the Serial.read() is inside the if(serial.available...
It works fine.

Just rigged it up in my truck and works like a charm (code is different from what is posted)

Thanks Pete

Matt

This is a badass idea...think i'll copy it when you get the code all ironed out.