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

Manipulator #7

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from
Open

Manipulator #7

wants to merge 11 commits into from

Conversation

Ryan-Bauroth
Copy link

Can someone make sure all this code looks good? I think it all should work but I am unsure about the init() function and just general IO things being correct. Thanks!

@Ryan-Bauroth Ryan-Bauroth self-assigned this Sep 9, 2023
Copy link
Collaborator

@a1cd a1cd left a comment

Choose a reason for hiding this comment

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

I made a bunch of comments. They have suggestions you can easily approve if you want to but make sure to read them first

private final RelativeEncoder encoder;
private final SparkMaxPIDController pid;

public ManipulatorIOSparkMax(){
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you want you can make motor id a parameter, but it will work this way as well

@a1cd a1cd linked an issue Sep 10, 2023 that may be closed by this pull request
Ryan-Bauroth and others added 2 commits September 12, 2023 14:41
Co-authored-by: Everett Wilber <71281043+a1cd@users.noreply.github.com>
Co-authored-by: Everett Wilber <71281043+a1cd@users.noreply.github.com>
Quick simple compilation failures
Copy link
Collaborator

@a1cd a1cd left a comment

Choose a reason for hiding this comment

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

Nice effort, still untested code and it is not used in robot container but it looks like it should work properly.

@a1cd
Copy link
Collaborator

a1cd commented Sep 12, 2023

@Ryan-Bauroth Merge it when you want to

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.

Manipulator
2 participants