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

Vision demo #170

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Vision demo #170

wants to merge 2 commits into from

Conversation

loafdog
Copy link
Contributor

@loafdog loafdog commented May 10, 2023

No description provided.

Copy link
Contributor

@ParkerMeyers ParkerMeyers left a comment

Choose a reason for hiding this comment

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

This should not be a PR, it should either be a draft or should stay in a separate branch. This is not being deployed/merged, its just for learning.

@@ -40,6 +40,11 @@ public final class Constants {

/** The voltage to turn the warning lights on. */
public static final double MIN_BATTERY_VOLTAGE = 12.5;

public static final double CAMERA_PITCH_RADIANS = 1.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

1 Radian is incorrect, it should be 0 (same as the horizion)

@@ -103,7 +104,8 @@ public RobotContainer() {
driveTrain.setDefaultCommand(
new DriveCommand(driveTrain, rightJoystick::getY, rightJoystick::getX));
} else {
driveTrain.setDefaultCommand(driveTrain.stopCommand());
// driveTrain.setDefaultCommand(driveTrain.stopCommand());
Copy link
Contributor

Choose a reason for hiding this comment

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

This code should not be push to main if this is being done. Instead of making a pull request, either convert this to a draft or close the PR and just use the branch.

private double startAngle;
private double startDistance;
private final SlewRateLimiter speedSlewRateLimiter =
new SlewRateLimiter(DriveConstants.SLEW_RATE_LIMITER / 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason this is being divided by 2? Should it be in constants separately?

private double startDistance;
private final SlewRateLimiter speedSlewRateLimiter =
new SlewRateLimiter(DriveConstants.SLEW_RATE_LIMITER / 2);
private double maxSpeed = 0; // in meters/sec
Copy link
Contributor

Choose a reason for hiding this comment

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

This should defiantly not be 0, -1 should be used for max speed

private double maxSpeed = 0; // in meters/sec

// Change this to match the name of your camera
PhotonCamera camera = new PhotonCamera("photonvision");
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be the name of the camera, not the name of the host. EG: Logitech_Pro_4260

Copy link
Contributor

Choose a reason for hiding this comment

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

This should also be private/final

// drivetrain.arcadeDrive(speed, -turnError);
}

// Called once the command ends or is interrupted.
Copy link
Contributor

Choose a reason for hiding this comment

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

Change to proper comments

/**
* Sets the max speed for driving straight
*
* @param maxSpeed max speed in meters/second. 0 indicates no speed limit.
Copy link
Contributor

Choose a reason for hiding this comment

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

should be -1

double targetWidth = Units.inchesToMeters(41.30) - Units.inchesToMeters(6.70); // meters
// See
// https://firstfrc.blob.core.windows.net/frc2020/PlayingField/2020FieldDrawing-SeasonSpecific.pdf
// page 197
Copy link
Contributor

Choose a reason for hiding this comment

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

There is built in methods for defining the 2023 field. No need to specify specific dimensions. Also I would use april tags for localiation

new Translation3d(tgtXPos, tgtYPos, Constants.TARGET_HEIGHT_METERS),
new Rotation3d(0.0, 0.0, 0.0));

double camDiagFOV = 170.0; // degrees - assume wide-angle camera
Copy link
Contributor

Choose a reason for hiding this comment

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

our current camera is 90.

new Rotation3d(0.0, 0.0, 0.0));

double camDiagFOV = 170.0; // degrees - assume wide-angle camera
double camPitch = Units.radiansToDegrees(Constants.CAMERA_PITCH_RADIANS); // degrees
Copy link
Contributor

Choose a reason for hiding this comment

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

im pretty sure this should be in radians

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