Learning the Command System; need help with a few advanced concepts

[19lmyers]

Recently my team’s controls mentor and I have been working together on a set of procedures and tools to teach our 100% rookie programming team before I head off to college. (I’ve been the sole programmer for four years and he’s only been with us for two, so we’re basically starting from scratch.)

In the process, we’ve been looking into the Command System, and in tweaking it to fit our needs I’ve come across a few roadblocks. Could any of you guys help clarify them?

Subsystems vs Commands: automated logic

For many of our mechanisms, we have some sort of automated control system that runs periodically - but we sometimes want to manually control the mechanism instead.

Is the proper solution to have said automation in the Subsystem’s periodic() method? In a method called by Commands that want it? Or in every Command that uses it? (It’s sort of a separation of concerns problem?)

In one case, we actually had a three-jointed arm that was either manually controlled or automated but the driver was supposed to have the option to manually adjust the position of one of the joints. I’m not sure at all where/how that would be implemented in the Command system.

(There’s actually another related question to this: would an arcadeDrive() method go in the Subsystem or a Command? I’m not sure every caller wants the arcade drive method, but then again I still use drive methods that only work when PID is enabled.)

Command structure: how and when?

One thing I believe in is consistency - a Command shouldn’t be tightly coupled to the implementation of the Subsystem it runs on. If so, then say I have a distance-based drive command for autonomous - how does it know to stop without accessing the drive encoders?

Where this becomes a problem is any “state” based automation system, like a lift with multiple positions or (sighs) the three-jointed arm. If the automation is done inside the Subsystem, then these Commands would only be able to set the position, and not be able to know when the mechanism’s reached the desired position. (Unless you run the automation in the Command, and then you’re back to the previous dilemma.)

And for that matter, do you really need a Command per state, like an OpenClaw and a CloseClaw Command, or a Command for each position, or can one parameterize them? Don’t we then end up exposing the Subsystem’s parameter type (which I often create an enum like Position for) to the Robot itself, which isn’t good in terms of coupling?

Speaking of states… do Commands start other Commands? I feel like Commands shouldn’t be managing their own state, but then is it Robot’s job to access auxiliary sensors to decide, say, to run a “tip protection drive” command instead of a normal drive command? But isn’t this just IterativeRobot again then?

Operator Interface

Ugh, I never liked querying buttons every loop, but the OI templates are frustrating. For the most part you can bind Commands to buttons (though what does whilePressed() mean? Does it constantly call initialize()?) but for the things (axes, analog triggers, the POV selector) you can’t.

Does it then violate separation of concerns to also add getters to the OI object exposing those, and passing it into Commands? Is there really no way to avoid coupling the Commands to the driver station layout without resorting to something really complex, like a lambda?

General syntax

In general, I want to be sure the syntax I’m using is both elegant and reasonable to teach relatively new programmers without compromising the effectiveness of the program. Yes, it’s a tall ask. No, I don’t know how to check myself on this.

---

For reference, the full code I’m working with is here.
(It’s sort of theoretical and probably won’t correspond to a real robot when it’s done.)

To those who actually made it to the end, thank you!

[Ty_Tremblay]

Each subsystem in command based java has a default command that is set up through setDefaultCommand(). This should be the command that your subsystem should run when it’s not supposed to be doing something else.

For example, our drivetrain runs BobDrive(), our elevator runs JoystickElevator(), and our pneumatics subsystem runs CompressorRun().

It’s worth noting that not every subsystem needs a default command. We used this in our cube collector to essentially have the collector continue doing whatever it was last told to do.

I recently did a presentation at our offseason event, Battle Of the Bay, going over command based java. It’s on our website: http://www.frc319.com/command-based-java and I’m happy to answer any questions you might have.

[GeeTwo]

What to do when no operator-originated commands: Use the default command as @Ty_Tremblay mentioned.

Where to put the arcadeDrive() method: I’ve seen it done both ways, but I definitely prefer to have the command access the joysticks and do any scaling, and have the subsystem run with “clean” directions. Even cleaner is to make a custom joystick interface class so the command is just being the traffic cop with the info; this means that every command which accesses the OI is using it the same way.

Separation of function regarding encoders: I agree with your concerns. A clean way to do this is for the drive train subsystem to provide one or more methods to provide abstracted encoder info to the command, e.g. distanceLeft() and distanceRight() for a drive train or height() for an elevator, which would an answer in problem space units (inches or feet), having worked past all the fiddly bits of encoder counts and gear ratios.

I’ve seen both simple and parameterized commands, and each has its uses. A neat thing to do is provide convenience functions to a parameterized command which drives to predefined positions, along the lines of:

Command goToGroundFloor() {
      return new moveElevatorTo(elevator.groundFloor);
}

