Competition code - common mistakes

[jpearman]

I’m going to cover some of the code mistakes we found at worlds 2022 in this topic. Before we get going I want to provide a VEXcode project that can be used to help demonstrate the issues and also provides a C++ class that can be (at least theoretically) included with any VEXcode C++ project to help debug.

The demo project is available here.

GitHub - jpearman/V5_CompetitionTest: class and demo code for debugging...

It includes a new class inside a header file called comp_debug.h, the class is very simple to add, include comp_debug.h after vex.h in main.cpp.

#include "vex.h"
#include "comp_debug.h"

and then create a global instance of it passing you competition class instance as a parameter.

// debug class
competition_debug Cdebug( Competition );

The competition_debug instance will add an overlay to your code with current competition status shown as well as a few other useful items, this is what it looks like when the robot is disabled.

Any output your code has will be clipped and drawn as usual.

[jpearman]

ok, now that I fixed the spelling of competition in all the screen shots

first topic is incorrect use of event handlers, something we have covered before but it seems everyone forgets.

The symptom is that, for example, an event handler on a controller button sometimes works and sometimes doesn’t. At worlds the most common use was for firing pneumatics, a button push to set one state and a second button push to revert back to original state.

simple code for a callback could look as follows.

void button_callback() {
    static  int calls = 0;
    Brain.Screen.setFont( mono40 );

    Brain.Screen.printAt( 100, 160, "Button %4d", ++calls );
}

This is just for demonstration purposes and prints the number of callbacks on the brain screen.

We often saw event registration at the beginning of driver control like this.

void usercontrol( void ) {
    Controller.ButtonA.pressed( button_callback );


    // more code
}

So what’s the problem with that ?

usercontrol can be called more than once.

The reason is that if the controller is connected (radio linked) to the robot and the field control is not connected (either new or legacy) then usercontrol will run. This is expected and what we usually want to happen when practicing. Once the field control is connected then the robot will usually be disabled unless the match has already started. When the driver control period starts then usercontrol will run again and the button_callback registered a second time, when controller button A is pressed button_callback will be called twice (and potentially more if field control was connected and disconnected more than once) and obviously if it is toggling a pneumatic solenoid nothing will actually happen (the solenoid will be enable and disabled very quickly or not at all).

Event registration should only happen once at the beginning of main. This would be the correct place for the code.

int main() {    
    Controller.ButtonA.pressed( button_callback );

    Competition.autonomous( autonomous );
    Competition.drivercontrol( usercontrol );

    pre_auton();

    while(1) {
        this_thread::sleep_for(10);
    }
}

The order that teams connected to the field and started their code varied and they had inconsistent results because of registering events in usercontrol.

[Mentor_355U]

@jpearman

So what’s the problem with that ?

usercontrol can be called more than once.

Would it be possible for VexCode to include a linter that would detect / warn against this (and any of these other common) anti-patterns before the compiler runs?

[jpearman]

Along the same lines.

incorrect use of local variables.

consider this code, a very simplified version of something we saw at worlds.

code

vex::pneumatics   pn(Brain.ThreeWirePort.A );

void usercontrol( void ) {
    int my_state = 0;

    while (true) {
      if( Controller.ButtonA.pressing() ) {
        if( my_state == 0 ) {
          my_state = 1;
        }
        else
        if( my_state == 1 ) {
          my_state = 0;
        }

        if( my_state == 1 ) {
          pn.open();
          Brain.Screen.printAt( 100, 100, "Open   " );
        }
        if( my_state == 0 ) {
          pn.close();
          Brain.Screen.printAt( 100, 100, "Closed " );
        }

        while( Controller.ButtonA.pressing() ) {
          this_thread::sleep_for(10);
        }
      }   

      this_thread::sleep_for(10);
    }
}

The idea is to toggle pneumatics open and closed (much more was involved in the student code)

The team may allow usercontrol to run before connecting field control and press button A to open the pneumatics, then connect field control. The first time usercontrol is run in the match the pneumatics do not close when the button is pressed. The mistake is assuming that a local variable will maintain state between calls to usercontrol, the solution for this is to make the variable my_state static or (worse) global. my_state then retains its value between calls.

static int my_state = 0;

[jpearman]

@Mentor_355U

Would it be possible for VexCode to include a linter that would detect / warn against this (and any of these other common) anti-patterns before the compiler runs?

I don’t know, probably not. There are also alternatives, I’m trying to simplify here. This topic

Competition code and user created tasks

We’ve had some feedback recently from teams who have had issues with competition control. I’ve looked at some of the team code and noticed that often problems are being caused by tasks started in either the driver or autonomous section of the program being allowed to run during the other phase of the match. This can occur with programs written in either VEXcode or PROS (and I assume RobotMesh, but I’m not planning to specifically discuss that programming environment in this topic) although the…

discussed the bStopAllTasksBetweenModes flag that can also remove event handlers but I didn’t explain or recommend anyone use that last week.

[jpearman]

Ok, last one I can think of for today.

motor setVelocity is evil…

consider this code.

void autonomous( void ) {
    m1.setVelocity( 100, percent );

    this_thread::sleep_for(1000);

    m1.spin( reverse );
} 

/*----------------------------------------------------------------------------*/

void usercontrol( void ) {
    m1.setVelocity( 0, percent );
    m1.spin( forward );

    while (true) {
      m1.setVelocity( Controller.Axis1.position(), percent );

      this_thread::sleep_for(10);
    }
}

The intent is for the motor to spin at 100 percent in reverse during autonomous after something else has happened. In reality the motor will often spin forwards at 100% immediately and then spin in reverse.

The problem is again that usercontrol can often run when the controller/field connection order is inconsistent. If user control runs, the motor is told to spin at 0 velocity and then we run into problems.

setVelocity has different functionality depending on whether the motor is stopped, when it means set the velocity to be used by the next spin command (with no velocity parameter), however, if the motor has already been told to spin then it means change velocity to a new value.

When auton runs after usercontrol and setVelocity used, the motor will move.

as I said, setVelocity is evil.

but the pattern comes from blocks where it was required to reduce the number of parameters each block had and overloads were not available.

You could stop the motor at the beginning of auton, but my preferred solution is to never use setVelocity and add velocities as parameters to the spin command.

void autonomous( void ) {
    this_thread::sleep_for(1000);

    m1.spin( reverse, 100, percent );
} 

/*----------------------------------------------------------------------------*/

void usercontrol( void ) {
    while (true) {
      m1.spin( forward, Controller.Axis1.position(), percent );

      this_thread::sleep_for(10);
    }
}

[jpearman]

Here’s one for the PROS users (and @Sylvie )

void initialize() {
    pros::lcd::initialize();
    
    // calibrate the IMU
    imu_1.reset();
    while( imu_1.is_calibrating() ) {
        pros::delay(10);
    }
    pros::lcd::set_text(7, "Cal Done");    
}

All is good, unless the inertial sensor is unplugged, then the code blocks forever it seems and auton/driver never get called.

Not an issue in VEXcode.

I need to look for a PROS API to test if the sensor is installed or not, but I don’t see one.

Edit:
Looks like solution is to check that the correct sensor is plugged in. It’s unfortunate that you need to remember the port number, doesn’t seem to be a way to get that from the imu instance.

	if( pros::c::registry_get_plugged_type(2) == pros::c::E_DEVICE_IMU ) {
		imu_1.reset();
		
		while( imu_1.is_calibrating() ) {
			pros::delay(10);
		}
	}

alternative solution proposed by @JoshJ

    if( imu_1.reset() != PROS_ERR ) {	
      while( imu_1.is_calibrating() ) {
        pros::delay(10);
      }
      pros::lcd::set_text(7, "Cal Done");
    }
    else {
      if( errno == ENODEV ) {
        pros::lcd::set_text(7, "No Imu device");
      }
    }

However, in both cases if IMU is disconnected during the loop waiting for calibration to finish, the code blocks.

[JoshJ]

I need to look for a PROS API to test if the sensor is installed or not, but I don’t see one.

The most convenient way imo would be to check the return value of reset(), which returns PROS_ERR if it was unsuccessful (if either nothing, or the wrong thing is plugged in on that port, alongside other errors). It also sets errno depending on what happened.

[jpearman]

Yes, that works as well.

The important point is that typically a team will blame field control for the error, but the code is at fault.

[Mentor_355U]

@jpearman

The important point is that typically a team will blame field control for the error, but the code is at fault

Is there a way to make this, and your prior post about thread issues, a “knowledge base” article? This is incredibly useful information, especially getting at assumptions which, in fairness seem reasonable without specifically thinking thru real world scenarios. I’m afraid in a year or two this post will be hard to find.

Maybe change the competition template to include comments that highlight the fact that at a competition a team’s routine may cause user control to run more than once?

Maybe include these in a set of examples of “what not to do” programs in the VexCode Examples?

[jpearman]

@Mentor_355U

Is there a way to make this, and your prior post about thread issues, a “knowledge base” article?

Perhaps we could document in more detail how/when the different competition phases will run for alternate connection and program execution order, but I would expect VEX will only want to do that for VEXcode. PROS actually behaves slightly differently IIRC, and really that’s for the PROS team to document. Related to this, I wish the PROS team had stuck around at worlds and provided some technical support to their users. The tech support team this year has almost no experience with PROS and turned away several teams with issues. It’s not possible to support the partner development solutions beyond just giving superficial advise.

[nenik]

I would add one more “field-control related” failure mode that my middle school team run into earlier in the season. First the symptoms:

Sometimes, somewhat rarely and only during tournaments, the robot would go very sloooooowly during the auton period (then work perfectly in driver control). Never seen in the lab before the tournament while using the controller-built-in match timer.

I’ve got the students to reproduce the problem in the lab (you always have to find a way to reproduce a problem, if you want to claim you have fixed it) using the wired manual competition switch using the following scenario (typically, they would plug in first, then start the program):

1. Start the program.
2. Plug in the (disabled) comp switch.
3. Switch to auton, enable.

From this point, after cursory look at the code, the explanation was rather easy, even though the mechanisms behind are rather involved. They have had a teleop routine that starts a thread per subsystem, one of those subsystems being the drivebase control. Here is what happens behind the scenes with the failure scenario:

1. The program starts. Robot is enabled in the driver mode, so the OS starts a new thread and runs the teleop() function in its context.
2. The teleop() function starts all its extra threads, drivebase control among them (which allows controlling the robot)
3. Cable gets plugged in, robot gets disabled.
4. Vex OS stops the driver thread running the teleop() function. All threads started from there keep running though.
5. While the robot is disabled, the drivebase thread keeps running, but joysticks read zeros and motor speed commands are ignored.
6. Auton mode is started. Vex OS starts a new thread and runs the auton() function in there. Their code starts to execute, which first tries to drive forward in a straigh line. That is a loop performing ramp-up, drive and ramp-down on approaching the target, thus calling setSpeed() every 20ms or so.
7. But the drivebase thread is running as well. Robot is in auton mode, so all the joysticks still read as zero, but its motor commands are now executed. Thus, the drivebase motors keep getting conflicting, interleaved/alternating calls of setSpeed(0) vs. setSpeed(<the right ramp speed>).
8. Given how V5 motor comms are implemented, the motor will randomly pick up one or the other last command, resulting in a very slow and jerky movement.
9. After the auton finishes (VexOS kills the auton thread) and driver is enabled, VexOS starts a thread for teleop(), where their code would try to start all the per-subsystem threads again. But since VexOS uses thread function identity (address) as a key, it first kills the old copies of the threads, then start them anew, for flawless driver control behavior.

The team first implemented a quick, if hacky, workaround using bStopAllTasksBetweenModes, though that was incompatible with their UI (telemetry) task, so they switched to the pattern where all previously infinite loops inside drive-control threads rather read:

   while (Competition.isDriverControl() && Competition.isEnabled())  {
    ....
   }

Since we were able to deterministically reproduce the problem before, we could verify the fix is working and reliable.
Also, from that point, they have exclusively used actual competition switch for testing, as it better matches the actual competition. Later in the season, they have also obtained the Timing Competition Switch from @Puzzlers100, which was a great help, since it allowed easily simulating the cable plug in and removal w/o actually having to disconnect the RJ45.

[lacsap]

@nenik

I would add one more “field-control related” failure mode that my middle school team run into earlier in the season. First the symptoms:

I think you are describing a user created problem. Here is the kb article on connection robot to the field:

Connecting to the V5 Competition Field

The steps start with the robot and controller(s) coming to the field powered off before connecting to the field.

Then you connect controller to field and power on the controller, and then the robot. You wait until the wireless connection is made before starting your competition program.

This assures you have a known state.

I think I need to make infographics for the fields for our middle school events. Or checklists to tattoo on drive team arms - that would be good documentation practice

[nenik]

@lacsap

think you are describing a user created problem.

User code problem, yes, that’s why the quotes around “field-control related”. Operator problem? To some extent (lack of consistncy), but if the SW can deal with that better, it should deal with it better.

@lacsap

Then you connect controller to field and power on the controller, and then the robot. You wait until the wireless connection is made before starting your competition program.

It’s not always possible to follow this protocol, especially for advanced teams. When you need to do some moves in pre-auton, like calibrate end-stop position, tension-up a mechanism, your pre-auton needs to run with robot enabled. In a similar manner, if you’ve got a controller-driven auton selector, robot needs to be enabled to pass through button clicks.
You might argue it’s bad robot design, but plenty of real-world “robots” are built with some form of self-calibration, for example because endstops or servo torque feedback are cheaper than absolute encoders.

Anyway, even if that protocol was a hard rule, you’d still want to test the protocol violation for robustness. It protects you a bit from cable glitches, but even the TM might temporarily enable the robots as part of field troubleshooting.

[jpearman]

Yes, you are describing almost exactly the issue of starting additional threads I described in this topic last year.

Competition code and user created tasks

We’ve had some feedback recently from teams who have had issues with competition control. I’ve looked at some of the team code and noticed that often problems are being caused by tasks started in either the driver or autonomous section of the program being allowed to run during the other phase of the match. This can occur with programs written in either VEXcode or PROS (and I assume RobotMesh, but I’m not planning to specifically discuss that programming environment in this topic) although the…

It’s difficult for vexos to solve this automatically as there are programs where the user may expect this behavior, so we leave it to the user to manage threads beyond the two default competition started ones. It’s also quite viable to skip using our competition class completely and monitor the competition status directly (well, you need the class to do that but you can skip registering the auton and/or driver threads) or handle the events in a different way (for which I would have to provide some additional documentation on how to do that).

When we created this back in 2017, we made the behavior more or match what would happen with cortex. If a controller is connected then driver will always run when field control is not connected, I felt this was more useful as it allows driving practice without needing a competition switch or moving the code to a non-competition template.

[nenik]

What I don’t like about the competition class (and some others) is how they loose context. It’s a C++ class, but mostly used as a namespace wrapper with C-like approach.
You can only register a no-parameter callback, which means a free-standing function somewhere. If you need to pass something down (trivial example: selected auton), you’re forced to use globals.
Thread/task at least allows you to pass down a pointer, so you can use a (class-)static trampoline that restores the context (this pointer) and forward the call to a member function, for example.

Imagine the competition class would instead look like this:

class competition {
[...]
protected:
    void runAuton() { _autonomous_callback(); }
    void runDriver() { _drivercontrol_callback(); }
}

(where the system would call those functions on a registered instance)
A more advanced programmer could then, instead of registering callbacks (which would still work for other users), subclass it to get access to their context of choice. For example like:

class CompetitionSupport : public competition {
    Robot& _robot;
    CompetitionSupport(Robot &r) : competition, _robot(r) {}
protected:
    void runAuton() override {  _robot.auton(); }
    void runDriver() override {  _robot.driver(); }
}