What's your function of a 'subsystem.java'?

Technical Programming
1

Hi guys. I’ve been working on some programming guides for our team recently. We used to have just one programmer and it wasn’t a big problem to program however he/she liked but now we have some people working together. It has gone pretty well so far but there’s something that confused me:

Should subsystem.java function just as an IO interface for controlling harware and leave all the calculations to commands & wherever else?
Or should it integrate calculations in itself and be called only to ‘change states’?

I’ve seen both practices when referring to several teams’ codes and both make sense to me. So what are the pros and cons of them respectively?

Personally I prefer the former one because it’s closer to our old way of doing the job.

2

So u mention IO interface so I assume your using Advantage Kit. If so, you will have multiple files per subsystem. Your subsystem class which extends SubsystemBase will be where u interact with the subsystem via commands. You then have ur IO interface and then the various classes that implement ur IO interface. These implementations will have ur actual hardware defined and the logic to interact with the HW to include updating ur IO inputs.

1 Like
3

I can be corrected, but it doesn’t sound like OP is referring to IO interfaces in the AdvantageKit/hardware abstraction sense. The question is about whether more responsibility should be designated to subsystems or commands. If control logic is designated to commands, the subsystem essentially becomes an interface for talking to hardware and nothing more.

Ultimately I think both approaches are perfectly reasonable, and it mostly boils down to personal preference. There’s also a bit of a middle ground where control logic is isolated to commands that are generated via factories in the subsystem, which keeps all of the logic in the same file.

5 Likes
4

Ya, that’s valid. I read IO interface and that’s where my mind went after spending all practice yesterday teaching our programmers how to use advantage kit :rofl:

2 Likes
5

The way I look at it the subsystem is responsible for all motor control logic. That includes all output calculations including pid and feed forward. Also unit conversions to what the commands will want. The subsystem presents a business interface to the commands. The business of the robot may be set elevator position, or travel at x speed whatever. The commands talk that language to the subsystem.

1 Like
6

My team certain leans toward the complex logic being in the Subsystem. Sometimes that means putting public functions in the Subsystem to do complex calcs (so that it is shared between commands). Sometimes that means putting state machines into the Subsystem periodic() method. (I know that is big no-no in some team’s view, but it works fine for us.)

7

A subsystem provides the MutEx for any resource that requires it - not just hardware.

I find the subsystem class is a handy place to store commands and triggers related to that subsystem but the commands do essentially all of the work of defining a state of the subsystem.

Regarding the subsystem periodoc() method - we use runBeforeCommands() and runAfterCommands() periodicals for data acquisition and logging and not for state calculations.

For commands that require more than one subsystem, I think the current recommendation is to minimize injecting subsystems into other subsystems and instead define command factories at a high-enough level where all the subsystems exist.

We see those higher level files, classes, methods get too large so we define an external abstract class that contains the static command factories that use more than one subsystem. Getters() are used to get the subsystems from where they were defined (used to be class RobotContainer and we are going to continue using that so class Robot isn’t cluttered).

1 Like
8

Yeah actually i was introducing AdvantageKit into our codes and thanks for ur suggestions anyway! :slight_smile:

9

Here’s a pretty long thread on this.

The short of it is, it’s complicated, and there are different types of Commands.

1 Like
10

So does that mean it goes like there’s several files for a subsystem, and IOXXX.java deals with HW communication and subsystem.java works as a command factory that provides commands like

//in subsystem.java

public Command setPositionCommand(double angleDegs) {…}
public Command intakeCommand() {…}

And there might be some higher level structure to provide commands including multiple subsystems or we just put them in the commands folder.

Correct me if I misunderstood.

1 Like
11

So basically it’s a matter of personal tastes? :confused:

12

Always has been

1 Like
13

I strongly recommend reading the thread in full. Unfortunately, it is not a light read; your original question has a fairly complex answer, and that thread has a few people debating it, illustrating many important points.

The short of it is: the two ways have pros and cons, but they are not balanced pros and cons; they are lopsided.

14

I am not familiar with AdvantageKit and the use of IOXXX.java. You can say it’s all a matter of personal taste but do adjust your tastes to the situation. Standards for style are important for organizations and you generally try hard to fit in. If there is a typical AK style, then consider using it so you and fellow AK users can more easily communicate.

A Tiny Bit Of Style Suggestions

Take advantage of features of a language that aim to mistake-proof your code. Don’t use features of a language that users are prone to mess up. A simple example is many commands made in a subsystem class have the requirements of the subsystem (this) automatically made for you. Commands made in the full-blown class technique or commands created outside of a subsystem rely on the user remembering to addRequirements or code the commands parameter for the requirement and I assure you that doesn’t always happen correctly.

Other stylistic considerations are bury the minutiae that make mechanisms work (the solution space) so you don’t have to think about that again. Make obvious the problem being solved (when and why the mechanism is being moved) - the problem space.

Other Threads

You might enjoy Bulletproof Software although again you have to wade through discussions. There are several other threads about best practices, style, etc.

My Examples

You can see what my team is teaching for organizing files, commands, subsystems in an example using a Romi. The repository is a work in progress - it’s the last lesson worked by the beginning students (accumulation of lessons up to this week and it is added to each week). [Complete project runs WPILib 2024]
GitHub - SirLanceABot/2024-Romi-Project

Someday you might want to take a look at some examples I compiled into this single repository. It’s not trivial to read the code, though. There are a half-dozen independent examples and I wanted them to be as isolated from each as possible so I succumbed to injecting the LED display subsystem into each otherwise almost standalone example. [complete project runs WPILib 2025-beta-2]
GitHub - tom131313/Examples

4 Likes
15

Hey there, greetings from Shenzhen!

There is no absolute right-or-wrong for code style, but I think most would argue that the following should be written in the subsystem code:

  • PID controllers.

  • Code to detect hardware faults and report them to dashboard/driverstation.

  • Triggers to expose boolean states to other subsystems, e.g.:

    // intake.java
public final Trigger hasNote = ...;

  • Some basic commands that can be composed into more complex commands later on, e.g.:

    // Arm.java
public Command runAngle(...) {
   return run(() -> ...);
}
// Flywheels.java
public Command runRPM(...) {
  return run(() -> ...);
}

// somewhere else
public Command aimAtSpeaker(Arm arm, Flywheels flywheels) {
  return arm.runAngle(...)
    .alongWith(flywheels.runRPM(...))
    .alongWith(drive.faceToSpeaker());
}

Sometimes, when commands are heavily composed, the default command may not be scheduled on time, causing control loops to get stuck. This is why some teams (including @jonahb55 s) like to put control loops in subsystem.periodic() and not in commands. However, there will be a one-period delay.

There has been a detailed discussion in this thread:

Hope this will help and good luck with your work!

3 Likes
16

Small note: from within subsystem scope, use the subsystem command factories instead of the ones on Commands. They declare the requirement for you.

6 Likes
17

to clarify where in the subsystem this should be written see Command-based help - #19 by Oblarg . The PID controller may be instantiated as a subsystem class field or may be instantiated within a command factory method local initiation. The use of the PID controller in both cases is completely within a Command (and not the subsystem.periodic()). The associated command factory may be defined within the subsystem and that is preferred as already mentioned twice in this thread - use the automatic subsystem requirements assignment feature instead of trying to always remember to do it yourself.

I’m not sure what you mean by a control loop gets stuck. I don’t think my team has ever experienced what I consider stuck. The referenced CD thread and sub-thread are a very hard read but if I’m guessing the situation correctly the use of Triggers for state changes of an FSM should make clean and useful code (my examples referenced above have two examples of Moore-like FSMs implementing the best-practice usage of triggers, I am hopeful).

1 Like
18

Sorry for the confusion. Let me clarify. Imagine you have a command that uses multiple subsystems, like this:

Command myCommand = arm.goToAngle(...)
        .alongWith(shooter.runFlyWheelsToRPM(...));

Due to how alongWith is defined, myCommand will only finish when both goToAngle and runFlyWheelsToRPM complete. Now, let’s assume goToAngle finishes in 1 second, while runFlyWheelsToRPM takes 3 seconds. During the remaining 2 seconds, nothing will run on arm since its default command isn’t rescheduled.

As a result, the last control voltage applied remains unchanged for those two seconds. This can lead to undesirable behavior - sometimes even damages your robot. Personally, I call this “getting stuck,” but I know there’s probably a more professional way to call it.

One could argue that there is the axProxy() for this, but

19

Right, and thanks for pointing that out! I’ve updated the example code.

20

You can even omit the super, and just call run!

1 Like