-
Notifications
You must be signed in to change notification settings - Fork 0
Climber code & generic comments #1
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
base: main
Are you sure you want to change the base?
Conversation
Contains Gabby's code for running the climber. Will likely require some tweaking of things like current limits once we can test it for real, but for the time being, it appears to be behaving correctly. Also includes some arbitrary thoughts and comments of different sections of code.
|
|
||
| public static final class Neo { | ||
| public static final double FREE_SPEED_RPMS = 5676.0; | ||
| public static final double FREE_SPEED_RPMS = 5676.0; //TODO: Move this outside of the CurrentLimit class |
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'll just remove this - it's not actually used.
|
|
||
| /* Robot frame width in meters */ | ||
| public static final double WIDTH = Units.inchesToMeters(28.125); | ||
| public static final double WIDTH = Units.inchesToMeters(28.125);//TODO: update |
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 all the frame and drivetrain measurements are accurate. I'll double check, but I'm pretty sure I measured them when the frame and drive base were completed. I'll need to update the weight, but that keeps changing - so as soon as we stabilize there I'll set. I'm sure we'll be at or near max.
| /* Pose estimate should not be reset until after this long after collision */ | ||
| public static final long MICROSECONDS_SINCE_COLLISION_THRESHOLD = 250000; //0.25 seconds | ||
|
|
||
| //TODO: Do we still need to tune these values? |
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.
Yes, these need to be tested / verified. The defaults used by SwerveDrivePoseEstimator are 0.1 M, 0.1 M, 0.1 Radians (approx 6 degrees).
| public static final double kElevatorKi = 0; | ||
| public static final double kElevatorKd = 0; | ||
|
|
||
| // TODO: Elevator has changed slightly, confirm values are still accurate. |
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'll ask Dillan about this. I ran Sys ID characterization on the Elevator on Monday and verified the new gains seemed correct when doing a couple test runs of the reachVerticalGoal command using the ProfiledPIDController.
| } | ||
|
|
||
| public static final class PathPlanner { | ||
| //TODO: Do we still need to tune these values? |
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.
Yes, no Pathplanner testing or tuning has been done yet. I've been waiting for the cameras to be mounted and wired on the competition robot.
| private final SendableChooser<Command> autoChooser; | ||
|
|
||
| /* | ||
| * TODO: if possible, I think we should remove startingPositionChooser, and try |
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.
Yeah, I'll get rid of this - just old crud carried over from the Eclipse codebase. I have the initial pose reading from vision already setup - see Drivetrain.resetPoseEstimateUsingVision()
| * @param value The value you want to modify | ||
| * @return The filtered value | ||
| * | ||
| * FIXME: Matt: I recommend we remove this, as it is arguably obselete by the generic version below. |
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.
Agreed, I'll remove as it is no longer actively used.
| /* | ||
| * I didnt write this and i dont remember what it does | ||
| * FIXME: Matt: I recommend we remove this if we don't remember what it does. | ||
| * For what its worth, we're not currently using it. |
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.
This actually is used in SwerveModule.setSteerAngle(). I'll test this out more and either document it better or remove it if I find it's not having any real effect. I think this had something to do with constraining the desired angle within 360 degrees of the current angle.
|
|
||
| public static void verifySparkMaxStatus(REVLibError revResult, int canID, String deviceName, String operation) { | ||
| if (revResult != REVLibError.kOk) { | ||
| //FIXME: Matt - I know nothing about resource leaks, but it sounds bad? |
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'll clean this up. The issue has less to do with the Alert object not being 'closed' via calling the close() method and more to do with potentially creating many new Alert objects if this check fails when being called quickly within a periodic() or loop. As it is, it's not even being used since it's only referenced in setSteerAngleAbsolute(), which isn't used either and is a helper for tuning swerve module rotation PID, so usage there would only be temporary anyway.
| //Note: field layout values can be obtained by examining the AprilTagFieldLayout .json file for the game | ||
| public static final double FIELD_LENGTH_METERS = 16.541; //x in field drawings (from 2024 game - update for 2025 if necessary) | ||
| public static final double FIELD_WIDTH_METERS = 8.211; //y in field drawings (from 2024 game - update for 2025 if necessary) | ||
| public static final double FIELD_LENGTH_METERS = 16.541; //x in field drawings (FIXME: from 2024 game - update for 2025 if necessary) |
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've removed these constants and replaced usage in VisionSystem with AprilTagFieldLayout.getFieldLength() and getFieldWidth(), which will be more accurate in the case we need to swap between default welded frame field size to the andymark field size.
Contains Gabby's code for running the climber. Will likely require some tweaking of things like current limits once we can test it for real, but for the time being, it appears to be behaving correctly.
Also includes some arbitrary thoughts and comments of different sections of code.