Problem with negitive numbers

[EHaskins]

I've been working on a velocity PID for motor control, but I'm having trouble.

When the PID calculation returns a positive value the code runs fine, but when it return a negitice value the motor output begins jumping appearently randomly from very high(5 digit) to very low(-5 digit) values.

Once this happens the PID reacts correctly, but due to the values that a far outside of normal it never recovers.

Here is the code I think contains the issue, but I can't find it. I added some comments to help explain the issue.

void MotorControl (unsigned char motor)
{
         
    //The printfs in CalculateSpeed() give correct data. 

	int speed = CalculateSpeed(motor);
	int output;
	//printf("speed on motor %d = %d\r\n", motor);
	//printf("Speed Goal = %d\r\n", speedGoal[motor]);

    // Last line in PID prints the PID output, and it always appears correctly. 

	output = PID(speed, speedGoal[motor], &pidInfo[motor]);
 	currentOutput[motor] += output;

	if (loopCount == 0)
	{
          //This is where the issue is obvious. It prints strange values within
          //the same loop as PID() returns a negitive value, and then until reset.

        
		printf("Output %d = %d\r\n\r\n\r\n", motor, (int)currentOutput[motor]);
	}
	switch (motor)
	{
		case 0:
			MOTOR_ONE = LimitInput(currentOutput[motor] + 127, 0, 255);
			//printf("Motor one set during %d\r\n", motor);
			break;
		case 1:
			MOTOR_TWO = LimitInput(currentOutput[motor] + 127, 0, 255);
			//printf("Motor one set during %d\r\n", motor);
			break;
	}
}

[usbcd36]

What do your functions do? Could the problem exist there?

What variable types are you using? Unsigned variables dropping below zero and incorrect casting can cause problems similar to what you are describing.

[dcbrown]

Sounds like arithmatic wrap/overflow in a byte value. Check all the variable sizes. The compiler will choose to keep arithmetic in byte value instead of promoting to integer like ansi standard.

[Jared Russell]

What type is "currentOutput[motor]"?

If currentOutput[motor] is, say, 0, and you try adding -5 to it, you will get overflow unless currentOutput[motor] is a signed type.

[Joe Ross]

the maximum size for a signed integer is 2^15-1 or 32767. Since you talk about large 5 digit numbers, are you sure you aren't overflowing an int?

[David Fort]

When people say, "overflow", they mean the number goes from its lowest value it can represent to its highest value (or vice versa).

For example, and unsigned char can represent values from 0 to 255.

If you add 1 to 255, you get 0.
If you subtract 1 from 0, you get 255.

Suppose in this expression:
currentOutput[motor] + 127
currentOutput is declared something like this:
unsigned char currentOutput[2];

Then if the value is more than 127, you are going to have trouble. I presume that isn't your case, but I used that example because there is something more interesting:

when you add two things and assign to a larger thing, the promotion to the larger thing doesn't happen until the assignment.

suppose the example above, plus
int x;
x=currentOutput[motor] + 127;

now, since x is an int, you wouldn't (might not) expect the expression currentOutput[motor] + 127 to overflow just because currentOutput[motor] is, say 200, because 200 + 127 is just 327, and that will easily fit in an int.
However, C, first it will add currentOutput[motor] + 127
using byte sized math, and THEN "widen" it to 16 bits to promote it to an integer type. By then, the overflow has occured, you wrapped back around positive infinity to zero, and you have the wrong value.

(small caveat: If the compiler thinks of 127 as an "int" instead of as a "unsigned char" in the example above, then it will first promote both operands of the + to ints. I don't remember the rules for integer literals off hand, and this compiler may well bend them to save some space anyway. To make the example more precise, and applicable to any C compiler (including ANSI compliant ones), then I could have used two unsigned chars, like intX = ucY + ucZ; In this case, the addition of ucY + ucZ might overflow a byte).

I apologize if I went overboard on the detail. This is the sort of thing that confuzles the kids on our team frequently, so I thought maybe some background would help.

David Fort
Team 1001
Hacksaw

[EHaskins]

Originally Posted by usbcd36

What do your functions do? Could the problem exist there?

What variable types are you using? Unsigned variables dropping below zero and incorrect casting can cause problems similar to what you are describing.

Here are the prototypes for the other function involved.

void Motor(char motor, int speed); //Sets the goal for the PID calculation
void ProcessDrive(void); //Performs PID calculations for all motors, and sends outputs
void MotorControl(unsigned char motor); //Performs PID calc for each motor. Called from ProcessDrive
PIDInfo GetNewPIDInfo (void); //Creates a PIDInfo struct using default values.
void InitializeDrive(void); //Performs drive system initialization. Called from UserInit
int CalculateSpeed(unsigned char motor); //Calculates the speed of a given motor 
int PID (int input, int goal, PIDInfo *info); //Performs PID calc using the supplied params.

As I tried to explain, I have several printfs thoughout the different function, so I am confident that the values are correct until the first printf in the code in my first post.

I'm familiar with overflows, and have had them catch me before. I assumed that was the issue, but I couldn't where the it was happening.

Originally Posted by Abwehr

What type is "currentOutput[motor]"?

If currentOutput[motor] is, say, 0, and you try adding -5 to it, you will get overflow unless currentOutput[motor] is a signed type.

It is defined here.

int currentOutput[NUMBER_OF_MOTORS];

I'm confident the overflow is occuring in one of these two lines(comments added), since the printf at the end of PID has the correct value, and the printf immediatly following these lines shows the incorrect value.

	output = PID(speed, speedGoal[motor], &pidInfo[motor]); //returns int
 	currentOutput[motor] += output; //These are both ints

I was able to determine that the switching from high to low values is an overflow by examining the terminal info, but I still can't identify the primary issue.

Does anyone see an issue with my logic here? What am I missing?

If anyone want to see the full code I'll send it. I would post it, but I don't like posting large amounts of broken code.

[Uberbots]

check your PID loop... maybe you inadvertedly got rid of a typecast.

also, are you sure you dont have any longs defined in there?

[dcbrown]

If you suspect overflow in currentOutput, then bump it from int to short long type to see if the symptoms change.

[Alan Anderson]

I don't see anything that defines the type of output. If it were an unsigned variable, it would give the behavior you describe.

[EHaskins]

Originally Posted by Uberbots

check your PID loop... maybe you inadvertedly got rid of a typecast.

also, are you sure you dont have any longs defined in there?

I did a find in my code, and found only one long. It is used in CalculateSpeed, to store a local copy of the encoder count. The output from CalculateSpeed appears correctly in the printfs in both CalculateSpeed and PID.

Originally Posted by Alan Anderson

I don't see anything that defines the type of output. If it were an unsigned variable, it would give the behavior you describe.

It is defined in the code in my first post, line 7. It is an int.

Originally Posted by dcbrown

If you suspect overflow in currentOutput, then bump it from int to short long type to see if the symptoms change.

I will try that next time I have access to the robot.