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

[WIP] Simulation Overhaul #742

Merged
merged 43 commits into from
Jun 18, 2023
Merged

[WIP] Simulation Overhaul #742

merged 43 commits into from
Jun 18, 2023

Conversation

amquake
Copy link
Member

@amquake amquake commented Jan 15, 2023

Putting this (living) draft up for feedback and testing. Some of the videos on this repo's readme show this simulation in action. This is all currently just from my perspective, and there's still lots more that could be added, so I would appreciate opinions on anything and everything.

What does this do?

  • Deprecates previous sim classes
  • Has a CameraProperties class for describing a camera's basic/calibration info, and performance values for simulation. Calibration values can be loaded from the config.json in the settings exported by photonvision.
  • OpenCVHelp provides convenience functions for using opencv methods with wpilib/photonvision classes, mainly to project 3d points to a camera's 2d image and perform solvePnP with the above camera calibration info.
    • TargetModels describe the 3d shape of a target, both for projecting into the camera's 2d image and use in solvePnP.
  • PhotonCameraSim uses camera properties to simulate how 3d targets would appear in its view, and has simulated noise, latency, and FPS. For apriltags, the best/alternate camera-to-target transform is also estimated with solvePnP.
    • VideoSimUtil has helper functions for drawing apriltags to a simulated raw and processed MJPEG stream for each camera using the projected tag corners.
  • VisionSystemSim stores VisionTargetSims and PhotonCameraSims, and is periodically updated with the robot's simulated pose. When updating, camera sims are automatically processed and published with their visible targets from their respective poses with proper latency.

How to test this


Possible Additions / Known Issues

  • Latency is not used correctly for moving vision targets because their poses are not saved over time.
  • Differentiate camera pipelines
    • Only view certain types of vision targets?
    • Change camera properties?
  • Video stream simulation: should on or off by default?
  • "Raw" video stream apriltag image warping is messy
    • Better antialiasing to work-around poor opencv warpPerspective results
    • Warp the entire tag image instead of just onto the detected corners
  • Expand target support
    • Get corners(?) / area of spherical targets
    • Perform solvePnP on non-apriltag targets
    • Draw non-apriltag targets to video stream
  • Allow processing of simulated video frames directly with wpilib apriltag bindings
    • Simulate actual camera image noise instead of random gaussian corner movement
    • Simulate motion blur
  • Draw field wireframe to video stream sim

Things that should be done to finish this PR

  • Documentation additions/changes
  • Naming/organization review
  • More test coverage
  • API changes based on user testing
  • C++ :)
  • Update sim examples

@mcm001 mcm001 marked this pull request as ready for review June 11, 2023 17:57
@mcm001 mcm001 requested a review from a team as a code owner June 11, 2023 17:57
@srimanachanta
Copy link
Member

All SIM-related content should be moved into a simulation folder following wpilib, also, with deprecated classes, it will be crowded

@mcm001 @amquake Thoughts on this?

if (result.empty()) {
BufferedImage buf;
try {
buf = ImageIO.read(resource);
Copy link
Member

Choose a reason for hiding this comment

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

no null check here? Is this assumed to be non-null based on the previous check?

Copy link
Member Author

Choose a reason for hiding this comment

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

if you mean result, it should always be at least an empty Mat. If you mean buf, I believe it should throw an exception when it can't read the desired image.

Copy link
Member

Choose a reason for hiding this comment

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

I mean resource, you bill check before but not beyond that if statement

Copy link
Member Author

Choose a reason for hiding this comment

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

If resource is null, ImageIO.read will throw an exception.

addVisionTargets(
"apriltags",
new VisionTargetSim(
tagLayout.getTagPose(tag.ID).get(), // preserve alliance rotation
Copy link
Member

Choose a reason for hiding this comment

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

Check if tag is present, getTagPose returns an optional

Copy link
Member Author

Choose a reason for hiding this comment

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

We are iterating through the tags that the tag layout reports through getTags() and then redundantly getting their pose from the layout again (for handling different alliance origins), so this should not be empty.


// use camera pose from the image capture timestamp
Pose3d lateRobotPose = getRobotPose(timestampCapture);
Pose3d lateCameraPose = lateRobotPose.plus(getRobotToCamera(camSim).get());
Copy link
Member

Choose a reason for hiding this comment

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

getRobotToCamera returns optional, camSim confirmed to be non-empty?

Copy link
Member Author

Choose a reason for hiding this comment

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

When adding a camera sim to the vision system sim, it is ensured that there is a transform. The only way for the getRobotToCamera method to return empty should be if the given camera sim does not exist in the vision system sim. In this case, we are iterating through the camera sims in this vision system sim.

srimanachanta and others added 4 commits June 11, 2023 15:19
See github for things I thought warranted further discussion
@amquake
Copy link
Member Author

amquake commented Jun 11, 2023

All SIM-related content should be moved into a simulation folder following wpilib, also, with deprecated classes, it will be crowded

@mcm001 @amquake Thoughts on this?

Does moving the deprecated sim classes into the org.photonvision.simulation package break team's code?

@mcm001
Copy link
Contributor

mcm001 commented Jun 18, 2023

Does moving the deprecated sim classes into the org.photonvision.simulation package break team's code?

Yes, but I think that it's OK as long as we add a note to the docs?

@mcm001 mcm001 merged commit f1cadc1 into PhotonVision:master Jun 18, 2023
19 of 20 checks passed
@mcm001 mcm001 deleted the sim-overhaul branch June 18, 2023 22:54
mcm001 pushed a commit that referenced this pull request Jul 24, 2023
- `PNPResults` can now be empty (`isPresent` = false)
- solvePNP methods actually handle errors and return empty `PNPResults`
  - This reveals an odd error where some inputs to `solvePNP_SQUARE()` resulted in an estimated transform with NaN values, and attempts to handle it
- Overwrites java changes from #817 since #742 had duplicate fixes
- Minor bugfixes
@amquake amquake mentioned this pull request Oct 10, 2023
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.

None yet

4 participants