Skip to content
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

Made Telemetry Preferences for motors #145

Merged
merged 10 commits into from
Feb 19, 2024
Merged

Conversation

HHQB
Copy link
Contributor

@HHQB HHQB commented Feb 9, 2024

I imported Telemetry Preferences so the subsystems, then added Telemetry Preferences for the motors in each subsystem that contains a motor. All that needs to be done is testing it in the sim (I can't run sim on the school computer.)

Made Telemetry Preferences for motors
@HHQB HHQB requested a review from GalexY727 February 9, 2024 01:15
@HHQB HHQB self-assigned this Feb 9, 2024
Copy link
Member

@GalexY727 GalexY727 left a comment

Choose a reason for hiding this comment

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

I am going insane. Is this a test? Is this a dream? Does our robot need to know where it isn't?

src/main/java/frc/robot/subsystems/Swerve.java Outdated Show resolved Hide resolved
@@ -31,6 +32,7 @@ public void configMotors() {
elevator.setPeriodicFramePeriod(PeriodicFrame.kStatus5, 65535);
elevator.getEncoder().setPositionConversionFactor(TrapConstants.ELEVATOR_POSITION_CONVERSION_FACTOR);
elevator.setPID(TrapConstants.TRAP_PID);
elevator.setTelemetryPreference(TelemetryPreference.NO_ENCODER);
Copy link
Member

Choose a reason for hiding this comment

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

THe elevator most definitely has an encoder

src/main/java/frc/robot/subsystems/elevator/Elevator.java Outdated Show resolved Hide resolved
@@ -33,6 +33,7 @@ public void configMotor() {
ShooterConstants.PIVOT_D,
ShooterConstants.PIVOT_MIN_OUTPUT,
ShooterConstants.PIVOT_MAX_OUTPUT);
pivot.setTelemetryPreference(TelemetryPreference.NO_ENCODER);
Copy link
Member

Choose a reason for hiding this comment

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

The pivot has an absolute encoder on it, not no encoder. WHY DOES NOTHING HAVE ENCODERS ANYMORE

@@ -30,6 +31,8 @@ public void configMotors() {
ShooterConstants.SHOOTER_D,
ShooterConstants.SHOOTER_MIN_OUTPUT,
ShooterConstants.SHOOTER_MAX_OUTPUT);
motorLeft.setTelemetryPreference(TelemetryPreference.NO_ENCODER);
motorRight.setTelemetryPreference(TelemetryPreference.NO_ENCODER);
Copy link
Member

Choose a reason for hiding this comment

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

THE SHOTOER HAS AN ECNODER WE NEED TO TALK TO IT TO TELL IT MOVE FAST!

Base automatically changed from sim-dev to main February 11, 2024 05:53
@GalexY727 GalexY727 changed the base branch from main to sim-dev February 14, 2024 05:30
@GalexY727 GalexY727 self-requested a review February 14, 2024 05:30
Copy link
Member

@GalexY727 GalexY727 left a comment

Choose a reason for hiding this comment

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

I am going to assume you forgot to push your changes, this is the same PR as the one I reviewed previously.

Comment on lines 361 to 366
public void configMotors() {
rearRight.setTelemetryPreference(TelemetryPreference.NO_ENCODER);
rearLeft.setTelemetryPreference(TelemetryPreference.NO_ENCODER);
frontLeft.setTelemetryPreference(TelemetryPreference.NO_ENCODER);
frontRight.setTelemetryPreference(TelemetryPreference.NO_ENCODER);
}
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be here. Configure the motors inside of MAXSwerveModule, not here. Also, we don't want swerve modules to be set to NO_ENCODER

@HHQB HHQB requested a review from GalexY727 February 18, 2024 01:04
Copy link
Member

@GalexY727 GalexY727 left a comment

Choose a reason for hiding this comment

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

A couple of telemetry preferences are absolute when they should be relative. Also, I am not seeing anything for the pivot, is this intentional?

elevator.getEncoder().setPositionConversionFactor(TrapConstants.ELEVATOR_POSITION_CONVERSION_FACTOR);
elevator.setPID(TrapConstants.TRAP_PID);
elevator.setTelemetryPreference(TelemetryPreference.ONLY_ABSOLUTE_ENCODER);
Copy link
Member

Choose a reason for hiding this comment

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

only relative

Comment on lines +34 to +35
motorLeft.setTelemetryPreference(TelemetryPreference.ONLY_ABSOLUTE_ENCODER);
motorRight.setTelemetryPreference(TelemetryPreference.ONLY_ABSOLUTE_ENCODER);
Copy link
Member

Choose a reason for hiding this comment

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

both only relative

@@ -38,6 +38,7 @@
import frc.robot.util.Constants.FieldConstants;
import monologue.Logged;
import monologue.Annotations.Log;
import frc.robot.util.Neo.TelemetryPreference;
Copy link
Member

Choose a reason for hiding this comment

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

unused import is that you

Copy link
Member

Choose a reason for hiding this comment

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

(IDC if you resolve this)

@GalexY727 GalexY727 merged commit f6f5f02 into sim-dev Feb 19, 2024
@GalexY727 GalexY727 deleted the Telementary-Preference branch February 19, 2024 10:25
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