-
Notifications
You must be signed in to change notification settings - Fork 0
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
35 create make determine waypoint command #39
base: main
Are you sure you want to change the base?
Conversation
case L1Coral: | ||
case L3Coral: | ||
case L4Coral: | ||
getCoralWaypoint(scoringSide); |
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.
Does this method exist?
@Override | ||
public void initialize() { | ||
List<PhotonPipelineResult> visionResults = vision.getResults(); | ||
double area = 0.0; |
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.
What will the area be used for?
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.
The area variable isn't used for anything. It only determines the april tag with the smallest area.
AcquisitionPositionSetpoint setpointLevel = OperatorState.getLastInput(); | ||
ScoringSide scoringSide = OperatorState.getScoringSide(); |
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.
Right now, I think we are assuming that the operator will need to press one of the scoring buttons before setting the waypoint. I would assume this means that we will need to disable the magic button until a scoring button is pressed.
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.
Also, if we decide to change the scoring or cancel it altogether, we must ensure that the current instance of this command gets canceled and a new command replaces it.
case 6: | ||
this.waypoint = scoringSide == ScoringSide.LEFT ? DriveWaypoints.LeftCoral22 | ||
: DriveWaypoints.RightCoral22; | ||
this.isMirrored = true; |
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.
I would set this boolean based on if it's greater than an April Tag judicial ID (i.e. this.isMirror = this.bestTag >= 17)
public static final double kMaxAccelerationMetersPerSecondSquared = 7; | ||
public static final double kMaxAngularSpeedRadiansPerSecond = Math.PI; | ||
public static final double kMaxAngularSpeedRadiansPerSecondSquared = Math.PI; | ||
|
||
public static final double kPXController = 1.0; | ||
public static final double kIXController = 0.35; | ||
public static final double kDController = 0.0; |
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.
I feel this should be called kDXController to be consistent with the other variable names.
@@ -8,8 +18,8 @@ public final class VisionConstants { | |||
public static final String kEnabled0Topic = "enable0"; | |||
public static final String kEnabled1Topic = "enable1"; | |||
|
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.
These variables should each have a k at the start of their names to remain consistent with the other variable names.
…ithub.com/LakotaRobotics1038/2025RobotProject into 35-create-make-determine-waypoint-command
case L1Coral: | ||
case L3Coral: | ||
case L4Coral: | ||
this.waypoint = get134CoralWaypoint(scoringSide); |
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.
Instead of passing in the scoringSide and performing the check for scoringSide within each case of the switch-case statement, we could pass in a boolean variable indicating whether the scoringSide is left or right and use that boolean to determine whether to use the right or left waypoint.
|
||
private Optional<DriveWaypoints> get134CoralWaypoint(ScoringSide scoringSide) { | ||
switch (this.bestId) { | ||
case 6: |
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.
Look in branch 35-create-make-determine-waypoint-command-RG for HashMap implementation.
driveTrain.getState().Speeds.vxMetersPerSecond, | ||
driveTrain.getState().Pose.getRotation()), | ||
new GoalEndState(0, Rotation2d.kZero))))); | ||
super.aButton.and(() -> determineWaypointCommand.getPose2d().isPresent()).whileTrue( |
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.
I think the way this is working right now is that when the "A" button is first pressed, we check whether there is a waypoint present to go towards. However, we call the command after this check, so there will not be a value associated with the Optional when the check is performed. Though, my logic might be wrong.
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.
I see that this check ensures the robot does not move until a waypoint is available. Perhaps, we can call driveToWaypoint every time the acquisition setpoint changes.
Pull Request
Issue Number
Closes #35
Closes #36
Comments