|
|
|
![]() |
|
|||||||
|
||||||||
![]() |
|
|
Thread Tools | Rate Thread | Display Modes |
|
|
|
#1
|
|||
|
|||
|
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 |
|
#2
|
||||
|
||||
|
Re: Reducing code complexity
If you just want to remove the if statements, here's a few things you can do:
Code:
if (xbox.isReleased(JStick.XBOX_A)) {
usingCheesy = !usingCheesy;
}
Code:
usingCheesy=usingCheesy^xbox.isReleased(JStick.XBOX_A); Code:
if(slowMode){
driveGear.set(false);
}else{
driveGear.set(normalGear);
}
Code:
driveGear.set(slowMode?false:normalGear); Code:
if (shooterMode == SHOOTER_MODE_VOLTAGE) {
lcd.println(DriverStationLCD.Line.kUser1,1,"Shooter mode:voltage ");
} else if (shooterMode == SHOOTER_BANG_BANG) {
lcd.println(DriverStationLCD.Line.kUser1,1,"Shooter mode:bangbang");
} else if (shooterMode == SHOOTER_COMBINED) {
lcd.println(DriverStationLCD.Line.kUser1,1,"Shooter mode:combined");
} else {
lcd.println(DriverStationLCD.Line.kUser1,1,"Shooter mode:????????");
}
Code:
{
const char *out;
switch(shooterMode){
case SHOOTER_MODE_VOLTAGE: out="Shooter mode:voltage "; break;
case SHOOTER_BANG_BANG: out="Shooter mode:bangbang"; break;
case SHOOTER_COMBINED: out="Shooter mode:combined"; break;
default: out="Shooter mode:????????";
}
lcd.println(DriverStationLCD.Line.kUser1,1,out);
}
|
|
#3
|
||||
|
||||
|
Re: Reducing code complexity
I just noticed that I turned one of the snippets from Java into C++. Oh well.
|
|
#4
|
|||
|
|||
|
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. |
|
#5
|
||||
|
||||
|
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. |
|
#6
|
||||
|
||||
|
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:
Quote:
|
|
#7
|
||||
|
||||
|
Re: Reducing code complexity
Quote:
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! |
|
#8
|
|||||
|
|||||
|
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. |
|
#9
|
||||
|
||||
|
Re: Reducing code complexity
Quote:
|
|
#10
|
|||
|
|||
|
Re: Reducing code complexity
Quote:
|
|
#11
|
|||
|
|||
|
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. Code:
public enum State {Initializing, Waiting, StartThrowing, Throwing, StartRetracting, Retracting}
Code:
public State state;
...
// This switch is run repeatedly, either in a while loop if you're in an iterative robot or as part of a command in the command based robot.
switch (state)
{
case Initializing:
// Do whatever one time initialization logic here
state = Waiting;
break;
case Waiting:
if (arm_is_triggered) { // however you're triggering the arm
state = StartThrowing;
}
break;
case StartThrowing:
// Do whatever one-time logic needed to start throwing -- e.g., open solenoid
state = Throwing;
break;
case Throwing:
if (done_throwing) { // however you're detecting throwing is done -- e.g., limit switch, timer
// do whatever's needed to stop throwing
state = StartRetracting;
}
break;
case StartRetracting:
// Do whatever is needed to start retracting
state = Retracting;
break;
case Retracting:
if (done_retracting) {
// do whatever's needed to stop retracting
state = Waiting;
}
break;
default:
printf("Unknown state!\n");
break;
}
|
|
#12
|
|||||
|
|||||
|
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. |
|
#13
|
||||
|
||||
|
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. |
|
#14
|
||||
|
||||
|
Re: Reducing code complexity
Quote:
Like many things in life I didn't really appreciate them until they were denied to me. |
|
#15
|
||||
|
||||
|
Re: Reducing code complexity
Quote:
|
![]() |
| Thread Tools | |
| Display Modes | Rate This Thread |
|
|