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

Shooter wheel maintainer command #58

Merged
merged 10 commits into from
Feb 3, 2024
Merged

Conversation

sVampified
Copy link
Contributor

@sVampified sVampified commented Feb 2, 2024

Why are we doing this?

ShooterWheelMaintainerCommand reads and controls values.
Asana task URL: https://app.asana.com/0/38541457243752/1206397614455703
Create ShooterWheelMaintainerCommand

Whats changing?

Questions/notes for reviewers

How this was tested

  • unit tests added
  • tested on robot

@sVampified sVampified requested a review from a team as a code owner February 2, 2024 03:43
double speed = wheel.getTargetValue();

if (wheel.isCalibrated()) {
wheel.setPower(1.0);
Copy link
Contributor

Choose a reason for hiding this comment

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

⭐ ⭐ ⭐ instead of setting full power, this should be running the PID on the goal speed to figure out what power to send

}

@Override
protected Double getHumanInput() {
Copy link
Contributor

Choose a reason for hiding this comment

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

@JohnGilb I'm wondering if we should have a more simple version of maintainer that doesn't have human input as a concept and then a more complicated version that does?


@Override
protected double getErrorMagnitude() {
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

⭐ ⭐ ⭐ this needs to return how far off the current speed is from the goal

super(wheel, pf, hvmFactory, 0, 0);
this.oi = oi;
this.wheel = wheel;
this.addRequirements(this.wheel);
Copy link
Contributor

Choose a reason for hiding this comment

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

⭐ ⭐ You don't actually have to add this requirement yourself, the BaseMaintainerCommand will do it for you because you pass in wheel on line 16 already

return 0.0;
}

protected boolean getErrorWithinTolerance() {
Copy link
Contributor

Choose a reason for hiding this comment

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

⭐ ⭐ now that we don't have custom error thresholds anymore, I think we don't need this method at all since the base one does this logic already: https://github.com/Team488/SeriouslyCommonLib/blob/main/src/main/java/xbot/common/command/BaseMaintainerCommand.java#L199

@sVampified sVampified merged commit bae2a94 into main Feb 3, 2024
1 check passed
@sVampified sVampified deleted the ShooterWheelMaintainerCommand branch February 3, 2024 19:36
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.

2 participants