RobotC - Bad programming habits


#1

Around four years ago I started a thread called “RobotC programming tips”. I’ve decided to start another called “RobotC - Bad programming habits” as I’ve recently seen multiple examples of the same bad programming techniques being used and it seems that we (the forum) should try and address these so that everyone can improve and learn.

(Many of these will also apply to EasyC, PROS or ConVEX)

**Using double negatives
**
Imaging you have just created a new robot drive, very simple, four wheels and four direct drive motors. You decide (as you have been doing VEX for six weeks now and are an expert :slight_smile: ) to add some sensors to this skyrise dominating robot and have read on the forum that using encoders on the wheels will allow super accurate autonomous code. So you add an IME to each of the two front wheels and start to write your program. First of all you create this code.

#pragma config(Motor,  port2,           DriveWheel_LF, tmotorVex393_MC29, openLoop)
#pragma config(Motor,  port3,           DriveWheel_LB, tmotorVex393_MC29, openLoop)
#pragma config(Motor,  port8,           DriveWheel_RB, tmotorVex393_MC29, openLoop)
#pragma config(Motor,  port9,           DriveWheel_RF, tmotorVex393_MC29, openLoop)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

task main()
{
    while( 1 )
        {
        motor DriveWheel_LF ] = vexRT Ch3 ];   
        motor DriveWheel_LB ] = vexRT Ch3 ];   
        motor DriveWheel_RF ] = vexRT Ch2 ];   
        motor DriveWheel_RB ] = vexRT Ch2 ];   
            
        wait1Msec(10);
        }
}

It’s just a simple tank drive control.

But you find that the robot spins, obvious, you need to reverse the direction of the wheels on one side. So you modify the code and send negative values to the right side motors.

#pragma config(Motor,  port2,           DriveWheel_LF, tmotorVex393_MC29, openLoop)
#pragma config(Motor,  port3,           DriveWheel_LB, tmotorVex393_MC29, openLoop)
#pragma config(Motor,  port8,           DriveWheel_RB, tmotorVex393_MC29, openLoop)
#pragma config(Motor,  port9,           DriveWheel_RF, tmotorVex393_MC29, openLoop)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

task main()
{
    while( 1 )
        {
        motor DriveWheel_LF ] =  vexRT Ch3 ];   
        motor DriveWheel_LB ] =  vexRT Ch3 ];   
        motor DriveWheel_RF ] = -vexRT Ch2 ];   
        motor DriveWheel_RB ] = -vexRT Ch2 ];   
            
        wait1Msec(10);
        }
}

Great, now the robot drives correctly. Time to start on the autonomous code. You write the following.

#pragma config(I2C_Usage, I2C1, i2cSensors)
#pragma config(Sensor, I2C_1,  ,               sensorQuadEncoderOnI2CPort,    , AutoAssign)
#pragma config(Sensor, I2C_2,  ,               sensorQuadEncoderOnI2CPort,    , AutoAssign)
#pragma config(Motor,  port2,           DriveWheel_LF, tmotorVex393_MC29, openLoop, encoderPort, I2C_1)
#pragma config(Motor,  port3,           DriveWheel_LB, tmotorVex393_MC29, openLoop)
#pragma config(Motor,  port8,           DriveWheel_RB, tmotorVex393_MC29, openLoop)
#pragma config(Motor,  port9,           DriveWheel_RF, tmotorVex393_MC29, openLoop, encoderPort, I2C_2)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

#include "Vex_Competition_Includes.c"   //Main competition background code...do not modify!

void pre_auton()
{
    bStopTasksBetweenModes = true;
}

task autonomous()
{
    nMotorEncoder DriveWheel_LF ] = 0;
    nMotorEncoder DriveWheel_RF ] = 0;
    
    // Go forwards
    motor DriveWheel_LF ] =  127;   
    motor DriveWheel_LB ] =  127;   
    motor DriveWheel_RF ] = -127;   
    motor DriveWheel_RB ] = -127;   
    
    // Wait until encoder has changed by 1000 counts
    while( ( nMotorEncoder DriveWheel_LF ] < 1000 ) && ( nMotorEncoder DriveWheel_RF ] < 1000 ) )
        wait1Msec(10);
    
    // Stop
    motor DriveWheel_LF ] = 0;   
    motor DriveWheel_LB ] = 0;   
    motor DriveWheel_RF ] = 0;   
    motor DriveWheel_RB ] = 0;   
}

task usercontrol()
{
    // add my driver code later
    while(1) wait1Msec(10);
}

Easy, drive forwards 1000 counts. But the code doesn’t work, the robot keeps driving. You break out the ROBOTC debugger and look at what the motors and sensors are doing and see the problem, the IME on the right wheel is counting backwards, you change the code.

    // Wait until encoder has changed by 1000 counts
    while( ( nMotorEncoder DriveWheel_LF ] < 1000 ) && ( nMotorEncoder DriveWheel_RF ] > -1000 ) )
        wait1Msec(10);

and the robot drives forward and stops.

What you have done here is create a double negative, sending negative control values to the motor has caused the encoder to count backwards and now you have to either negate that number or reverse the logical tests to account for it. Very soon the code gets complicated as more steps to the autonomous code are added.

What you should do

Use the motor reverse flag in motors&sensors setup.

You want to always try and have the following setup.

**Send positive control values to motors to make the robot go forwards.
When the robot goes forward (by sending those positive control values) the encoders you have should increment.
**
Here is the correct version of the autonomous code.

#pragma config(I2C_Usage, I2C1, i2cSensors)
#pragma config(Sensor, I2C_1,  ,               sensorQuadEncoderOnI2CPort,    , AutoAssign)
#pragma config(Sensor, I2C_2,  ,               sensorQuadEncoderOnI2CPort,    , AutoAssign)
#pragma config(Motor,  port2,           DriveWheel_LF, tmotorVex393_MC29, openLoop, encoderPort, I2C_1)
#pragma config(Motor,  port3,           DriveWheel_LB, tmotorVex393_MC29, openLoop)
#pragma config(Motor,  port8,           DriveWheel_RB, tmotorVex393_MC29, openLoop, reversed)
#pragma config(Motor,  port9,           DriveWheel_RF, tmotorVex393_MC29, openLoop, reversed, encoderPort, I2C_2)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

#include "Vex_Competition_Includes.c"   //Main competition background code...do not modify!

void pre_auton()
{
    bStopTasksBetweenModes = true;
}

task autonomous()
{
    nMotorEncoder DriveWheel_LF ] = 0;
    nMotorEncoder DriveWheel_RF ] = 0;
    
    // Go forwards
    motor DriveWheel_LF ] =  127;   
    motor DriveWheel_LB ] =  127;   
    motor DriveWheel_RF ] =  127;   
    motor DriveWheel_RB ] =  127;   
    
    // Wait until encoder has changed by 1000 counts
    while( ( nMotorEncoder DriveWheel_LF ] < 1000 ) && ( nMotorEncoder DriveWheel_RF ] < 1000 ) )
        wait1Msec(10);
    
    // Stop
    motor DriveWheel_LF ] = 0;   
    motor DriveWheel_LB ] = 0;   
    motor DriveWheel_RF ] = 0;   
    motor DriveWheel_RB ] = 0;   
}

task usercontrol()
{
    // add my driver code later
    while(1) wait1Msec(10);
}

If gearing is such that the left hand side need reversing then so be it but always try and achieve.

Positive motor values => forwards => incrementing encoders.

The same applies to lift systems, try and achieve the following

Positive motor values => lift going up => incrementing encoders.

Trust me, creating autonomous (or PID) code gets easier if you do this.


#2

Functions are your friends

Post #1 contains the worst programming bad habit I see, repeating the same lines of code over and over again when they should be placed into a function. For example

    motor DriveWheel_LF ] =  127;   
    motor DriveWheel_LB ] =  127;   
    motor DriveWheel_RF ] =  127;   
    motor DriveWheel_RB ] =  127;   

and

    motor DriveWheel_LF ] = 0;   
    motor DriveWheel_LB ] = 0;   
    motor DriveWheel_RF ] = 0;   
    motor DriveWheel_RB ] = 0;   

anytime that you start using the same multiple lines of code but with slightly different parameters you need to think “function”. If we were to rewrite this it would look as follows.

void
DriveAtSpeed( int leftSpeed, int rightSpeed )
{
    motor DriveWheel_LF ] =  leftSpeed;   
    motor DriveWheel_LB ] =  leftSpeed;   
    motor DriveWheel_RF ] =  rightSpeed;   
    motor DriveWheel_RB ] =  rightSpeed;   
}

and our rewritten autonomous code would be like this


task autonomous()
{
    nMotorEncoder DriveWheel_LF ] = 0;
    nMotorEncoder DriveWheel_RF ] = 0;
    
    // Go forwards
    DriveAtSpeed( 127, 127 );
    
    // Wait until encoder has changed by 1000 counts
    while( ( nMotorEncoder DriveWheel_LF ] < 1000 ) && ( nMotorEncoder DriveWheel_RF ] < 1000 ) )
        wait1Msec(10);
    
    // Stop
    DriveAtSpeed( 0, 0 );
}

You might even decide to have another layer and create the following.

void
DriveForwards( int speed )
{
    DriveAtSpeed( speed, speed );
}

void
DriveStop()
{
    DriveAtSpeed(0,0);
}

task autonomous()
{
    nMotorEncoder DriveWheel_LF ] = 0;
    nMotorEncoder DriveWheel_RF ] = 0;
    
    // Go forwards
    DriveForwards( 127 );
    
    // Wait until encoder has changed by 1000 counts
    while( ( nMotorEncoder DriveWheel_LF ] < 1000 ) && ( nMotorEncoder DriveWheel_RF ] < 1000 ) )
        wait1Msec(10);
    
    // Stop
    DriveStop();
}

The code is more descriptive, you are less likely to make mistakes, it’s just all around a better way to program.

Please try and use functions.


#3

Tasks are not functions

A function (or subroutine as it’s sometimes called) is a reusable section of software. Some functions are part of the ROBOTC library, for example, wait1Msec is a function that halts the program flow for the specified number of milliseconds. Some functions are ones that you write yourself, see the examples in post #2.

tasks are different, they are not functions and you should not use them in that way.

A task contains a series of code statements that will appear to execute at the same time as code in other tasks. A ROBOTC task is similar to a thread and (although less so) a process in other operating systems.

This is not how a task should be used.

task
DriveForwards()
{
    motor DriveWheel_LF ] =  127;   
    motor DriveWheel_LB ] =  127;   
    motor DriveWheel_RF ] =  127;   
    motor DriveWheel_RB ] =  127;   
}

task
DriveStop()
{
    motor DriveWheel_LF ] =  0;   
    motor DriveWheel_LB ] =  0;   
    motor DriveWheel_RF ] =  0;   
    motor DriveWheel_RB ] =  0;   
}


task autonomous()
{
    nMotorEncoder DriveWheel_LF ] = 0;
    nMotorEncoder DriveWheel_RF ] = 0;
    
    // Go forwards
    startTask( DriveForwards );
    
    // Wait until encoder has changed by 1000 counts
    while( ( nMotorEncoder DriveWheel_LF ] < 1000 ) && ( nMotorEncoder DriveWheel_RF ] < 1000 ) )
        wait1Msec(10);
    
    // Stop
    startTask( DriveStop );
}

A simple rule of thumb would be

**If a task does not have a while statement then it should be a function.
**

Not always true, a task could be used for any time consuming code where you want to be able to do other things as well, but in most of the programming for VEX it’s close enough.

So when should you use tasks ?

Anytime you need concurrent functionality.

Displaying status on an LCD while you drive the robot.
PID control loops.
Controlling the robots drive system, lift system and intake systems independently.
A safety emergency stop system.
Autonomous code where two things need to happen simultaneously, example, drive forwards whilst lifting an arm to a preset height.


PROS Mutitasking
#4

Stop your motors jittering

It seems like almost everyday we get a new question about motors that are jittering on the robot. The cause is setting the motor to multiple different values within the driver control infinite loop. Typical code may look like this.

#pragma config(Motor,  port1,           MotorA,        tmotorVex393_HBridge, openLoop)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

task main()
{
    while(1)
        {
        // Really bad code that causes motors to jitter
        if( vexRT Btn8U ] )
            motor MotorA ] = 127;
    
        if( vexRT Btn8D ] )
            motor MotorA ] = -127;
    
        motor MotorA ] = 0;
        
        wait1Msec(10);
        }
}

The motor is first set to +127, then set to -127, then set to 0. These all happen very quickly and it just depends on which value happens to get sent to the motor that determines what it does.

So we all know the fix, I left out all the “else” statements, it should look like this.

#pragma config(Motor,  port1,           MotorA,        tmotorVex393_HBridge, openLoop)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

task main()
{
    while(1)
        {
        // No jitter now
        if( vexRT Btn8U ] )
            motor MotorA ] = 127;
        else
        if( vexRT Btn8D ] )
            motor MotorA ] = -127;
        else
            motor MotorA ] = 0;
        
        wait1Msec(10);
        }
}

But what if we wanted to add some joystick control as well, typically this might start off as follows.

#pragma config(Motor,  port1,           MotorA,        tmotorVex393_HBridge, openLoop)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

task main()
{
    while(1)
        {
        // Button control
        if( vexRT Btn8U ] )
            motor MotorA ] = 127;
        else
        if( vexRT Btn8D ] )
            motor MotorA ] = -127;
        else
            motor MotorA ] = 0;
        
        // oops, jitter again
        if( abs(vexRT Ch2 ]) > 10 )
            motor MotorA ] = vexRT Ch2 ];
        else
            motor MotorA ] = 0;
        
        wait1Msec(10);
        }
}

But now we have jitter on the motor again. The problem, as before, is that we are using if-then-else statements to conditionally send values to the motor but keep overriding out previous decision. So we (well, the bad habit programmer) try and fix this by complicating the logic like this.

#pragma config(Motor,  port1,           MotorA,        tmotorVex393_HBridge, openLoop)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

task main()
{
    while(1)
        {
        // Button control
        if( vexRT Btn8U ] )
            motor MotorA ] = 127;
        else
        if( vexRT Btn8D ] )
            motor MotorA ] = -127;
        else
            {       
            // fixed again !
            if( abs(vexRT Ch2 ]) > 10 )
                motor MotorA ] = vexRT Ch2 ];
            else
                motor MotorA ] = 0;
            }
            
        wait1Msec(10);
        }
}

and so on, as we add presets, a partner joystick, whatever, it gets harder to stop those motors jittering.

There is a better way, use a variable, set the motor only once. A rewrite of the above may look as follows.

#pragma config(Motor,  port1,           MotorA,        tmotorVex393_HBridge, openLoop)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

task main()
{
    int     MotorA_Value;

    while(1)
        {
        // Clear to start with
        MotorA_Value = 0;
        
        // Button control
        if( vexRT Btn8U ] )
            MotorA_Value = 127;

        if( vexRT Btn8D ] )
            MotorA_Value = -127;

        // Joystick control
        if( abs(vexRT Ch2 ]) > 10 )
            MotorA_Value = vexRT Ch2 ];
        
        // Finally - send to the motor here
        motor MotorA ] = MotorA_Value;

        wait1Msec(10);
        }
}

There is still priority to the control, joystick will override button 8D and that will override button 8U, but what the priority is can be determined by the order of the statements. The final value of the variable is only sent to the motor once, it is impossible for there to be any motor jitter (the code may not work but that’s another story).


#5

Keep your code well formatted

What’s wrong with this program?

#pragma config(Motor,  port1,           MotorA,        tmotorVex393_HBridge, openLoop)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

task main(){int MotorA_Value;while(1){MotorA_Value=0;if(vexRT[Btn8U])MotorA_Value=127;if(vexRT[Btn8D])MotorA_Value=-127;if(abs(vexRT[Ch2])>10)MotorA_Value=vexRT[Ch2];motor[MotorA]=MotorA_Value;wait1Msec(10);}}

Actually nothing, it runs just fine, but it’s really hard to tell just by looking at it.

or this version?

#pragma config(Motor,  port1,           MotorA,        tmotorVex393_HBridge, openLoop)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

task main()
{
int     MotorA_Value;
while(1)
{
MotorA_Value = 0;
if( vexRT Btn8U ] )
MotorA_Value = 127;
if( vexRT Btn8D ] )
MotorA_Value = -127;
if( abs(vexRT Ch2 ]) > 10 )
MotorA_Value = vexRT Ch2 ];
motor MotorA ] = MotorA_Value;
wait1Msec(10);
}
}

That’s the same code.

Here’s another version.

#pragma config(Motor,  port1,           MotorA,        tmotorVex393_HBridge, openLoop)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

task main()
{
int     MotorA_Value;
while(1)
{
MotorA_Value = 0;

        if( vexRT Btn8U ] )
    MotorA_Value = 127;

    if( vexRT Btn8D ] )
            {
    MotorA_Value = -127;
}

if( abs(vexRT Ch2 ]) > 10 )
            MotorA_Value = vexRT Ch2 ];

    motor MotorA ] = MotorA_Value;

wait1Msec(10);
        }
}

Hopefully you get the point I’m trying to make. Well formatted code is both easier to read and debug. Use tabs to show the level of conditional statements and loops. Add comments to explain what you are intending to do, even if the code is not quite working yet.

Here is what I would like to see.

#pragma config(Motor,  port1,           MotorA,        tmotorVex393_HBridge, openLoop)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

task main()
{
    int     MotorA_Value;
    
    while(1)
        {
        // Clear to start with
        MotorA_Value = 0;

        // Button control
        if( vexRT Btn8U ] )
            MotorA_Value = 127;

        if( vexRT Btn8D ] )
            MotorA_Value = -127;

        // Joystick control
        if( abs(vexRT Ch2 ]) > 10 )
            MotorA_Value = vexRT Ch2 ];

        // Finally - send to the motor here
        motor MotorA ] = MotorA_Value;

        wait1Msec(10);
        }
}

Not the only way of formatting, the style you choose is not important, many companies enforce their own guidelines (eg. google link ) and my own style is now a little old school but please try and write (and post of the forum) good looking, well formatted code.


#6

+1

Sophomore year, I wrote a very complex code in RobotC that i decided (for some dumb reason) should be exactly 100 lines, when it should have been around 250. When another team member started work on it too, he was very annoyed, and convinced me of the error of my ways.

RobotC users, look for the Icon at the top of the screen that looks like this:https://vexforum.com/attachment.php?attachmentid=9005&stc=1&d=1423355418
It will auto format you code, Other IDE’s have this too, usually ctrl+shift+f in eclipse(ConVex and PROS). Use It, it helps a lot.
Capture.PNG


#7

Sometimes I think the students participating in VEX would be good at this competition.
The International Obfuscated C Code Contest


#8

YAY another thing to distract me from homework! I need a break!


#9

THANK YOU! I always cringe when I see the lack of functions in autonomous. in fact i’m planning on making a library during the summer called Lazy ID:10T Lib that has these function already there so lazy programmers don’t have to.


#10

@jpearman I was just wondering about a claw, should positive motor values be closing the claw or opening it??


#11

I don’t think it really matters with a claw, the last time I programmed a clawbot I set encoder position 0 as closed and the open position ended up at around 500 counts. That code is at the end of this thread.
https://vexforum.com/t/clawbot-improvements/23723/1


#12

Sorry for resurrecting an old thread, but just wanted to thank @jpearman for the coding tips on this thread! We were struggling with motor sputtering issues while attempting to setup a partner remote system in which both remotes could drive the robot. We have setup a partner remote several times in the past, but never in such a way that both students could actually drive the robot (only the main remote would control the drive).

Our TP robot has the roller intake and catapult on the ‘front’ and the cap turner and cap lift on the ‘back’. We were attempting to setup the two remotes with intuitive controls so that when the ‘cap driver’ is driving, his/her robot will drive ‘forward’, and then when the '‘catapult driver’ is driving, the controls will reverse and the ‘catapult driver’ can also drive ‘forward’. We struggled with the motor stutter issue for a while, so I started searching for an answer in the VEX Forum and ran across this thread! We tried the sample code @jpearman provided and it worked perfectly!

Thanks again to @jpearman and all others who have contributed to this thread! I certainly hope we can return the favor someday and contribute something to this forum that will help others.


#13

I’m actually really glad it got brought back to life - I’ll definitely be referring back! :slight_smile:


#14

Thumbs up. With a constant stream of new students, this should probably be a guide so its easy to access.