Brightscript port of QRCode Generator#110
Open
ahwayakchih wants to merge 18 commits intokazuhikoarase:masterfrom
Open
Brightscript port of QRCode Generator#110ahwayakchih wants to merge 18 commits intokazuhikoarase:masterfrom
ahwayakchih wants to merge 18 commits intokazuhikoarase:masterfrom
Conversation
Mostly straightforward port of JavaScript version of QRCode Generator created by Kazuhiko Arase. Some of the components are split a bit differently and some internal names are changed. Public API stays similar, changed only to match "Roku-way" of doing things: `typeNumber` and `errorCorrectionLevel` are specified through field values, `make` function should be called after `addData` calls. Only core QRCode handling is ported. Plus printing "ASCII" version to either string or console log.
It was called just from a single place inside `QRCode` anyway. Instead of calling `isDark` for every point, get all `modules` array first and then just access items directly. Test run using `brs` before change: ``` real 0m20,842s user 0m0,215s sys 0m0,089s ``` Test run using `brs` after change ``` real 0m6,401s user 0m0,223s sys 0m0,081s ```
First try to run on Roku device discovered multiple issues:
- fix shadowed variable and use of uninitialized var reported by linter
- prefix unused variables reported by Roku
- fix Rooibos test
- change `type` to `typeNumber` variable, `type` is used by Roku
- rename `QRData_Number` to `QRData_Numeric` directory name
- rename `QRData_8BitByte` to `QRData_Byte` directory name
- moved common parts of test to `QRCodeTest` component
- moved `QRPrinter` out of `QRCode` directory, as it's not a "core" code
Also: QR is generated super fast on Roku (even on Roku Express).
A LOT faster than running with `brs` (<600ms vs >6s :D)!
It generates PNG image and puts it on screen.
Name test components start with `test` word to make them less likely to be confused as required part of the library. Stop exposing QRMath functions from `QRCode` component. That was for testing only, so move that to `TestQRCode` component.
Not necessary for test channel, but for potentially turning it into full-blown downloadable component library.
Instead of adding them to the `TestQRCode`, which made test runs to be a lot longer than necessary.
No need to setup whole env, if we just want to run some basic tests in CI pipeline.
Rooibos warned that it will auto-extend from `Group` which is renderable and we do not want that in this case.
This allows to still set different `width` and `height` if needed, and better matches loading regular images.
It's a bit more involved now, but only if one needs that. Removed `toLog`, since it was just a `toString` with `print` called. Removed `toString`, exposing `toASCII` directly instead. Now printing QRCode can be done either through calling the function, or similarly to `QRPoster`: by setting `qrcode` or `text` field.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi,
I created an initial port of JS version of qrcode-generator to BrightScript (used to write apps for Roku devices).
Would you be interested in merging this into main qrcode-generator repo? Or should it be a separate "fork" project?
I'll probably keep updating code with time (and use in my apps). I think i'll extract some parts to separate components or merge them into main QRCode component. ot sure yet :).
In any case, this works ok as is. To make a full test you'd have to have a Roku device at hand, but "core" functionality (generating QRCode data, plus turning it into ASCII-like render, same as with JS version) can be tested using
brs.I included test and
package.jsonthat will use@hulu/rocatest runner - all you need is to have a node.js installed (or running in container or VM :)).brs(used byroca) implements only parts of BrightScript and SceneGraph APIs. Generating QRCode with it is A LOT slower than when running it on Roku device, so don't be scared if test runs over 6 seconds. On device it generates data in less than 600ms and turns it into PNG image in another ~350ms (or prints to console as ASCII in ~45ms).Here's output from
npm testcommand:Here's console output from Roku device running example channel:
Please not that the spacing between blocks depends on the font. In local Terminal i see no spacing, but in this PR's description i see some - depends on font config used on GitHub.
And here's a screenshot from the same run:
