Skip to content

Feature/multi qr code reader #376

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

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
fc690ea
WIP code
Dec 1, 2020
09c4ea3
WIP: commit meant to save the code
odahcam Dec 2, 2020
9d0fae6
dependecies: updated sharp
odahcam Dec 2, 2020
9a7527c
tests(multi qrcode): fix array equals assertion
odahcam Dec 2, 2020
abeafd7
image loading fix
odahcam Dec 2, 2020
7e7c13f
tests corrections
odahcam Dec 4, 2020
abcff1a
new finder pattern finder
odahcam Dec 4, 2020
e0ae737
added some notes
odahcam Dec 4, 2020
9f073f9
working on overloads
odahcam Dec 7, 2020
f2a398d
Merge branch 'feature/multi-qr-code-reader' of https://github.com/zxi…
odahcam Dec 7, 2020
384c84a
test: fix overload usage
odahcam Dec 7, 2020
562277e
Merge branch 'master' into feature/multi-qr-code-reader
odahcam Dec 9, 2020
f5212ac
fix implementation and overload names
odahcam Dec 22, 2020
ebd9756
fix spacing in func declarations
odahcam Dec 22, 2020
6efcdca
implemented overloading pattern
odahcam Dec 23, 2020
9e92e55
fix general build/test errors
odahcam Dec 23, 2020
e3c15b6
Merge branch 'master' into feature/multi-qr-code-reader
odahcam Apr 11, 2021
e5aac7c
updates and added info about porting overloads
odahcam Apr 11, 2021
49a0363
fixed tests build
odahcam Apr 11, 2021
07ec4e7
Merge branch 'master' into feature/multi-qr-code-reader
odahcam Apr 11, 2021
3599b38
ts-node launches
odahcam May 29, 2021
7c0beab
Remove duplicate import and add type to assertion to allow tests to run
shahrin-shahrulzaman-rakuten Aug 11, 2021
fa890ef
Remove duplicate import and add type to assertion to allow tests to run
shahrin-shahrulzaman-rakuten Aug 11, 2021
9f32092
Merge pull request #464 from shahrin014/fix/feature/multi-qr-code-reader
werthdavid Aug 11, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 88 additions & 67 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,151 +5,172 @@
"type": "node",
"request": "launch",
"name": "Unit Tests",
"program": "${workspaceFolder}/node_modules/mocha-webpack/bin/mocha-webpack",
"program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
"args": [
"./src/**/*.spec.ts",
"--colors",
"--require",
"ts-node/register",
"--require",
"tsconfig-paths/register",
"-u",
"tdd",
"--timeout",
"999999",
"--webpack-env",
"dbg",
"--webpack-config",
"webpack.config.test.js"
"--colors",
"--recursive",
"./src/**/*.spec.ts"
]
},
{
"type": "node",
"request": "launch",
"name": "Unit Tests - ts-node",
"name": "Code 39 Tests",
"program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
"args": [
"--require",
"ts-node/register",
"--require",
"tsconfig-paths/register",
"-u",
"tdd",
"--timeout",
"999999",
"--colors",
"--recursive",
"./src/test/**/*.spec.ts"
],
"internalConsoleOptions": "openOnSessionStart"
},
{
"type": "node",
"request": "launch",
"name": "Code 39 Tests",
"program": "${workspaceFolder}/node_modules/mocha-webpack/bin/mocha-webpack",
"args": [
"./src/test/core/oned/Code39*.spec.ts",
"--colors",
"--timeout",
"999999",
"--webpack-env",
"dbg",
"--webpack-config",
"webpack.config.test.js"
]
},
{
"type": "node",
"request": "launch",
"name": "EAN 13 Tests",
"program": "${workspaceFolder}/node_modules/mocha-webpack/bin/mocha-webpack",
"program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
"args": [
"./src/test/core/oned/Ean13*.spec.ts",
"--colors",
"--require",
"ts-node/register",
"--require",
"tsconfig-paths/register",
"-u",
"tdd",
"--timeout",
"999999",
"--webpack-env",
"dbg",
"--webpack-config",
"webpack.config.test.js"
"--colors",
"--recursive",
"./src/test/core/oned/Ean13*.spec.ts",
]
},
{
"type": "node",
"request": "launch",
"name": "PDF417 Tests",
"program": "${workspaceFolder}/node_modules/mocha-webpack/bin/mocha-webpack",
"program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
"args": [
"./src/test/core/pdf417/",
"--recursive",
"--colors",
"--require",
"ts-node/register",
"--require",
"tsconfig-paths/register",
"-u",
"tdd",
"--timeout",
"999999",
"--webpack-env",
"dbg",
"--webpack-config",
"webpack.config.test.js"
"--colors",
"--recursive",
"./src/test/core/pdf417/**/*.spec.ts"
]
},
{
"type": "node",
"request": "launch",
"name": "QR Code Tests",
"program": "${workspaceFolder}/node_modules/mocha-webpack/bin/mocha-webpack",
"program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
"args": [
"./src/test/core/qrcode/",
"--recursive",
"--colors",
"--require",
"ts-node/register",
"--require",
"tsconfig-paths/register",
"-u",
"tdd",
"--timeout",
"999999",
"--webpack-env",
"dbg",
"--webpack-config",
"webpack.config.test.js"
"--colors",
"--recursive",
"./src/test/core/qrcode/**/*.spec.ts"
]
},
{
"type": "node",
"request": "launch",
"name": "Data Matrix Tests",
"program": "${workspaceFolder}/node_modules/mocha-webpack/bin/mocha-webpack",
"program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
"args": [
"./src/test/core/datamatrix/",
"--recursive",
"--colors",
"--require",
"ts-node/register",
"--require",
"tsconfig-paths/register",
"-u",
"tdd",
"--timeout",
"999999",
"--webpack-env",
"dbg",
"--webpack-config",
"webpack.config.test.js"
]
"--colors",
"--recursive",
"./src/test/core/datamatrix/**/*.spec.ts"
],
"internalConsoleOptions": "openOnSessionStart"
},
{
"type": "node",
"request": "launch",
"name": "Aztec 2D Tests",
"program": "${workspaceFolder}/node_modules/mocha-webpack/bin/mocha-webpack",
"program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
"args": [
"./src/test/core/aztec/",
"--recursive",
"--require",
"ts-node/register",
"-u",
"tdd",
"--timeout",
"999999",
"--colors",
"--recursive",
"./src/test/core/aztec/**/*.spec.ts"
],
"internalConsoleOptions": "openOnSessionStart"
},
{
"type": "node",
"request": "launch",
"name": "StringBuilder tests",
"program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
"args": [
"--require",
"ts-node/register",
"--require",
"tsconfig-paths/register",
"-u",
"tdd",
"--timeout",
"999999",
"--webpack-env",
"dbg",
"--webpack-config",
"webpack.config.test.js"
]
"--colors",
"--recursive",
"./src/test/core/util/StringBuilder.spec.ts"
],
"internalConsoleOptions": "openOnSessionStart"
},
{
"type": "node",
"request": "launch",
"name": "Aztec 2D Tests - ts-node",
"name": "Result tests",
"program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
"args": [
"--require",
"ts-node/register",
"--require",
"tsconfig-paths/register",
"-u",
"tdd",
"--timeout",
"999999",
"--colors",
"--recursive",
"./src/test/core/aztec/**/*.spec.ts"
"./src/test/core/Result.spec.ts"
],
"internalConsoleOptions": "openOnSessionStart"
}
Expand Down
95 changes: 84 additions & 11 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Initial port from 3.3.1-SNAPSHOT on May 2017 by Adrian Toșcă (@aleris).

