-
Notifications
You must be signed in to change notification settings - Fork 0
[EPIC-6378] Prepare/restructure for stable RC #19
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
base: main
Are you sure you want to change the base?
Conversation
… sync across Java/Python/C wrappers
if (ec != SCANBOTSDK_OK) { fprintf(stderr, "get_detection_result: %d: %s\n", ec, error_message(ec)); goto cleanup; } | ||
|
||
scanbotsdk_document_detection_result_get_status(detect_result, &status); | ||
printf("Document Detection Status = %d\n", (int)status); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is int representation of the status useful for the user?
if (ec != SCANBOTSDK_OK) { fprintf(stderr, "classifier_run: %d: %s\n", ec, error_message(ec)); goto cleanup; } | ||
|
||
ec = scanbotsdk_document_classifier_result_get_status(result, &status); | ||
printf("Classifier Status: %d\n", (int)status); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are int statuses here and below meaningful for the user?
if (ec != SCANBOTSDK_OK) { fprintf(stderr, "get_check: %d\n", ec); return ec; } | ||
|
||
scanbotsdk_credit_card_scanning_result_get_detection_status(result, &status); | ||
printf("Document Detection Status = %d\n", (int)status); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Int status doesn't provide user with any useful information
examples/c/src/main.c
Outdated
else if (strcmp(category, "classify") == 0) { | ||
if (!file_path) { print_usage(argv[0]); return 1; } | ||
|
||
scanbotsdk_image_t *image = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
image is not freed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactored with
scanbotsdk_image_free(image);
at the end of condition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to have all free calls in cleanup block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Add snippets for nodejs wrapper
… commens and separated functions
…s for scanner/result/etc.
examples/c/src/main.c
Outdated
else if (strcmp(category, "classify") == 0) { | ||
if (!file_path) { print_usage(argv[0]); return 1; } | ||
|
||
scanbotsdk_image_t *image = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to have all free calls in cleanup block
process_word(words[k]); | ||
} | ||
|
||
free(words); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
words are not freed in case of early outs, there are no early outs now, but they can be added in future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated to free inside cleanup:
process_line(lines[j]); | ||
} | ||
|
||
free(lines); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as words above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated to free inside cleanup:
process_block(blocks[i]); | ||
} | ||
|
||
free(blocks); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as words above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated to free inside cleanup:
|
||
inner_cleanup: | ||
scanbotsdk_document_quality_analyzer_result_free(analyse_result); | ||
if (ec != SCANBOTSDK_OK) break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (ec != SCANBOTSDK_OK) {
goto cleanup;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced with proposed snippet
scanbotsdk_barcode_scanner_result_get_barcodes_size(result, &count); | ||
fprintf(stdout, "%zu barcodes found in frame %zu\n", count, frame_number); | ||
|
||
scanbotsdk_barcode_item_t **barcodes = malloc(count * sizeof(*barcodes)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use calloc in other examples, let's use it here as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced with calloc
|
||
size_t count = 0; | ||
ec = scanbotsdk_medical_certificate_patient_info_box_get_fields_size(patient_info, &count); | ||
if (ec != SCANBOTSDK_OK) { fprintf(stderr, "get_fields_size: %d: %s\n", ec, error_message(ec)); return ec; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
goto cleanup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced with goto cleanup
char *text_input = get_flag(argc, argv, "--text"); | ||
char *license_arg = get_flag(argc, argv, "--license"); | ||
|
||
// TODO Add your Scanbot SDK trial license key here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this second option of specifying license directly in the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed that yes, it make sense
public class DetectBarcodesSnippet { | ||
public static void run(ImageRef image) throws Exception { | ||
// Make sure you have a valid license | ||
if(ScanbotSDK.getLicenseInfo().getStatus() != LicenseStatus.OKAY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you manually check it here? If there are problems with license, an exception will be thrown. Same for all other snippets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed license check in all the places (java snippets so far)
if(ScanbotSDK.getLicenseInfo().getStatus() != LicenseStatus.OKAY) | ||
return; | ||
|
||
BarcodeFormatCommonConfiguration formatConfig = new BarcodeFormatCommonConfiguration(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need specifically this config in the example? It's not passed anywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my bad, updated snippet with BarcodeFormatCommonConfiguration in use
try ( | ||
BarcodeScanner scanner = new BarcodeScanner(config) | ||
) { | ||
BarcodeScannerResult result = scanner.run(image); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BarcodeScannerResult can contain images so should be within try-with-resources as well. You can have multiple objects created in the same try-with-resources clause
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved into try with resources
try( | ||
CheckScanner scanner = new CheckScanner(config) | ||
) { | ||
CheckScanningResult result = scanner.run(image); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The result can contain images, should be in try-with-resources
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved into try with resources
try( | ||
CreditCardScanner scanner = new CreditCardScanner(config) | ||
) { | ||
CreditCardScanningResult result = scanner.run(image); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The result can contain images, should be in try-with-resources
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved into try with resources
); | ||
} | ||
|
||
ScanbotSDK.deregisterDevice(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use DeviceSession, no need to call ScanbotSDK.deregisterDevice() and ScanbotSDK.waitForDeviceDeregistrationCompletion(15000); manually
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced with comment to call such operations in case if DeviceSession no in use
|
||
ScanbotSDK.initialize(scanbotLicenseKey, System.getProperty("user.dir")); | ||
|
||
try (DeviceSession deviceSession = new DeviceSession(15_000)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add explanation why is it needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comment on top of this operation
} | ||
} | ||
|
||
if (doc.getChildren() != null && !doc.getChildren().isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to check lists for nullability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed check for nullability
String typeFullName = (type != null) ? type.getFullName() : "—"; | ||
System.out.printf("Type: %s (%s)%n", typeName, typeFullName); | ||
|
||
if (doc.getFields() != null && !doc.getFields().isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to check lists for nullability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed check for nullability
switch (category) { | ||
case "scan": { | ||
if (file == null && resource == null) { ExampleUsage.print(); return; } | ||
try (ImageRef image = Utils.createImageRef(file, resource)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add calls to ImageRefProfiler after work is done to check if we properly set all the try-with-resources
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added profiler operations and a comment at the bottom
No description provided.