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 #10

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

Vision #10

wants to merge 37 commits into from

Conversation

TheFlameFish
Copy link
Contributor

Adds vision to the robot with real and sim implementations.

Vision real was tested yesterday but has not been tested on the latest version.
Vision sim has been tested and appears to be functional with the modifications to moduleIOSim described in #9, although said modification has not been committed to this branch so sim will currently fail.

…ion, made field bounds check a little more tolerant on the Z axis.
…it adapt to any # of cameras), made default stdDevs (if the apriltag is not a known ID) very large so it'll be essentially discarded.
@TheFlameFish
Copy link
Contributor Author

Tested on real dory. Appears to be functioning well.

Copy link
Member

Choose a reason for hiding this comment

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

looks like it is unused in drive, i dont see any reason to keep these additions unless you have another one

Copy link
Member

@Ace5tar Ace5tar left a comment

Choose a reason for hiding this comment

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

looks good, I dont see any actual implementation outside of the subsysthem though, so getting it to integrate with odometry is still something that needs to be done

@TheFlameFish
Copy link
Contributor Author

looks good, I dont see any actual implementation outside of the subsysthem though, so getting it to integrate with odometry is still something that needs to be done

It's instantiated in drive and adds vision measurements to the drivetrain's pose estimator if that's what you mean

@TheFlameFish
Copy link
Contributor Author

Judging by issues we're having while testing pathing, the camera offsets may be off.

@@ -54,6 +55,8 @@ public class Drive extends SubsystemBase {
private final SysIdRoutine moduleSteerRoutine;
private final SysIdRoutine driveRoutine;

private AprilTagVision aprilTagVision = new AprilTagVision(poseEstimator);
Copy link
Member

Choose a reason for hiding this comment

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

not sure why this exists, seems to serve no purpose?

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 think it serves a purpose in that it instantiates vision. If you're referring to it being instantiated in drive instead of RobotContainer, that's mostly because it requires a drivetrain pose estimator which is instantiated by drive since it requires some suppliers from drive.
I think me and @nolanhergert had a conversation about instantiating it in RobotContainer vs in Drive and Nolan convinced me it would be better to do it in drive. However, neither of us can remember exactly why we made this decision. If preferred, it could be instantiated in RobotContainer (in which case we would instantiate Drive and then give AprilTagVision a reference to Drive's pose estimator).

Copy link
Member

@Ace5tar Ace5tar left a comment

Choose a reason for hiding this comment

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

other than the seemingly unneccessary code in Drive.java, looks good

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