Pros haaallp!!!!

I recently decided to switch to PROS for this competition season. I have enjoyed the lack of restriction, but I’ve recently encountered a very serious problem. Our robot has taken a very HAL’esque turn recently and will occasionally enter a state wherein it no longer responds to the Vex controller yet still runs motors. Powering down the controller does nothing once this has set in nor does disabling via a competition switch. As of yet, nothing has been found to solve this short of powering down the robot manually. My initial assumption was that it was a problem with the VexNet system interacting with PROS, later testing revealed that it is either some horrible programming mistake on my part or a problem within the PROS system. My team foresees a scenario where this happens in a competition setting and we risk disqualification.
I can post my code upon request. Any help solving this problem would be greatly beneficial and appreciated (I would prefer to keep my current program in PROS rather than convert back to an officially supported language).

hugs and kisses:p,
hshultz324

We would love to help you, but in order to do that we’re going to need some actual code to look at…

sure thing!


int rpmRight=0;
int rpmLeft=0;

int locations] = {7000 ,6000, 10000};
int targetRPM = 7200;

int driverCheck = 0;


int max(int a, int b)
{
	if(a>b){ return a; }
	else{ return b; }
}


void getRightRPM(void *ignore)
{
	while(1)
	{
		rpmRight = (encoderGet(rightEncoder)*33);
		encoderReset(rightEncoder);

		lcdPrint(uart1, 1, "Right: %d RPMs", rpmRight);

		delay(20);
	}
}

void getLeftRPM(void *ignore)
{
	while(1)
	{
		rpmLeft = (encoderGet(leftEncoder)*33);
		encoderReset(leftEncoder);

		lcdPrint(uart1, 2, "Left:  %d RPMs", rpmLeft);

		delay(20);
	}
}


void maintainRPM(void *ignore)
{
	int speedRight = 0;
	int speedLeft = 0;

	while(1)
	{
		float kp=0.05;

		int diffRight = targetRPM - rpmRight;
		int diffLeft = targetRPM - rpmLeft;

		speedRight += kp*diffRight;
		speedLeft  += kp*diffLeft;

		if(speedRight>127){speedRight=127;}
		else if(speedRight<30){speedRight=30;}

		if(speedLeft>127){speedLeft=127;}
		else if(speedLeft<30){speedLeft=30;}

		motorSet(2,speedRight);
		motorSet(3,speedRight);
		motorSet(4,speedLeft);
		motorSet(5,speedLeft);

		if(driverCheck==1  ||  analogRead(1)>1500)
		{
			motorSet(1, -127);
		}
		else
		{
			motorSet(1, 0);
		}

		delay(50);
	}
}


void operatorControl()
{

	taskCreate(maintainRPM, TASK_DEFAULT_STACK_SIZE, NULL, TASK_PRIORITY_DEFAULT);
	taskCreate(getRightRPM, TASK_DEFAULT_STACK_SIZE, NULL, TASK_PRIORITY_DEFAULT);
	taskCreate(getLeftRPM, TASK_DEFAULT_STACK_SIZE, NULL, TASK_PRIORITY_DEFAULT);

	while (1)
	{

		int joystickX = joystickGetAnalog(1, 4) + joystickGetAnalog(1, 1);
		int joystickY = joystickGetAnalog(1, 3) + joystickGetAnalog(1, 2);

		motorSet(6,   joystickY-joystickX);
		motorSet(7,   joystickY+joystickX);
		motorSet(8,  -joystickY-joystickX);
		motorSet(10,  joystickY-joystickX);


		motorSet(9, 127);


		if(joystickGetDigital(1, 8, 1)==1)//joy, button gr, button
		{
			targetRPM = locations[0];
		}
		else if(joystickGetDigital(1, 8, 8)==1)
		{
			targetRPM = locations[1];
		}
		else if(joystickGetDigital(1, 8, 4)==1)
		{
			targetRPM = locations[2];
		}

		else if(joystickGetDigital(1, 7, 1)==1)
		{
			targetRPM = 0;
		}



		if(joystickGetDigital(1, 6, 1)==1)
		{
			driverCheck = 1;
		}
		else
		{
			driverCheck = 0;
		}

		// Joystick and motor values refreshed every 20 ms
		delay(20);
	}
}

you have the symtoms of corrupt firmware/stack corruption normally this just leads to the processor calling the PROS error handler and rebooting but in verry rare cases instead of panicking it will put the processor in an infinite loop like what you are seeing

the only place where there could be stack corruption in that code is this part:


lcdPrint(uart1, 1, "Right: %d RPMs", rpmRight);

the formatted string “Right: %d RPMs” has the potential to go over 16 characters, what the LCD can support
im guessing the cause of the problem is lcdPrint was poorly coded and uses sprintf instead of snprintf, allowing the buffer to overflow

i am testing to see if the issue is happening to me also

Oh, okay. Thanks! I was having problems earlier when the rpms would get over 10000 (overflow the 16 characters), I thought I had that fixed though. If it’s as simple as the lcdPrint statement then it’s an easy fix.

Great catch Pixel, I’ve reformatted your code while going through it, mostly just unnecessary polish.

int rpmRight = 0;
int rpmLeft = 0;
int targetRPM = 7200;
int driverCheck = 0;

#define NO_RPM 0
#define SLOW_RPM 6000
#define MED_RPM 7000
#define FAST_RPM 10000

int max(int a, int b)
{
	return (a > b) ? a : b;
}

int clamp(int value, int min, int max)
{
	return (value > max) ? max : (value < min) ? min : value;
}

