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

Structured Append #132

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Structured Append #132

wants to merge 8 commits into from

Conversation

kousu
Copy link

@kousu kousu commented Apr 2, 2019

Fixes #128 .

@kousu
Copy link
Author

kousu commented Apr 2, 2019

Here's some open questions:

  • what should the API look for this? The current version adds an optional .structuredAppend to result objects with .M and .N subobjects. "Structured Append" is the technical name from the spec, and it matches what qrencode and zxing-cpp call this feature, but it's very abstract; I would rather it be something like .numbering.page, .numbering.of.
  • the M and N are 1-based (see the QR spec (ISO 18004) section 9.2), which is annoying to work with in Javascript (and most languages). I've conformed to the spec, up-shifting the numbers so that 0<M<=N<=16, but maybe that's a mistake? Or maybe I should upshift N but not M so that you have 0<=M<N<=16, python-style?
  • I couldn't figure out how to use jest to test dependent facts because it wants to take everything as callbacks that it runs in parallel. To get around this I added loadPngSync() and used assert in a couple of places instead of expect(). I'm not proud of this but I couldn't see another way. Help?

…rite "M/N" page numberings.

Some of the output.jsons were hand-massaged to be what they should be,
rather than what generate-test-data currently would give,
because this feature doesn't work yet.

amen.mp3 is public domain from
https://sampleswap.org//samples-ghost/DRUM%20LOOPS%20and%20BREAKS/161%20to%20180%20bpm/128[kb]161_amenvar3.aif.mp3
It's important that this is done sync, not async, because there's a stateful
part that places parts into an array by their page number, and if the separate
subparts run async from each other the ordering gets messed up.
- I remembered that the parity byte exists, so I test that too
- bumped the output data down into a sub-member (result.structuredAppend.{M,N,parity}) and gave it a TypeScript type
- bumped M and N up to be 1-based, following the QR spec
The assert line is weird. In structured-append-test.ts I can use
```
import { strict as assert } from 'assert';
```

But here I can't and I have to use
```
import * as assert from 'assert';
```
(ref: https://stackoverflow.com/questions/39883960/what-is-the-typescript-2-0-es2015-way-to-import-assert-from-node-js)

Oh well??
As you can see from the diff, this only adds missing byte chunks
and the new structured append chunks, plus 15 newly passing tests :)

Previously, in 7d113b2, I had generated
the expected output using https://github.com/glassechidna/zxing-cpp/.
Now, the expected output was made by jsQR.
The fact that the diff shows no change in that part of the output.jsons
means this feature branch is correct :)
@kousu
Copy link
Author

kousu commented Apr 2, 2019

Another issue:

if I try to use import { strict as assert } from 'assert'; and then regenerate the test-cases, I get "Module '"assert"' has no exported member 'strict'. ":

$ ./node_modules/.bin/ts-node --project tests/ tests/generate-test-data.ts

./jsQR/node_modules/ts-node/src/index.ts:307
        throw new TSError(formatDiagnostics(diagnosticList, cwd, ts, lineOffset))
              ^
TSError: ⨯ Unable to compile TypeScript
src/decoder/decodeData/index.ts (5,10): Module '"assert"' has no exported member 'strict'. (2305)
    at getOutput (./jsQR/node_modules/ts-node/src/index.ts:307:15)
    at ./jsQR/node_modules/ts-node/src/index.ts:336:16
    at Object.compile (./jsQR/node_modules/ts-node/src/index.ts:498:11)
    at Module.m._compile (./jsQR/node_modules/ts-node/src/index.ts:392:43)
    at Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Object.require.extensions.(anonymous function) [as .ts] (./jsQR/node_modules/ts-node/src/index.ts:395:12)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)

But this doesn't happen if I just ./node_modules/.bin/jest --runInBand structured-append which has the same offending line in it. How is that possible? Is generate-test-data.ts running in a stricter TypeScript mode?

@kousu kousu changed the title [WIP] Structured append Structured Append Apr 2, 2019
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.

1 participant