Robot 1 - For review only #3
Conversation
I'm merging everything for robot 1
There was a problem hiding this comment.
One comment overall, let's consider taking advantage of the new Java Units Library as much as we can, to avoid unit errors.
| public static final int DEVICE_ID_SHOOTER_RIGHT = 51; | ||
| public static final int DEVICE_ID_SHOOTER_LEFT = 50; | ||
| public static final int DEVICE_ID_ACTUATOR_MOTOR = 52; | ||
| public static final int SHOOTER_VELOCITY_OFFSET = 0; |
There was a problem hiding this comment.
The naming doesn't seem right on this. I think it's a tolerance, not an offset.
| public static final int DEVICE_ID_SHOOTER_LEFT = 50; | ||
| public static final int DEVICE_ID_ACTUATOR_MOTOR = 52; | ||
| public static final int SHOOTER_VELOCITY_OFFSET = 0; | ||
| public static final int WRIST_POSITION_OFFSET = 0; |
There was a problem hiding this comment.
Two problems here:
- This constant is used as a tolerance in the subsystem, not an offset.
- There needs to be an offset configured in the subsystem, but it is not.
| public static final int DEVICE_ID_ACTUATOR_MOTOR = 52; | ||
| public static final int SHOOTER_VELOCITY_OFFSET = 0; | ||
| public static final int WRIST_POSITION_OFFSET = 0; | ||
| public static final int SHOOTER_MOTOR_RATIO = 1; |
| } | ||
|
|
||
| public static class ClimbConstants { | ||
| public static DeadbandFilter CLIMB_DEADBAND_FILTER = new DeadbandFilter(-.25, .5); |
| public static class ClimbConstants { | ||
| public static DeadbandFilter CLIMB_DEADBAND_FILTER = new DeadbandFilter(-.25, .5); | ||
|
|
||
| public static final int DEVICE_ID_FIRST_STAGE_CLIMB = 12; |
| * | ||
| * @return wrist velocity in radians per second | ||
| */ | ||
| public double getWristVelocity() { |
There was a problem hiding this comment.
I guess this is fine, but why would we use it?
| } | ||
|
|
||
| public boolean checkShooterSpeed(double shooterSpeedGoal) { | ||
| return (Math.abs(getWristVelocity() - shooterSpeedGoal) <= SHOOTER_VELOCITY_OFFSET); |
There was a problem hiding this comment.
This logic is not correct. It's checking the velocity of the wrist, not the velocity of the shooter.
| return Units.rotationsToDegrees(actuatorEncoder.getVelocity()); | ||
| } | ||
|
|
||
| public boolean checkWristPosition(double radiansToRotate) { |
There was a problem hiding this comment.
I think it would be more useful to have a method that returns true if the wrist is at the previously requested velocity. Having it this way will be hard to deal with in a command. The same applies to the checkShooterSpeed method.
|
|
||
| public void actuatorRotate(double radians) { | ||
| var target = MathUtil.clamp(radians, LIMIT_BOTTOM_RADIANS, LIMIT_TOP_RADIANS); | ||
| new TrapezoidProfile.State(target, 0.0); |
There was a problem hiding this comment.
This method doesn't actually do anything
| * them for every robot loop. Combining them into a single method allows us to minimize | ||
| * the writes to switch ports on the multiplexer. | ||
| */ | ||
| public class MultiplexedColorSensor { |
There was a problem hiding this comment.
We're not going to have color sensors on a multiplexer
This is a PR for reviewing Robot 1. It won't be merged to
main.