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

isCancelled should be set to true as a initial value #15

Open
WeIio opened this issue Jul 3, 2017 · 10 comments
Open

isCancelled should be set to true as a initial value #15

WeIio opened this issue Jul 3, 2017 · 10 comments

Comments

@WeIio
Copy link

WeIio commented Jul 3, 2017

Thanks for your share firstly.
Before start authenticate every time, we should check the FingerPrint whether is started, if yes, cancel it. So if do cancel when FingerPrint is not started, we will get an error.
In general, the status of service should always be cleared or closed before service is started.
is it right?

@jariz
Copy link
Owner

jariz commented Jul 3, 2017

Not really sure what you're asking here.
isCancelled gets set to false every time the authentication starts.
isCancelled means the authentication was explicitly cancelled, either by the application itself, or by the system for whatever reason.
It does not indicate whether authentication has already started yes/no.
If you want to check if authentication has started, you can easily keep track of this in your own application state. (for example, by keeping a reference to the promise returned by authenticate(); if it still exists, clear the reference & cancel authentication)
Hope that helps.

@WeIio
Copy link
Author

WeIio commented Jul 3, 2017

Thanks for your answer.
You seems not really understand what I say.
In your codes,there is a line :boolean isCancelled = false;
So If I call isAuthenticationCanceled before start authenticate , it return false , and application will trade it as authenticate is started, but not actually

@nathan-k
Copy link

nathan-k commented Jul 3, 2017

@jariz Is there a reason why you use the boolean isCancelled instead of cancellationSignal.isCanceled()?

@nathan-k
Copy link

nathan-k commented Jul 4, 2017

Thought I would quickly mention that I had to make a small change in order to get cancellation to work consistently. Line 147 of FingerprintModule.java sometimes gets executed, throwing a "this shouldn't happen" AssertionError. This is because promise is sometimes null when errorCode == FINGERPRINT_ERROR_CANCELED

@WeIio
Copy link
Author

WeIio commented Jul 4, 2017

@jariz
[AssertionError. This is because promise is sometimes null when errorCode == FINGERPRINT_ERROR_CANCELED]
Maybe nathan-k said is why I asked you to change isCancelled variable. When cancel authenticate which is not started actually, 'null' error will happen.(Now initial isCancelled is false, so application will treat it as authenticate is already running)

@nathan-k
Copy link

nathan-k commented Jul 4, 2017

@WeIio He gave you a good solution to the problem that you were describing.

@WeIio
Copy link
Author

WeIio commented Jul 4, 2017

@nathan-k
Thanks.
I had figured out what he said. He had give me a solution that how to judge authentication status by keeping a reference to the promise returned by authenticate() and check it if I need.But I don't think it's a good choice.
In general, In init phase of a class, we always set variables to disabled. For example, there is an OpenFile class. Set isopen variable to false in init, When call openFile function or method, set isopen to true, that looks make sense.
Additionally, users who take this repository won't need to update or add much more codes to check whether authentication is started

@nathan-k
Copy link

nathan-k commented Jul 4, 2017

@WeIio Ok, I see your point. I don't see the behavior that you describe in the docs for FingerprintManagerCompat, which is probably why it's not in this package:

React Native Fingerprint Android is mostly just a set of bindings to Android FingerprintManager.
Alas, it's very low level.

@jariz
Copy link
Owner

jariz commented Jul 4, 2017

@nathan-k
I honestly can't remember why I'm not using cancellationSignal.isCancelled.
If you're in the mood to file a PR, I'd gladly accept it. (please test if everything works well with the example app)

@WeIio
Still can't really decipher what you're talking about, but we're gonna get rid of isCancelled completely, so I'm guessing that solves the problem you're having.

@CHGHA
Copy link

CHGHA commented Nov 27, 2017

i find this probleme
Error:(68, 0) Could not read script 'F:\react-native-fingerprint-android-master\example\node_modules\react-native\react.gradle' as it does not exist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants