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

Fixed importing QR codes #5747

Merged
merged 2 commits into from
Oct 5, 2023
Merged

Fixed importing QR codes #5747

merged 2 commits into from
Oct 5, 2023

Conversation

grzesiek2010
Copy link
Member

@grzesiek2010 grzesiek2010 commented Sep 19, 2023

Closes #5748

What has been done to verify that this works as intended?

I've tested the changes manually and also added automated tests.

Why is this the best possible solution? Were any other approaches considered?

I don't know how zxing works under the hood but obviously using DecodeHintType.PURE_BARCODE to FALSE caused problems with scanning some QR codes. That parameter seems to be redundant so I juts got rid of it.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

I think we can focus on testing importing QR codes. I was able to reproduce the issue with the one attached on the forum https://forum.getodk.org/t/importing-a-qr-with-odk-collect-does-the-size-of-the-image-matter/43071/1 and with the one I used in tests: https://github.com/getodk/collect/pull/5747/files#diff-08ee19f4a8d2e6438261e7192493809104b12fe010229802bf088ee63723fc09
Please play with settings/generating qr codes and see if you can do the same on your own. Generally, if a QR code is shared by ODK Collect via email for example it works well but if you take a screenshot it's not always the case.

Do we need any specific form for testing your changes? If so, please attach one.

No.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No.

Before submitting this PR, please make sure you have:

  • run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.
  • verified that any new UI elements use theme colors. UI Components Style guidelines

@grzesiek2010 grzesiek2010 marked this pull request as ready for review September 20, 2023 09:34
@grzesiek2010 grzesiek2010 changed the title Added tests Fixed importing QR codes Sep 20, 2023
@dbemke
Copy link

dbemke commented Oct 5, 2023

Tested with Success!

Verified on device with Android 8.1, 10

Verified cases:

@srujner
Copy link

srujner commented Oct 5, 2023

Tested with Success!

Verified on device with Android 5.1, 12,13

@grzesiek2010 grzesiek2010 merged commit 80b7019 into getodk:master Oct 5, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some QR codes can't be recognized when imported
4 participants