### Approach

The Java files are transformed using regexps for some obvious syntax transformation (see ./autotransform) and then modified manually.
The Java files are transformed using RegExps for some obvious syntax transformation, see `` for a starting point.

Using http://www.jsweet.org was considered but rejected because of loosing type information early on (for example
number versus int is essential for bitwise operations), language style and older TypeScript version.
Expand All @@ -50,6 +50,84 @@ number versus int is essential for bitwise operations), language style and older
| `byte[]` | `Uint8ClampedArray` |
| `int[]` | `Int32Array` |

### Java numbers to TS numbers

- Take care of `int` -> `number` (integer to number) port when doing bitwise transformation especially `<<`. Do a `& 0xFFFFFFFF` for ints, a &0xFF for bytes.
- Take care of array initialization, in Java `new Array(N)` initializes capacity NOT size/length.
- Use `Math.floor` for any division of `int`s otherwise the `number` type is a floating point and keeps the numbers after the dot.
- For `float`/`number` to `int` casting use `Math.trunc`, to replicate the same effect as Java casting does.

### Porting overloads

> [but don't rewrite JavaScript to be Java](https://github.com/zxing-js/library/pull/376#commitcomment-44928885)

Strong words and you should agree, so we're in favor of mixing implementations using a prefered order:

1. Don't implement overloading if not needed but document it.
2. Missing argument handling and then calling a (one) implementation method (this is easily preferable than 3 bellow).
3. Missing argument handling and calling vastly (multiple) different implementations as the arguments matches.

All this in favor of keeping the interfaces similar to Java and the code as close as possible for porting and debugging. Both should be very well commented in the code so they explain why they're there and what they're doing.

> [Most of the contributors to this library will most likely have a JavaScript background rather than Java.](https://github.com/zxing-js/library/pull/376#commitcomment-44928885)

Yeah but most will have to have a very good understanding of both languages so they can port the `core` and porting is terrible hard when code doesn't matches. For new modules **not based** in the Java version we're **against** the use of overloading pattern, JavaScript simply doesn't fits it well and should be avoided in here.

> You can find more on this discussion in [this Pull Request](https://github.com/zxing-js/library/pull/376).

#### Examples

Based on the rules set above, this is where we land, first with a simpler yet effective approach:

```typescript
constructor(arg1: any);
constructor(arg1: any, arg2: any);
constructor(arg1: any, arg2: any, arg3: any);
constructor(arg1: any, arg2?: any, arg3?: any) {
if (arg2 == null) arg2 = {};
if (arg3 == null) arg3 = {};
return constructorImpl(arg1, arg2, arg3)
}

constructorImpl(arg1: any, arg2: any, arg3: any) {
/* Implementation code */
}
```

And less preferred if more advanced logic needed:

```typescript
constructor(arg1: any);
constructor(arg1: any, arg2: any);
constructor(arg1: any, arg2: any, arg3: any);
constructor(arg1: any, arg2?: any, arg3?: any) {
if (arg3 != null) return constructorImpl(arg1, arg2, arg3);
if (arg2 != null) return constructorOverload2(arg1, arg2);
return constructorOverload1(arg1)
}

private constructorOverload1(
arg1: any,
) {
return this.constructorOverload2(arg1, {});
}

private constructorOverload2(
arg1: any,
arg2: any,
) {
return this.constructorImpl(arg1, arg2, {});
}

private constructorImpl(
arg1: any,
arg2: any,
arg3: any,
) {
/* Implementation code */
}
```

## Types

### Java types
Expand All @@ -62,17 +140,12 @@ https://docs.oracle.com/javase/tutorial/java/nutsandbolts/datatypes.html
- `int` has 32 bits, signed, so `int[]` transforms to `Int32Array`.
- `char` has 2 bytes, so `char[]` transforms to `Uint16Array`.
- `long` has 64 bit two's complement `integer`, can be signed or unsigned.
- `float[]` can be ported to `Float32Array`.
- `double[]` can be ported to `Float64Array`.

### JavaScript's TypedArray

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray

## Things to look for

- Take care of `int` -> `number` (integer to number) port when doing bitwise transformation especially `<<`. Do a `& 0xFFFFFFFF` for ints, a &0xFF for bytes.
- Take care of array initialization, in Java `new Array(N)` initializes capacity NOT size/length.
- Use `Math.floor` for any division of ints otherwise the `number` type is a floating point and keeps the numbers after the dot.
- For `float` to `int` casting use `Math.trunc`, to replicate the same effect as Java casting does.
Read about JavaScript TypedArray [here](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray).

## Encoding

Expand All @@ -89,8 +162,8 @@ Will became: `StringEncoding.decode(<ByteArray>, encoding)`.
- `common/AbstractBlackBoxTestCase.java`
- `Cp437` not supported by TextEncoding library see `DecodedBitStreamParserTestCase`.
- Replace `instanceof` with something more robust.
- Simplify double `null !== <something> && undefined !== <something>` checks.
- Simplify double `<something> !== null && <something> !== undefined` checks.

----
---

Most of things here are opinions and were written by the first porter, please feel free to discuss and help us to make it better.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@
"nyc": "^15.1.0",
"rollup": "^2.8.2",
"seedrandom": "^2.4.4",
"sharp": "^0.22.1",
"sharp": "^0.26.3",
"shx": "0.3.2",
"sinon": "^7.2.7",
"terser": "^5.3.7",
Expand Down
5 changes: 1 addition & 4 deletions src/core/MultiFormatReader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,7 @@ export default class MultiFormatReader implements Reader {
* @throws NotFoundException Any errors which occurred
*/
/*@Override*/
// public decode(image: BinaryBitmap): Result {
// setHints(null)
// return decodeInternal(image)
// }
public decode(image: BinaryBitmap): Result;

/**
* Decode an image using the hints provided. Does not honor existing state.
Expand Down
3 changes: 2 additions & 1 deletion src/core/Reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ interface Reader {
* @throws NotFoundException if no potential barcode is found
* @throws ChecksumException if a potential barcode is found but does not pass its checksum
* @throws FormatException if a potential barcode is found but format is invalid
* @override decode
*/
// decode(image: BinaryBitmap): Result /*throws NotFoundException, ChecksumException, FormatException*/
decode(image: BinaryBitmap): Result;

/**
* Locates and decodes a barcode in some format within an image. This method also accepts
Expand Down
Loading