frc-4931 / 2016-Robot

Code for the 2016 season for FRC Team 4931
MIT License
0 stars 12 forks source link

Drive subsystem #2

Closed agausmann closed 8 years ago

agausmann commented 8 years ago

This PR adds a DriveSystem class to the robot. It is given an arcade-style drive interface and has an Orientation that specifies which side of the robot is considered to be "forward." I also added a set of inputs to the Robot class that are used to control the robot's motion and orientation.

Execution of this code is likely to throw a NullPointerException due to the lack of field assignment in Robot; however, we cannot assign meaningful values until we know what kinds of motor controllers are used and what ports are assigned to I/O.

On Sunday, we had a discussion about different ways to implement subsystems. Zach favored implementations where the components of a subsystem are accessed directly by the user. However, I prefer implementations that abstract the control of subcomponents by providing methods for the actions you would want the user to take. For example, I provide methods like setSpeed() and stop() instead of letting the user individually control the left and right motors. This can provide the user with a descriptive interface and has the potential to reduce redundancy in code, especially for complex calculations like arcade drive. Speaking of the complexity of code, we both agreed that any continuous/repetitive/looping/whatever you call it logic (like waiting for a switch to trigger) should be implemented as commands instead of methods. Thus, there is no such method void driveForwardFor(double seconds) or void driveForwardUntil(BooleanSupplier trigger) as such would require continuous checking of a condition.

agausmann commented 8 years ago

I made the proposed changes to the code in DriveSystem, and I removed the unassigned fields in Robot. The code should execute without error now.

rhauch commented 8 years ago

Looks great. I did add a few comments above. Also, how about a unit test? It could act as a regression test to ensure the turn speed is not negated when the direction is flipped.

rhauch commented 8 years ago

@AGausmann: Can you please squash the commits into one?

agausmann commented 8 years ago

Squashed.

rhauch commented 8 years ago

@AGausmann: Fantastic!