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

Rework arm implementation and API #34

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from
Draft

Conversation

charlesmackenzie
Copy link

@charlesmackenzie charlesmackenzie commented Nov 30, 2023

I'm attempting to address the first part of #31 by aggressively refactoring the arm code. Changes focus on understandably and straightforwardness.

Need to do:

  • Implement a state machine to manage telescope retraction (after drawing a diagram)
  • Add javadoc
  • Add some unit tests? This might will become another PR
  • Run on the actual robot

Notes:

  • AdvantageKit implementation will take place in another PR. YOLO

@charlesmackenzie charlesmackenzie added the enhancement New feature or request label Nov 30, 2023
@charlesmackenzie charlesmackenzie changed the base branch from main to 2024-Phoenix-Swerve December 11, 2023 23:07
@charlesmackenzie charlesmackenzie changed the base branch from 2024-Phoenix-Swerve to main December 11, 2023 23:08
Copy link
Member

@jkleiber jkleiber left a comment

Choose a reason for hiding this comment

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

Half of a first pass. I'm a bit confused how all of the arm stuff works together in this PR since arm subsystem looks deleted

Copy link
Member

Choose a reason for hiding this comment

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

To avoid merge conflicts, I would just delete this file since @redPlover is deleting it

Copy link
Member

Choose a reason for hiding this comment

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

probably should delete this file since @redPlover is deleting it in his branch

Copy link
Member

Choose a reason for hiding this comment

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

Probably best to delete this one too since it's being deleted in #35

Copy link
Member

Choose a reason for hiding this comment

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

this file is being deleted in #35 and isn't related to the arm - best move is to revert all changes or delete it to avoid merge conflicts

private final WristSubsystem wrist = new WristSubsystem();
private final IntakeSubsystem intake = new IntakeSubsystem();
// private final Drive drive = new Drive();
private final ArmSubsystem arm;
Copy link
Member

Choose a reason for hiding this comment

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

It looks like ArmSubsystem has been deleted, so it isn't clear how this will work

@jkleiber jkleiber mentioned this pull request Dec 13, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants