Conversation
|
I recommend renaming the pipeline from "Capstone" to "TeamShippingElement" ("TSE" for a shorter one?) |
I was thinking about that but then I realized FFTSEDetector would not look like the best name for a class. Or we could just separate our vision out into different packages for each season and I could Remove the FF. |
JarnaChao09
left a comment
There was a problem hiding this comment.
Everything else looks fine, the math checks out (assuming raw moments), should be ready to merge once the 3.2 and 1.6 get resolved
|
|
||
| public Placement getPlacement() { | ||
| if (capstonePipeline.getCentroid() != null) { | ||
| if (capstonePipeline.getCentroid().x > WIDTH - (WIDTH / 3.2)) |
There was a problem hiding this comment.
Explanation of where the 3.2 magic number came from.
Also, will this value change based on camera location? If so, is there a way to make it a tunable value by the user
core/vision/src/main/java/com/arcrobotics/ftclib/vision/FFCapstoneDetector.java
Outdated
Show resolved
Hide resolved
Yeah, I was just messing around and forgot to change it back, so now it is less than 1/3 of the screen is left and greater than 2/3 is right. Also, should I make the value based on a percentage of the screen or just pixels? |
I would say store it in pixels and just make helper functions that take in a percentage instead and then have it calculate it based on that. Some users may want more fine control while others will be like "ehhh screw it, set it to 50%" |
|
Just made a change. |
core/vision/src/main/java/com/arcrobotics/ftclib/vision/FFCapstonePipeline.java
Show resolved
Hide resolved
| // Draws contours onto the frame from the list created above. During this it searches for | ||
| // the biggest contour and saves it to the variable biggest. | ||
| if (hierarchy.size().height > 0 && hierarchy.size().width > 0) { | ||
| MatOfPoint biggest = new MatOfPoint(); | ||
| for (int index = 0; index >= 0; index = (int) hierarchy.get(0, index)[0]) { | ||
| Imgproc.drawContours(frame, contours, index, new Scalar(88, 0, 0), 2); | ||
| if (index == 0) | ||
| biggest = contours.get(index); | ||
| else if (contours.get(index).size().area() > contours.get(index - 1).size().area()) | ||
| biggest = contours.get(index); | ||
| } |
There was a problem hiding this comment.
You should clarify what is meant here by "contours."
Should be ready for release
There was a problem hiding this comment.
Thresholds are always something that should be left as transparent as possible. Giving the user options to define their own thresholds is important. I would look into some way of giving the user a way to define their own threshold calculations, whether it is through a new constructor, or maybe using lambdas for threshold calculation instead.
In line 220 of the detector I have the option to set the threshold. Is that what you mean or do you mean setting the logic of the threshold as well? |
|
no, I was thinking more so you are able to change the formula used. Instead of width / 3, it would be width / 4 and the user could supply this equation through either a lambda or just doing the calculation in their own function and supplying the result. |
* fix readme * fix: contributing guidelines outdated errors * update PR template * Add Upgraded Motor Groups (#164) * feat: upgraded motor groups * fix: add stop motor when button is released * fix: missing follower logic in set() * change x on L106 to motor to be more descriptive Co-authored-by: dansman805 <[email protected]> * change x on L113 to motor to be more descriptive Co-authored-by: dansman805 <[email protected]> * feat: verbose comments on flywheel sample * fix: change NotNull JB annotation to NonNull from androidx * fix: missing comment for clearing the bulk cache in sample Co-authored-by: Purav Datta <[email protected]> Co-authored-by: dansman805 <[email protected]> * add: new method to directly power mecanum drive wheels (#186) * Update Mecanum Sample (#187) * fix: mecanum sample to be readable and useful * add: drawings and descriptions throughout sample * fix: model of robot frame * feat: update SDK dependencies * fix: Bumped eocv to 1.5.0 and fixed openCameraDeviceAsync Impls (#193) * fix: Bumped eocv to 1.5.0 and fixed breaking changes * fix: Changed the vision release version to 2.0.0 * fix: Better error handling of camera opening * fix: Surrounded the callback with the lock * fix: issues with reset in RevIMU (#195) * fix: issues with ramsete controller ending before command runs (#194) * Min and Max flipped. Clarify AngleUnit type in parameter name (#201) min and max arguments flipped on call to constructor. Consider renaming parameter to minDegree and maxDegree to clarify to caller that AngleUnit.DEGREES is assumed. * Introduce Rate Feature to Motor Encoders (#196) * feat: clean up motor group inversion and add encoder rate * fix: typo in javadoc comment * fix: set last state before current state (#204) * Capstone Vision for 2021 Season (#200) * Added capstone detector and pipeline * Updated values on lines 119 and 117 of Capstone Detector * Added setters to tune area of detection * Released hierarchy Mat and now gives definition of contour * Released all matrices and fully tested pipeline Should be ready for release * Vector2d Normalization (#205) * fix: updated issue templates (#203) * feat: modify README and dependencies for v2.0 release (#206) Co-authored-by: Purav Datta <[email protected]> Co-authored-by: dansman805 <[email protected]> Co-authored-by: Lunerwalker2 <[email protected]> Co-authored-by: Kevin Sheck <[email protected]> Co-authored-by: Dolphin2Point1 <[email protected]> Co-authored-by: Ethan Leitner <[email protected]>
Pull Requests
Please note that we accept pull requests from anyone, but that does not mean it will be merged.
What kind of change does this PR introduce?
This PR adds a pipeline and detector people can use in order to find the position of their capstones. The pipeline will look for the largest contour and return its center point. The detector will then determine the position of the objects based on the center points relative to their position on the screen.
Did this PR introduce a breaking change?
A breaking change includes anything that breaks backwards compatibility either at compile or run time.
Please make sure your PR satisfies the requirements of the contributing page