-
Notifications
You must be signed in to change notification settings - Fork 0
Arm logic #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Arm logic #16
Changes from all commits
26ede8b
b969772
b93b24d
8bcbe19
cbf1359
d3546dd
4b60ce6
f327b94
bd98958
eb64ed0
024fb62
d1c4d2d
4dd87f5
379867e
1b9c09f
4820ccc
bbe7ac4
140137d
2b0037f
482bd8f
1aeb7bb
07b24f6
94c264b
936826d
9b9fc93
c2591b7
8159e68
dc3319b
d748b5e
248ca5c
3a8dc54
c0db5e1
43757f9
62fcd32
440dc79
fae2066
33beb6d
cf68be9
d1c7edc
5bf8194
bd19f72
4ea93e3
df0a3f4
fb1233d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,10 +2,12 @@ | |
|
||
import com.ctre.phoenix6.controls.DynamicMotionMagicVoltage; | ||
import com.ctre.phoenix6.controls.VoltageOut; | ||
import edu.wpi.first.math.MathUtil; | ||
import edu.wpi.first.math.geometry.*; | ||
import edu.wpi.first.units.Units; | ||
import edu.wpi.first.wpilibj.sysid.SysIdRoutineLog; | ||
import edu.wpi.first.wpilibj2.command.sysid.SysIdRoutine; | ||
import frc.trigon.robot.RobotContainer; | ||
import frc.trigon.robot.subsystems.MotorSubsystem; | ||
import lib.hardware.phoenix6.cancoder.CANcoderEncoder; | ||
import lib.hardware.phoenix6.cancoder.CANcoderSignal; | ||
|
@@ -81,31 +83,29 @@ public void sysIDDrive(double targetVoltage) { | |
armMasterMotor.setControl(voltageRequest.withOutput(targetVoltage)); | ||
} | ||
|
||
public boolean isArmAboveSafeAngle() { | ||
return getAngle().getDegrees() >= ArmConstants.MAXIMUM_ARM_SAFE_ANGLE.getDegrees(); | ||
} | ||
|
||
public boolean atState(ArmConstants.ArmState targetState, boolean isStateReversed) { | ||
if (isStateReversed) | ||
return this.targetState == targetState && atTargetAngle(isStateReversed); | ||
return this.targetState == targetState && atAngle(subtractFrom360Degrees(targetState.targetAngle)); | ||
return atState(targetState); | ||
} | ||
|
||
public boolean atState(ArmConstants.ArmState targetState) { | ||
return this.targetState == targetState && atTargetAngle(); | ||
return this.targetState == targetState && atAngle(targetState.targetAngle); | ||
} | ||
|
||
public boolean atTargetAngle(boolean isStateReversed) { | ||
public boolean atTargetAngle() { | ||
if (isStateReversed) { | ||
final double currentToTargetStateDifferenceDegrees = Math.abs(ArmConstants.FULL_ROTATION.minus(targetState.targetAngle).minus(getAngle()).getDegrees()); | ||
return currentToTargetStateDifferenceDegrees < ArmConstants.ANGLE_TOLERANCE.getDegrees(); | ||
return atAngle(subtractFrom360Degrees(targetState.targetAngle)); | ||
} | ||
return atTargetAngle(); | ||
} | ||
|
||
public boolean atTargetAngle() { | ||
final double currentToTargetStateDifferenceDegrees = Math.abs(targetState.targetAngle.minus(getAngle()).getDegrees()); | ||
return currentToTargetStateDifferenceDegrees < ArmConstants.ANGLE_TOLERANCE.getDegrees(); | ||
return atAngle(targetState.targetAngle); | ||
} | ||
|
||
public boolean hasGamePiece() { | ||
return ArmConstants.COLLECTION_DETECTION_BOOLEAN_EVENT.getAsBoolean(); | ||
public Rotation2d getAngle() { | ||
return Rotation2d.fromRotations(angleEncoder.getSignal(CANcoderSignal.POSITION)); | ||
ShmayaR marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
void setTargetState(ArmConstants.ArmState targetState) { | ||
|
@@ -120,7 +120,7 @@ void setTargetState(ArmConstants.ArmState targetState, boolean isStateReversed) | |
|
||
if (isStateReversed) { | ||
setTargetState( | ||
ArmConstants.FULL_ROTATION.minus(targetState.targetAngle) | ||
subtractFrom360Degrees(targetState.targetAngle) | ||
, targetState.targetEndEffectorVoltage | ||
); | ||
return; | ||
|
@@ -132,40 +132,58 @@ void setTargetState(ArmConstants.ArmState targetState, boolean isStateReversed) | |
|
||
void setTargetState(Rotation2d targetAngle, double targetVoltage) { | ||
setTargetAngle(targetAngle); | ||
setTargetVoltage(targetVoltage); | ||
setEndEffectorTargetVoltage(targetVoltage); | ||
} | ||
|
||
void prepareForState(ArmConstants.ArmState targetState) { | ||
prepareForState(targetState, false); | ||
void setArmState(ArmConstants.ArmState targetState) { | ||
setArmState(targetState, false); | ||
} | ||
|
||
void prepareForState(ArmConstants.ArmState targetState, boolean isStateReversed) { | ||
void setArmState(ArmConstants.ArmState targetState, boolean isStateReversed) { | ||
this.isStateReversed = isStateReversed; | ||
this.targetState = targetState; | ||
|
||
if (isStateReversed) { | ||
setTargetAngle(ArmConstants.FULL_ROTATION.minus(targetState.targetAngle)); | ||
setTargetAngle(subtractFrom360Degrees(targetState.targetAngle)); | ||
return; | ||
} | ||
setTargetAngle(targetState.targetAngle); | ||
} | ||
|
||
private Rotation2d getAngle() { | ||
return Rotation2d.fromRotations(angleEncoder.getSignal(CANcoderSignal.POSITION)); | ||
void setEndEffectorState(ArmConstants.ArmState targetState) { | ||
setEndEffectorTargetVoltage(targetState.targetEndEffectorVoltage); | ||
} | ||
|
||
private void setTargetAngle(Rotation2d targetAngle) { | ||
armMasterMotor.setControl(positionRequest.withPosition(targetAngle.getRotations())); | ||
armMasterMotor.setControl(positionRequest.withPosition(Math.max(targetAngle.getRotations(), calculateMinimumArmSafeAngle().getRotations()))); | ||
} | ||
|
||
private Rotation2d calculateMinimumArmSafeAngle() { | ||
final boolean isElevatorAboveSafeZone = RobotContainer.ELEVATOR.isElevatorAboveSafeZone(); | ||
final double heightFromSafeZone = RobotContainer.ELEVATOR.getElevatorHeightFromMinimumSafeZone(); | ||
final double cosOfMinimumSafeAngle = MathUtil.clamp(heightFromSafeZone / ArmConstants.ARM_LENGTH_METERS, 0, 1); | ||
return isElevatorAboveSafeZone | ||
? Rotation2d.fromRadians(0) | ||
: Rotation2d.fromRadians(Math.acos(cosOfMinimumSafeAngle)); | ||
} | ||
|
||
private void setTargetVoltage(double targetVoltage) { | ||
private void setEndEffectorTargetVoltage(double targetVoltage) { | ||
ArmConstants.END_EFFECTOR_MECHANISM.setTargetVelocity(targetVoltage); | ||
endEffectorMotor.setControl(voltageRequest.withOutput(targetVoltage)); | ||
} | ||
|
||
private boolean atAngle(Rotation2d targetAngle) { | ||
final double currentToTargetAngleDifferenceDegrees = Math.abs(targetAngle.minus(getAngle()).getDegrees()); | ||
return currentToTargetAngleDifferenceDegrees < ArmConstants.ANGLE_TOLERANCE.getDegrees(); | ||
} | ||
|
||
private static Rotation2d subtractFrom360Degrees(Rotation2d angleToSubtract) { | ||
return Rotation2d.fromDegrees(Rotation2d.k180deg.getDegrees() * 2 - angleToSubtract.getDegrees()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't change anything, it just overcomplicates it for no reason. Just revert it to the way it was before. I don't care enough about this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ezra does |
||
} | ||
|
||
private Pose3d calculateVisualizationPose() { | ||
ShmayaR marked this conversation as resolved.
Show resolved
Hide resolved
|
||
final Transform3d armTransform = new Transform3d( | ||
new Translation3d(), | ||
new Translation3d(0, 0, RobotContainer.ELEVATOR.getPositionMeters()), | ||
new Rotation3d(0, getAngle().getRadians(), 0) | ||
); | ||
return ArmConstants.ARM_VISUALIZATION_ORIGIN_POINT.transformBy(armTransform); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,11 +9,13 @@ | |
import edu.wpi.first.units.Units; | ||
import edu.wpi.first.wpilibj.sysid.SysIdRoutineLog; | ||
import edu.wpi.first.wpilibj2.command.sysid.SysIdRoutine; | ||
import frc.trigon.robot.RobotContainer; | ||
import frc.trigon.robot.subsystems.MotorSubsystem; | ||
import org.littletonrobotics.junction.Logger; | ||
import frc.trigon.robot.subsystems.arm.ArmConstants; | ||
import lib.hardware.phoenix6.talonfx.TalonFXMotor; | ||
import lib.hardware.phoenix6.talonfx.TalonFXSignal; | ||
import lib.utilities.Conversions; | ||
import org.littletonrobotics.junction.Logger; | ||
|
||
public class Elevator extends MotorSubsystem { | ||
private final TalonFXMotor masterMotor = ElevatorConstants.MASTER_MOTOR; | ||
|
@@ -23,7 +25,7 @@ public class Elevator extends MotorSubsystem { | |
ElevatorConstants.DEFAULT_MAXIMUM_VELOCITY, | ||
ElevatorConstants.DEFAULT_MAXIMUM_ACCELERATION, | ||
ElevatorConstants.DEFAULT_MAXIMUM_ACCELERATION * 10 | ||
).withEnableFOC(ElevatorConstants.FOC_ENABLED); | ||
).withEnableFOC(ElevatorConstants.FOC_ENABLED); | ||
private ElevatorConstants.ElevatorState targetState = ElevatorConstants.ElevatorState.REST; | ||
|
||
public Elevator() { | ||
|
@@ -75,14 +77,40 @@ public void updatePeriodically() { | |
Logger.recordOutput("Elevator/CurrentPositionMeters", getPositionMeters()); | ||
} | ||
|
||
public double getPositionMeters() { | ||
return rotationsToMeters(getPositionRotations()); | ||
} | ||
|
||
public boolean isElevatorAboveSafeZone() { | ||
return getPositionMeters() >= ElevatorConstants.MAXIMUM_ELEVATOR_SAFE_ZONE_METERS; | ||
} | ||
|
||
public double getElevatorHeightFromMinimumSafeZone() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not? its a constant |
||
return getPositionMeters() - ElevatorConstants.MINIMUM_ELEVATOR_SAFE_ZONE_METERS; | ||
} | ||
|
||
void setTargetState(ElevatorConstants.ElevatorState targetState) { | ||
this.targetState = targetState; | ||
scalePositionRequestSpeed(targetState.speedScalar); | ||
if (targetState == ElevatorConstants.ElevatorState.LOAD_CORAL) { | ||
masterMotor.setControl(positionRequest.withPosition(metersToRotations(targetState.targetPositionMeters))); | ||
return; | ||
} | ||
setTargetPositionRotations(metersToRotations(targetState.targetPositionMeters)); | ||
} | ||
Comment on lines
92
to
100
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainOne more guard: prevent future bypasses of the clamp Search for any other direct masterMotor.setControl(positionRequest.withPosition(...)) writes in Elevator to ensure all paths route through setTargetPositionRotations(). Also applies to: 102-114 🏁 Script executed: #!/bin/bash
rg -nP --type=java -C2 'setControl\s*\(\s*positionRequest\.withPosition\(' src/main/java/frc/trigon/robot/subsystems/elevator/Elevator.java Length of output: 535 Prevent clamp bypass in LOAD_CORAL branch src/main/java/frc/trigon/robot/subsystems/elevator/Elevator.java — the LOAD_CORAL branch in setTargetState directly calls masterMotor.setControl(positionRequest.withPosition(...)) (line 96), bypassing the clamp in setTargetPositionRotations (line 103). Replace that direct call with setTargetPositionRotations(metersToRotations(targetState.targetPositionMeters)) or apply the same Math.max clamp before calling masterMotor.setControl. 🤖 Prompt for AI Agents
|
||
|
||
void setTargetPositionRotations(double targetPositionRotations) { | ||
masterMotor.setControl(positionRequest.withPosition(targetPositionRotations)); | ||
masterMotor.setControl(positionRequest.withPosition(Math.max(targetPositionRotations, calculateMinimumSafeElevatorHeightRotations()))); | ||
} | ||
|
||
private double calculateMinimumSafeElevatorHeightRotations() { | ||
final double armCos = RobotContainer.ARM.getAngle().getRadians(); | ||
final double elevatorHeightFromArm = Math.cos(armCos) * ArmConstants.ARM_LENGTH_METERS; | ||
final double minimumElevatorSafeZone = ElevatorConstants.MINIMUM_ELEVATOR_SAFE_ZONE_METERS; | ||
final double minimumSafeHeightMeters = (RobotContainer.ARM.isArmAboveSafeAngle() | ||
? 0 : elevatorHeightFromArm) | ||
+ minimumElevatorSafeZone; | ||
return metersToRotations(minimumSafeHeightMeters); | ||
} | ||
|
||
private Pose3d getFirstStageComponentPose() { | ||
|
@@ -117,10 +145,6 @@ private void scalePositionRequestSpeed(double speedScalar) { | |
positionRequest.Jerk = positionRequest.Acceleration * 10; | ||
} | ||
|
||
private double getPositionMeters() { | ||
return rotationsToMeters(getPositionRotations()); | ||
} | ||
|
||
private double rotationsToMeters(double positionsRotations) { | ||
return Conversions.rotationsToDistance(positionsRotations, ElevatorConstants.DRUM_DIAMETER_METERS); | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix wrap-around in isArmAboveSafeAngle (Rotation2d.getDegrees is [-180,180)).
Angles >180° read as negative and will fail this check. Normalize before comparing.
📝 Committable suggestion
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anytime the arm is above 90 degrees itll ignore the logic so it doesnt matter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ShmayaR Thank you for the clarification about the system behavior above 90°.
I understand that your current system logic bypasses this safety check when the arm is above 90°, which would indeed make the wrap-around issue less critical in practice. However, I'd like to point out a couple of considerations:
Latent bug risk: If the system behavior ever changes or if the arm physically operates in the 180°+ range, angles like 270° would read as -90° and incorrectly return
false
when they should returntrue
.Code robustness: Even if the current logic prevents problematic scenarios, defensive programming would make the code more maintainable and less prone to future bugs.
Could you clarify what specifically happens when the arm reaches angles above 180°? Does the system have hard stops, or are there other safety mechanisms that prevent operation in that range?
The fix would be minimal and wouldn't impact performance, but I understand if it's not a priority given your current system constraints.
🐰💭
✏️ Learnings added