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 the skeleton for score note command #20

Merged
merged 7 commits into from
Jan 16, 2024
Merged

Conversation

Griffin9881
Copy link
Contributor

Pull Request


Issue Number

Closes #16

Comments

made the scoring constants skeleton

@Griffin9881 Griffin9881 self-assigned this Jan 13, 2024
@Griffin9881 Griffin9881 linked an issue Jan 13, 2024 that may be closed by this pull request
9 tasks

@Override
public void execute() {
scoring.runRoller(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

we could replace the speed parameter with a default speed

Copy link
Member

Choose a reason for hiding this comment

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

This should at least be a constant

matunkar
matunkar previously approved these changes Jan 13, 2024

@Override
public boolean isFinished() {
if (timer.get() > ScoringConstants.kSecondsToScore) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (timer.get() > ScoringConstants.kSecondsToScore) {
return timer.get() > ScoringConstants.kSecondsToScore;

Seconds to score should be passed in as a parameter to the constructor. We also need a second constructor that takes no parameters. That way this can be
return this. secondsToScore ? timer.get() > this.secondsToScore : false
Which allows us to run the command until operator stops pressing the button in teleop and have auton end the command after a set time

@@ -14,6 +14,8 @@ public class ScoringConstants {
public static final double kMaxSpeed = 1;
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the k from everything except p I and d


@Override
public boolean isFinished() {
return (this.secondsToScore == 0 ? timer.get() > this.secondsToScore : false);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return (this.secondsToScore == 0 ? timer.get() > this.secondsToScore : false);
return this.secondsToScore == 0 ? timer.get() > this.secondsToScore : false;

These parenthesis are unnecessary


@Override
public void execute() {
scoring.runRoller(0);
Copy link
Member

Choose a reason for hiding this comment

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

This should at least be a constant

@reediculous456 reediculous456 merged commit 9872d4a into main Jan 16, 2024
1 check passed
@reediculous456 reediculous456 deleted the 16-score-note-command branch January 16, 2024 00:33
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.

Score Note Command
3 participants