Adding ability to consume barcodes in BarcodeCaptureActivity #267

adam-hurwitz · 2017-09-16T00:24:46Z

Hi Google Team!

I tinkered with the camera’s barcode detection and scanning capabilities in order to create the ScannerApp sample showcasing smooth transitioning between barcode scanning modes while keeping the camera running. This results in seamless toggling between barcode types which is also useful for switching between any type of camera detection.

This commit reflects a portion of the modifications I made to the Google Vision sample app in order to return the barcode results on the Activity level via an interface. This allows seamless toggling between scanner types in the ScannerApp because then the returned data is passed down to the child ViewPager to show for the corresponding scanner view.

Here is a more in-depth breakdown of this use case. I have conducted manual QA testing and do not see any UI bugs or errors in the logs. I'd love your feedback.

Thanks!

Adam

googlebot · 2017-09-16T00:24:49Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

adam-hurwitz · 2017-09-16T00:34:10Z

I signed it!

On Fri, Sep 15, 2017 at 5:24 PM googlebot ***@***.***> wrote: Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 *Please visit https://cla.developers.google.com/ <https://cla.developers.google.com/> to sign.* Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks. ------------------------------ - If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data <https://cla.developers.google.com/clas> and verify that your email is set on your git commits <https://help.github.com/articles/setting-your-email-in-git/>. - If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. - In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#267 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADwXHHOvPhdHw2yA914opRDlC52x45Bsks5sixVWgaJpZM4PZpCB> .

-- ASH

googlebot · 2017-09-16T00:34:12Z

CLAs look good, thanks!

liuyl · 2017-09-21T16:58:07Z

...src/main/java/com/google/android/gms/samples/vision/barcodereader/BarcodeGraphicTracker.java

@@ -26,25 +29,43 @@
 * to an overlay, update the graphics as the item changes, and remove the graphics when the item
 * goes away.
 */
-class BarcodeGraphicTracker extends Tracker<Barcode> {
+public class BarcodeGraphicTracker extends Tracker<Barcode> {


Can this still be package-private?

This needs to be public in order for an Activity to implement the UpdateBarcodeListener interface within BarcodeGraphicTracker.

liuyl · 2017-09-21T17:02:24Z

...src/main/java/com/google/android/gms/samples/vision/barcodereader/BarcodeGraphicTracker.java

-    BarcodeGraphicTracker(GraphicOverlay<BarcodeGraphic> overlay, BarcodeGraphic graphic) {
-        mOverlay = overlay;
-        mGraphic = graphic;
+    public UpdateBarcodeListener updateBarcodeListener;


Please remove the blank line above, make the variable private-access, and rename the variable to mUpdateBarcodeListener here and below.

liuyl · 2017-09-21T17:06:23Z

...src/main/java/com/google/android/gms/samples/vision/barcodereader/BarcodeGraphicTracker.java

     */
    @Override
    public void onNewItem(int id, Barcode item) {
+


Please remove this redundant blank line, as well as the one at the end of this method.

liuyl · 2017-09-21T17:07:59Z

...src/main/java/com/google/android/gms/samples/vision/barcodereader/BarcodeGraphicTracker.java

+
+    public interface UpdateBarcodeListener {
+        @UiThread
+        void getBarcode(Barcode barcode);


Please add Javadoc for this method.

liuyl · 2017-09-21T17:08:04Z

...src/main/java/com/google/android/gms/samples/vision/barcodereader/BarcodeGraphicTracker.java

        mGraphic.setId(id);
+
+        updateBarcodeListener.getBarcode(item);


The name "getBarcode" is kind of confusing. If it's only called when a new Barcode is detected, please consider rename it to "onNewBarcode" or something more self explanatory.

liuyl · 2017-09-21T17:08:43Z

...src/main/java/com/google/android/gms/samples/vision/barcodereader/BarcodeTrackerFactory.java

@@ -24,17 +26,22 @@
 * Factory for creating a tracker and associated graphic to be associated with a new barcode.  The
 * multi-processor uses this factory to create barcode trackers as needed -- one for each barcode.
 */
-class BarcodeTrackerFactory implements MultiProcessor.Factory<Barcode> {
-    private GraphicOverlay<BarcodeGraphic> mGraphicOverlay;
+public class BarcodeTrackerFactory implements MultiProcessor.Factory<Barcode> {


Please keep it package-private if possible.

In order to keep a project's classes organized the BarcodeTrackerFactory is public so it can live in a separate package for the Camera utilities and then the Activities / Fragments can access these methods from a separate package.

In the sample below, the BarcodeActivity implements BarcodeGraphicTracker and BarcodeTrackerFactory from a parent package which is why they are public.

liuyl · 2017-09-21T17:09:18Z

...src/main/java/com/google/android/gms/samples/vision/barcodereader/BarcodeTrackerFactory.java

-    private GraphicOverlay<BarcodeGraphic> mGraphicOverlay;
+public class BarcodeTrackerFactory implements MultiProcessor.Factory<Barcode> {
+    private GraphicOverlay<BarcodeGraphic> graphicOverlay;
+    Context context;


private-access please.

liuyl · 2017-09-21T17:09:53Z

...src/main/java/com/google/android/gms/samples/vision/barcodereader/BarcodeTrackerFactory.java

    }

    @Override
    public Tracker<Barcode> create(Barcode barcode) {
-        BarcodeGraphic graphic = new BarcodeGraphic(mGraphicOverlay);
-        return new BarcodeGraphicTracker(mGraphicOverlay, graphic);
+


Please remove this blank line and the blank line at the end of the method.

liuyl · 2017-09-21T17:11:16Z

...src/main/java/com/google/android/gms/samples/vision/barcodereader/BarcodeGraphicTracker.java

+        this.mGraphic = mGraphic;
+
+        if (context instanceof UpdateBarcodeListener) {
+            updateBarcodeListener = (UpdateBarcodeListener) context;


this.mUpdateBarcodeListener = (UpdateBarcodeListener) context;

liuyl · 2017-09-21T17:18:21Z

...rc/main/java/com/google/android/gms/samples/vision/barcodereader/BarcodeCaptureActivity.java

@@ -57,7 +57,7 @@
 * rear facing camera. During detection overlay graphics are drawn to indicate the position,
 * size, and ID of each barcode.
 */
-public final class BarcodeCaptureActivity extends AppCompatActivity {
+public final class BarcodeCaptureActivity extends AppCompatActivity implements BarcodeGraphicTracker.UpdateBarcodeListener {


I think "BarcodeUpdateListener" is more nature and consistent with "BarcodeCaptureActivity", "BarcodeGraphicTracker".

adam-hurwitz · 2017-09-22T17:16:23Z

Hi Yulong,

Thank you for the code review! I've made all of the suggested revisions above except for the public vs. package-private adjustment in order to keep the UI and Camera logic in separate packages.

For instance, in order to keep a project's classes organized the BarcodeTrackerFactory is public so it can live in a separate package for the Camera utilities and then the Activities / Fragments can access these methods from a separate package.

In the sample below, the BarcodeActivity implements BarcodeGraphicTracker and BarcodeTrackerFactory from a parent package which is why they are public.

Let me know if any further changes need to be made in order to accept the pull request.

Have a great weekend!

Adam

liuyl · 2017-09-25T18:32:39Z

...src/main/java/com/google/android/gms/samples/vision/barcodereader/BarcodeTrackerFactory.java

@@ -25,16 +27,19 @@
 * multi-processor uses this factory to create barcode trackers as needed -- one for each barcode.
 */
 class BarcodeTrackerFactory implements MultiProcessor.Factory<Barcode> {
-    private GraphicOverlay<BarcodeGraphic> mGraphicOverlay;
+    private GraphicOverlay<BarcodeGraphic> graphicOverlay;
+    private Context context;


I think you accidentally renamed the mGraphicOverlay variable. Can you change it back and also rename context to mContext to be consistent with the rest of the code?

…ng interface too BarcodeGraphicTracker

adam-hurwitz · 2017-09-26T03:50:43Z

Thanks for calling out the variable names. I updated my pull request accordingly.

liuyl · 2017-09-29T19:07:31Z

I merged the pull request, thank you for the contribution!

liuyl reviewed Sep 21, 2017

View reviewed changes

adam-hurwitz force-pushed the master branch from 9a5c618 to 78d1899 Compare September 22, 2017 17:10

liuyl suggested changes Sep 25, 2017

View reviewed changes

Adding ability to consume barcodes in BarcodeCaptureAcitivity by addi…

fe42d73

…ng interface too BarcodeGraphicTracker

adam-hurwitz force-pushed the master branch from 78d1899 to fe42d73 Compare September 26, 2017 03:46

liuyl merged commit 2ce3132 into googlesamples:master Sep 29, 2017

Adding ability to consume barcodes in BarcodeCaptureActivity #267

Adding ability to consume barcodes in BarcodeCaptureActivity #267

Uh oh!

Conversation

adam-hurwitz commented Sep 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

googlebot commented Sep 16, 2017

Uh oh!

adam-hurwitz commented Sep 16, 2017 via email

Uh oh!

googlebot commented Sep 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adam-hurwitz Sep 22, 2017 • 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!

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!

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!

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!

adam-hurwitz commented Sep 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adam-hurwitz commented Sep 26, 2017

Uh oh!

liuyl commented Sep 29, 2017

Uh oh!

Uh oh!

adam-hurwitz commented Sep 16, 2017 •

edited

Loading

adam-hurwitz Sep 22, 2017 •

edited

Loading