Skip to content

Bump Autoalign#37

Open
Dillpickle9756 wants to merge 6 commits intomainfrom
feature/bump-autoalign
Open

Bump Autoalign#37
Dillpickle9756 wants to merge 6 commits intomainfrom
feature/bump-autoalign

Conversation

@Dillpickle9756
Copy link
Contributor

No description provided.

Copy link
Member

@spellingcat spellingcat left a comment

Choose a reason for hiding this comment

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

looks great overall. lmk when you're available to come to the field and you can test it pretty soon

Comment on lines +623 to +625
((Math.abs(getPose().getX() - 4.62) < 2) || (Math.abs(getPose().getX() - 11.91) < 2))
&&
((getPose().getY() > 5.04 && getPose().getY() < 6.07) || (getPose().getY() > 2 && getPose().getY() < 3.01)))
Copy link
Member

Choose a reason for hiding this comment

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

i'd recommend throwing these numbers for the coordinates (4.62, 11.91 etc) into FieldUtils.java as constants (so similar to the BLUE_HUB_POS that's already there) and then referencing them by name here to minimize the amount of "magic numbers" floating around in our code

modifyJoystick(driver.getLeftX())
* SwerveSubsystem.SWERVE_CONSTANTS.getMaxLinearSpeed()));

while(swerve.isCloseToBump()){
Copy link
Member

Choose a reason for hiding this comment

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

this is the right logic, although the while loop doesn't explicitly create a trigger object around this boolean value

Suggested change
while(swerve.isCloseToBump()){
new Trigger(swerve::isCloseToBump).whileTrue(swerve.bumpAlign etc...)

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