void getRightRPM(void * ignore)
{
	while(true)
	{
		rpmRight = encoderGet(rightEncoder) * 33;
		encoderReset(rightEncoder);

		lcdPrint(uart1, 1, "Right: %d RPMs", rpmRight);

		delay(20);
	}

}

void getLeftRPM(void * ignore)
{
	while(true)
	{
		rpmLeft = encoderGet(leftEncoder) * 33;
		encoderReset(leftEncoder);

		lcdPrint(uart1, 2, "Left:  %d RPMs", rpmLeft);

		delay(20);
	}

}

void maintainRPM(void * ignore)
{
	int speedRight = 0;
	int speedLeft = 0;

	while(true)
	{
		float kp = 0.05;

		int diffRight = targetRPM - rpmRight;
		int diffLeft = targetRPM - rpmLeft;

		speedRight += kp * diffRight;
		speedLeft  += kp * diffLeft;

		speedRight = clamp(speedRight, 30, 127);
		speedLeft = clamp(speedLeft, 30, 127);

		motorSet(2, speedRight);
		motorSet(3, speedRight);
		motorSet(4, speedLeft);
		motorSet(5, speedLeft);

		motorSet(1, (driverCheck || analogRead(1) > 1500) ? -127 : 0);

		delay(50);

	}

}

void operatorControl()
{
	taskCreate(maintainRPM, TASK_DEFAULT_STACK_SIZE, NULL, TASK_PRIORITY_DEFAULT);
	taskCreate(getRightRPM, TASK_DEFAULT_STACK_SIZE, NULL, TASK_PRIORITY_DEFAULT);
	taskCreate(getLeftRPM, TASK_DEFAULT_STACK_SIZE, NULL, TASK_PRIORITY_DEFAULT);

	while(true)
	{
		int joystickX = joystickGetAnalog(1, 4) + joystickGetAnalog(1, 1);
		int joystickY = joystickGetAnalog(1, 3) + joystickGetAnalog(1, 2);

		motorSet(6,   joystickY - joystickX);
		motorSet(7,   joystickY + joystickX);
		motorSet(8,  -joystickY - joystickX);
		motorSet(10,  joystickY - joystickX);

		motorSet(9, 127);

		// joy, button gr, button
		if(joystickGetDigital(1, 8, 1))
			targetRPM = SLOW_RPM;
		else if(joystickGetDigital(1, 8, 8))
			targetRPM = MED_RPM;
		else if(joystickGetDigital(1, 8, 4))
			targetRPM = FAST_RPM;
		else if(joystickGetDigital(1, 7, 1))
			targetRPM = NO_RPM;

		driverCheck = joystickGetDigital(1, 6, 1);

		// Joystick and motor values refreshed every 20 ms
		delay(20);

	}

}

Out of curiosity why are you doing this?

int joystickX = joystickGetAnalog(1, 4) + joystickGetAnalog(1, 1);
int joystickY = joystickGetAnalog(1, 3) + joystickGetAnalog(1, 2);

i just tested on my cortex and it doesnt look like lcdPrint is causing a buffer overflow, :confused: my last guess would be that you arent allocating enough stack space to your threads

That doesn’t make sense, he isn’t doing any recursion in his threads.

My driver uses the left stick, I use the right stick when I drive. It’s just a way to use either one.

Not really knowledgeable about tasks in PROS, but could it be that he is setting the LCD in two tasks without a mutex/semaphore?

You aren’t using IMEs right? I don’t see them in the code, but it kinda sounds like behavior that can result from an IME static shock.

quadrature encoders, not IMEs

the cortex only has one core and PROS code does not run in a VM so race conditions are impossible, threads are basically just co-routines

Not really true. The scheduler can cause a context switch at any time, if one of those functions needs to access data in a non-atomic fashion then that could cause a problem. I assume all the PROS functions are thread safe (I don’t actually know) and, for example, do not use any static variables or things like that.

You make some bold claims here that are incorrect. I am including the exact source code below for lcdPrint:


// lcdPrint - Convenience method that performs snprintf() and then lcdSetText()
void lcdPrint(FILE *lcdPort, uint8_t line, const char *fmt, ...) {
	char buffer[17];
	// Pass to vsnprintf
	va_list args;
	va_start(args, fmt);
	vsnprintf(buffer, 17, fmt, args);
	va_end(args);
	// Ensure that there are no null-terminator issues
	buffer[16] = 0;
	lcdSetText(lcdPort, line, buffer);
}

And the code for lcdSetText:


// lcdSetText - Sets a line (1 or 2) of text on the LCD to the specified null-terminated string
// Up to 16 characters will actually be transmitted; this function is thread safe, but
// concurrent access to the same line of the same LCD will cause garbage text to be displayed
void lcdSetText(FILE *lcdPort, uint8_t line, const char *buffer) {
	// Blit to the buffer, automatically dumped on the idle task
	uint32_t port = (uint32_t)lcdPort - 1, i;
	line--;
	if (port < 2 && line < 2) {
		// Find pointer to location
		char *scr = &(lcd[port].screen[0]);
		if (line) scr += 16;
		for (i = 0; i < 16; i++) {
			// Space pad to 16 characters
			if (*buffer)
				*scr++ = *buffer++;
			else
				*scr++ = ' ';
		}
	}
}

You are incorrect here. In any scenario where multiple threads are running it is possible to have a race condition unless each thread is mutually exclusive.

hshultz324 - Have you read the API carefully to see the notes on lcdPrint vs lcdSetText? Try converting your calls of lcdPrint to lcdSetText as lcdPrint utilizes vsnprintf which is an expensive call to be making every 20ms in your two threads.

Thanks for all the help everyone! I never thought that an LcdPrint Statement could cause this much trouble. I’ll switch over to LcdSetText and see if that helps.