Skip to content

Subsystem/comp turret#33

Open
apembleton wants to merge 7 commits intomainfrom
subsystem/comp-turret
Open

Subsystem/comp turret#33
apembleton wants to merge 7 commits intomainfrom
subsystem/comp-turret

Conversation

@apembleton
Copy link
Contributor

No description provided.

Copy link
Contributor

@SCool62 SCool62 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good, but you still need to write a TurretIOSim.

public static double HOOD_GEAR_RATIO = 24.230769;
public static Rotation2d HOOD_MAX_ROTATION = Rotation2d.fromDegrees(40);
public static Rotation2d HOOD_MIN_ROTATION = Rotation2d.fromDegrees(2);
public static double TURRET_GEAR_RATIO = (12.0 / 42.0) * (16.0 / 32.0) * (10.0 / 85.0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you've checked this with cad?

/** Add your docs here. */
public class TurretIO {
@AutoLog
public static class TurretIOInputs {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also log velocity here

public double turretSupplyCurrentAmps = 0.0;
public double turretVoltage = 0.0;
public double turretTempC = 0.0;
// _TODO: Input reall values
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wdym?

private VelocityVoltage velocityVoltage = new VelocityVoltage(0.0).withEnableFOC(true);

public TurretIO(TalonFXConfiguration talonFXConfiguration, CANBus canbus) {
turretMotor = new TalonFX(11, canbus);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shooter hood is already ID 11. Comp CAN ids haven't been decided yet so for now give it a large random number (like 40 or smth) for now

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw this is so that the ids don't clash during sim

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ig its not on this branch so maybe it doesn't matter but better avoid the problem now then fix it later

Copy link
Member

@spellingcat spellingcat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall looks really great! just a few things and then it will be there :)

inputs.turretStatorCurrentAmps = turretStatorCurrent.getValueAsDouble();
inputs.turretSupplyCurrentAmps = turretSupplyCurrent.getValueAsDouble();
inputs.turretTempC = turretTemp.getValueAsDouble();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sure you also refersh the velocity status signal and update its value here

Comment on lines +18 to +19
public TurretIO turretIO;
public TurretIOInputsAutoLogged turretIOInputs = new TurretIOInputsAutoLogged();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make these private instead of public

turretphysicssSim.setInputVoltage(motorSim.getMotorVoltage());
turretphysicssSim.update(deltaTime);
motorSim.setRawRotorPosition(turretphysicssSim.getAngularPositionRotations() * turretphysicssSim.getGearing());
motorSim.setRotorVelocity(turretphysicssSim.getAngularVelocityRPM()/60 * turretphysicssSim.getGearing());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
motorSim.setRotorVelocity(turretphysicssSim.getAngularVelocityRPM()/60 * turretphysicssSim.getGearing());
DCMotorSim turretPhysicsSim =

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants