-
Notifications
You must be signed in to change notification settings - Fork 374
ctb-android-changes #96
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks for the PR. I've added some preliminary comments. Also, if you could please provide a description in each of the 3 PRs for these changes, others will have more context.
@@ -121,26 +121,26 @@ private native void nScanFrame(byte[] data, int frameWidth, int frameHeight, int | |||
Log.i(Util.PUBLIC_LOG_TAG, "card.io " + BuildConfig.PRODUCT_VERSION + " " + BuildConfig.BUILD_TIME); | |||
|
|||
try { | |||
loadLibrary("cardioDecider"); | |||
System.loadLibrary("cardioDecider"); |
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.
You should probably rebase to the latest master
. This change should not be 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.
This appears to have slipped through FileMerge. Easy fix.
@@ -6,6 +6,9 @@ | |||
|
|||
<uses-permission android:name="android.permission.CAMERA"/> | |||
<uses-permission android:name="android.permission.VIBRATE"/> | |||
<!-- We need this for tessdata--> | |||
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" /> |
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 does tessdata
need these extra permissions? Is the data not stored within the SDK itself?
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.
tessdata is an external directory read in by Tesseract. It needs to be included with the app/assets/bundle. With android the only way to do this is have it on the sd card or emulated storage build into most devices. Read/write external in this case does not mean external storage but rather the external emulated storage.
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.
There should be no need to write it to external storage, the internal private storage for the app should be fine. I've had to do something similar with an OpenVPN binary that needs to read a configuration file from disk, it does need to be written to storage instead of the assets directory so it can be accessed as a file, but from the point of view of the app, internal storage should be ok.
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.
This is an issue with tesseract, not an inability to read from assets. C/C++ can do what you described. However to my knowledge Tesseract specifically does not. It is platform agnostic so it cannot use AssetManager/NDK without referencing android specific apis. I will look into adding this functionality with some flags/magic if that is the desired. It should also be noted that this is not writing to external storage. It is internal to the phone, not the apk. The write permission is required to do that. It may require code changes to tesseract. The current solution is the agreed method outside of changing the tesseract source. Having said that I am in agreement that this is sort of a stupid solution to something you would think would be more straight forward as it is with ios version.
Some relevant discussion on the topic: http://stackoverflow.com/questions/23174318/how-to-get-a-full-path-for-an-android-resource-file-init-tesseract-ocr-in-andro
@@ -50,6 +50,18 @@ | |||
import io.card.payment.ui.Appearance; | |||
import io.card.payment.ui.ViewUtil; | |||
|
|||
|
|||
/* Tesseract Imports */ | |||
import android.os.Environment; |
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.
Imports should follow the standard sorting.
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.
Will do.
* @throws Exception | ||
*/ | ||
private void prepareDirectory(String path) { | ||
|
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.
Extra newline
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.
It will be removed.
* Entry point for tessdata adapted from TessTwo | ||
*/ | ||
private void tessSetup() { | ||
DATA_PATH = getExternalFilesDir(null) + "/Tesseract/";//getFilesDir()+ "/Tesseract/";//getExternalFilesDir(null) + "/Tesseract/";//Environment.getExternalStorageDirectory().toString() + "/Tesseract/"; |
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 can't you load the data from the assets themselves, without having to copy to another location?
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.
This is complicated issue for both android itself and tesseract. In iOS this can be read directly from the bundle, the same is not true for android assets. In order to read for tesseract it must be copied to external storage. Having said that in order to read the file as a stream, which should be doable directly from assets the tesseract input / engine might need to be modified. I suspect this is non-trivial. I suspect it can be done if that is a requirement.
See refs below for more detail
http://stackoverflow.com/questions/24129719/android-cant-find-file-in-assets-folder
http://stackoverflow.com/questions/33220378/tesseract-tessdata-folder-not-found-when-put-in-assets-folder
@kmfree I tried your code CTB changes, It is not working, generating some unknown 16 digit number instead of card number. Is there any enhancements ? |
See most recent Readme for description: https://github.com/CTBConsulting/Card.io-for-Capital-One-vertical-Cards/blob/dev/README.md