HELP: Robot Crash in Autonomous

[Shadow503]

I wrote some code for a simple table based autonomous mode, but whenever we ran it, the robot completely crashed (random motors will thrash around a split second before the thing finally dies).

I figure there must a seg fault caused by to the way I'm using a struct pointer, but I don't know enough C++ to get this coded the right way.

Anyone got a spare moment to look over the code? Thanks for your time!

auto_commands_2009.h:

//autonomous commands
#define AUTO_STOP			0
#define AUTO_DRIVE_4WHEEL	1
#define AUTO_DRIVE_PIVOT	2
#define AUTO_DRIVE_TANK		3
#define AUTO_DRIVE_CRAB		4


/*
struct commands
{
	int steer_mode;
	double time;
	double speed;
	double angle;
};
*/
struct Command
{
	int command;	//what command to run
	double param1;	//usually time
	double param2;	//usually speed
	double param3;	//usually angle
};

struct Command command_list_0[] = {	//DEFAULT - STAY STOPPED
		//   Command        parm 1     	parm 2   	parm 3  
		//	Steer mode		Time(Sec)	Speed		Angle
		{AUTO_STOP,       	0.0,        0.0,     	0.0}		
};

struct Command command_list_1[] = {

//   Command              parm 1     	parm 2   	parm 3  
//	Steer mode				Time(Sec)	Speed		Angle
{AUTO_DRIVE_4WHEEL,       	1.0,        10.0,     	0.0},
{AUTO_DRIVE_PIVOT,         	1.0,        10.0,     	10.0},
{AUTO_DRIVE_4WHEEL,         1.0,        10.0,     	10.0},
{AUTO_DRIVE_PIVOT,         1.0,        0.0,      	0.0},
{AUTO_STOP,          		1.0,       0.0,     	0.0}};
. . .

autonomous_2009.cpp:

/**
 * Call this method to drive
 * @param speed a double describing the speed		-15fps to +15fps
 */
void Autonomous::procTable(int AutoMode) {
	// TODO Finish autonomous code
	//		static INT32 autoSec = (INT32)GetClock() + 1;
	curTime+= TIME_PER_CYCLE;
	currentStateTime += TIME_PER_CYCLE;
	struct Command *currentCommand;
	
	//set the commandlist
	switch (AutoMode) {
	case AUTOMODE_0:		//Auton mode 0
		currentCommand = command_list_0;
		break;
	case AUTOMODE_1:		//Auton mode 0
		currentCommand = command_list_1;
		break;
	case AUTOMODE_2:		//Auton mode 0
		currentCommand = command_list_2;
		break;
	case AUTOMODE_3:		//Auton mode 0
		currentCommand = command_list_3;
		break;
	case AUTOMODE_4:		//Auton mode 0
		currentCommand = command_list_4;
		break;
	
	
	default:
		currentCommand = command_list_0;
		break;
	}
	
	//behave based on the current command
	switch (currentCommand[cur_command].command) {
	case AUTO_STOP:			//robot stopped
	default:		
		reqDriveSpeed = 0.0;				// Requested Drive speed
		reqAngle = 0.0;						// Requested steer angle
		reqDriveMode = STEER_MODE_4WHEEL;	// Requested Drive Mode
		if(currentCommand[cur_command].param1 != 0 && currentStateTime >= currentCommand[cur_command].param1 ) {	//time to move on to next state
			if(cur_command + 1 < sizeof(currentCommand)) {	//there is another command to run
				cur_command++;	//increment to next command
				currentStateTime = 0; //move on to next state
			}
		}		
		break;
	}
}

[Sentient]

I'm not sure if this is the root of your problem, but I don't believe you are using structs properly, and I think it's strange the compiler didn't catch it. You only need the "struct" keyword before Command when you declare the struct. So, you don't need so say

struct Command command_list_0[] = {	//DEFAULT - STAY STOPPED
		//   Command        parm 1     	parm 2   	parm 3  
		//	Steer mode		Time(Sec)	Speed		Angle
		{AUTO_STOP,       	0.0,        0.0,     	0.0}		
};

but rather

Command command_list_0[] = {	//DEFAULT - STAY STOPPED
		//   Command        parm 1     	parm 2   	parm 3  
		//	Steer mode		Time(Sec)	Speed		Angle
		{AUTO_STOP,       	0.0,        0.0,     	0.0}		
};

The only place you need the struct keyword is here, when you declare the struct:

struct Command
{
	int command;	//what command to run
	double param1;	//usually time
	double param2;	//usually speed
	double param3;	//usually angle
};

You did this quite a bit. I hope this fixes the problem. C++ may be doing something weird and unpredictable because of all those extra struct keywords.

[Shadow503]

Thanks! I'll try that during the next build window!

[Pancake]

If you're looping the Autonomous::procTable(int AutoMode), make sure to delete the *currentCommand pointer.

And if you're using WPILib, I fail to see the point for using structs...

[heydowns]

Originally Posted by Shadow503

/**
 * Call this method to drive
 * @param speed a double describing the speed		-15fps to +15fps
 */
void Autonomous::procTable(int AutoMode) {

// ........

			if(cur_command + 1 < sizeof(currentCommand)) {	//there is another command to run
				cur_command++;	//increment to next command
				currentStateTime = 0; //move on to next state
			}

// .........

}

This is at least part of the problem you are experiencing. "sizeof(currentCommand)" will not give the size of the commands array. sizeof evaluates at compile time to the size of a type, or, when used with a variable, the type of the variable. currentCommand's type is "struct Command *". So, the sizeof (regardless of which command table is chosen to have currentCommand point to) will always evaluate to the size of a struct Command pointer (probably 4). This could be greater than the actual size of your command table, causing your program to potentially walk off the end of an array.

The easiest way to change this would be to add another command type that signifies the end of the command table. When your autonomous code sees that command as it processes the table, it knows it has reached the last entry.

[byteit101]

also if you do this

 type *varname;
...
varname=value;

you assign varname's address at memory the value of value
you need to do this

type *varname;
...
*varname=value;
OR
type *varname;
...
varname=&value;

EDIT: if you do int* evilval; evilval=anynumber, especialy low ones like 0; and then later *evilval = another number; another number is going into the memory address anynumber crashing the computer, which is what i did when i was not thinking earlier in the season

[heydowns]

Originally Posted by byteit101

also if you do this

 type *varname;
...
varname=value;

you assign varname's address at memory the value of value
you need to do this

type *varname;
...
*varname=value;
OR
type *varname;
...
varname=&value;

Generally speaking, yes. However, OP's assignment of the only pointer he is using is correct:

struct Command *currentCommand;
// ...
currentCommand = command_list_X;

command_list_X is an array of "struct Command". Assigning to a pointer using an array (of the same base type) is perfectly legitimate and will simply result in the pointer pointing to the base of the array.