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

added code format to see type of code scanned #633

Merged
merged 8 commits into from
Apr 30, 2024

Conversation

android-imdad
Copy link
Contributor

Summary

Added codeFormat to see what kind of code is being scanned, useful when there is code specific validation that needs to be done.

How did you test this change?

Tested example apps on both IOS and Android, no visual change for screenshot.

Copy link
Contributor

@DavidBertet DavidBertet left a comment

Choose a reason for hiding this comment

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

I was planning to do it next week, thanks @android-imdad!

A few feedbacks, but it's looking great. Can you also check the overall syntax, spaces, blank lines, file header, to be consistent with other places?

ios/ReactNativeCameraKit/SimulatorCamera.swift Outdated Show resolved Hide resolved
android/src/main/java/com/rncamerakit/CodeFormat.kt Outdated Show resolved Hide resolved
src/Camera.d.ts Outdated Show resolved Hide resolved
@android-imdad
Copy link
Contributor Author

Hi, yes will check these comments in a few days

android-imdad and others added 3 commits February 18, 2024 11:50
Changed supported barcode types to list of CodeFormat

Co-authored-by: David Bertet <[email protected]>
Added annotation for int type

Co-authored-by: David Bertet <[email protected]>
@android-imdad
Copy link
Contributor Author

@DavidBertet I fixed the indentation, accepted your changes and added types. Please review and let me know if there are any concerns.

@DavidBertet
Copy link
Contributor

@DavidBertet I fixed the indentation, accepted your changes and added types. Please review and let me know if there are any concerns.

Thanks @android-imdad. Have you tried it? I doubt the iOS part compiles at all.

When I said to replace AVMetadataObject.ObjectType by CodeFormat, there is more than just changing a type. It has to be reflected all along the chain. On both camera & simulator, as well as in CameraProtocol

@android-imdad
Copy link
Contributor Author

@DavidBertet I fixed the indentation, accepted your changes and added types. Please review and let me know if there are any concerns.

Thanks @android-imdad. Have you tried it? I doubt the iOS part compiles at all.

When I said to replace AVMetadataObject.ObjectType by CodeFormat, there is more than just changing a type. It has to be reflected all along the chain. On both camera & simulator, as well as in CameraProtocol

My bad, nope didn't try it after the change. Will test it and do a commit with the fixes.

@android-imdad
Copy link
Contributor Author

@DavidBertet Please check now, I made the changes and tested in both IOS and Android, seems to be working as expected.

Copy link
Contributor

@DavidBertet DavidBertet left a comment

Choose a reason for hiding this comment

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

I couldn't test for now, 2 minor feedbacks but the code looks right.

Thanks @android-imdad for your contribution!

metadataOutput.metadataObjectTypes = filteredTypes
let filteredTypes = supportedBarcodeType.filter { type in availableTypes.contains(type.toAVMetadataObjectType()) }

metadataOutput.metadataObjectTypes = filteredTypes.map { $0.toAVMetadataObjectType() }
Copy link
Contributor

Choose a reason for hiding this comment

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

We are doing toAVMetadataObjectType twice for the same objects. Something like that should do the same

let filteredTypes = supportedBarcodeType
  .map { $0.toAVMetadataObjectType() }
  .filter { availableTypes.contains($0) }

metadataOutput.metadataObjectTypes = filteredTypes

@@ -184,14 +186,16 @@ class CameraView: UIView {

// Scanner
if changedProps.contains("scanBarcode") || changedProps.contains("onReadCode") {
let supportedFormats = supportedBarcodeType.map { CodeFormat.fromAVMetadataObjectType($0) }
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use CodeFormat for supportedBarcodeType line 23, so no need to map it here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes I should have just done that :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DavidBertet check now

src/Camera.d.ts Outdated Show resolved Hide resolved
@scarlac scarlac merged commit 234e7f8 into teslamotors:master Apr 30, 2024
3 of 4 checks passed
@scarlac
Copy link
Collaborator

scarlac commented Apr 30, 2024

Thanks @android-imdad and @DavidBertet !

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.

3 participants