Programming Issue: Lift

My lift isn’t working and the program seems fine. The motors worked earlier and don’t seem to have any issues with it. The weight of the lift isn’t too much for the motors to handle. Please reply if you can help. My team has a competition in 2 days. So we need it urgently.

Code:

In the future please indent your code properly (there’s an autoformat button somewhere in RobotC) and include a minimal sample - in other words, the least amount of code you can to demonstrate the problem.

Your problems become much more apparent with properly formatted code:

task usercontrol()
{
	motor[Drive1] = vexRT[Ch2];
	motor[Drive2] = vexRT[Ch2];
	motor[Drive3] = vexRT[Ch3];
	motor[Drive4] = vexRT[Ch3];

	{
		//Lift
		if(vexRT[Btn5U] == 1) //If button 5U is pressed:
		{
			motor[port6] = 127; //lift arm
			motor[port7] = 127;
			motor[port8] = 127;
			motor[port9] = 127;
		}
		else if(vexRT[Btn5D] == 1) //If button 5D is pressed:
		{
			motor[port6] = -127; //go down
			motor[port7] = -127;
			motor[port8] = -127;
			motor[port9] = -127;
		}
		else //If neither buttons 5U or 5D are pressed:
		{
			motor[port6] = 0; //stop motor port 5
			motor[port7] = 0;
			motor[port8] = 0;
			motor[port9] = 0;
			
			//Intake
			if(vexRT[Btn6U] == 1) //If button 6U is pressed:
			{
				motor[port1] = 127; //Take In
				motor[port10] = -127;
			}
			else if(vexRT[Btn6D] == 1) //If button 6D is pressed:
			{
				motor[port1] = -127; //Spit Out
				motor[port10] = 127;
			}
			else //If neither buttons 6U or 6D are pressed:
			{
				motor[port1] = 0; //stop motor port 5
				motor[port10] = 0;

				// This is the main execution loop for the user control program. Each time through the loop
				// your program should update motor + servo values based on feedback from the joysticks.
	
				// .................................................. ...................................
				// Insert user code here. This is where you use the joystick values to update your motors, etc.
				// .................................................. ...................................

				UserControlCodePlaceholderForTesting(); // Remove this function call once you have "real" code.
			}
		}
	}
}

So let’s figure this out. The first thing you need to do is make sure you’re running the whole thing in a while loop. Otherwise, it only runs once really fast which doesn’t do you very much good. You also want to remove the “UserControlCodePlaceholderForTesting” line because that line will sit there forever. It’s just intended to be temporary. So here that part is fixed:

task usercontrol()
{
	
	while (true)
	{
		// Drive
		motor[Drive1] = vexRT[Ch2];
		motor[Drive2] = vexRT[Ch2];
		motor[Drive3] = vexRT[Ch3];
		motor[Drive4] = vexRT[Ch3];

		//Lift
		if(vexRT[Btn5U] == 1) //If button 5U is pressed:
		{
			motor[port6] = 127; //lift arm
			motor[port7] = 127;
			motor[port8] = 127;
			motor[port9] = 127;
		}
		else if(vexRT[Btn5D] == 1) //If button 5D is pressed:
		{
			motor[port6] = -127; //go down
			motor[port7] = -127;
			motor[port8] = -127;
			motor[port9] = -127;
		}
		else //If neither buttons 5U or 5D are pressed:
		{
			motor[port6] = 0; //stop motor port 5
			motor[port7] = 0;
			motor[port8] = 0;
			motor[port9] = 0;
			
			//Intake
			if(vexRT[Btn6U] == 1) //If button 6U is pressed:
			{
				motor[port1] = 127; //Take In
				motor[port10] = -127;
			}
			else if(vexRT[Btn6D] == 1) //If button 6D is pressed:
			{
				motor[port1] = -127; //Spit Out
				motor[port10] = 127;
			}
			else //If neither buttons 6U or 6D are pressed:
			{
				motor[port1] = 0; //stop motor port 5
				motor[port10] = 0;
			}
		}
	}
}

That code should work, but you may notice that your intake code will only work if the arm is not moving. This may be desirable, but I typically prefer that the driver have full control - you never know when they may need to control the intake while the arm is moving. Judging by the comments, I believe that was the intention anyways:

task usercontrol()
{
	
	while (true)
	{
		// Drive
		motor[Drive1] = vexRT[Ch2];
		motor[Drive2] = vexRT[Ch2];
		motor[Drive3] = vexRT[Ch3];
		motor[Drive4] = vexRT[Ch3];

		//Lift
		if(vexRT[Btn5U] == 1) //If button 5U is pressed:
		{
			motor[port6] = 127; //lift arm
			motor[port7] = 127;
			motor[port8] = 127;
			motor[port9] = 127;
		}
		else if(vexRT[Btn5D] == 1) //If button 5D is pressed:
		{
			motor[port6] = -127; //go down
			motor[port7] = -127;
			motor[port8] = -127;
			motor[port9] = -127;
		}
		else //If neither buttons 5U or 5D are pressed:
		{
			motor[port6] = 0; //stop motor port 5
			motor[port7] = 0;
			motor[port8] = 0;
			motor[port9] = 0;
		}

		//Intake
		if(vexRT[Btn6U] == 1) //If button 6U is pressed:
		{
			motor[port1] = 127; //Take In
			motor[port10] = -127;
		}
		else if(vexRT[Btn6D] == 1) //If button 6D is pressed:
		{
			motor[port1] = -127; //Spit Out
			motor[port10] = 127;
		}
		else //If neither buttons 6U or 6D are pressed:
		{
			motor[port1] = 0; //stop motor port 5
			motor[port10] = 0;
		}
	}
}

Now for the autonomous portion, that should work as is but I would suggest removing the “AuonomousCodePlaceholderForTesting” line. Doesn’t hurt to have it there, but kind of unnecessary. It just makes the autonomous sit in the thread at the end instead of exiting.

Please keep in mind I don’t have RobotC on any machine right now so I just wrote this in notepad (well, gedit) real quick - there may be small mistakes in it but the logic should be more or less correct.

You should definitely consider writing a helper function to set those motor values. Even if all the function does is set all four at once, it would at least make this a whole lot more readable.

void setLiftMotors(int a, int b, int c, int d) {
	motor[port6] = a;
	motor[port7] = b;
	motor[port8] = c;
	motor[port9] = d;
}

// Then...

if(vexRT[Btn5U] == true) setLiftMotors(127, 127, 127, 127);

Or since you often set the same value to all those motors…

void setAllLiftMotors(int a) {
	motor[port6] = a;
	motor[port7] = a;
	motor[port8] = a;
	motor[port9] = a;
}

// Then...

if(vexRT[Btn5U] == true) setAllLiftMotors(127);

-Cody

The way I accomplish this in my code is a task I name autondrivesimplification

task autondrivesimplification
while 1{
motor[mDriveL2]=motor[mDriveL1];
motor[mDriveL3]=motor[mDriveL1];
motor[mDriveL4]=motor[mDriveL1];
motor[mDriveR2]=motor[mDriveR1];
motor[mDriveR3]=motor[mDriveR1];
motor[mDriveR4]=motor[mDriveR1];
}

This just allows my autonomous only to have to set the right side of drive power and the left side of drive power. I also use this task for polling sensor values and any other commands that need to be run constantly throughout autonomous.

This isn’t a great way of doing that because it wastes a ton of processing time. Just make a function which sets the left and right sides.

It is nice for polling values as well.

For encoders I use a variable RE for Right encoder value
Inside my autondrivesimplification I can poll the sensor values and average all the encoders on the right side of the drive and set this value to RE. This needs to happen constantly for RE to be used as an accurate encoder value.

This can also be done for the intake:


void setIntakeMotors(int a) {
	motor[port1] = -a; //Positive number intakes balls
	motor[port10] = a; //Positive number shoots balls out
}

But having a whole task for that is such a waste! What’s wrong with something like this?


#define RE (SensorValue(enc1) / 2 + SensorValue(enc2) / 2)

Then you don’t have an entire task running all the time polling and averaging values - you just average them when you need them and you can still use your RE “variable” (it’s actually a preprocessor).

Note that you should not include a semicolon at the end.

Also note that I did not test this and may have made a stupid mistake.

In the RobotC help file they use square brackets with SensorValue instead of parenthesis. I think both work though.
And one thing to help with integer division precision is to add first and divide later.


#define RE ((SensorValue[enc1] + SensorValue[enc2]) / 2)

It is just a matter of preference though.

The problem with function-like macros is that they don’t preform any variable type-checks. You can get some very goofy results using function-like macros that can be difficult to track down.

Also, they are very easy to mess up and often won’t work in every case. All three of the following “should” preform in the same way, yet due to the pre-processing nature of #define, the first two have major fallback cases where they return incorrect results. This can lead to a nightmare of debugging.

[Code]#define SQUARE(z) z * z

#define SQUARE(z) (z) * (z)

#define SQUARE(z) ((z) * (z))
[/CODE]

If you don’t feel comfortable working with macros then just use a function:

int RE()
{
  return (SensorValue(enc1) / 2 + SensorValue(enc2) / 2);
}

Either way is a bajillion times more efficient then using a task to constantly calculate the values.

By not using a task and instead calling a function or using a macro, one is trading processing speed for code rigidity. There are rare cases in which this code can “fall through the cracks” of a logic statement (such as an if/else if/else statement resulting in the incorrect result). While rare, it will happen, more often as the values change more rapidly. Keep this in mind if one plans on using this method in a critical logic statement.

Doesn’t a task also rely on logic statements? Even more so than a function? You have the whole thing in a while(true) loop, and there’s no delay. Regardless, I believe DaPolice’s issue has been resolved. As Hal would say, this conversation can serve no purpose.

I was simply trying to give advice for those who go back and reference these threads long after we leave them. I apologize if what I said came of in any other way. I’m confident everyone who posted here is capable for creating amazing code that works great and I am not trying to change their methods. I am just trying to present my recommendations for those who will come later when we are gone.

As far as my previous post, an example may help.


if(re()<0)
     do_a;
else if(re()==0)
     do_b;
else                        //re()>0
     do_c;

Supose in the first “if” re() returns 0, so the logic statement moves on. Now imagine that the “else if” re() returns 5. This will cause do_c to occur, when do_b should have in the first place. Now this may seem unlikely, but it can and will occur. In most cases it wouldn’t matter.
However, there are some cases where such an error would be critical. This is what I am attempting to point out.

Wait, what? I don’t follow you at all.

Either method will work exactly the same in a logic statement. I believe what you are referring to is something like the following, correct me if I’m wrong.

if (RE == 5)
  doSomething();
else if (RE == 6)
  doSomethingElse();
else if (RE == 7)
  oneMoreThing();
else
  singASong();

So basically RE can theoretically change right after evaluation of the first if statement which might make complex statements fail in confusing ways. This problem would happen whether you use the task method, the macro method, or the function method because the value is being recalculated constantly. In most student code this constant recalculation wouldn’t cause any harm but I suppose there are some esoteric situations where it might.

In those situations, you should really do something like this:

 int temp = RE;
if (temp == 5)
  doSomething();
else if (temp == 6)
  doSomethingElse();
else if (temp == 7)
  oneMoreThing();
else
  singASong();

That way you know you are working with a constant value - whatever RE was when the code chunk started to be executed. RE could have been a macro, recalculated constantly in a task, or a function. Heck, it could even just have the statement right there.

Seriously, using a task to constantly recalculate values like really is just a bad idea. It has many fallbacks, like take the following example for instance:

task doStuff()
{
  while(true)
  {
     RE += 5;
     RE *= 2;

     wait(10);
  }
}

Perhaps RE needs to have both operations done to it before you should access the variable. Code in another task might access it only after adding 5 but before multiplying by 2 which could lead to erroneous results. Sticking to one thread removes this complexity. I would think finding problems like this is much more difficult than realizing your order of operations is funky and adding parentheses around a macro. A function has none of these problems.

Using a second thread to recalc constantly isn’t just slower - it’s a total waste of system resources which could be used for other things. It’s more complex and harder to debug. A macro might be a bit complex I suppose, but I can’t see many realistic situations where a task would be advantageous over a function for something like this.

PS - Sorry, second hijacked thread this week :o.

** ==== EDIT ====**

This will happen with the task method just as much, as I explained above.

Well there I go thinking again :rolleyes:, I had in my head assigned different priority levels to the tasks and forgotten that when tasks have the same priority level they get timeslices so you’ll still run into the same problem. Anyways, you said what I was trying to better than I did, so thanks.

I probably shouldn’t tell daniel about the other 2 tasks I run in my autonomous huh? :stuck_out_tongue:

IIRC, even if they’re different priorities there still is time slicing, the higher priority task is just given more time. And not a problem - I just didn’t want folks reading the forum to think that this was a good example. That’s not to say it doesn’t work, there’s just better and simpler ways to do the same thing.

Let’s hear what they do haha - now I’m curious :cool:

Two tasks one to monitor each side of the drive as it approaches an encoder value.
I have a function start the task whenever I set a new encoder goal. Allows actual autonomous code to be extremely simple.
R(1000);
L(1000);
would drive forward until each encoder hit 1000 and stop also it runs them separately so speed differences are accounted for.

The reason I do this instead of the same value for right encoder and left encoder is it allows me to say
R(1000);
L(5);

The code for this could be done in 1 task rather than 2 but for my purposes it is a ton simpler for error finding and readability. :smiley:

Now that’s an acceptable reason to use tasks :slight_smile: