Command Based Programming (Threads?)

[nathanzentner08]

I was under the assumption that when you used the a Command Group and added parallel that it would handle them as separate threads. I was testing this out with my group and when I put a Timer.delay in the execute method of one command, it held up all execution. Is this multi-threaded based? Are there other ways to run things within threads within the WPILibJ?

[Arhowk]

If you're using Timer.delay than you're fundamentally missing the purpose of the command based structure... essentially you should be split into three different parts (the code before the wait, the wait itself, and the code after the wait) and they should all be non-interruptive commands.

[Ben Wolsieffer]

Command-based programming is just an extension of iterative that makes it easier to schedule and trigger actions. It gives you most of the benefits of multithreading without the added complexity of worrying about deadlocks and race conditions.

All commands run in the same thread, so they can't ever block. When commands are run, they are added to a queue which the scheduler executes iteratively.

[Ether]

they should all be non-interruptive commands.
What does "non-interruptive" mean?

[TFleig78]

What does "non-interruptive" mean?



He probably meant interruptible.
From the wpilib website:

When a command is scheduled, the Scheduler checks to make sure that no other commands are using the same subsystems that the new command requires. If one or more of the subsystems is currently in use, and the current command is interruptible, it will be interrupted and the new command will be scheduled. If the current command is not interruptible, the new command will fail to be scheduled.

[Ether]

What does "non-interruptive" mean?

He probably meant interruptible.

You probably meant non-interruptible?

If the current command is not interruptible, the new command will fail to be scheduled.

So if all the commands should be "non-interruptive", and non-interruptive means non-interruptible, then all new commands will fail to be scheduled if any command is presently running?

[TFleig78]

You probably meant non-interruptible?



So if all the commands should be "non-interruptive", and non-interruptive means non-interruptible, then all new commands will fail to be scheduled if any command is presently running?




Yes, I meant non-interruptible.

And yes, the currently running command would have to end before any other command that requires the same subsystem could be scheduled. This is only true if you set the command's interruptible property to false.

[gixxy]

The way commands are run in the Command Based Structure are via a queue. The execute method of each scheduled command is executed in sequence (I assume in the order added to the queue). This means that each command's execute method shouldn't contain anything that will sleep the thread (delays) or otherwise tie the thread up for long periods of time (intensive camera calculations). Such actions will lead to decreased robot responsiveness.

There are many other ways to implement non-blocking timers, if you describe what it is you mean I can help point you in the right direction.

I believe by "non-interruptive" Arhowk meant non-blocking. As far as interrupts on a command, they are only used when a new command is added to the queue that uses the SAME subsystem as another queued command. So, if the first is not interruptible, the second command will not run, if it is, the first command will be interrupted and the second will take over. Command Interrupts do not refer to commands using different subsystems in any way.

[Ether]

I believe by "non-interruptive" Arhowk meant non-blocking.

@Arhowk: is that what you meant?

[Arhowk]

@Arhowk: is that what you meant?




yes... :( by non-interruptive, I meant code that does not engage the use of the java stall commands (Timer.delay, Object.wait, Thread.sleep)

[notmattlythgoe]

The way the Command Based Structure is designed is to keep a list of all currently running commands. Every ~20ms each of these commands will be executed one at a time. Each of the commands should be designed to execute very quickly so it does not delay the execution of the other commands that are being executed. This means, if you delay the execution by some means this will cause the other commands after it in the list to wait on the delayed command.

[nathanzentner08]

Our thought was to have an image processing Command, that would have taken a bit more time and processing power, that ran in a parallel Command Group, but since it isn't really parallel, then the main thread would be stuck in the Image Processing command until it returned.

This non-threaded functionality could affect our driving (Essentially Everything) because if the thread is working on image processing then it is not available to change speed on our motors. (This is just a simple example.)

Im coaching the team and talking about parallel processing and trying to show an example based on the API that is given. The more I work with it, the more I find it isn't really there.

It's alright that it is not in there, but the documentation says:

"... instead of waiting for the child to finish, a CommandGroup will have it run at the same time as the subsequent Commands. ..."

[notmattlythgoe]

Our thought was to have an image processing Command, that would have taken a bit more time and processing power, that ran in a parallel Command Group, but since it isn't really parallel, then the main thread would be stuck in the Image Processing command until it returned.

This non-threaded functionality could affect our driving (Essentially Everything) because if the thread is working on image processing then it is not available to change speed on our motors. (This is just a simple example.)

Im coaching the team and talking about parallel processing and trying to show an example based on the API that is given. The more I work with it, the more I find it isn't really there.

It's alright that it is not in there, but the documentation says:

"... instead of waiting for the child to finish, a CommandGroup will have it run at the same time as the subsequent Commands. ..."

It will run it similar to how commands for different subsystems would run at the "same time". If you execute method takes a long time to execute it will slow everything else down.

Last night we created a separate thread that will run our vision processing so it doesn't slow down the processing of commands. I can assist you with this if you'd like.

[Ether]

If you execute method takes a long time to execute it will slow everything else down.

Within the command-based architecture, what is the recommended way to implement a delay (for example, in a firing sequence).

[notmattlythgoe]

Within the command-based architecture, what is the recommended way to implement a delay (for example, in a firing sequence).




I would suggest using a WaitCommand in a CommandGroup. Below is an example that will fire, then wait 1 second before firing again and then finishing the CommandGroup.


public FiringCommandGroup extends CommandGroup {

public FiringCommandGroup() {
addSequential(new FireCommand());
addSeqiential(new waitCommand(1));
addSequential(new FireCommand());
}
}

Another way to do it would be to check the time passed in the execute method of a command to see if the period of time you want to wait has passed.

Commands also have a timeSinceInitialixed() method that will return the time in seconds since the command was initialized(aka started). This could be used to check to see if a command has been running for a certain period of time.

[Ether]

Thanks for the response. I hope you don't mind some follow-up questions:

I would suggest using a WaitCommand in a CommandGroup. Below is an example that will fire, then wait 1 second before firing again and then finishing the CommandGroup.


public FiringCommandGroup extends CommandGroup {

public FiringCommandGroup() {
addSequential(new FireCommand());
addSeqiential(new waitCommand(1));
addSequential(new FireCommand());
}
}

What does the waitCommand() do, "under the hood". i.e., how is it implemented?

Another way to do it would be to check the time passed in the execute method of a command to see if the period of time you want to wait has passed.

Is there a "recommended" or "best practice" way to implement that approach?

[notmattlythgoe]

Thanks for the response. I hope you don't mind some follow-up questions:



What does the waitCommand() do, "under the hood". i.e., how is it implemented?



Is there a "recommended" or "best practice" way to implement that approach?




I haven't looked at the wait command under the hood, but this is how I believe it works. I can look under the hood tonight at the source code and see if I'm correct.


public class WaitCommand extends Command {

private double waitTime;

public WaitCommand(double waitTime) {
this.waitTime = waitTime;
}

public boolean isFinished() {
return timeSinceInitialized >= waitTme;
}
}



As to the second question. Something like this would work for a single command doing multiple things based on time.


public void execute() {
if (timeSinceInitialized() < sometime) {
do something;
} else if(timeSinceInitialized() < some bigger time) {
do something 2;
} else {
do something 3;
}
}

[Ether]

I haven't looked at the wait command under the hood, but this is how I believe it works. I can look under the hood tonight at the source code and see if I'm correct.


public class WaitCommand extends Command {

private double waitTime;

public WaitCommand(double waitTime) {
this.waitTime = waitTime;
}

public boolean isFinished() {
return timeSinceInitialized >= waitTme;
}
}


I don't read much Java code. Is this the correct interpretation of what that code is supposed to do?

- the first time it is executed it notes the system time so that it can compute the value of "timeSinceInitialized" each time it is executed.

- it gets executed every 20ms as long as it returns "isFinished()" as false

- it returns "isFinished()" as false as long as "timeSinceInitialized" is less than the desired "waitTime" that was passed as a parameter

- as long as it is returning "isFinished()" as false, any commands that are waiting for it to finish will not execute.

- when "timeSinceInitialized" is greater than or equal to the desired "waitTime", it returns "isFinished()" as true, which results in it being removed from the list of commands to be executed every 20ms. Any commands that were waiting for it to finish will now execute.

[notmattlythgoe]

I don't read much Java code. Is this the correct interpretation of what that code is supposed to do?

- the first time it is executed it notes the system time so that it can compute the value of "timeSinceInitialized" each time it is executed.

- it gets executed every 20ms as long as it returns "isFinished()" as false

- it returns "isFinished()" as false as long as "timeSinceInitialized" is less than the desired "waitTime" that was passed as a parameter

- as long as it is returning "isFinished()" as false, any commands that are waiting for it to finish will not execute.

- when "timeSinceInitialized" is greater than or equal to the desired "waitTime", it returns "isFinished()" as true, which results in it being removed from the list of commands to be executed every 20ms. Any commands that were waiting for it to finish will now execute.




Yes, that would be the correct process from my interpretation.

[Ether]

As to the second question. Something like this would work for a single command doing multiple things based on time.


public void execute() {
if (timeSinceInitialized() < sometime) {
do something;
} else if(timeSinceInitialized() < some bigger time) {
do something 2;
} else {
do something 3;
}
}


Again, is this the correct interpretation of the above code?

- "something" will be executed every 20ms as long as "timeSinceInitialized()" is less than "sometime"

- "something 2" will be executed every 20ms as long as "timeSinceInitialized()" is greater than or equal to "sometime" and less than "sometime bigger time"

- "something 3" will be executed every 20ms as long as "timeSinceInitialized()" is greater than or equal to "some bigger time"

[notmattlythgoe]

Again, is this the correct interpretation of the above code?

- "something" will be executed every 20ms as long as "timeSinceInitialized()" is less than "sometime"

- "something 2" will be executed every 20ms as long as "timeSinceInitialized()" is greater than or equal to "sometime" and less than "sometime bigger time"

- "something 3" will be executed every 20ms as long as "timeSinceInitialized()" is greater than or equal to "some bigger time"

Perfect.

[Ether]

So this codeA:

public FiringCommandGroup extends CommandGroup {

public FiringCommandGroup() {
addSequential(new FireCommand1());
addSeqiential(new waitCommand(1));
addSequential(new FireCommand2());
}
}

...is quite different than this codeB:
public void execute() {
if (timeSinceInitialized() < sometime) {
do something;
} else if(timeSinceInitialized() < some bigger time) {
do nothing;
} else {
do something2;
}
}

...because codeA executes FireCommand1() once, then waits, then executes FireCommand2() once, and exits.

whereas codeB executes "something" repeatedly, then waits, then executes "something2" repeatedly.

Yes?

[notmattlythgoe]

So this codeA:

public FiringCommandGroup extends CommandGroup {

public FiringCommandGroup() {
addSequential(new FireCommand1());
addSeqiential(new waitCommand(1));
addSequential(new FireCommand2());
}
}

...is quite different than this codeB:
public void execute() {
if (timeSinceInitialized() < sometime) {
do something;
} else if(timeSinceInitialized() < some bigger time) {
do nothing;
} else {
do something2;
}
}

...because codeA executes FireCommand1() once, then waits, then executes FireCommand2() once, and exits.

whereas codeB executes "something" repeatedly, then waits, then executes "something2" repeatedly.

Yes?




Correct. I can write up something that would be similar to code A in the second style if you'd like.

[Ether]

Correct. I can write up something that would be similar to code A in the second style if you'd like.

Yes, thank you, that is what I was asking originally.

[notmattlythgoe]

Yes, thank you, that is what I was asking originally.




Apologies, here you go.


public class ShootCommand extends Command {

public void initialize() {
logic for fire1;
}

public void execute() {

}

public boolean isFinished() {
return timeSinceInitialized() > 1;
}

public void end() {
logic for fire 2;
}
}


1. Run fire1 logic when command starts.
2. Do nothing for 1 second.
3. After 1 second return true in isFinished().
4. When isFinished() returns true the end() method is called.
5. Run logic for fire2 in end.

Keep in mind, the logic for both fire1 and fire2 will need to execute quickly. Meaning no Timer.delays or long running loops.

[Ether]

public class ShootCommand extends Command {

public void initialize() {
logic for fire1;
}

public void execute() {

}

public boolean isFinished() {
return timeSinceInitialized() > 1;
}

public void end() {
logic for fire 2;
}
}


1. Run fire1 logic when command starts.
2. Do nothing for 1 second.
3. After 1 second return true in isFinished().
4. When isFinished() returns true the end() method is called.
5. Run logic for fire2 in end.


OK thanks that's pretty clear. Is there any performance difference between the approach shown above and the code in post#15? Which approach do you prefer, and why (or does it depend on context)?

[Arhowk]

OK thanks that's pretty clear. Is there any performance difference between the approach shown above and the code in post#15? Which approach do you prefer, and why (or does it depend on context)?




Performance-wise the latter would be better, since the post in number #15 has three elements in the list whereas the post that you quoted only as one element in the list. Every 20ms, every element in the list is enumerated upon and conditions for execution are checked. This performance boost is negligible unless command groups are in excess of hundreds of commands, which is extremely uncommon (the best I got was 75, though I re-made the entire command based framework from scratch so some of the commands were syntaxical (wait, if, else, etc.))

However, the former approach (#15) is the preferred option for modularity sake. Integrating the wait with both executions turn the command, not into a modular command, but rather a defined sequence. If the sequence is regular (e.g. load hopper, wait, shoot) than it would be preferential to use a CommandGroup, for readability sake. If the sequence is irregular (e.g. turn 15 degrees, wait, shoot at low velocity) than the command will only be used once and more commands of similar conditions (e.g. turn 30 degrees, wait, shoot at high velocity) would have to be developed. In this case, it would be easier to make a turn command, a wait command, and a shoot command and plug them in in a similar fashion to post #15.

[notmattlythgoe]

OK thanks that's pretty clear. Is there any performance difference between the approach shown above and the code in post#15? Which approach do you prefer, and why (or does it depend on context)?




Honestly, the performance differences would between the two would be so minuscule they probably aren't measurable.

My team and I prefer the method I provided in post #15. It's easier to read, easier to modify, and more modular. Here is an example from our competition code from last season. Notice how easy it is to read exactly what the ShootSeries command does.


public class ShootSeries extends CommandGroup {

public ShootSeries() {
addParallel(new PickUpDeploy(PickUp.DEPLOY, 0, PickUp.CLOSE));
addSequential(new WaitCommand(0.25));
addSequential(new SetShooterPosition(Shooter.FIRE));
addSequential(new WaitCommand(0.2));
addSequential(new SetShooterPosition(Shooter.PRIME));
addSequential(new WaitCommand(0.1));
addParallel(new ResetArmCommand());
}
}



public class PickUpDeploy extends CommandBase {

private boolean deploy;
private double rollerSpeed;
private boolean openWings;

public PickUpDeploy(boolean deploy, double rollerSpeed) {
this(deploy, rollerSpeed, PickUp.CLOSE);
}

public PickUpDeploy(boolean deploy, double rollerSpeed, boolean openWings) {
requires(pickUp);
this.deploy = deploy;
this.rollerSpeed = rollerSpeed;
this.openWings = openWings;
}

protected void initialize() {
pickUp.deployArm(deploy);
pickUp.deployCatch(openWings);
}

protected void execute() {
if(oi.isRollerOn()){
pickUp.setRollerSpeed(rollerSpeed);
}else{
pickUp.setRollerSpeed(0);
}
}

protected boolean isFinished() {
return false;
}

protected void end() { }

protected void interrupted() { }
}


public class SetShooterPosition extends CommandBase {

public boolean shooterPosition;

public SetShooterPosition(boolean shooterPosition) {
requires(shooter);
this.shooterPosition = shooterPosition;
}

protected void initialize() { }

protected void execute() {
shooter.primeShooter(shooterPosition);
}

protected boolean isFinished() {
return true;
}

protected void end() { }

protected void interrupted() { }
}



public class ResetArmCommand extends CommandBase {

public ResetArmCommand() {
requires(pickUp);
}

protected void initialize() { }

protected void execute() { }

protected boolean isFinished() {
return true;
}

protected void end() { }

protected void interrupted() { }
}



public class PickUp extends Subsystem {

public static final boolean DEPLOY = true;
public static final boolean RETRACT = false;
public static final boolean OPEN = true;
public static final boolean CLOSE = false;

private Solenoid pickUpSolenoid1 = new Solenoid(PropertyReader.getProperty("PICKUP_SOLENOID_MODULE", 2), PropertyReader.getProperty("PICKUP_SOLENOID_CHANNEL_A", RobotMap.pickUpSolenoid1));
private Solenoid pickUpSolenoid2 = new Solenoid(PropertyReader.getProperty("PICKUP_SOLENOID_MODULE", 2), PropertyReader.getProperty("PICKUP_SOLENOID_CHANNEL_B", RobotMap.pickUpSolenoid2));

private Solenoid wingSolenoid = new Solenoid(PropertyReader.getProperty("WING_SOLENOID_MODULE", 2), PropertyReader.getProperty("WING_SOLENOID_CHANNEL", RobotMap.wingSolenoidChannel));

private SpeedController upperPickUp = new Talon(PropertyReader.getProperty("PICKUP_TALON_CHANNEL", RobotMap.upperPickUpRoller));

private double powerLevel;

private RollingAverager pickUpAverager = new RollingAverager(10, 0);

public void initDefaultCommand() {
setDefaultCommand(new PickUpDeploy(RETRACT, 0, CLOSE));
}

public void setRollerSpeed(double power) {
powerLevel = power;
pickUpAverager.addValue(powerLevel);
upperPickUp.set(pickUpAverager.getAverage());
}

public void deployArm(boolean deploy) {
pickUpSolenoid1.set(!deploy);
pickUpSolenoid2.set(deploy);
}
public void deployCatch(boolean deploy) {
wingSolenoid.set(deploy);
}
}



public class Shooter extends Subsystem {
public String currentHot;

public Solenoid blockerPole1 = new Solenoid(PropertyReader.getProperty("BLOCKER_SOLENOID_MODULE", 1), PropertyReader.getProperty("BLOCKER_POLE_CHANNEL_A", RobotMap.blockerPolePort1));
public Solenoid blockerPole2 = new Solenoid(PropertyReader.getProperty("BLOCKER_SOLENOID_MODULE", 1), PropertyReader.getProperty("BLOCKER_POLE_CHANNEL_B", RobotMap.blockerPolePort2));

public static final boolean FIRE = true;
public static final boolean PRIME = false;

private Solenoid shooterSolenoid1 = new Solenoid(PropertyReader.getProperty("SHOOTER_SOLENOID_MODULE", 1), PropertyReader.getProperty("SHOOTER_SOLENOID_CHANNEL_A", RobotMap.shooterSolenoid1Port));
private Solenoid shooterSolenoid2 = new Solenoid(PropertyReader.getProperty("SHOOTER_SOLENOID_MODULE", 1), PropertyReader.getProperty("SHOOTER_SOLENOID_CHANNEL_B", RobotMap.shooterSolenoid2Port));

public void initDefaultCommand() {
setDefaultCommand(new SetShooterPosition(Shooter.PRIME));
}

public void primeShooter(boolean prime) {
//System.out.println("IN PRIME SHOOTER " + prime);
shooterSolenoid1.set(prime);
shooterSolenoid2.set(prime);
}
}


If interested, here (https://code.google.com/p/aerial-assist/source/browse/#svn%2Fbranches%2FZeta%20Working%20Branch%2FZeta%2 0Practice) is the entirety of our code from last season, with some offseason additions.

Also, here (https://code.google.com/p/recycle-rush/) is our code from this season so far.