Added ability to import a barcode from a local image #317
Conversation
| contents = qrCodeResult.getText(); | ||
| format = qrCodeResult.getBarcodeFormat().name(); | ||
| } catch (NotFoundException e) { | ||
| Log.i(TAG, "No barcode was found"); |
There was a problem hiding this comment.
Should this exception be logged? The e.printStackTrace() would not be captured, right?
There was a problem hiding this comment.
No, I don't believe it should be. All it says is that there was no barcode in the image - which isn't truly an exception.
Now that I think about it - maybe a toast would be useful?
The printStacktrace() was left there accidentally. I'll remove it.
| @@ -368,12 +368,21 @@ | |||
| android:padding="10.0dip" | |||
| android:layout_width="fill_parent" | |||
| android:layout_height="wrap_content" | |||
| android:id="@+id/barcodeCaptureLayout"> | |||
| android:id="@+id/barcodeCaptureLayout" | |||
| android:baselineAligned="false"> | |||
There was a problem hiding this comment.
If I understand correctly from
this was likely necessary to get the buttons to align up correctly, correct?
There was a problem hiding this comment.
Yes. The new button spans two lines on narrower devices and the default is to align by text, not by their bounding boxes.
| bitmap = MediaStore.Images.Media.getBitmap(this.getContentResolver(), intent.getData()); | ||
| } catch (IOException e) { | ||
| Log.e(TAG, "Error getting the image data"); | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
Same here, should this be logged instead?
There was a problem hiding this comment.
Right, printStacktrace() is, in fact, logged, but without the TAG. I'll move it into the Log.e call.
|
Pretty cool! Are you up for adding some Robolectric tests for this new feature? Much of the app has automated tests, which helps prevent breakage and regressions. |
| } | ||
|
|
||
| // In order to decode it, the Bitmap must first be converted into a pixel array... | ||
| int[] intArray = new int[bitmap.getWidth()*bitmap.getHeight()]; |
There was a problem hiding this comment.
Do you know what the failure mode is if a user selects an image which "is too big"? For example, in generating the barcode for display I've sometimes encountered issues with memory:
There was a problem hiding this comment.
I don't actually know... I looked around and I'm still not sure about an OOM exception, however I did stumble upon people having issues with codes in huge images not being recognized.
I'll do some more testing and maybe add a downscale step before processing.
|
Sure. I'm not familiar with Robolectric, but I'll look into it. |
|
I'm quite interested in this feature too. I don't normally do Android development but I'm willing to try to help if useful |
|
I got a bunch of uni work dumped on me so I haven't had any time to work on this. It looks like I'll be clear beginning of December, so you can expect this finished around then. @TheLastProject thanks for the offer. I'm not sure GitHub would be very happy about mixing remote branches in one PR, but it's been a while since I've tried. If you can wait the two weeks, it's probably not worth the hassle of setting up Android Studio 😅 |
|
Okay, I'll just wait then :) |
|
I've been working on testing image importing by generating a barcode image, saving it and sending a fake pick intent response to the app. I have discovered, however, that the version of Robolectric used here doesn't actually implement image compression and is as such unable to save images to disk in a way that can read back. It seems that the feature was only implemented in v4.3.1. While the actual migration to v4 and up is not too difficult, it does, however, require compiling on SDK 28 or higher. Should I try to migrate the project to a newer version of Robolectric? |
|
I actually did a migration to Roboelectric 4 in my pull request in the hope it would fix things for me. It sadly didn't, but you may still want to take a look at 0a9611e to save yourself some time if you end up going that path |
The Play Store has a target SDK requirement which updates each year. As of Nov 1 this app will need to compile against SDK 28, else an update cannot be pushed to the Play Store. https://support.google.com/googleplay/android-developer/answer/113469#targetsdk If you would be so kind as to update it to 28 (or 29) that would be much appreciated. I'm also fine with updating the version of Robolectric being used to the latest. |
To follow-up, #334 recently was accepted which updates the SDK version compiled against to 29 and updates Robolectric to 4.0.2. If you are willing to upgrade the version of Robolectric used from 4.0.2 to 4.3.1 go for it. I've not looked into it; hopefully nothing that the existing tests use is deprecated and removed between those two versions. If you run into snags, let me know. |
Adds an "Import from image" button next to "Capture card" that does exactly what was described in #248 - allows the user to pick an image from their device that contains the barcode they want to add.
Signed-off-by:
Miha Frangež<[email protected]>This change is