Skip to content

Intake assist#148

Open
scotch-tape wants to merge 12 commits into
mainfrom
intake-assist
Open

Intake assist#148
scotch-tape wants to merge 12 commits into
mainfrom
intake-assist

Conversation

@scotch-tape

Copy link
Copy Markdown
Contributor

No description provided.

@codeNinja-1 codeNinja-1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great! I added some comments for potential improvements to think about.

Comment on lines +35 to +37
double kP = 0.5; // Not tuned

double maxAssist = 1.0; // Not tuned

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It would be nice if there were tunables for these.

Comment on lines +68 to +69
double correctedVelocityX = (-velocityY / robotSpeed) * assist;
double correctedVelocityY = (velocityX / robotSpeed) * assist;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These variables' names are a bit confusing, because it sounds like these will be the output of this method, not a vector added to the input velocity. Perhaps call this correctionVelocityX and correctionVelocityY instead?

// the robot is moving away from the fuel

double speedScale =
Math.min(1.0, robotSpeed / 2.0); // 2.0 m/s should be tuned to the speed at where the assist

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also a tunable for this would be nice.

Comment on lines +168 to +170
public void setAssistFunction(UnaryOperator<ChassisSpeeds> assistFunction) {
this.assistFunction = assistFunction;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If we expect to typically use this in a startEnd command that sets the assist function to a value at start and sets it to null at end, it could be nice to have a convenience method that wraps this in a command.

* perpendicular to the robot's current velocity, based on the distance to the fuel clump and how
* much the robot is moving towards it, and adds that to the robot's velocity output.
*/
public ChassisSpeeds calculateAdjustedVelocity(ChassisSpeeds currentVelocity) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You're doing a lot of vector operations in this method. Maybe consider using WPILib's Vector class to simplify the code.

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