Reducing code complexity
 

One of the problem with our teams code this year was that teleop had a huge amount of if statements for each element of the control system (i.e. if this button is pressed, perform this function). This seems like bad practice and the the code isn't as maintainable as it could be.

Link to the code is here. Have any other teams struggled with this? How have you gotten around it?

Thanks,
Daniel, Team 1836

Re: Reducing code complexity
 

If you just want to remove the if statements, here's a few things you can do:
can become:
can become:
Also, this:
can become:

Re: Reducing code complexity
 

I just noticed that I turned one of the snippets from Java into C++. Oh well.

Re: Reducing code complexity
 

SoftwareBug2.0 makes some good suggestions, but I think they treat the symptoms rather than the root cause.

In many FRC games, the primary challenge when programming the robot is code organization. (Occasionally an especially challenging problem like vision tracking pops up, but even then there are enough tutorials around that the algorithm itself is not the obstacle.) I suggest using the command-subsystem style from the beginning of a project. It seems like overkill at the beginning when code is simple. However, the separation between the robot subsystems, higher-level commands, and operator interface it provides makes situations like those you complain about occur less frequently, if at all. It also makes the program far easier to change in the time pressure of competition.

Re: Reducing code complexity
 

I really don't recommend using the ? or ^ operators just to make your code smaller. It will just make the code harder to read and understand which is far more important than how big it is. The switch statement is a good idea though.

Instead, how about moving all of the code related to the shooter into a function and all of the code related to the drivetrain into another one, etc.

What we do is take the idea a little farther and use a separate class for each subsystem and that class even contains the speed controller objects, the sensors, etc. Within our classes though, there are plain old 'if' statements.

Re: Reducing code complexity
 

Implementing the command-subsystem paradigm using the base classes included in WPIlibJ also allows you to use event-driven programming using the Button classes, which can replace most - if not all - of your if statements with a couple lines at program initialization in the form:

Re: Reducing code complexity
 

I agree with Ziv - move to a cleaner extensible framework such as the command-based framework. It's not perfect and there is a bit of a learning curve but it does help tremendously with code modularity.
The biggest problem with the simple-loop organization is that there is no isolation between different robot functions - a small change in one place is fairly likely to affect other operations.
Moving to a modular approach means code chunks are more isolated and that is a lifesaver when you are fixing code 3 minutes before you match starts with no time to test!

Re: Reducing code complexity
 

On the LV side of things, we use a few tricks to keep code organized which should port fairly well to any language:

-We STRICTLY separate the operating code from the HMI/Auton code. This isn't quite like the command/action garbage of the command framework, we JUST abstract the actual button inputs, debouncing/rising-edge triggering, and scaling before passing the data into the code. For example, our drivetrain code has inputs of desired shift state (enumerated), and doubles for left/right drivetrain powers. The slab subsystem has a desired state (enumerated), which it controls to.

-HMI code just takes buttons from the button data and switches or scales it to the units used in the primary code, and sends it to the primary code. Auton operates slightly differently, since Auton is procedural by the nature of Beescript, but the end result is data passed to the primary code blocks in the same task.

-We organized all of the code into a single RT thread. While this initially seems like a bad idea, we could organize data transfer far more efficiently and deterministically by passing data through wires and data structures, and guaranteeing order of operations through the code. We read all of the data from the FPGA, scale it into bus_inputs, and make that bus available to all subsystems and HMI/Auton. Likewise, the subsystems all have access to bus_outputs which they write the outputs they are responsible for, and at the end of the iteration the data is all scaled to raw units and written to the FPGA. bus_state is used to store the publicly-accessible state information for each system, and is the primary means of data-passing between subsystems. Since we are single-threaded, we don't have to worry about missing events from other threads due to timing jitter, temptations to write bad code using blocking calls, or the order of operations in actions between multiple threads. We also don't have to deal with data access and data issues in read-modify-write operations because the code is single-threaded.

-All three busses are stored in shift registers and data in them is carried over between iterations. This allows a VI which executes last (e.g. the roller system) to send data to a system which executes before it, but 10ms later. This is commonly used to reset the input diagnostics which are checked first, and also allows items to be calculated at a lower frequency than the main loop frequency (e.g. new DS data is received at a lower frequency than the control loop, but it uses latest data every loop and iterates the loop anyway).

-All systems operate using either math algorithms or state-machines. Several use both. State-machines are a highly recommended way to reduce clutter by grouping desires to change state and actions to perform when changing states into a single area of the code, and resulting in a definitive state for this iteration. If the state is enumerated, you can use it to do table lookups for positions, motor command values, etc. and keep calibrations in simple data tables, which is more organized then writing them directly in code.

Re: Reducing code complexity
 

I agree with the bee. I recommend reprogramming your bot, OP, using Finite State Machines during the offseason. The process very much reduces complexity, and leaves no memory leaks. Diagramming each subsystem of your bot will give you exactly what needs to go into the code.

Re: Reducing code complexity
 

The code you provided actually seems pretty clean to me; the logic is clear, and I can't think of any direct improvements that can be made. I will however echo others' suggestion to use the Command-based setup, as it makes button-mapping code much easier to maintain. Internally, the logic is the same, but it's encapsulated in nice things like button.whenPressed(). As an example, here's our OI class, which handles input from the drivers. A snippet from it shows that it can simplify things greatly:

Re: Reducing code complexity
 

My earlier suggestions were indeed fixing only superficial problems, and you probably want to do more than just what I pointed out. I'll get to some suggestions about that in a second.

But first let me say that I disagree about what the primary problem is in organizing robot code. It is understanding the underlying system and its limitations. You have to discover the flaws in the underlying system, and know how to work around them. For example, there was a time years ago, when you couldn't call atan2() because it would take so long that you'd cause the watchdog timer to go off. So then we had to implement lookup tables. Similarly, my team has hit several WPIlib bugs this year. And we discovered that the amount of bandwidth that was supposed to be available to each robot really wasn't.

Now on to some higher-level code suggestions: apalrd is on the right track. You don't need to use seperate classes and functions for everything, but you there are several different layers that make sense to use.

Here are three possible layers:
1) Reading buttons
2) Generating goals for each of the subsystems
3) Running each of the subsystems

(In autonomous mode, layers 1-2 would be replaced but 3 would be unchanged)

This year, my team actually used the command based stuff, and I can't recommend it with a straight face. You can kind of get towards a problem some call "callback hell".

I understand there are some differences in opinion once you get toward style stuff, but here's an example of how I like to see code. This controls a 30-pt climber, which sadly had some mechanical issues and didn't make it onto our robot.
A little bit about the mechanism: There are two sets of slides, each of which is controlled by a motor, and each of which has a limit switch at the top and bottom.

Going back to concrete, lower level suggestions, about the quickest change to improve the code would be to just start your function with a bunch of lines of the form:
There are a few places where the same code reading the inputs is repeated. Whenever that happens, alarm bells should be going off in your head.

Re: Reducing code complexity
 

I know a bit about finite state machines because I've been learning Erlang, but how would you recommend implementing one in Java?

Re: Reducing code complexity
 

Consider an arm that's going to throw something. When it first starts up, it needs to initialize, and then it waits. When it throws, it needs to launch a process, and then waits for a signal that throwing is complete, and then retracts. It then waits for the next throwing command.

This is typed in code that's mostly Java, but not compiled or tested.

An enum that represents your states.

then in your logic

Re: Reducing code complexity
 

In LV and C I usually use a typedef enum to store the state, and a switch for the states. LV it cleaner in this with a case structure.

The state in C is usually stored as a global variable, with an accessor function to adhere to double-buffering (storing the previous state when the set state is different from the current state).

In LV we store the state in bus_state which is a data structure which is essentially global to us. Alternatively you could store it in a shift register locally or as a global variable.

Since a typedef enum can be used as a uint, you can use it to index an array. We use this frequently to determine the state of outputs or setpoints based on state without additional code to explicitly set the output.

Re: Reducing code complexity
 

Create separate classes for all your systems. Even if you're only going to be creating one object, it's still worth it.

Since we end up using Tank Drive every year, I made a TankDrive class last year. We just create an object with the appropriate parameters, call drive.drive(); in our main teleop loop, and forget about it. It's that simple.

Re: Reducing code complexity
 

As a team that uses Java (based on ME) I'm deeply envious of your casual use of the enum word.
Like many things in life I didn't really appreciate them until they were denied to me.

Re: Reducing code complexity
 

It's possible to emulate enums using regular Java classes (though the code becomes a bit more verbose and not quite as readable). In addition to the method outlined in the linked article, you can also use static final int variables, such as are used in WPIlibJ, though you lose type safety that way.

Re: Reducing code complexity
 

I'm not sure how I would write code without the enum. It's possibly one of the most useful programming constructs I have ever used.

Lookup/Interpolation tables are also my favorites. I'm surprised I don't see them more in FRC.

As a note to LV users in this thread, you can make a typedef enum in LV by making a custom control (go to New instead of New VI and it's an option), set it to 'strict type def' and drop an enum. Anywhere you use it, it will auto-update to the type def, so you can change the control (.ctl file) and all of the constants and controls will update. When you create it, the order will define the associated number (it's to the right of the name), which is what you get if you use the enum as a uint. A similar method works for typedef struct, you just need a Cluster full of stuff.

Re: Reducing code complexity
 

I'm aware of the class approach but I don't see that the added complexity is warranted.
We do use static final int and a naming convention to simulate enum, but of course you lose the benefit of syntactic typing constraints.

My point was to subtly indicate that some of the earlier suggestions don't translate directly to the current platform due to Java ME limitations.

Re: Reducing code complexity
 

My Java skills are a bit crusty but this should give you an idea.

Re: Reducing code complexity
 

I'm curious what sort of horrible difficulties you ran into with the command-based template. I used it this year and it greatly simplified organizing and modularizing code compared to working with the SimpleTemplate. The biggest headaches I ran into was forgetting to statically initialize subsystems to NULL, making it repeat fire discs when the trigger was held, and remembering that commands attached to events are actual object instances that are created once at program initialization. The latter was probably the biggest headache since it meant reworking the shoot command so it would check the joystick inputs and look up the selected setpoint itself instead of getting instantiated with it. The pluses were pretty significant, since it greatly simplified event sequencing for autonomous and firing sequences. I think the biggest limitation is that you can't easily implement a true general purpose Finite State Machine using Commands and CommandGroups since CommandGroups are just static lists of Commands to run one after the other(ish). No transitioning from one state to a variety of others based off inputs and such.

So, what's your beef with command based stuff?

Re: Reducing code complexity
 

There's nothing saying you can't make your own Command to do it, though. CommandGroups are just Commands implementing a special case of a state machine. Also, an interesting thing to note is that if you use requires(), each Subsystem acts as a state machine with Commands as the states (and it's really easy to add a state for these, as it just involves adding requires() to a Command's constructor).

Re: Reducing code complexity
 

Yeeeees... I can see how you could make that work, though I think you'd end up with either one huge command or your state machine logic spread over a half dozen files. I'll have to think on it a little more now that there's not a robot to finish already, dangit.

Re: Reducing code complexity
 

Check out the the CommandGroup source code for some inspiration... Though personally, I'm a fan of the "Subsystem-as-State-Machine" model.

Re: Reducing code complexity
 

I highly recommend separating your code into classes per subsystem. In terms of condensing your code you will see the most benefit if you do that - it will also make your code easier to read while some of the other methods will save very little space and make your code more difficult to read.

Re: Reducing code complexity
 

Just a hint: take full use of classes and inheritances.

Re: Reducing code complexity
 

My major beef with the command based stuff is that it makes it harder to understand the system as a whole. If you have commands that don't complete immediately, it can be hard to know exactly what is happening. You end up at a place with neither a simple set of variables that tells you everything that's going on nor a straightforward callgraph to follow.

Here's a more minor problem: Our autonomous mode became

rather than:
Because it was so much easier to just make it a list of commands. So we took longer to shoot things than we needed to. People have mentioned how to work around this in this thread, but it was enough that it didn't happen.

I get that it's also easier to do the steps linearly in non-command based code, but it's not much different. You'd go from:

to

Re: Reducing code complexity
 

This will fulfill the second sequence:

Re: Reducing code complexity
 

This thread reminds me why I dislike Java and the command architecture for embedded systems.


I've always designed the code so we can pass desired setpoints in a single auton function then terminate. That way, we send the Set command in the script, and the subsystem listens and controls to that commanded value indefinitely (or until disable->enable transition).

This way, the subsystem holds all of the code required to fully run a mechanism, and the rest of the code just is just a button map or auton command. I can test the subsystem individually by commanding setpoint directly, and verify results using all of the data present in the subsystem.

In our C code for Vex, each subsystem gets a single C and two H files (since RobotC dosen't exactly work like normal C, the data is in a H with externs in the other H, if we used 'real' C the data would be in a C, code in a C, and externs/prototypes/typedefs in a H). Main (which runs the main task and HMI) gets another C, and Auton has a few files (autosel, routines, psc, nav). All of the code fits easily in a single directory, and totals ~1400 lines (lots of comments and diagnostics).

Re: Reducing code complexity
 

We used 3 different methods with the command system.
1)Send a setpoint and exit immediately while holding the setpoint.
2)Send a setpoint, wait for it to get there, then exit and keep holding.
3)Send a setpoint, wait for it to get there, then exit and stop holding

Not sure why you think there's only one way to write things with the command system.

Re: Reducing code complexity
 

Here is the enum pattern that I like using with Java ME:

The single biggest challenge when writing code is development time and testing. Everything I do, I do with the thought that "I might need to tweak this in between matches, so I better make it easy to understand and find what I need". If I bonk my head and forget the code entirely, or if I get hit by a bus and the team finds a new programming mentor, I want to be sure that it is fairly obvious what I am doing.

To that end, here is my general technique for writing code (C++ or Java) for FRC:

* I love using scripting - reading properties from a file, reading autonomous modes from a file, etc. You should never need to recompile your code to change a PID gain! Just be sure to put your properties files into source control along with your code.

* You MUST use source control. CVS, SVN, git...don't really care which, but USE IT!

* I only use 2 threads in my user code (well, 3...I let the compressor have its own thread too). One thread does the typical periodic loop stuff. The other thread runs at a high speed (100Hz this year) and does only real-time control stuff (filtering and control loops). For this reason, I dislike using the built-in Command templates and PID controller. More threads = more potential for problems (race conditions, deadlock, synchronization, etc.)

* I love whitespace and useful comments. I hate binary operators and "if ? then : else" syntax - it is too easy to misread. I always put "{ ... }" on my if/else logic...even if it is only one line's worth of code.

* I dislike putting any constants (other than obvious mathematical tautologies...like a circumference being equal to pi*diameter) inline in code. Instead, I put all of my constants (even things like PWM ports) in a single Constants.h/Constants.java file. If I think I will ever need to change the value on the fly, I will put it in a properties file instead.

* My preferred architecture uses three primary interfaces: Subsystems, Loops, and Autonomous States. Subsystems are things like "drive", "arm", "shooter". Loops are things like "arm position controller", "drive straight controller", "auto aim shooter speed controller". Autonomous States are things like "drive for a specified distance", which in many cases involves activating a control loop for a given subsystem (but having these loops separate from the autonomous logic lets me invoke the same behavior during teleop...e.g. arm moving to setpoints, or auto aiming)

* Each subsystem gets a class/file. Each autonomous mode "state" gets its own class/file. Each control loop gets its own class/file.

* At most one autonomous state is active at any time. If I want to do two things at once, then I make a new state that does both. It's just less error prone this way.

* At most one control loop is active for a given subsystem at any time (e.g. I have a drive straight controller, a turn in place controller, a vision-aided aiming controller for the drive...only one at a time!).

* All variables and methods get descriptive names. All member variables start with "m". All function arguments start with "a". All constants start with "k". All variables that represent a real world quantity get units appended to their name, e.g. "mDesiredDistanceMeters".

* For utilities that don't interact directly with WPIlib, I prefer to write code that is valid for both J2SE and J2ME. For example, our trajectory generation library can be copied and pasted into a J2SE project so I can test it.

* I put ALL driver input processing into a single method that is called by teleopPeriodic. But the logic in this method is fairly simple (lots of if/else stuff, but it calls out to methods for specific subsystems to actually do anything).

* I only have one method per motor to actually set the PWM output. This method is then called by other methods in a subsystem. This way, if I need to reverse a direction I only have to worry about a single negative sign.

Re: Reducing code complexity
 

Joe's example gives a good demonstration of how the command-based framework lets you abstract away the details and see the high level function of the robot.

It's not the only way to do it -- you can certainly write your own framework -- but it's good enough that you can instead focus on robot functionality. You also get the benefit of others knowing how it works and helping out if needed.

Re: Reducing code complexity
 

Interesting. That seems simpler then doing this: http://wpilib.screenstepslive.com/s/...g-two-commands

Re: Reducing code complexity
 

There are advantages of doing that over what I suggested. WaitForChildren waits for the termination of all parallel commands. If you add it to something that includes parallel commands intended to run over a longer period of time, you can have issues. And really, their complexity is pretty similar:

Re: Reducing code complexity
 

What would you do if you wanted to have a variable update when steps finished, so that you could display it or something?

I guess you'd wrapperize all of the command classes?

Is there some easier way that I'm missing?