Usually the case where you want one command to call another can be managed using command groups. Sensor driven things can be started with a trigger.

whilePressed() does not call initialize() while the command is running. As such, it only calls initialize() repeatedly if the command exits quickly.

getters: Unless you really NEED to put the details outside of the class which exposes them, getters and putters should return “sanitized” or “problem space” numbers rather than raw numbers. (A generalization of what I said about separation of function regarding encoders.)

[cpapplefamily]

Operator Interface

Ugh, I never liked querying buttons every loop, but the OI templates are frustrating. For the most part you can bind Commands to buttons (though what does whilePressed() mean? Does it constantly call initialize() ?) but for the things (axes, analog triggers, the POV selector ) you can’t.

@GeeTwo touched on parts of this already.
How I understand the differences of the whenPressed, whenReleased, and whileHeld are to when the command is started and is complete.

In all cases the the initialize() method is called once followed by the execute() every robot periodic scan until something interrupts it.

whenPressed will start the command “when Pressed” and continue until the
isFinished() returns true
or
another command is scheduled that requires the same subsystem.

isFinished will be a boolean statement comparing some DIO input, analog value, encoder count, or a timeout for example

whenReleased is the same as above only will start the command when the button is “released”

We have used these two state for example last season our driver would hold down a button to start our HatchAcquire_Step_1_3_Command_Group sequence and when they release that button the HatchAcquire_Step_4_6_Command_Group would run.

The end() and interrupted() methods are similar but called by different conditions.

end() is called when the isFinished() returns true so you can clean up after yourself or turn off motors as you may require.

The interrupted() method is call if a new command is scheduled the requires the same subsystem before the new command takes over the subsystem. Often we call the end() method inside of the interrupted() because they likely will do the same thing.

whileHeld calls the initialize() once then the execute() like the other states but when the button is released the interrupted() method will get called completing the command.

To treat the analog inputs as button you will need to create classes that extend the button class. We have used code from 1519 Mechanical Mayhem Here is a link to our utils https://github.com/GraniteCityGearheads3244/2019_Jupiter/tree/master/src/main/java/org/usfirst/frc3244/Jupiter2019/util There are also a few AND OR utils that help if two inputs are required to start a command like an ENABLE switch to arm the warhead. Or with the OR util if two buttons (Driver or CoDriver) call the same command you don’t want the second button press to cancel first call and restart the command.

[cpapplefamily]

Command groups is where I feel things get fun. Running two command in parallel and in sequence. There are a few gotcha’s in there when having long sequences of both you may have to add waitForChildren() in the group before moving onto the next series of commands.

You also have conditional commands that when used will run one of two choices at that moment based on a “condition”. In most cases when the commands are created at robot.init() the commands will capture any conditions your trying to test at that moment and not what might be important later as the robot is operated. Here is an example we had last season. https://github.com/GraniteCityGearheads3244/2019_Jupiter/blob/master/src/main/java/org/usfirst/frc3244/Jupiter2019/commands/DriveToggleShifter.java

[tkdberger]

Is the proper solution to have said automation in the Subsystem’s periodic() method? In a method called by Commands that want it? Or in every Command that uses it? (It’s sort of a separation of concerns problem?)

I personally try to avoid using periodic to actually change things in the “real world” - it’s fine to e.g. put debugging in there, but it probably shouldn’t send anything to motor controllers/solenoids.
Our solution for ongoing tasks is a “default command” that runs whenever something else isn’t going on (other replies have already discussed how to do this.)

(There’s actually another related question to this: would an arcadeDrive() method go in the Subsystem or a Command? I’m not sure every caller wants the arcade drive method, but then again I still use drive methods that only work when PID is enabled.)

Probably a default command.

say I have a distance-based drive command for autonomous - how does it know to stop without accessing the drive encoders?

It uses a getWhatever method of the subsystem. The susystem can handle all the unit conversions if you want it to. This also solves the issue of a command not knowing when the mechanism has reached a desired position, since you can compare the getWhatever return value to your target in isFinished.

And for that matter, do you really need a Command per state

Nope! You can make a command’s constructor take a parameter like any other class, and have some logic in execute (or initialize or whatever) that acts on that parameter.

For the most part you can bind Commands to buttons […] but for [other] things (axes, analog triggers, the POV selector) you can’t.

Check out POVButton and possibly Trigger

Does it then violate separation of concerns to also add getters to the OI object exposing those, and passing it into Commands?

You shouldn’t need to expose button values to Commands unless it’s something not started from OI (e.g. our drivetrain has a default Drive command that gets stick values from OI. It gets other button values from there as well, but many of them could be implemented with a state stored in the drivetrain subsystem and some whenPressed and whenReleased trickery.)

… did I miss anything?

