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

UPC_A support on iOS? #176

Closed
timdmackey opened this issue Dec 6, 2018 · 22 comments
Closed

UPC_A support on iOS? #176

timdmackey opened this issue Dec 6, 2018 · 22 comments

Comments

@timdmackey
Copy link

With the demo app, even though the app is set to scan EAN_13 codes, it still scans UPC_A codes and just adds a zero to the beginning of them. In fact, this isn't surprising given that EAN_13 is a superset of UPC_A, and 12-digit UPC_A codes can be converted to an equivalent EAN_13 code by prepending a zero to the code.

This leads me to wonder, why is it that UPC_A codes aren't supported on iOS? What is preventing it? Is it possible to add support? Or should I be using EAN_13 for both, and treating all UPC_A codes as EAN_13?

@EddyVerbruggen
Copy link
Owner

I think it actually does support UPC_A because of what you mentioned. So with EAN_13 enabled you can also scan UPC_A, according to this: https://developer.apple.com/documentation/avfoundation/avmetadataobjecttypeean13code?language=objc

Do you think updating the readme would suffice?

@timdmackey
Copy link
Author

timdmackey commented Dec 6, 2018 via email

@timdmackey
Copy link
Author

timdmackey commented Dec 6, 2018 via email

@EddyVerbruggen
Copy link
Owner

Interesting findings, thanks. I'm personally not interested in UPC-A so don't hold your breath for me doing this any time soon, but I'll flag it as 'help wanted' and perhaps on a rainy day I can take a stab at it myself.

@timdmackey
Copy link
Author

timdmackey commented Dec 6, 2018 via email

@EddyVerbruggen EddyVerbruggen added this to the 3.0.0 milestone Jan 9, 2019
EddyVerbruggen added a commit that referenced this issue Jan 9, 2019
@EddyVerbruggen
Copy link
Owner

This is now past of 3.0.0 which I'll likely publish tomorrow.

Details: as you suggested I've compared and aligned with Android, and have done what's mentioned at that SO post you linked to. Seems to work pretty well.

Because this is breaking change (a leading '0' will be dropped for EAN_13 codes which are actually UPC_A codes) so I decided to bump the major version.

@timdmackey
Copy link
Author

Ooh, fantastic! Thanks for doing this.

@timdmackey
Copy link
Author

Looking over this, I think it would be better if the zero was only stripped when UPC_A is specified in the formats value. If the format is specified as EAN_13, then the zero should be kept.

Since the two codes are functionally equivalent, the difference comes down to how the barcode value is stored and displayed. A developer who is working with EAN_13 will expect that their data always has 13 digits, while a developer working with UPC_A only will expect 12 digits.

Of interest, I came across this article today (Are UPC and EAN the Same), which give a lengthy explanation of how and why UPC_A is equivalent to EAN_13. This image from the article is particularly helpful, showing how the barcode is identical and only the printed numbers or the data stored in the computer system are different:
image

@timdmackey
Copy link
Author

With this change, I suggest also making a note in the documentation that you cannot use both UPC_A and EAN_13 at the same time, that you must choose one or the other.

EddyVerbruggen added a commit that referenced this issue Jan 10, 2019
@EddyVerbruggen
Copy link
Owner

@timdmackey Thanks for the great assistance!

I actually intended to check the formats the develop passed in but forgot about it, so thanks!

The best way to be consistent with Android is reporting UPC_A when a leading 0 is detected and either no formats have been requested ("scan everything") or UPC_A has been requested explicitly. If you also specify EAN_13 then still the 0will be stripped when the plugin thinks it's actually UPC_A. Again, that's how Android behaves as well.

Note that this no longer is a "breaking change" so bumping to 2.8.0 will suffice. Only users who don't pass in formats (not recommended anyway) may now get an "EAN_13" reported as UPC_A, with one less 0. Not a biggie IMO.

@timdmackey
Copy link
Author

timdmackey commented Jan 10, 2019

The best way to be consistent with Android is reporting UPC_A when a leading 0 is detected and either no formats have been requested ("scan everything") or UPC_A has been requested explicitly.

That sounds good

If you also specify EAN_13 then still the 0 will be stripped when the plugin thinks it's actually UPC_A. Again, that's how Android behaves as well.

Do you mean if the developer specifies EAN_13 and not UPC_A that the zero will still be stripped? I'm surprised that's what Android does, but if so it's important to keep things consistent between iOS and Android if you're going to have this feature.

@EddyVerbruggen
Copy link
Owner

EddyVerbruggen commented Jan 10, 2019

Argh! To complicate things, I just found out that on Android, when specifying formats as EAN_13 it will still scan UPC_A and report it as UPC_A, with a stripped leading 0. So that's actually what I had on iOS as well, before on my last commit. 🤔

@EddyVerbruggen
Copy link
Owner

I'm going to see what ML Kit does under similar circumstances, and report back here.

@timdmackey
Copy link
Author

Argh, that's frustrating. I wonder if having a special flag for it would be a solution? Something like:

ConvertUPCAtoEAN13: true / false

If you don't want to break anything, you can let the default behaviour follow Android (strip the zero), but allow the developer to keep 13 digits if they use the flag.

@timdmackey
Copy link
Author

timdmackey commented Jan 10, 2019

Looking at your zxing library port, it appears that the EAN_13 decoder adds a leading zero to UPC_A codes: EAN13Reader.java#L117, while the UPC_A decoder returns the code as is and just verifies that the first digit is a zero: UPCAReader.java#L77. As for which scanner is used, I'm unsure where that is determined.

Also, the MultiFormat decoder checks whether UPC_A has been specified as aa format, and only strips the leading zero if UPC_A has been requested. Otherwise, it leaves the code as 13 digits: MultiFormatUPCEANReader.java

@EddyVerbruggen
Copy link
Owner

EddyVerbruggen commented Jan 10, 2019

There's quite a difference between these two plugins, and I like what ML Kit does better, because it does exactly what you'd expect. The differences are:

  • ML Kit doesn't scan UPC_A if only EAN_13 is specified, and vice versa.
  • This plugin does, and when EAN_13 is specified while UPC_A is scanned, iOS reports it as EAN_13 and adds a 0.

See the details below, tested with these barcodes: EAN_13 and UPC_A.

Barcode Scanner plugin

This is how this plugin behaves including the changes for this issue:

Format specified Format scanned Android iOS
UPC_A UPC_A
UPC_A EAN_13
EAN_13 UPC_A ✅reported as UPC_A ❌reported as EAN_13, adds leading 0
EAN_13 EAN_13
EAN_13, UPC_A UPC_A ✅reported as UPC_A ✅reported as UPC_A
EAN_13, UPC_A EAN_13 ✅reported as EAN_13 ✅reported as EAN_13

Firebase ML Kit

Format specified Format scanned Android iOS
UPC_A UPC_A
UPC_A EAN_13 ✅no scan ✅no scan
EAN_13 UPC_A ✅no scan ✅no scan
EAN_13 EAN_13
EAN_13, UPC_A UPC_A ✅reported as UPC_A ✅reported as UPC_A
EAN_13, UPC_A EAN_13 ✅reported as EAN_13 ✅reported as EAN_13

Now what?

When changing the implementation to my previous commit (returning scanned EAN_13 codes on iOS as UPC_A in case the decoded result has a leading 0), we get this:

Format specified Format scanned Android iOS
UPC_A UPC_A
UPC_A EAN_13 ✅reported as EAN_13 ✅reported as EAN_13
EAN_13 UPC_A ✅reported as UPC_A ✅reported as UPC_A
EAN_13 EAN_13
EAN_13, UPC_A UPC_A ✅reported as UPC_A ✅reported as UPC_A
EAN_13, UPC_A EAN_13 ✅reported as EAN_13 ✅reported as EAN_13
ANY UPC_A ✅reported as UPC_A ✅reported as UPC_A
ANY EAN_13 ✅reported as EAN_13 ✅reported as EAN_13

@timdmackey Do you have any concerns with specific barcodes (other than the two examples I tested with)?

Btw, I also tried this barcode and this one, and on both iOS and Android, with both this plugin and ML Kit they now get reported as UPC_A with value 052600112751.

@timdmackey
Copy link
Author

timdmackey commented Jan 10, 2019

Oh wow, ML Kit must be reading not just the scanlines but also the digits? I don't think there's any other way to distinguish the two!
Durr, that's not true. This whole discussion has been about how UPCA has an equivalent EAN codes, so of course it can be identified by just the value! Brainfart.

EddyVerbruggen added a commit that referenced this issue Jan 10, 2019
@timdmackey
Copy link
Author

timdmackey commented Jan 10, 2019

Good call testing those other codes with extra zeros.

I think what you have now is good. The Firebase ML Kit implementation makes sense, so I think it probably makes sense for this plugin to behave similarly. i.e.- always report UPC_A as UPC_A, and don't add a leading zero. If a developer wants to treat UPC_A as a 13-digit EAN, it's easy enough for them to add the digit themselves.

edit: answered my own question about the last chart—specifying EAN or UPC enables both codes

@timdmackey
Copy link
Author

Took a look at the new commit, and it looks great. Thanks for working through this!

@timdmackey
Copy link
Author

I guess this is a breaking change again! I think this is the correct solution though.

Because this is breaking change (a leading '0' will be dropped for EAN_13 codes which are actually UPC_A codes) so I decided to bump the major version.

@EddyVerbruggen
Copy link
Owner

You're right, I decided to go with 3.0.0 again (and just published it to npm). Thanks a million for all the support with this nasty issue!

@timdmackey
Copy link
Author

Happy to help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants