-
Notifications
You must be signed in to change notification settings - Fork 139
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
Fix need to rotate android device to landscape to scan 1D barcodes, also fix focus and auto focus not being implemented #52
base: main
Are you sure you want to change the base?
Conversation
Looks like a nice start, i need to take a closer look :) |
Can this get merged please? |
Hey, just curious if this is ready to be merged in or if there is some physical testing that needs to be done let me know and I am willing to do some testing on physical hardware. |
@Captnwalker1 The indentation looks wonky on Github. You might want to run the file through a formatter. |
var factory = new SurfaceOrientedMeteringPointFactory(1f, 1f); | ||
var fpoint = factory.CreatePoint(.5f, .5f); | ||
var action = new FocusMeteringAction.Builder(fpoint,FocusMeteringAction.FlagAf) | ||
//.DisableAutoCancel() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either remove this or uncomment it.
@@ -63,6 +65,7 @@ public void Connect() | |||
// Frame by frame analyze | |||
imageAnalyzer = new ImageAnalysis.Builder() | |||
.SetDefaultResolution(new Android.Util.Size(640, 480)) | |||
.SetOutputImageRotationEnabled(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be set based on some input?
I don't want it to auto-rotate if I initialise this:
new CameraBarcodeReaderView
{
Options = new BarcodeReaderOptions
{
AutoRotate = false,
Multiple = false,
}
}
@@ -71,9 +74,64 @@ public void Connect() | |||
|
|||
UpdateCamera(); | |||
|
|||
}), ContextCompat.GetMainExecutor(Context.Context)); //GetMainExecutor: returns an Executor that runs on the main thread. | |||
AutoFocus(); | |||
setupAutoFocusTimer(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like it adds a continuous autofocus that's never canceled, even when you explicitly call one of the other functions.
As much as I need part of this functionality, I think this PR needs rework. It adds three separate features in one PR.
Also the current implementation seems messy:
This should probably be at least three separate PRs The commit history of this PR is also quite dirty. Multiple commits whose changes cancel out should never exist in a PR, and be rewritten. |
Will anyone re-work this PR as @Ghostbird requested? I need the fix for the barcode so we won't need to rotate the device in order to scan a simple barcode. |
After I get some obligations off my plate, if this is still has no movement, I may take a look at it and give it a shot. However, if someone else wants to take it, please do. |
Simple fix using android ImageAnalysis Builder to auto-rotate images if needed.
https://developer.android.com/reference/androidx/camera/core/ImageAnalysis.Builder#setOutputImageRotationEnabled(boolean)
Implement Focus and AutoFocus via TimerTask