[retrodaredevil]

Is the proper solution to have said automation in the Subsystem’s periodic() method? In a method called by Commands that want it? Or in every Command that uses it? (It’s sort of a separation of concerns problem?)

For choosing whether something should go in a command or not, think if you want to reuse that piece of code somewhere else or in another command. Usually, you will want to so you should put that logic in the subsystem.

For something like a boom/lift/elevator you will usually want to be able to do something like boom.middleHeight() or boom.setSpeed(speed). This year we accomplished this by a simple “Lift Mode” shown here. Also, most of the time you won’t even have state like that. Last year we just called different methods periodically instead of actually storing state like seen in that link.

The reason we put code in periodic instead of just calling a different method was because periodic() was called, well, periodically. We didn’t have to worry about setting the speed back to 0 because it would automatically do that if we forgot to call it. It doesn’t matter much for manually setting the speed, but if you’re implementing a PID loop in auto and stop calling a setSpeed() method all of a sudden, it will keep going with the last speed. That’s something to be careful about when designing your code.

would an arcadeDrive() method go in the Subsystem or a Command?

I think that this could go either way. It probably doesn’t matter because you’re most likely only going to be using the arcadeDrive() code in a single command. If you decide to use that code for autonomous, you could probably put it in the subsystem. Something unique about arcadeDrive is that the code for that doesn’t really have to know much about the implementation. It just has to change x and y to left and right speeds. If you really wanted to, you could abstract that to the point where it’s just a static helper method that knows nothing about other parts of your program.

However, there are more complex arcadeDrive methods which may make a command a more viable option.

Does it then violate separation of concerns to also add getters to the OI object exposing those, and passing it into Commands? Is there really no way to avoid coupling the Commands to the driver station layout without resorting to something really complex, like a lambda?

If you’re worried about coupling commands with driver station layout, you could pass all the buttons/axi as constructor parameters. Unless the command is very generic, this probably isn’t the best option.

So, although we heavily used a custom command based system this year, all of our operator code was basically iterative. This is a choice for you to choose what’s best and what you’re more comfortable with. If you need to have even a little bit complex code, passing your entire OI into a command is probably the best option. Even though it was basically iterative, we had the operator code in its own command. If you’re going to use whileHeld or whenPressed, the commands in those should be very, very simple, otherwise I recommend a more iterative approach.

By passing your entire OI object into something like your drive command, it will make adding something like a “fine control” button easier in the future as you don’t have to add more constructor parameters.

In general, I want to be sure the syntax I’m using is both elegant and reasonable to teach relatively new programmers without compromising the effectiveness of the program. Yes, it’s a tall ask. No, I don’t know how to check myself on this.

In my opinion, I don’t think new programmers should have to do everything in a command based way. As long as you can teach them to separate their code in a reasonable way, I think the code they produce will be maintainable and readable.

but for the things (axes, analog triggers, the POV selector ) you can’t.

You can actually create your own Button or Trigger subclass and map it to one of those 3 things.

Disclaimer: We didn’t actually use the official command framework. My action-lib library was simple and effective at getting the advantages that you can get with the built in command framework. But we mostly just used it to do driver control after auto finished and used it heavily for auto. In my opinion, the biggest advantage you get from commands is just auto. Some teams only use commands for auto and use something more basic for driving.

[Oblarg]

By passing your entire OI object into something like your drive command, it will make adding something like a “fine control” button easier in the future as you don’t have to add more constructor parameters.

Note that doing this will also tightly couple your command to your OI object, however, which can be undesirable.

Generally, I recommend having some sort of intermediate abstraction layer between the joystick objects and a drive command. Lambdas work well.

[retrodaredevil]

While I do agree that there should be an intermediate layer of abstraction, being able to pass an object in would work out nicer as you require more buttons. Something like this might make adding buttons less of a pain:

interface DriverControls {
    double getX();
    double getY();
    boolean isFineControl();
    // able to easily add whatever you desired here and implement in your subclass(es)
}

Unless I need something super generic, I dislike passing parameters (in this case a bunch of lambdas) into a constructor when I know I may have to add more parameters down the line. This is why I like to wrap controls in its own object: It makes it less tightly coupled and is very easy to extend. This also keeps you from having a constructor call with a ton of parameters. Might as well bunch it into an object or interface to make it cleaner.

By doing that you can easily change your implementation by passing another implementation of DriveControls. And even if you started adding methods to DriveControls unrelated to the drive command, it still wouldn’t be that tightly coupled as you can just change the implementation if you really need to.

[Oblarg]

I strongly recommend handling isFineControl and similar features in their own commands.

[19lmyers]

Thanks everyone for the input, it’s really helping us figure this out. I have a few more questions though:

Parameters

