Elevator subsystem #57
Conversation
| public default void updateInputs(ElevatorInputs inputs) {} | ||
|
|
||
| public default double getElevatorPosition() { | ||
| return 0.0; | ||
| } | ||
|
|
||
| public default void setElevatorPosition(double position) {} | ||
|
|
||
| public default void setVolts(double volts) {} | ||
|
|
||
| public default double getVolts() { | ||
| return 0.0; |
There was a problem hiding this comment.
Make docstrings for all of these methods
| elevatorConfig.Slot0.kS = ElevatorConstants.ELEVATOR_S; | ||
| elevatorConfig.Slot0.kV = ElevatorConstants.ELEVATOR_V; | ||
| elevatorConfig.Slot0.kA = ElevatorConstants.ELEVATOR_A; | ||
| elevatorConfig.Slot0.kG = ElevatorConstants.ELEVATOR_G; |
There was a problem hiding this comment.
isn't g to like account for gravity? I remember messing with this for charles
| public double getElevatorPosition() { | ||
| return elevatorInterface.getElevatorPosition(); | ||
| } | ||
|
|
||
| public double getVolts() { | ||
| return elevatorInterface.getVolts(); | ||
| } | ||
|
|
||
| public void setElevatorPosition(double position) { | ||
| elevatorInterface.setElevatorPosition(position); | ||
| } | ||
|
|
||
| public void setVolts(double volts) { | ||
| elevatorInterface.setVolts(volts); | ||
| } |
| } | ||
|
|
||
| public double getVolts() { | ||
| return elevatorInterface.getVolts(); |
There was a problem hiding this comment.
you can also return inputs.elevatorVolts or whatever it is
| } | ||
|
|
||
| public double getElevatorPosition() { | ||
| return elevatorInterface.getElevatorPosition(); |
There was a problem hiding this comment.
you can also use the autologged inputs for this
| elevatorConfig.Slot0.kS = ElevatorConstants.ELEVATOR_S; | ||
| elevatorConfig.Slot0.kV = ElevatorConstants.ELEVATOR_V; | ||
| elevatorConfig.Slot0.kA = ElevatorConstants.ELEVATOR_A; | ||
| elevatorConfig.Slot0.kG = ElevatorConstants.ELEVATOR_G; |
There was a problem hiding this comment.
You need to configure the GravityType
| elevatorConfig.Slot0.kS = ElevatorConstants.ELEVATOR_S; | ||
| elevatorConfig.Slot0.kV = ElevatorConstants.ELEVATOR_V; | ||
| elevatorConfig.Slot0.kA = ElevatorConstants.ELEVATOR_A; | ||
| elevatorConfig.Slot0.kG = ElevatorConstants.ELEVATOR_G; |
| } | ||
|
|
||
| @Override | ||
| public void updateInputs(ElevatorInputs inputs) { |
There was a problem hiding this comment.
i.e. BaseStatusSignal.refreshAll(signals....)
| ElevatorConstants.MAX_HEIGHT, | ||
| ElevatorConstants.SIMULATE_GRAVITY, | ||
| ElevatorConstants.MIN_HEIGHT, | ||
| null); |
There was a problem hiding this comment.
is the null for stddevs? if so you can remove it
|
|
||
| @Override | ||
| public double getElevatorPosition() { | ||
| return m_elevatorSim.getPositionMeters(); |
There was a problem hiding this comment.
your units for the position methods (and the inputs) in the sim and physical implementations are inconsistent. Make sure they are the same so you can just call the getElevatorPosition function in Elevator.java without having to worry about units.
They should all be the same units, unless u wanna do conversions and stuff.
| public void setElevatorPosition(double position) { | ||
| m_pidController.setSetpoint(position); | ||
| double output = m_pidController.calculate(getElevatorPosition(), position); | ||
| double feedforward = m_feedforward.calculate(m_pidController.getSetpoint()); |
There was a problem hiding this comment.
isn't feedforward supposed to have velocity as the input?
|
|
||
| /** Creates a new ElevatorHardware. */ | ||
| private final TalonFX leaderMotor = new TalonFX(ElevatorConstants.ELEVATOR_LEADER_MOTOR_ID); | ||
|
|
| import org.littletonrobotics.junction.Logger; | ||
|
|
||
| public class Elevator extends SubsystemBase { | ||
| /** Creates a new Elevator. */ |
There was a problem hiding this comment.
what
the constructor creates a new elevator. not the elevator interface.
Last branch is too cooked, told Max C to make another branch so he can fix most of my mistake ;)