Never, Never, Never, Ever Use CommandRobot

Technical Programming
#1

Hope I got your attention…

I am talking about the WPILib VSCode template CommandRobot (and the old Eclipse template)…it should be renamed to UntestableCommandRobot.

Watch my rant “The UntestableCommandRobot”.

Stay tuned for a video series that describes a better way. Subscribe to our channel to get more as we publish.

Spartan Robotics - Team 997

I will also note that as I comb recent commits to WPILib itself, good things are appearing on the testing front for this year…very excited.

Chuck Benedict
Mentor
Spartan Robotics
Team 997

#2

As mentor for a team that makes heavy use of the command-based framework (and one who also does not particularly care for the “standard” implementation thereof), I guess I’ll respond.

For starters, I agree that the template is not very good. It doesn’t do a good job of outlining the underlying design of the framework, nor does it give any indication that there is a fair amount of flexibility in its use. The documentation provided by ScreenSteps to match the template is inadequate, and suffers from many of the same problems. However, I think many of your specific criticisms are off-base, or at the very least not adequately articulated.

The lack of “test” mode being set up for you is really, really minor. Our team has never used the supported test mode for testing anyway, because it’s rather clunky and we’ve always found it easier to just modify our ordinary teleop or autonomous code. This isn’t a real strike against the testability of command-based, and it certainly doesn’t indicate that “the author didn’t even consider testing to be an important element.”

That subsystems are declared as global variables is a valid gripe in certain contexts, but not really for the reasons you state. If you don’t intend to re-use any of your robot code, you’re not really going to get burned by having global (or singleton, if you want to be a *bit *fancier) subsystems.

Where the subsystems-as-global-variables design really hamstrings you is in code re-use; commands written in the “standard” command-based style are rigidly-coupled to the subsystems they use, and thus can rarely be used again on future robots. The best way to address this is to inject your subsystems into your commands, and write your commands so that your subsystems are hidden behind interfaces - but for a team that’s just starting out (which is the situation in which one is most-likely to use the given templates), this is a fair bit of overhead for seemingly-nebulous gains.

What you do in your video, however, is worse than both - you instantiate your subsystems as private nonstatic fields of Robot.java, but then fail to mention the (substantial!) ways the rest of the code has to change to accommodate this. The changes you make cannot simply be made in isolation. You cannot simply justify them by mentioning, in an abstract sense, how globally-scoped fields are undesirable in an OO language. You have to explain how, in this specific case, globally-scoped subsystems are a bad idea, and then explain how to use the framework if you change this. A team that naively tries to encapsulate their subsystems within robot.java without a solid understanding of what they’re doing is going to find it very difficult to write working command-based code.

It is true that globally-scoped subsystems make it a bit more annoying to unit-test your code. As far as unit testing code in an FRC context is concerned, this is the least of your problems - doing such is already a massive pain due to the tremendous amount of overhead required to write stubs for all of the dependencies. Properly-encapsulating your subsystems will not help this. I really do not recommend unit testing in FRC, for mostly this reason - it will not, on the whole, save you any time and effort, which is the primary purpose of adopting a tool.

The way OI is handled is indeed kind of bad, and does result in some teams every year snarling up the ordering of their static init. However, your complaint about students un-commenting the commented-out example code seems a bit spurious - if you actually read the comments in question, they’re clearly there to document the functionality of the Button class, and not as code that is intended to be un-commented and used in-place. Screensteps very clearly indicates that said code should be placed explicitly in OI’s constructor.

There are a lot of ways that template could be made better - in all honesty, I’d advocate removing the template entirely and instead increasing the number of example robots provided on the WPILib github, to demonstrate different ways to use the framework (global subsystems, singleton subsystems, private/injected subsystems, etc). But it’s extremely alarmist to claim that no one should ever use the CommandRobot template in its current state, especially when such a claim is mainly predicated on its apparent incompatibility with unit testing, of all things.

1 Like
#3

Oblarg already addressed many of the problems with your video as well as the command-based framework. That said, I think that the command-based framework is still an extremely useful framework that many teams benefit from? Could it use refactoring? Sure. Do the docs suck? Yes, yes, and I don’t think I’m the only who thinks that. But wholesale scrapping the framework? That sounds like alarmist rhetoric that will do most teams more harm than good, and the top tier teams that might benefit from your video already do modify subsystems and commands to their liking. In the end, any valid points you’ve made are negated by the alarmist command to never use a very usable framework that significantly raises the software floor in FRC.

edit: finished sentence

#4

I agree that I do not go into the reasons for some of the changes I recommended, and I will be doing so in subsequent videos. And I am not advocating for the complete scrapping of the command robot architecture…far from it. The template demonstrates poor practices, that then live on as students perpetuate those practices. But I have to COMPLETELY disagree with the notion that automated, repeatable unit testing is not valuable. And the template’s perpetuation of those poor practices make unit testing all but impossible.

#5

I never said not to use the framework. I am talking about the CommandRobot template. Perpetuating the coding practices hinted at within the template makes unit testing impossible.

#6

Ah I see. I conflated the template with the framework.

#7

Unit-testing in an FRC context means writing stubs for basically all of the WPILib hardware APIs. Do you really think most teams would benefit from doing this? Unit testing is an invaluable tool within a fairly narrow problem-space. Most FRC teams are not operating in that problem-space.

Global subsystems don’t make unit testing impossible, either; there’s no reason you can’t replicate the necessary global state in your test environment.

#8

If you would indulge me, I have a student wanting to unit test some timing logic he put in a command. Of course, no roboRio readily available for him to do the testing. Unit testing that I helped him enable got him to exercising his code, without waiting. The video is here: https://www.youtube.com/watch?v=ulPdoDsleJg

#9

If you are injecting subsystems into commands, you can indeed mock up subsystems instead of the underlying WPILib objects to test the commands.

That’s still a comparatively large amount of overhead, and only works on pieces of code that are separated from the underlying hardware by a layer of abstraction that you have written (you won’t be able to unit-test the subsystems in this way, unless you shunt the WPILib classes down another level, which doesn’t really solve the problem).

To more-thoroughly unit-test a command, you’d also need to mock up various quite-chewy pieces of WPILib code like the scheduler. That’s really not something I think is a good use of time for, well, anyone.

I seriously doubt this will actually save time and effort for most teams, compared to simpler testing methodologies (such as simply inserting print statements and running the thing on a spare RIO). Unit testing is more or less the last thing I’d invoke as a reason to avoid global state in an FRC program, especially when said global state does actually represent large robot components (with properly-encapsulated internal complexity) for which global accessibility makes some conceptual sense.

#10

Like I said, this would not be without controversy, but I stand by my assertion that finding ways for students to get quick feedback on their code is critical (spare roboRio’s for 6 devs not really practical), and so I am dedicated to meeting that challenge with helpful techniques. And healthy debate is great…

#11

This is an unfortunate way to start a post. Unit testing is important in code development, but it is not required to make a functional FRC robot. Writing a post title like this could lead people to think you’re an expert, and actually never use a useful framework for teaching high school robotics.

Be careful with clickbait titles. We’re here to help people learn, not to drive video views.

#12

adding flame to this thread

Then use RobotPy. We’ve had easy unit/integration testing and simulation on your laptop since at least 2014. :yikes:

From those many years of experience, I’ve seen that most people just end up using the simulation to verify their code instead of writing unit tests because stuff just changes too much, and nobody likes writing tests.

#13

Or you can try SnobotSim. It handles mocking out the HAL stubs for you. Comes with a GUI to set up your connections and simulations, boilerplate motor sims, and the ability to run the robot in teleop mode, with joystick support.

I started to make examples for how you can use it this year, but haven’t added tests to it yet. Last year I had my kids work on making test suites, although in practice they are more like blackbox or integration tests rather than unit tests. Still a huge benefit for my kids, even if most of the time they tested their logic “live” in the teleop sim. I know I’ve simulated CommandBased robot’s before, but I don’t think I’ve tried unit testing them. All those singletons would probably still cause some headaches

#14

Chuck

