Should you have State in Subsystems? (Java)

I am attempting to develop a better framework for my team’s code over the summer, and I am looking at different usages of the Command-Based system. One thing that we really struggled with this past season was figuring out the logical divide between commands and subsystems. As an example, this past season my team had a shooter with variable shoot speed, shoot angle, and feeder wheel speed. We had “presets”—combinations of shoot speed, angle, and feed speed—that we had tuned to be accurate for each side of the speaker. I decided to implement these as enumerated states within the shooter subsystem, and only have the set of possible enumerated states accessible through its public API. Looking back, this was a bad decision, because we also wanted to move the climber manually at times, and adjust the setpoints during a match, and take manual control of the motor to reset it, and all of this meant we had to set up a complex chain of logic within the subsystem that was hard to control and resulted in a lot of complex and tangled state between the subsystem and its commands. But, to this day, I’m not sure if putting all the state in the commands would really have been a better option due to the amount of data that should really be encapsulated within the subsystem.
I have recently seen teams such as 2910 who use a superstructure abstraction which centerally manages the state of every subsystem and allows commands to request a state change through it, rather than interacting with the subsystems directly. This seems good, but aren’t there cases where having only one possible state could get really unwieldy? Like, say you’re in the auto-shooting state but you want to put the climbers up, for example. Do you need one state for AutoShooting and another state for AutoShootingAndMovingClimber? It seems like you could easily end up with dozens of states which have a lot of redundancy
Anyway, I’d love to hear your thoughts.

1 Like

The pattern that is recommended by WPILib is the command factory pattern. Each subsystem has methods that return commands, and those methods serve as a public interface of available actions. Thus, the details of the states/commands are in the subsystem class, but all control of the subsystem is through those commands.

3 Likes

Why did this conflict with the shooter state machine?

I prefer subsystems doing more to hide their implementation details.

In the case of a climber that can either be manually ran or that could use a control loop I think we would have 3 commands:

public Command cManualUp() { }
public Command cManualDown() {}
public Command cControlLoop(DoubleSupplier setpoint) {}

A shooter that needs to be ‘side aware’ could have commands like this:

public Command cShootLeft() {}
public Command cShootRight() {}
public Command cShootCenter() {}
1 Like

These are all different subsystems on my team’s robot so there isn’t much interaction especially with the climber and the others.

The enumerated states could live in a Supplier or Triggers possibly for the four subsystems. There are varieties of Command shortcuts that include decisions and selections so that, also, might be a place for your shooting strategy data table.

1 Like

This is a philosophy that greatly influenced how I organized things this year.

Something else you might want to consider is managing your states through public Triggers (which you can also be using to control states inside the subsystem). This way, you can keep most individual functionality in just a subsystem that returns a few specific commands and using the public triggers to create command classes or compositions. Then you do not need to unnecessarily create instances of specific commands for the subsystem.

For what its worth, we are on our second year of fully using our own state machine framework (SMF). You can check out how we do it in our 2024 comp bot (GitHub - CCShambots/Shambots5907_24CompBot: Shambots competition bot code for the 2024 season) code that uses our shared library ShamLib (GitHub - CCShambots/ShamLib)

3 Likes

This is pretty nifty; probably the best explicit state machine I’ve seen this year.

I do think you should be able to automate away most of the registerTransition boilerplate, though; it seems like redundant info.

Thank you! I agree with your registerTransition() comment. We transitioned this year from primarily relying on running commands in our “transitions” to running “StateCommands” instead, which made the transitions somewhat useless…

Potentially in the future we’d move to just assuming all transitions are allowed and only explicitly defining transitions when we need a specific action for a specific combination of transitions, and/or setting any disallowed transitions that should be presented. Does that sound like a reasonable idea?

1 Like

Yeah, that’s more or less what I was going to suggest.

I’m on the lookout for a best practice of a shooting parameters table and since I suggested triggers and suppliers here’s a quick and dirty possibility. There are pros and cons to this example. Maybe others here will suggest the ideal way. In a too quick of a look I didn’t notice a table lookup type of problem solution in @Jedi_4. Maybe they solved it.

You didn’t hint at how many enumerated states you have and that could drive the technique to use.

In this example the field is divided into three shooting zones and the Xbox controller right trigger axis simulates where you are shooting from. It needs the usual Main and Robot loop with RobotContainer instantiation. It runs in simulation, too.

Java Code
package frc.robot;

import edu.wpi.first.wpilibj2.command.Command;
import edu.wpi.first.wpilibj2.command.Commands;
import edu.wpi.first.wpilibj2.command.SubsystemBase;
import edu.wpi.first.wpilibj2.command.button.CommandXboxController;
import edu.wpi.first.wpilibj2.command.button.Trigger;
import edu.wpi.first.math.MathUtil;
import edu.wpi.first.units.Angle;
import edu.wpi.first.units.Distance;
import edu.wpi.first.units.Measure;
import edu.wpi.first.units.Velocity;

import static edu.wpi.first.units.Units.Degrees;
import static edu.wpi.first.units.Units.Feet;
import static edu.wpi.first.units.Units.FeetPerSecond;

import java.util.function.Supplier;

public class RobotContainer {

  private final CommandXboxController m_driverController =
      new CommandXboxController(0);

  /** The container for the robot. Contains subsystems, OI devices, and commands. */
  public RobotContainer() {
    // Configure the trigger bindings
    configureBindings();
  }

  private void configureBindings() {

    double fullLength = 54.0; // units conversion - joystick to field length

    new Trigger(
        ()->MathUtil.isNear(
              Feet.of(40.0).in(Feet),
              Feet.of(m_driverController.getRightTriggerAxis()*fullLength).in(Feet),
              Feet.of(15.).in(Feet))
    ).onTrue(Commands.parallel( // whileTrue doesn't work - commands likely cancel each other resulting in almost nothing
        Commands.print("\nfull court"),
        ShooterAngle.setAngle(()->Degrees.of(40.)),
        ShooterSpeed.setSpeed(()->FeetPerSecond.of(15.)),
        FeederSpeed.setSpeed(()->FeetPerSecond.of(3.)))
      );

    new Trigger(
        ()->MathUtil.isNear(
              Feet.of(20.0).in(Feet),
              Feet.of(m_driverController.getRightTriggerAxis()*fullLength).in(Feet),
              Feet.of(5.).in(Feet))
    ).onTrue(Commands.parallel( // whileTrue doesn't work - commands likely cancel each other resulting in almost nothing
        Commands.print("\nmid-line"),
        ShooterAngle.setAngle(()->Degrees.of(30.)),
        ShooterSpeed.setSpeed(()->FeetPerSecond.of(15.)),
        FeederSpeed.setSpeed(()->FeetPerSecond.of(2.)))
      );

    new Trigger(
        ()->MathUtil.isNear(
              Feet.of(7.5).in(Feet),
              Feet.of(m_driverController.getRightTriggerAxis()*fullLength).in(Feet),
              Feet.of(7.5).in(Feet))
    ).onTrue(Commands.parallel( // whileTrue doesn't work - commands likely cancel each other resulting in almost nothing
        Commands.print("\nup close"),
        ShooterAngle.setAngle(()->Degrees.of(60.)),
        ShooterSpeed.setSpeed(()->FeetPerSecond.of(5.)),
        FeederSpeed.setSpeed(()->FeetPerSecond.of(1.)))
      );
  }

  public static class ShooterAngle extends SubsystemBase {
    public static Command setAngle(Supplier<Measure<Angle>> angle) {
      return Commands.runOnce(()->System.out.println(angle.get().toLongString()));
    }
  }

  public static class ShooterSpeed extends SubsystemBase {
    public static Command setSpeed(Supplier<Measure<Velocity<Distance>>> speed) {
      return Commands.runOnce(()->System.out.println(speed.get().toLongString()));
    }
  }

  public static class FeederSpeed extends SubsystemBase {
    public static Command setSpeed(Supplier<Measure<Velocity<Distance>>> speed) {
      return Commands.runOnce(()->System.out.println(speed.get().toLongString()));
    }
  }

  public static class Climber extends SubsystemBase {}
}
/*
up close
1.0 Foot per Second
60.0 Degree
5.0 Foot per Second

mid-line
15.0 Foot per Second
2.0 Foot per Second
30.0 Degree
40.0 Degree
15.0 Foot per Second

full court
3.0 Foot per Second

mid-line
15.0 Foot per Second
2.0 Foot per Second
30.0 Degree

up close
1.0 Foot per Second
60.0 Degree
5.0 Foot per Second

up close
1.0 Foot per Second
60.0 Degree
5.0 Foot per Second

mid-line
15.0 Foot per Second
2.0 Foot per Second
30.0 Degree
40.0 Degree
15.0 Foot per Second

full court
3.0 Foot per Second

mid-line
15.0 Foot per Second
2.0 Foot per Second
30.0 Degree

up close
1.0 Foot per Second
60.0 Degree
5.0 Foot per Second
*/

We just do this with InterpolatingDoubleTreeMap:

We then pass our pivot and shooter speeds commands Suppliers such as ()->PIVOT_MAP.get(distanceToSpeaker())

2 Likes

Essentially similar to my team’s including half of it commented out and the students got a kick out of the name InterpolatingDoubleTreeMap!
I was trying to work in the Trigger as an example of the table look up. Maybe good for small tables and not so good for large tables?

Why would you have a trigger involved?

1 Like

Thanks for posting your code. I commend you on a program that can be read and fairly well understood without much time-consuming effort.

Use of the short form of commands, also, should help readability somewhat.

I didn’t notice you checking the “from state” before “transitioning” to the “new state” so maybe you already allow all transitions?

I agree that many transitions are unique to a “from” and “new” state combination so checking “from” isn’t useful. Games that allow the robot to handle multiple game pieces are likely to benefit from some robot internals limited use of the “from” - “transition” - “new” for game piece storage and shuttling control.

I think you mean specific combination of states (from state - transition - new state).

[edit: found one check for the “from state” - DetermineRingStatusCommand.]

We normally don’t explicitly check the start state (or “from state”) of a transition because of the way we store and look up transitions. When a StateMachine is requested to transition to a new state, it looks for a transition based on the current state, (i.e. if there is not a transition from the current state to the requested state, no transition will be found and thus nothing will happen).

Yeah, our DetermineRingStatusCommand makes more checks because of the many different cases of sensor trips we could have while indexing (we have three proximity sensors on our robot and can accept rings from our ground intake and from our shooter)

Yep, you’re right. I’ll make the edit :slight_smile:

We did have a lookup table. My other programmer (who graduated this year) wrote that part of the code, but it worked quite well for us. I believe we copied it from somewhere…

We define it in our Constants file - around line 236. The way ours worked was by performing the trig based on our physical robot parameters and distance to the speaker and then applying an offset from the lookup table (distance to angle offset lookup).