fix: Bumped eocv to 1.5.0 and fixed openCameraDeviceAsync Impls by Lunerwalker2 · Pull Request #193 · FTCLib/FTCLib

Lunerwalker2 · 2021-10-15T02:17:17Z

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?

Fix, updates the EasyOpenCv version from 1.4.4 to 1.5.0.

Did this PR introduce a breaking change?

A breaking change includes anything that breaks backwards compatibility either at compile or run time.

Yes, a little: While the EasyOpenCv update contains a breaking change with the openCameraDeviceAsync() method, most uses of that method are contained in the library and not exposed to the user. However, certain vision example programs, specifically _UGContourRingPipelineJavaExample and UGContourRingPipelineKtExample, are not in the library and technically would fall under breaking change?

The docs will need to be updated to include that the user must copy over the new libOpenCvAdndroid453.so from OpenCv-Repacked once this has been pushed out (thx Noah)

Please make sure your PR satisfies the requirements of the contributing page

ItsHarper · 2021-10-15T02:18:40Z

This is also a breaking change in that it requires a new OpenCV native library to be placed on the RC.

Lunerwalker2 · 2021-10-15T02:21:12Z

I believe that someone was going to add that to the docs, although I'll edit it into the PR text to be concise about it, thanks.

JIceberg · 2021-10-15T02:40:53Z

Can you update the README with the new document information and update the FTCLib version in the gradle file to 2.0.0?

Jaiming724 · 2021-10-15T02:48:29Z

Can you also make a note in the readme about no longer needing to upgrade to java version 1.8. Something like, Since SDK 7.0, there is no longer a need to upgrade the java version since it is 1.8 by default.

JIceberg · 2021-10-15T03:33:59Z

Can you also make a note in the readme about no longer needing to upgrade to java version 1.8. Something like, Since SDK 7.0, there is no longer a need to upgrade the java version since it is 1.8 by default.

Different PR 😊

JIceberg · 2021-10-15T15:23:47Z

+            @Override
+            public void onError(int errorCode){
+
+            }


Are we going to do nothing here? Same for the one in the pipeline example.

We can deal with it if we want. From what I can find, there's only two possible errors.

Assuming we want to tell the user about the error, how should we go about it? That detector class doesn't currently take in any telemetry object, only the hardware map. Should we make the init() method return something?

*That particular detector class doesn't have a telemetry; the contour ring one does take a telemetry, but it's not a required argument in that either.

Are these hardfault errors? Like, if they fail, there's no recovery? If there's no possible recovery for these errors, I would throw a runtime exception with the error code. If not, log it using Log.

I don't think they will crash the program, but the camera opening will definitely fail. The two errors shown in the image don't look to me like they will go away if we just try again, like in the CAMERA_OPEN_ERROR_POSTMORTEM_OPMODE, which I assume is thrown if the opening is called during stop() in an opmode.
The other error could be anything, but I would also think that it would just persist if another opening was attempted. Either way, the program is unlikely to be able move on as both of the errors would seem to suggest a failure to start the stream, so we might want to throw a runtime exception.

Returning a boolean is a great idea. I'm a fan of allowing user code to react to errors.

That sounds great. @Lunerwalker2 can you whip up an implementation of that in these detectors? In future detectors as well we'll need to include this style of implementation for error handling.

Sure, I was about to do it. In the UGContourRingDetector, the user passes in an optional telemetry instance. For that class, should I use telemetry.log(); in addition to the global warning?

The warning will show up at the top of the telemetry area, so that seems redundant.

Good point, I'll leave it out then

Lunerwalker2 · 2021-10-16T00:28:11Z

If you see anything wrong here, please do say so because some of it is new territory for me. Thanks @Windwoes for all the help.

Lunerwalker2 · 2021-10-16T01:07:29Z

To clarify on the changes I made, instead of returning a boolean, there is now an internal state enum that represents the detector. The init() function will now only run if the state is NOT_CONFIGURED. Once ran, the state is INITIALIZED. Upon successful onOpened(), the state is changed to RUNNING. Upon an error that calls onError, the state is INIT_FAILURE_NOT_RUNNING.
User code can poll getDetectorState() to see the current state of the detector.

Windwoes · 2021-10-16T01:08:04Z

Isn't this a massive race condition? The return values of these functions can change at any time (whenever a new frame comes in). I think you want to grab a snapshot of these at one moment in time and then run the logic chain.

    public Stack getStack() {
        if (Math.abs(ftclibPipeline.getTopAverage() - ftclibPipeline.getBottomAverage()) < ftclibPipeline.getThreshold()
                && (ftclibPipeline.getTopAverage() <= 100 && ftclibPipeline.getBottomAverage() <= 100)) {
            return Stack.FOUR;
        } else if (Math.abs(ftclibPipeline.getTopAverage() - ftclibPipeline.getBottomAverage()) < ftclibPipeline.getThreshold()
                && (ftclibPipeline.getTopAverage() >= 100 && ftclibPipeline.getBottomAverage() >= 100)) {
            return Stack.ZERO;
        } else {
            return Stack.ONE;
        }
    }

Also why is the camera hardcoded to BACK?

Jaiming724 · 2021-10-16T16:22:57Z

That is a race condition. I do not think we ever ran into an issue because the frames do not change much after randomization. We will keep that in mind for the new pipelines. I could be misinterpreting the issue, but it is set to back by default and the user can set the input to a webcam if they chose.

JIceberg

Approved for EOCV update. Open a new PR for fixing the race condition.

* 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]>

fix: Bumped eocv to 1.5.0 and fixed breaking changes

6f9fd32

fix: Changed the vision release version to 2.0.0

32317a2

JIceberg reviewed Oct 15, 2021

View reviewed changes

fix: Better error handling of camera opening

224a547

fix: Surrounded the callback with the lock

9fd4ace

JIceberg approved these changes Oct 17, 2021

View reviewed changes

Jaiming724 approved these changes Oct 18, 2021

View reviewed changes

Lunerwalker2 merged commit d693786 into FTCLib:dev Oct 18, 2021

Conversation

Lunerwalker2 commented Oct 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Requests

What kind of change does this PR introduce?

Did this PR introduce a breaking change?

Uh oh!

ItsHarper commented Oct 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lunerwalker2 commented Oct 15, 2021

Uh oh!

JIceberg commented Oct 15, 2021

Uh oh!

Jaiming724 commented Oct 15, 2021

Uh oh!

JIceberg commented Oct 15, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ItsHarper Oct 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lunerwalker2 commented Oct 16, 2021

Uh oh!

Lunerwalker2 commented Oct 16, 2021

Uh oh!

Windwoes commented Oct 16, 2021

Uh oh!

Jaiming724 commented Oct 16, 2021

Uh oh!

JIceberg left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Lunerwalker2 commented Oct 15, 2021 •

edited

Loading

ItsHarper commented Oct 15, 2021 •

edited

Loading

ItsHarper Oct 15, 2021 •

edited

Loading