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

Corrected action taken after successfully storing the fingerprint in enroll example. #69

Merged
merged 4 commits into from Jan 24, 2024

Conversation

miselico
Copy link
Contributor

@miselico miselico commented Nov 6, 2019

A return statement after successfully storing the fingerprint was missing. Added a return value of 0.
When the fingerprint is taken successfully, the program should restart instead of continuing to take the fingerprint; amended.
The program prints dots to indicate it is waiting, removed a newline being printed after each of these in one case.

A return statement after succesfully storing the fingerprint was missing. Added a return value of 0.
When the fingerprint is taken succesfully, the program should restart instead of continuing to take the fingerprint; amended.
The program prints dots to indicate it is waiting, removed a newline being printed after each of these in one case.
@caternuson
Copy link
Contributor

caternuson commented Jan 24, 2024

A return statement after successfully storing the fingerprint was missing.

Not missing, it's the return true after the conditional block. That would then cause the while (! getFingerprintEnroll() ); to exit and then the main loop() would restart.

The program prints dots to indicate it is waiting, removed a newline being printed after each of these in one case.

Yah, that's annoying. The second read loop doesn't do that.

If this PR is still being monitored, please provide more info about the motivation for the added return.

@miselico
Copy link
Contributor Author

That return true was added in 31585c9 , half a year after my pull request...

@miselico
Copy link
Contributor Author

Let me pull the latest to my branch,

@miselico
Copy link
Contributor Author

I'd still like the return in the if block more, because it is the only case in which that return is used, but that's a matter of preference.

int error = 1;
while (error){
error = getFingerprintEnroll();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is functionally equivalent. Why the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has been a long time. I think I changed it because it took me a long time to figure out what the original actually meant.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, for now plz make the PR just the fix for the "." prints.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Rolling back code clarification.
@caternuson
Copy link
Contributor

Thanks. LGTM.

@caternuson caternuson merged commit c127475 into adafruit:master Jan 24, 2024
8 checks passed
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

Successfully merging this pull request may close these issues.

None yet

2 participants