Skip to content

Limelight#9

Open
ac8274 wants to merge 7 commits intodevelopfrom
Limelight
Open

Limelight#9
ac8274 wants to merge 7 commits intodevelopfrom
Limelight

Conversation

@ac8274
Copy link
Contributor

@ac8274 ac8274 commented Jan 30, 2025

I am under the water...
Please help me ...
waleed

@ac8274 ac8274 requested a review from orira22 January 30, 2025 21:39
@ac8274 ac8274 changed the base branch from main to develop January 30, 2025 21:40
Copy link
Contributor

@orira22 orira22 left a comment

Choose a reason for hiding this comment

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

Overall good - there are some comments to fix

public static final Pose2d ZONE_FIVE_LEFT = new Pose2d();
public static final Pose2d ZONE_FIVE_RIGHT = new Pose2d();
public static final Pose2d ZONE_SIX_LEFT = new Pose2d();
public static final Pose2d ZONE_SIX_RIGHT = new Pose2d();
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to define the poses

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i dont know which are they.
the other ori told me not to bother yet

public static boolean isAngleBetween(double myAngle, double startAngle, double endAngle) {
myAngle = (myAngle + 360) % 360;
startAngle = (startAngle + 360) % 360;
endAngle = (endAngle + 360) % 360;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you add 360?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because the angle returned is between -180 and 180 so if it is negative turn it positive

else
{
return Constants.ZONE_ONE_LEFT;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommended to turn into ternary operator

Copy link
Contributor Author

@ac8274 ac8274 Feb 5, 2025

Choose a reason for hiding this comment

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

that's the problem this is the ternary function cause you need to check in which zone you are, the function that checks returns bool which can not be converted to a specific zone.

}
else
{
return Constants.ZONE_THREE_RIGHT;
Copy link
Contributor

Choose a reason for hiding this comment

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

ZONE_LEFT instead of right again

final CommandPS5Controller driverConntroller = new CommandPS5Controller(0);

private final SwerveSubsystem drivebase = new SwerveSubsystem(new File(Filesystem.getDeployDirectory(),
public static final SwerveSubsystem drivebase = new SwerveSubsystem(new File(Filesystem.getDeployDirectory(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Use singletone instead of public static

Copy link
Contributor Author

Choose a reason for hiding this comment

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

check whatsapp


// Called once the command ends or is interrupted.
@Override
public void end(boolean interrupted) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to stop the robot at the end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


// Returns true when the command should end.
@Override
public boolean isFinished() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to add a condition for stoping in isFinished

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

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.

3 participants