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

5390 correctly handle gs1 2d data matrix format #5401

Open
wants to merge 3 commits into
base: v2.4.0
Choose a base branch
from

Conversation

jmbrunskill
Copy link
Contributor

Fixes #5390

πŸ‘©πŸ»β€πŸ’» What does this PR do?

I tried to make minimal changes for this, but it should allow us to scan GS1 formatted barcodes and create assets from them.

πŸ’Œ Any notes for the reviewer?

Maybe/probably should remove the parsing of human readable string? Figured I'd leave it there for now as it might come in handy, and I didn't want to have to re-create data for all my tests cases and aiming for minimal changes here :)

Thoughts on this?

πŸ§ͺ Testing

  • Using either Android (with camera), or Desktop (with 2d Barcode Scanner)
  • Open the cold chain equipment section, and click the Scan button (this doesn't show on web)
  • Scan one of the barcodes below
  • You should be prompted to create a new asset
  • If you accept, it should show a new asset with the serial number, asset type, and Warenty date populated
  • The Installation date should default to today
  • Scanning the same barcode again, should navigate you to the right asset (once it has been created)

NOTE, scanning these 2d Matrixes in github dark mode or with a black background doesn't work well...
NOTE: in general scanning 2d Matrixes seems much less reliable than QR Codes, need to have camera straight on for example

Scan a GS1 formatted Data Matrix here (Note: or create your own for testing)

PQS Asset Type: E003/002
Serial Number: S12345678
https://bwipjs-api.metafloor.com/?bcid=gs1datamatrix&text=(01)09506000134352(11)241007(21)S12345678(241)E003/002(3121)820000(3131)670000(3111)630000(90)001(91)241007-310101
image

PQS Asset Type: E004/055
Serial Number: S1
https://bwipjs-api.metafloor.com/?bcid=gs1datamatrix&text=(01)09506000134352(11)241007(21)S1(241)E004/055(3121)820000(3131)670000(3111)630000(90)001(91)241007-310101

image

PQS Asset Type: E004/055
Serial Number S2
https://bwipjs-api.metafloor.com/?bcid=gs1datamatrix&text=(01)09506000134352(11)241007(21)S1(241)E004/055(3121)820000(3131)670000(3111)630000(90)001(91)241007-310101

image

πŸ“ƒ Documentation

  • Part of an epic: documentation will be completed for the feature as a whole
  • No documentation required: no user facing changes or a bug fix which isn't a change in behaviour
  • These areas should be updated or checked:
    1.
    2.

@jmbrunskill jmbrunskill linked an issue Nov 14, 2024 that may be closed by this pull request
@github-actions github-actions bot added this to the v2.4.1 milestone Nov 14, 2024
@github-actions github-actions bot added bug Something is borken Priority: Must Have The product will not work without this Team Piwakawaka James, LachΓ©, Carl feature: GAPS labels Nov 14, 2024
@jmbrunskill jmbrunskill changed the base branch from develop to v2.4.0 November 14, 2024 00:25
Copy link

github-actions bot commented Nov 14, 2024

Bundle size difference

Comparing this PR to main

Old size New size Diff
4.97 MB 4.97 MB 189 B (0.00%)

Copy link
Contributor

@lache-melvin lache-melvin left a comment

Choose a reason for hiding this comment

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

Working great! Thanks James πŸš€

Maybe/probably should remove the parsing of human readable string? Figured I'd leave it there for now as it might come in handy, and I didn't want to have to re-create data for all my tests cases and aiming for minimal changes here :)

I feel ok leaving it there, it's pretty isolated and comes in handy for testing if nothing else 😁

@lache-melvin
Copy link
Contributor

HAHA kidding wait I built it I didn't test hold fire

@lache-melvin
Copy link
Contributor

#exposed

Copy link
Contributor

@lache-melvin lache-melvin left a comment

Choose a reason for hiding this comment

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

Alas:
Screenshot_20241114_135559_Open mSupply

@jmbrunskill jmbrunskill marked this pull request as ready for review November 14, 2024 03:15
@lache-melvin
Copy link
Contributor

Ah - I think parseBarcode isn't returning anything? This is the ScanResult in handleScanResult:
Screenshot_20241114_162720_Open mSupply

@lache-melvin
Copy link
Contributor

I think I see the issue, doing a quick test build now, will update you :)

@lache-melvin lache-melvin self-assigned this Nov 14, 2024
Copy link
Contributor

@lache-melvin lache-melvin left a comment

Choose a reason for hiding this comment

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

Ok there are two issues:

The parseResult being called in the electron scanning code, but not for android:

if (hasNativeBarcodeScanner) {
setIsScanning(true);
const timeout = setTimeout(async () => {
await stopScan();
if (!hasElectronApi) error(t('error.unable-to-read-barcode'))();
}, SCAN_TIMEOUT_IN_MS);
// Check camera permission
await BarcodeScannerPlugin.checkPermission({ force: true });
// make background of WebView transparent
BarcodeScannerPlugin.hideBackground();
const result = await BarcodeScannerPlugin.startScan(); // start scanning and wait for a result
clearTimeout(timeout);
setIsScanning(false);
BarcodeScannerPlugin.showBackground();
callback(result);
}

However, I added the parseResult call there, and still the gs1 result of parseBarcode is coming back empty... don't have time to investigate further tonight sorry, I think a tomorrow problem 😁

@jmbrunskill
Copy link
Contributor Author

jmbrunskill commented Nov 14, 2024

However, I added the parseResult call there, and still the gs1 result of parseBarcode is coming back empty... don't have time to investigate further tonight sorry, I think a tomorrow problem 😁

Dang, was hopeful! Thanks for testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is borken feature: GAPS Priority: Must Have The product will not work without this Team Piwakawaka James, LachΓ©, Carl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Correctly handle GS1 2D data matrix format
2 participants