Modified default code gives "code violation"

[scottmso]

I've been modifying the default code to try to get a basic "joystick sensitivity reducer" working. I'm not planning on using this in our final robot as it significantly reduces the amount of power we can use. What it's supposed to do is subtract 127 from the joystick value and divide the result by two. Here's the code:

/* Variable declarations (at top of program) below */
unsigned char joystick_input = 127;
char joystick_input_2 = 0;
char drive_output = 0;
unsigned char drive_output_2 = 127;

/*******************************************************************************
* FUNCTION NAME: Slow_Drive
* PURPOSE:       "Halves" the input from the controllers.
* CALLED FROM:   Default_Routine
* ARGUMENTS:     unsigned char joystick_input - input joystick value 
* RETURNS:       unsigned char drive_output_2
*******************************************************************************/

unsigned char Slow_Drive(unsigned char joystick_input)
{
	joystick_input_2 = joystick_input - 127;
	drive_output = joystick_input_2 / 2;
	drive_output_2 = drive_output + 127;
	
	return drive_output_2;
}

void Default_Routine(void)
{
  
pwm01 = Slow_Drive(p1_y);
printf("Slow_Drive for pwm01 = %d",joystick_input); /* printfs are for debugging purposes */
pwm02 = Slow_Drive(p2_y);
printf("Slow_Drive for pwm02 = %d",joystick_input);

/* pwm01 and pwm02 below removed, rest of code left intact */
}

Whenever I put the code on the robot I get a "code violation" (flashing red program LED) or the robot controller will go back into program mode. Right now I'm thinking the problem might be with variable types, but where? Or does anyone see any particular problems? I know I may not have commented enough so feel free to ask any questions. Any advice appreciated - thanks!

[X-Istence]

char joystick_input_2 = 0;
char drive_output = 0;

Why are these straight up char's and not unsigned?

[Joe Ross]

I don't see anything in the code you posted that would cause a code violation. It would help if you posted your entire Default_Routine and Process_Data_From_Master_uP

Also, since you need joystick_input_2 and drive_output to be signed, it makes your code easier to understand and modify if you declare them as "signed char". Also, I can't remember if the C18 default for char is signed or unsigned.

[Keith Watson]

Originally Posted by X-Istence

char joystick_input_2 = 0;
char drive_output = 0;

Why are these straight up char's and not unsigned?

Because of the line,

joystick_input_2 = joystick_input - 127;

joystick_input can be 0 to 255 so the result of this line can be a negative value, so the variables being assigned to must be signed.

Have you commented out the calls to SlowDrive() to prove that is where the runtime error comes from?

I don't know what this particular compiler does but I wonder about this line,

	joystick_input_2 = joystick_input - 127;

joystick_input is unsigned, so the temporary variable created is unsigned, then 127 is subtracted from that, then the temporary is assigned to joystick_input_2. Maybe that is it? To test this theory change that one line to this,

	joystick_input_2 = joystick_input;
	joystick_input_2 = joystick_input_2 - 127;

The first line handles the type change then the second line does exactly what you expect.

[heydowns]

You probably get code errors because of your printf statements:

unsigned char joystick_input = 127;
 // ...
printf("Slow_Drive for pwm01 = %d",joystick_input); 
// ...
printf("Slow_Drive for pwm02 = %d",joystick_input);

You are asking printf to print an integer type (by using %d). But you are only passing it an "unsigned char" data type. In brief, this is in error -- details are below.
To fix, try passing it an unsigned integer explicitly by doing the following:

printf("Slow_Drive for pwm01 = %u", (unsigned int)joystick_input);

The %u says "print as unsigned". The cast to unsigned int will ensure that the arguments are decoded correctly within the printf function.

For those interested, the long explanation:
printf takes a variable number and type of arguments. This is contrary to most functions, which have a very specified number and type of arguments (void f(int x, int y) takes two ints, for example).

Printf relies on you supplying the correct format characters in the initial string argument to be able to decode the subsequent arguments. This is so that it can grab the right number of bytes off the block of memory representing the passed arguments. If you put a format character for int (%d), it will grab sizeof(int) bytes. If you put a format character for char (%c), it will grab sizeof(char) bytes. And so on.

The original poster's code specified %d as the format string, but passed a char. This means the printf code went to grab 2 bytes (sizeof int), but, in reality, the function was only passed 1 byte (sizeof unsigned char). This resulted in printf accessing memory outside the space allocated to the function's arguments.

In this case, it resulted in red light of doom..... But that isn't necessarily always the case. The results here really are undefined and unpredictable. There is a chance it would just run, probably printing out incorrect results for the passed unsigned char.

Another common mistake with printf is when printing long variables. You need to use %ld (for long) or %lu (for unsigned long). If you accidentally use %d for long, you'll likely just get wrong output, but not red light of death.

[X-Istence]

Ah, thank you Keith.

[Keith Watson]

Originally Posted by X-Istence

Ah, thank you Keith.

Was that it?

[devicenull]

I've got a suggestion for you, if you have the old joysticks with the little wheel on the side, scale your drive up or down based on that. It is probably the most useful function we had on our controls last year.

Also, for scaling, try this:

newval = (oldval-127)*(%scale)+127;

Where %scale is a value 0.0 - 1.0 representing the percent to scale it down by. We used this with the wheel last year to slow it down. This way saves two variables, which might not seem like much now, but it's probably good to try to save as much space as possible.

[X-Istence]

Originally Posted by Keith Watson

Was that it?

I am not the one asking the question, I just thanked you for telling me why it was not unsigned.