8000 RobotPoseEstimator Enhancements by hadley31 · Pull Request #677 · PhotonVision/photonvision · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

RobotPoseEstimator Enhancements #677

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Jan 3, 2023
Merged

RobotPoseEstimator Enhancements #677

merged 16 commits into from
Jan 3, 2023

Conversation

hadley31
Copy link
Contributor
@hadley31 hadley31 commented Jan 1, 2023 •
edited
Loading

Update RobotPoseEstimator class to accept a List object instead of ArrayList to allow for flexible user implementation.

@hadley31 hadley31 requested a review from a team as a code owner January 1, 2023 14:22
@hadley31 hadley31 mentioned this pull request Jan 1, 2023
Closed
@hadley31 hadley31 changed the title Use List for RobotPoseEstimator constructor RobotPoseEstimator Enhancements Jan 1, 2023
@hadley31
Copy link
Contributor Author
hadley31 commented Jan 1, 2023

Closes #678

mcm001
mcm001 reviewed Jan 1, 2023
View reviewed changes
mcm001
mcm001 previously approved these changes Jan 1, 2023
View reviewed changes
@hadley31 hadley31 force-pushed the fix/pose-estimator-list branch from 9e74fea to 6c2ecb0 Compare January 2, 2023 14:59
gerth2
gerth2 approved these changes Jan 3, 2023
View reviewed changes
Copy link
Contributor
@gerth2 gerth2 left a comment

Choose a reason for hiding this comment

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

Looks pretty solid. I'm not an expert in some of the language features used here but it looks directionally correct. Since the testcases are still passing, I'm fine with merging it. I'll try picking it up later this week into an example proejct

@gerth2 gerth2 merged commit 0b5256d into PhotonVision:master Jan 3, 2023
@mcm001 mcm001 mentioned this pull request Jan 4, 2023
Closed
@hadley31 hadley31 deleted the fix/pose-estimator-list branch January 11, 2023 14:48
MrRedness pushed a commit to MrRedness/photonvision that referenced this pull request Feb 8, 2023
@hadley31 @mcm001 @MrRedness
RobotPoseEstimator Enhancements (PhotonVision#677)
800a6df 
* Use List for RobotPoseEstimator constructor

* Update `RobotPoseEstimator` constructor to accept wpilib `AprilTagFieldLayout` java

* Initial cpp changes

* Java return optional from update

* Fix java test

* Clean up strategy switch

* small lint

* Actually link to vision_shared

* Fix auto optimized imports

* format

* report error

* small method changes

* format and clean up

Co-authored-by: Matt <matthew.morley.ca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gerth2 gerth2 gerth2 approved these changes

@mcm001 mcm001 mcm001 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hadley31 @gerth2 @mcm001
0