-
Notifications
You must be signed in to change notification settings - Fork 0
14 indexer subsystem #15
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
base: develop
Are you sure you want to change the base?
Conversation
|
Build errors will be fixed when #16 is merged, then pulled into this branch. @bjmcternan if you are able to approve #16 then this can get merged |
OK Let's merge #16 first then. We do not want to merge code that cannot be built and run |
src/main/java/frc/robot/subsystems/indexer/IndexerIOTalonFX.java
Outdated
Show resolved
Hide resolved
bjmcternan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add some comments
|
As a means of making the PR a bit easier to read, you can (temporarily) make the PR target 9-fix-async-logger-library, so that github will only show the new commits that are intended for this PR. You just need to change it back after that branch is closed. Though I see that there's a merge commit with that branch here... it would be better to rebase this branch onto that one rather than merging back and forth, which better reflects what we want the final commit state to be. |
…update naming conventions to be correct
NathanEdg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I don't think we need to comment every instance of "rps", as its a known unit
|
#9 is merged |
| @@ -0,0 +1,32 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are there updates to path planner?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be rebased, that is from a commit from #9.
Add functionality for an indexer motor that spins when a beam break sensor detects a game piece being indexed.