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

Misc error checking #70

Merged
merged 3 commits into from
Jun 28, 2018
Merged

Misc error checking #70

merged 3 commits into from
Jun 28, 2018

Conversation

cozmo
Copy link
Owner

@cozmo cozmo commented Jun 28, 2018

Fixes #68 and #53.

We were seeing 2 intermittent errors.

  1. Sometimes getting Uncaught RangeError: Invalid typed array length: Infinity in extract. This was caused by the moduleSize calculation in locate returning 0, so returning a QR of infinite size (edgeLength/moduleSize). We now explicitly check for this case and bail out of locator. This fixes part of Error in chrome 64.0.3282.186 Win 10 #53.
  2. Sometimes we'd return malformed data, either manifesting as random unicode characters or an empty byte array. Both of these cases were caused when we couldn't pull off enough codewords from the matrix. We now explicitly check that we pull off at least the number of codewords as we expect.

No changes to the end to end tests which is good, and have added explicit test cases for these failure cases.

let dimension: number;
let moduleSize: number;
try {
const computedDimension = computeDimension(topLeft, topRight, bottomLeft, matrix);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: You could use destructuring here.

({ dimension, moduleSize } = computeDimension(topLeft, topRight, bottomLeft, matrix));

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I was trying to do that but didn't have the syntax right. Added in a8ceef6.

@@ -62,6 +62,10 @@ function computeDimension(topLeft: Point, topRight: Point, bottomLeft: Point, ma
sum(countBlackWhiteRun(topRight, topLeft, matrix, 5)) / 7
) / 4;

if (moduleSize === 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless 0 is a magic number being returned by something, it'd probably be better to switch this to something like if (moduleSize < 1) { or whatever the smallest reasonable module size is to avoid float rounding issues.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So at least with the test case this was triggering because countBlackWhiteRun was returning [0,0,-1,0,0] which basically means the finder patterns were too small to extract distances from. We could try throwing deeper in the stack but it felt reasonable to just check for an explicit 0 here. Thoughts?

@cozmo cozmo merged commit 428e717 into master Jun 28, 2018
@cozmo cozmo deleted the error-checking branch June 28, 2018 22:10
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.

Incorrectly finds QR codes and returns malformed data
2 participants