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

Add support for multi-eci encoding for PDF417 #1506

Merged
merged 5 commits into from Mar 5, 2022

Conversation

AlexGeller1
Copy link
Contributor

By default multi-ECI encoding is disabled so that the encoding behavior is identical with the previous version.
The implementation shares the classes that convert character input to a byte/ECI stream with the data-matrix encoder.
This PR also fixes an issue that the PDF417 decoder incorrectly decoded input with a leading ECI and no explicit latch to TEXT encoding.

- Fixed issue that the PDF417 decoder incorrectly decoded input with a leading ECI and no explicit latch to TEXT encoding
@srowen
Copy link
Contributor

srowen commented Mar 4, 2022

The default behavior stays the same after this change?

@AlexGeller1
Copy link
Contributor Author

The default behavior stays the same after this change?

Absolutely

@lgtm-com
Copy link

lgtm-com bot commented Mar 4, 2022

This pull request introduces 4 alerts when merging e7313cd into 4bd257e - view on LGTM.com

new alerts:

  • 2 for Spurious Javadoc @param tags
  • 1 for Useless comparison test
  • 1 for Useless null check

@lgtm-com
Copy link

lgtm-com bot commented Mar 4, 2022

This pull request introduces 1 alert when merging 9aec766 into 4bd257e - view on LGTM.com

new alerts:

  • 1 for Useless null check

@srowen
Copy link
Contributor

srowen commented Mar 4, 2022

Have a look at the code alerts, probably really minor stuff

@AlexGeller1
Copy link
Contributor Author

Yes, thanks. Should be good now.

@srowen srowen merged commit 92854d4 into zxing:master Mar 5, 2022
@srowen srowen added this to the 3.5.0 milestone Mar 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants