Add barcode detector for ML Vision #646

azihsoyn · 2018-06-30T14:16:39Z

No description provided.

bparrishMines · 2018-07-03T00:06:34Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+      visionImage.imageFile.path,
+    );
+
+    final List<TextBlock> blocks = <TextBlock>[];


I believe you want List<VisionBarcode> barcodes <VisionBarcode>[]; right?

Woops... It's my fault.

bparrishMines · 2018-07-03T14:11:58Z

packages/firebase_ml_vision/ios/Classes/BarcodeDetector.m

+                          return;
+                        }
+
+                        // Scaned barcode


Small nitpick:
"Scanned"

bparrishMines · 2018-07-03T14:13:18Z

packages/firebase_ml_vision/ios/Classes/BarcodeDetector.m

+static FIRVisionBarcodeDetector *barcodeDetector;
+
+ (void)handleDetection:(FIRVisionImage *)image result:(FlutterResult)result {
+  if (barcodeDetector == nil) {


Small nitpick:
I believe if (barcodeDetector) { should work.

Did you mean if (!barcodeDetector) { ?

bparrishMines · 2018-07-03T14:21:49Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+  final VisionBarcodeCalendarEvent calendarEvent;
+  final VisionBarcodeDriverLicense driverLicense;
+
+  VisionBarcode._(Map<dynamic, dynamic> _data)


Typically, we place the constructor at the top of the class in dart.

bparrishMines · 2018-07-03T14:24:07Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+          _data['width'],
+          _data['height'],
+        ),
+        rawValue = _data['raw_value'] != null ? _data['raw_value'] : null,


If you're aim is to set rawValue to null if _data['raw_value'] equals null. I believe you can just use the line rawValue = _data['raw_value'],

You are right!

bparrishMines · 2018-07-03T14:24:29Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+          _data['height'],
+        ),
+        rawValue = _data['raw_value'] != null ? _data['raw_value'] : null,
+        displayValue =


same as rawValue

bparrishMines · 2018-07-03T14:32:16Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

  }
 }

-class BarcodeDetectorOptions {}
+class VisionBarcode {


Could you add dartdoc documentation explaining what the class is for? Also could you add documentation for all the public variables?

Typically, you can just copy and paste the documentation used on the firebase website for Barcodes.

You can look at TextDetector as well for documentation examples.

bparrishMines · 2018-07-03T14:34:27Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+            : VisionBarcodeDriverLicense._(_data['driver_license']);
+}
+
+// ios:


Instead of the links, you can just copy and paste their documentation, such as

'This option specifies the barcode formats that the library should detect.'

Also Dartdoc documentation uses /// instead of //

bparrishMines · 2018-07-03T14:35:25Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+// ios
+//   https://firebase.google.com/docs/reference/ios/firebasemlvision/api/reference/Classes/FIRVisionBarcodeEmail
+// android
+//   https://firebase.google.com/docs/reference/android/com/google/firebase/ml/vision/barcode/FirebaseVisionBarcode.Email


Same as VisionBarcodeFormat documentation comment

bparrishMines · 2018-07-03T14:37:17Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+class VisionBarcodeEmail {
+  VisionBarcodeEmail._(Map<dynamic, dynamic> data)
+      : type = VisionBarcodeEmailType.values.elementAt(data['type']),
+        address = data['address'] != null ? data['address'] : null,


For address, body, and subject. You should just be able to use variableName = data['variableName']

bparrishMines · 2018-07-03T14:41:01Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+  final VisionBarcodeEmailType type;
+}
+
+enum VisionBarcodeEmailType {


Could you add a little documentation for the enums in the class as well. The enum is straight forward so you could just simply use /// The type of email for [VisionBarcodeEmail]

azihsoyn · 2018-07-03T15:01:24Z

Thank you for your kind review!

I'll fix documentation and test code!

azihsoyn · 2018-07-04T14:12:33Z

Yay! I finally did it!

bparrishMines · 2018-07-09T18:28:47Z

packages/firebase_ml_vision/test/firebase_ml_vision_test.dart

+        ]);
+
+        final VisionBarcode barcode = barcodes[0];
+        expect(barcode.boundingBox, const Rectangle<num>(1, 2, 3, 4));


This should be Rectangle<int>, right? Can you change all the Point<num> to Point<int> as well?

Yes. I referred TextDetector test. (https://github.com/flutter/plugins/blob/master/packages/firebase_ml_vision/test/firebase_ml_vision_test.dart#L100)

bparrishMines · 2018-07-09T19:22:00Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

@@ -4,18 +4,599 @@

 part of firebase_ml_vision;

+/// Detector for performing optical character recognition(OCR) on an input image.


I believe this was copied from the TextDetector. Can you update this for the barcode detector?

bparrishMines · 2018-07-09T19:22:33Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

@@ -4,18 +4,599 @@

 part of firebase_ml_vision;

+/// Detector for performing optical character recognition(OCR) on an input image.
+///
+/// A barcode detector is created via getVisionBarcodeDetector() in [FirebaseVision]:


barcodeDetector()

bparrishMines · 2018-07-09T19:23:24Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart


+  /// Closes the barcode detector and release its model resources.


'release' => 'releases'. I made the same mistake on TextDetector :)

bparrishMines · 2018-07-09T19:23:57Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

  }

+  /// Detects barcode in the input image.
+  ///
+  /// The OCR is performed asynchronously.


Does the barcode detector use OCR?

bparrishMines

The tests look great! I added more comments mainly discussing some small documentation changes.

bparrishMines · 2018-07-09T19:25:45Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+  final Rectangle<int> boundingBox;
+
+  /// Returns barcode value as it was encoded in the barcode.
+  /// Structured values are not parsed, for example: 'MEBKM:TITLE:Google;URL://www.google.com;;'.


Leave a space between the top comment and the rest of the comments. Effective dartdocs Do the same for similar comments below.

bparrishMines · 2018-07-09T19:29:29Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+  /// Returns null if nothing found.
+  final String rawValue;
+
+  /// Returns barcode value in a user-friendly format.


Since this is a variable, I believe it is better to say The barcode value in a user-friendly format. Returns implies that it is a method. Same for similar comments on variables below.

You're right.

bparrishMines · 2018-07-09T19:31:10Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+  /// and implement your own parsing logic.
+  final VisionBarcodeValueType valueType;
+
+  /// Gets parsed email details (set iff [valueType] is [VisionBarcodeValueType.Email].


Missing closing parentheses at the end. You can remove Gets and just have Parsed email details ...

bparrishMines · 2018-07-09T19:35:28Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+        body = data['body'],
+        subject = data['subject'];
+
+  /// Gets email's address.


Remove Gets. Same for body and subject

Using The email's address would be a good choice.

bparrishMines · 2018-07-09T19:40:03Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

  }
 }

-class BarcodeDetectorOptions {}
+/// Represents a single recognized barcode and its value.
+class VisionBarcode {


I think using Barcode rather than VisionBarcode would be sufficient. It would also match the other Detectors.

I think you can remove Vision from all the other classes as well, since it makes the class names longer, but does not help in terms of clarity.

I was wondering what to do about it! I fix it😁

bparrishMines · 2018-07-10T19:45:45Z

packages/firebase_ml_vision/ios/Classes/BarcodeDetector.m

+    [emails addObject:@{
+      @"address" : email.address,
+      @"body" : email.body,
+      @"subjec" : email.subject,


bparrishMines · 2018-07-10T19:47:51Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+            ? null
+            : BarcodeDriverLicense._(_data['driver_license']);
+
+  /// Gets the bounding rectangle of the detected barcode.


I think you forgot to remove this "Gets"

bparrishMines · 2018-07-10T19:48:12Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+  /// Returns null if the bounding rectangle can not be determined.
+  final Rectangle<int> boundingBox;
+
+  /// Returns barcode value as it was encoded in the barcode.


I think you forgot to remove this "Returns"

bparrishMines · 2018-07-10T19:49:55Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+/// Barcode format constants - enumeration of supported barcode formats.
+class BarcodeFormat {
+  final int value;
+  const BarcodeFormat._(this.value);


nitpicky: Constructor should be at the top.

bparrishMines · 2018-07-10T19:50:11Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+
+/// Barcode format constants - enumeration of supported barcode formats.
+class BarcodeFormat {
+  final int value;


Forgot comment for this variable.

bparrishMines · 2018-07-10T19:56:47Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+            BarcodeWiFiEncryptionType.values.elementAt(data['encryption_type']);
+
+  final String ssid;
+  final String password;


Can you include comments for ssid and password variables?

bparrishMines · 2018-07-10T19:56:49Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+  final String ssid;
+  final String password;
+
+  /// Gets the encryption type of the WIFI


bparrishMines · 2018-07-10T19:57:07Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+      : latitude = data['latitude'],
+        longitude = data['longitude'];
+  final double latitude;
+  final double longitude;


Can you include comments for latitude and longitude variables?

bparrishMines · 2018-07-10T20:02:04Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+        jobTitle = data['job_title'],
+        organization = data['organization'];
+
+  /// Gets contact person's addresses.


You use Gets for the variables in this class and all the classes below this one.

bparrishMines · 2018-07-10T20:30:31Z

packages/firebase_ml_vision/lib/src/barcode_detector.dart

+  /// The four corner points in clockwise direction starting with top-left.
+  ///
+  /// Due to the possible perspective distortions, this is not necessarily a rectangle.
+  final List<Point<int>> cornerPoints;


For the classes that use a List variable, we don't want the user to be able to modify the list. final only keeps cornerPoints from being assigned a new List.

I typically just create a new list with a getter:

final List<Point<int>> _cornerPoints; List<Point<int>> get cornerPoints => List<Point<int>>.from(_cornerPoints);

j0nscalet · 2018-07-12T21:57:11Z

@azihsoyn Happy to help with this if need be, based on the review looks like it's on the 5 yard line though. (Been using your unofficial library, made to switch the the official one, only to realize barcode detector is not implemented 😆)

azihsoyn · 2018-07-12T23:30:16Z

Hi, @j0nscalet!

My unofficial library also implemented barcode detector (of course the other detector).

However, your switch is correct 😁

bparrishMines

Good to submit

azihsoyn · 2018-07-13T00:55:54Z

Great thanks @bparrishMines!

j0nscalet · 2018-07-13T17:20:39Z

This is sweet! Thank you @bparrishMines and @azihsoyn for your hard work on this. Our team will put it to good use. 😄 💪

implement android barcode detector

1c7ab5e

googlebot added the cla: yes label Jun 30, 2018

azihsoyn added 12 commits June 30, 2018 23:41

implement barcode scan for iOS

b54eacb

format

e6f7d46

format

60e4b58

format

ebfa762

format

23d3b91

format

d6ac947

analyze

8d41bf5

analyze

2e8ca15

travis format

3ae514c

format

a6e02b6

format

b22051f

format

a4ba472

azihsoyn changed the title ~~[WIP] Add barcode detector for ML Vision~~ Add barcode detector for ML Vision Jun 30, 2018

bparrishMines self-requested a review July 2, 2018 16:25

bparrishMines reviewed Jul 3, 2018

View reviewed changes

azihsoyn added 3 commits July 3, 2018 12:03

fix android

cfcd828

fix for iOS

469c578

implement barcode painter

8900078

bparrishMines reviewed Jul 3, 2018

View reviewed changes

fixed redundant code acording to review

a21f575

azihsoyn added 4 commits July 4, 2018 00:57

add documentation

6ea198d

add comment and fix format

5275351

add test and fix detector response

bea5e75

add test and fix implement

6806621

azihsoyn mentioned this pull request Jul 4, 2018

Fix barcode_detector azihsoyn/flutter_mlkit#30

Closed

fix analyze

9e10a96

kroikie added the flutterfire label Jul 9, 2018

bparrishMines reviewed Jul 9, 2018

View reviewed changes

bparrishMines mentioned this pull request Jul 9, 2018

Setup detectors to be called with options #652

Merged

azihsoyn added 2 commits July 10, 2018 23:11

fix doc comments

b3b691c

remove Vision prefix

6107e49

bparrishMines reviewed Jul 10, 2018

View reviewed changes

fix bug and docs

5632606

bparrishMines approved these changes Jul 13, 2018

View reviewed changes

bparrishMines merged commit 3edd01b into flutter:master Jul 13, 2018

azihsoyn deleted the feature/barcode_detector branch July 13, 2018 00:53

kmorkos pushed a commit to kmorkos/plugins that referenced this pull request Jul 19, 2018

Add barcode detector for ML Vision (flutter#646)

c96648a

julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020

Add barcode detector for ML Vision (flutter#646)

72ab756

		@@ -4,18 +4,599 @@

		part of firebase_ml_vision;

		/// Detector for performing optical character recognition(OCR) on an input image.


		/// Closes the barcode detector and release its model resources.

Add barcode detector for ML Vision #646

Add barcode detector for ML Vision #646

Conversation

azihsoyn commented Jun 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bparrishMines Jul 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azihsoyn commented Jul 3, 2018

azihsoyn commented Jul 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bparrishMines left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j0nscalet commented Jul 12, 2018 • edited Loading

azihsoyn commented Jul 12, 2018

bparrishMines left a comment

Choose a reason for hiding this comment

azihsoyn commented Jul 13, 2018

j0nscalet commented Jul 13, 2018

bparrishMines Jul 3, 2018 •

edited

Loading

bparrishMines left a comment •

edited

Loading

j0nscalet commented Jul 12, 2018 •

edited

Loading