Where would you store constant values that are reused multiple times, but aren’t objects (i.e. max speed, etc)? Putting them in RobotMap doesn’t make any sense, as that violates the loose coupling, but multiple classes would need to know the max speed (as the drive system requires input in terms of speed).

Disabling subsystems

This may be a holdover from IterativeRobot, but should I still use a disable() method on my Subsystems? Is there a better way to do it instead?

Also, why do samples run the scheduler in disabledPeriodic()?

Default commands

…how do you set a default command?

[Oblarg]

Where would you store constant values that are reused multiple times, but aren’t objects (i.e. max speed, etc)? Putting them in RobotMap doesn’t make any sense, as that violates the loose coupling, but multiple classes would need to know the max speed (as the drive system requires input in terms of speed).

This is a hard problem. The “correct” solution is “in a config file,” but that involves a significant amount of overhead. You’ll have to try different things and see what feels right for your team and situation.

This may be a holdover from IterativeRobot, but should I still use a disable() method on my Subsystems? Is there a better way to do it instead?

Also, why do samples run the scheduler in disabledPeriodic()?

Commands can be made to run when disabled; you should really call the scheduler from robotPeriodic.

…how do you set a default command?

Call the setDefaultCommand method in Subsystem.

[19lmyers]

Call the setDefaultCommand method in Subsystem .

Should that be in the constructor, or in initDefaultCommand()?

[Oblarg]

The superclass constructor calls initDefaultCommand() so it makes literally no difference.

It’s worth noting that if a lot of this seems clunky and weird, that’s because it is, and that’s why I’ve rewritten the whole darn thing for 2020.

[retrodaredevil]

Where would you store constant values that are reused multiple times, but aren’t objects (i.e. max speed, etc)? Putting them in RobotMap doesn’t make any sense, as that violates the loose coupling, but multiple classes would need to know the max speed (as the drive system requires input in terms of speed).

You can have a package private Constants class and pass the values to different constructors as they need them. As @Oblarg said, try different things. There’s no single solution for this.

If you want to change it on Shuffleboard, then using a lambda might work well.

This may be a holdover from IterativeRobot, but should I still use a disable() method on my Subsystems? Is there a better way to do it instead?

If you need to do something on a certain subsystem when the robot disables, this is probably a good option. To be honest I’d be interested to know if anyone else does something more “command like” for this.

Also, why do samples run the scheduler in disabledPeriodic()?

My guess would be that if a team wants to use an LED command or something, it wouldn’t stop when the robot disables. I kind of wonder why samples don’t just run the scheduler in robotPeriodic(). Maybe there’s a reason for that I don’t now.

how do you set a default command?

https://wpilib.screenstepslive.com/s/currentCS/m/java/l/599742-default-commands

[Oblarg]

Maybe there’s a reason for that I don’t now.

robotPeriodic is a fairly recent addition, so the “reason” is just inertia.

[cpapplefamily]

@19lmyers my first exsposure to Command Base robot was through Brad Miller 's Robotbuilder series. https://www.youtube.com/playlist?list=PLYA9eZLlgz7t9Oleid2wtlgnvhGObeKzp It is six ~15min videos on using RobotBuilder that builds out the backbone of the command Base robot. Its from 2013 but most of the steps are still relevant. Its a bit easier now.

I have also hosted a few talks about Robotbuilder at Jumpstart that have been hosted on our YouTube page. I basically preformed the robot builder 6 videos live for a class and using VSCode.
Part 1 https://www.youtube.com/watch?v=sA8fIKZyPiA&t=228s
Part 2 https://www.youtube.com/watch?v=0sIghcMyGhk

[19lmyers]

I have another question regarding dependency management. It’s not entirely related to the Command System itself, but it seems to be playing a major part in it.

As part of our code architecture, we’ve chosen to pass dependencies through constructors instead of statically accessing member variables of the Robot class.
(For example, our Subsystems are passed SpeedController objects created in RobotMap, and our Commands are passed the Subsystems they require.)

The problem that occurs with this system is when we want to create a Command, either in a Subsystem (with setDefaultCommand()) or in our OI class (for Button binding).
These commands often have Subsystem dependencies which aren’t needed by the classes they’re created in, and I don’t want to clutter up the code by passing them in through the constructor.

How have other teams not using static access solved this problem?
(I know about dependency injection frameworks, but that’s far too complicated for my use case.)

[Oblarg]

Try to create all of your commands in one place (the place where you’ve created your subsystems). You can call setDefaultCommand from outside of a subsystem (it’s a public method), so there’s no strict need to have the command declared in the subsystem.

[UnofficialForth]

Do you have an online repository for you code that we could look at? I’m interested to see your code.

Edit: Nevermind, after some more looking, I found your github.