Thanks for the comments and video. You have a lot of good ideas there. A few things to mention, this is a beta test which is, by definition, not finished. We’ve been working on improving the sample programs in vscode and the robot builder generated code. So there is still time to fix things.

Something I’d ask is that you create issues on GitHub so we can potentially address those concerns in the final release. That would benefit all teams and not just the ones who run across this post and videos. Even better, we accept pull requests from community members who want to make the final version better. Many mentors and students have contributed to the project in that way. You can find the vscode project here: https://github.com/wpilibsuite/vscode-wpilib.

With that said, in the final version, a lot of the “global variable” cases will be gone, including the RobotMap.java file in favor of creating all the sensors and actuators in the classes. Other public static variables will also go away.

It’s true that we haven’t made the sample code completely unit-testable so you need some hardware but there are a number of integration testing features built in. Subsystems automatically appear on the dashboards in test mode so you can operate the actuators, read sensor values and tune PID control systems. By writing commands to the dashboard interface, they automatically get tied to buttons that will run the command by itself allowing easy integration testing. And a number of other features like that.

As I said, you have a lot of good ideas that would ideally be incorporated into the project and so I’d encourage you to help out with that. We’re always looking to making the tools better reflect styles and methods that would make the students better programmers.

Thanks again for your input and hope to see some of those ideas as contributions or issues. BTW if you do decide to do some pull requests please let us know through an issue before you start so we don’t step on each other’s changes.

Thanks again
Brad Miller

#15

Alright, I said I would follow up with a response that addresses my rant. Here is the video The TestableCommandRobot. Here is link to code https://github.com/Team997Coders/TestableRobot. Feedback welcome and happy testing!

#16

Thanks Brad. I just added a post that includes an extensive example of the kind of things I think would be helpful. Please use what you think might be helpful. My whole objective was to elevate the subject of testing, given the state of the art. I guess I did that, for better or worse. I have not been able to formulate my example into a VSCode plugin…I guess I would like some feedback from others interested in this subject, after review of the work already done, on what should go into such a plugin. Or I supposed I could jam my example, as is, into a forked version and then your team could hack up as you see beneficial.

WPI does a tremendous job building the resources provided to help students produce incredible machines. I wish I was only so lucky back in high school. I am seeing many great things on the testing front in recent WPILib commits. Thanks for all that you do.

#17

I think RobotPy is great work. I have used elements from it in other projects. But Java and C++ are what is officially supported. Java is what our lead mentor has chosen.

Second, I disagree that nobody likes writing tests. All too often I see programming sub-team members sitting on their hands, waiting to access to the robot. Why? Mechanical is busy building it. And putting yourself in the seat of a new student, they have little confidence about what they are doing. Testing gives them an outlet to get instant feedback. It builds confidence. After I went through the gyrations on my videos with a student, he was very excited to be able to exercise his code. So just because those of us with experience hate to write tests, that does not mean the practice should not be promoted and it can be done in a way that builds student excitement.

#18

Correct, it is not required to build a functional robot. We have not done it in the past, and I doubt many do. That does not make it right. And my point is that even if you want to some testing, the starting point steers you in a direction that makes it all but impossible. I think as mentors, we owe it to our students to show them good SDLC practices. My provocative title was meant to draw attention to what I view is a big opportunity for improvement on many fronts.

#19

Here is my take on the whole topic:

I do not think writing unit tests to test the functionality of commands/subsystems, etc. is really worth it. The effort that goes into writing stubs / simulating the HAL is something that our team personally doesn’t find worth the time because we usually use another robot from a previous year to test these kinds of things.

I do think that writing tests to test other logical components of your code is helpful. For example, when we implemented a path following algorithm over the offseason (when we met less frequently), we wrote some tests with a visualizer to make sure that the algorithm was doing what we intended it to do. This was very helpful because we didn’t have to waste time debugging the faults in our implementation; instead, we spent more time tuning our gains on the robot itself.

This is just my personal opinion. If unit testing everything is really advantageous for your team and helps with increasing the quality of software, go for it!

#20

Yes, you do need some kind of scheduler to effectively unit test a command. Not that hard to do, and I created one for that purpose in my example (see later post in this thread).