Skip to content

Conversation

@ky28059
Copy link
Member

@ky28059 ky28059 commented Feb 25, 2023

this.driveSubsystem = driveSubsystem;

recognitionCamera = new PhotonCamera("Microsoft_LifeCam_HD-3000");
turnPID = new PIDController(0.2/35, 0.0, 0.0);
Copy link
Member Author

Choose a reason for hiding this comment

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

I'd use a ProfiledPIDController for this; see FollowPathCommand.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe after #76 gets tuned / merged this can use the setDrivePowersWithHeadingLock() method and not have to implement its own PID as well.

private final PIDController turnPID;
PhotonCamera recognitionCamera;

PhotonPipelineResult result;
Copy link
Member Author

Choose a reason for hiding this comment

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

Doesn't have to be a state?

PhotonCamera recognitionCamera;

PhotonPipelineResult result;
boolean hasTargets;
Copy link
Member Author

Choose a reason for hiding this comment

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

Also doesn't have to be a state

public void execute() {
result = recognitionCamera.getLatestResult();
hasTargets = result.hasTargets();
target = result.getBestTarget();
Copy link
Member Author

Choose a reason for hiding this comment

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

What does this object look like if hasTargets is false? Does the command end execution at all if that happens?

public class VisionAlignmentCommand extends CommandBase {
private final BaseSwerveSubsystem driveSubsystem;

private double angularPower;
Copy link
Member Author

Choose a reason for hiding this comment

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

Also doesn't have to be a state; you only need to store things in fields if you need to reference them across multiple methods or you need the previous value when rerunning the same method.

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