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 QRs without a termination byte #80

Merged
merged 2 commits into from
Jul 24, 2018
Merged

Conversation

cozmo
Copy link
Owner

@cozmo cozmo commented Jul 22, 2018

#77 reports an issue scanning a QR code without the termination byte. Feels safe to return the data we have assuming we've completely exhausted the data stream, even if we don't encounter the termination bit. This matches the behavior of ZXIng.

Also added the image as a test case.

@cozmo cozmo requested a review from jefff July 22, 2018 18:14
@cozmo cozmo mentioned this pull request Jul 22, 2018
@cozmo
Copy link
Owner Author

cozmo commented Jul 22, 2018

Hmm, reading this makes it seem like we've misunderstood under what circumstance a termination byte would show up, and also how (aka it could just be 1 0?)

Need to investigate a bit more.

@cozmo
Copy link
Owner Author

cozmo commented Jul 22, 2018

Updated in 93d2eb9 to follow the spec which says

The end of data in the symbol is signalled by the Terminator sequence of 0 bits, as defined in Table 2, appended to the data bit stream following the final mode segment. The Terminator shall be omitted if the data bit stream completely fills the capacity of the symbol, or abbreviated if the remaining capacity of the symbol is less than 4 bits.

@cozmo cozmo merged commit b3bac44 into master Jul 24, 2018
@cozmo cozmo deleted the no-termination-byte branch July 24, 2018 03:41
@nathanhoel
Copy link

@cozmo This fix does not appear in the 1.2 package that was pushed to NPM February 2019 - would you be able to have this added to NPM?
This specific issue is causing a large number of our QR codes to fail because the string we use happens to be really close to the exact capacity of the stream.

@nathanhoel
Copy link

I actually see the problem - the /dist folder is out of date, it has not been updated for 2 years missing many of the changes since then.

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

Successfully merging this pull request may close these issues.

2 participants