Skip to content

Template upload/download fixes, misc improvements #28

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Mar 4, 2021
Merged

Template upload/download fixes, misc improvements #28

merged 8 commits into from
Mar 4, 2021

Conversation

admiralmaggie
Copy link

Per our discussion, I have made the following changes:

  1. Added a compare method to match two fingerprints templates in char buffers. compare_templates
  2. Added a new function to read system parameters. read_sysparam
  3. Added a soft reset function. soft_reset
  4. Improved debugging functions. _print_debug
  5. Added an example to demonstrate how to upload/download templates and store them in a file. examples\fingerprint_template_file_compare.py
  6. Corrected bugs in _get_data and _send_data functions.

additional support for template comparasion
fdcaf76 
bugfixes for send/get data packet
new example for template upload/download
bugfix for LED behaviour
d0279ee
jerryneedell
jerryneedell requested changes Feb 16, 2021
View reviewed changes
Copy link
Contributor

@jerryneedell jerryneedell left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank for for this PR -- The clean-up and added features are very nice.
I tested this on a Raspberry Pi 4 and it worked well.
Also tested basic function on GrandCentral_M4 -- no issues
edited to add:
It looks like the functions added should also work on the other supported FingerPRint sensors.
The example has some R503 specific code (settings the LED). Would other changes be necessary for a different sensor?
Should the example be names as specifically for the R503.

I think the CI failure was due to missing License/Copyright lines in the example.
I don't have any other concerns.

adafruit_fingerprint.py Outdated
@@ -25,7 +25,12 @@
https://github.com/adafruit/circuitpython/releases
"""

from micropython import const
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why this was added.

Copy link
Author

Choose a reason for hiding this comment

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

I'm running these on Windows PC so needed a way to get around the micropython requirements. Any suggestion on how to get around it?

Copy link
Contributor

@jerryneedell jerryneedell Feb 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure I understand - I thought the micropython modue was satisfied buy the Blinka interface -- at least, it is on a Raspberry Pi.
Does the example work an a Windows system?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah -- I see -- the import of board is also commented out in the example so it is not using any CircuitPython specific modules. I have no objection to the try/except -- just curious about it.

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I looked into this further, the issue was that my venv was missing Blinka. I thought setup had installed but apparently not. I removed the pseudo code. The examples work on both Linux (Ubuntu) and Windows 10.

jerryneedell
jerryneedell reviewed Feb 16, 2021
View reviewed changes
examples/fingerprint_template_file_compare.py Outdated
"""Compares a new fingerprint template to an existing template stored in a file
This is useful when templates are stored centrally (i.e. in a database)"""
print("Waiting for finger print...")
finger.set_led(color=3, mode=1)
Copy link
Contributor

@jerryneedell jerryneedell Feb 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The LED control is specifically to the R503 sensor -- should this example be limited to the R503
Will it work on other supported sensors like the ZFM-20 sensor? It appears to have the same functions but i don't have access to one to test.

Copy link
Author

Choose a reason for hiding this comment

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

Good point. Not sure about this. We can designate the example as R503.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hopefully another reviewer has one and can test it. In any case, we may need an r503 version for the LED, so I'd suggest just changing the name of this example to reflect that.

Copy link
Author

Choose a reason for hiding this comment

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

I wrapped the LED call with a try/except block. Now, Theoretically, it should simply get ignored if a board doesn't support it.

@jerryneedell
Copy link
Contributor

Now I think you need to run "black" on the files to make CI happy....
https://learn.adafruit.com/improve-your-code-with-pylint
I'm just the messenger ;-)

@admiralmaggie
Copy link
Author

Now I think you need to run "black" on the files to make CI happy....
https://learn.adafruit.com/improve-your-code-with-pylint
I'm just the messenger ;-)

Now I think you need to run "black" on the files to make CI happy....
https://learn.adafruit.com/improve-your-code-with-pylint
I'm just the messenger ;-)

Oh boy... Let me see...

@admiralmaggie
Copy link
Author

@jerryneedell Okay, finally got the CI finishing without errors. Needed both Pylint corrections and Black formatting.

jerryneedell
jerryneedell approved these changes Feb 17, 2021
View reviewed changes
Copy link
Contributor

@jerryneedell jerryneedell left a comment

Choose a reason for hiding this comment

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

Tested updates on RPi 4 -- Looks good to me.

adafruit_fingerprint.py Outdated
@@ -25,7 +25,12 @@
https://github.com/adafruit/circuitpython/releases
"""

from micropython import const
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah -- I see -- the import of board is also commented out in the example so it is not using any CircuitPython specific modules. I have no objection to the try/except -- just curious about it.

@jerryneedell jerryneedell requested a review from a team February 17, 2021 13:12
@jerryneedell
Copy link
Contributor

This looks fine to me -- I'll let others have a day or two to review/comment then will merge.

@jerryneedell
Copy link
Contributor

It would be very helpful is someone with a different fingerprint sensor than the R503 could test this.

@jerryneedell
Copy link
Contributor

@giacomoverdi It sounds like you are still using the old library.
I just copied the new adafruit_fingerprint.py to the same folder I ran fingerprint_template_file_compare.py from so it is the one that gets used.

@admiralmaggie
Copy link
Author

Yeah, that comes across as if the library adafruit_fingerprint.py has not been updated to the latest version. Can you double check?

@giacomoverdi
Copy link

Thanks. Now it's working! It's amazing!! Is there any way to load the template0.dat to the fingerprint reader?

@admiralmaggie
Copy link
Author

@giacomoverdi You mean permanently load it into a flash location?

@giacomoverdi
Copy link

giacomoverdi commented Feb 19, 2021 via email

@admiralmaggie
Copy link
Author

admiralmaggie commented Feb 19, 2021 •
edited
Loading

You should be able to use finger.store_model() to store the template into a flash location. Basically, read the template file, send it to a buffer (1 or 2 - called slots in the code) using finger.send_fpdata() . Then use finger.store_model() and reference the buffer number used previously (1 or 2) and a flash location (called location).

@giacomoverdi
Copy link

The template0.dat file is only a fingerprint? or fingers print are appended in template0.dat?

@admiralmaggie
Copy link
Author

Just one fingerprint.

@jerryneedell
Copy link
Contributor

@admiralmaggie I was hoping to get confirmation from someone that that the changes do not cause any issues with the non-r503 sensors. I don't see anything gin the code that should so if you are OK with it, I think we can merge this and make a new release. Are there any other changes you want to make first?

@admiralmaggie
Copy link
Author

@jerryneedell I think I'm good for now. I did order a ZFM-20 from Adafruit but missed the UPS deadline on Friday. I should it get it early next week. I can run a few tests then.

@jerryneedell
Copy link
Contributor

@jerryneedell I think I'm good for now. I did order a ZFM-20 from Adafruit but missed the UPS deadline on Friday. I should it get it early next week. I can run a few tests then.

That would be great. We can wait until then to merge it.

@giacomoverdi
Copy link

Another question: is there a way to set a system id (or module address) for recognize each fingerprint reader?

@admiralmaggie
Copy link
Author

@giacomoverdi Right now, the address is hardcoded to 0xFFFFFFFF. We could potentially make it user configurable but not sure the use case for it though...

@giacomoverdi
Copy link

Can you help me to make that? I need to assign each readers to each reles.

@giacomoverdi
Copy link

I have 4 readers and 4 rele. Each reader must be connected to one rele

@admiralmaggie
Copy link
Author

admiralmaggie commented Feb 22, 2021 •
edited
Loading

@giacomoverdi The current module is not really designed to handle multiple sensors at once and I don't think it would be an easy task to make those changes. Also, It is risky to have multiple sensors on a shared UART since there is no built in collision management protocol (nothing prevents sensors from talking at once). It might be easier to connect each sensor to a low cost MCU and then "network" the MCUs with Ethernet/Wifi/RS422/RS485 etc (or use an MCU with 4 available UART port).

@admiralmaggie
Copy link
Author

admiralmaggie commented Feb 22, 2021 •
edited
Loading

@jerryneedell I contacted Grow Technology Co. and asked for an updated datasheet since some statements didn't make sense (i.e. buffer size was incorrect). They sent me an updated one. This has some changes compare to one hosted on Adafruit. One major change is that it support up to 6 characteristic buffers (instead of 2). This means we can create a fingerprint template out of 6 scans instead of 2 that we are currently doing. This improves accuracy. Tricky change to implement so I will hold off making any changes for now.

R503 fingerprint module user manual.pdf

@jerryneedell
Copy link
Contributor

@admiralmaggie Wow! Thanks for the updated sheet. Do you know which version your sensor will support? From Register 0x3A? I guess any change would have to check that ?

@admiralmaggie
Copy link
Author

@jerryneedell At this point, It should support all versions. I think if we decide to support the 6 buffer capability, then we need to check the version/model.

@jepler jepler merged commit 05a2c8e into adafruit:master Mar 4, 2021
@jerryneedell
Copy link
Contributor

@jepler Did you do any testing with this. If so , which sensor?

@jerryneedell
Copy link
Contributor

jerryneedell commented Mar 4, 2021 •
edited
Loading

@admiralmaggie Did you get your ZFM-20 sensor to test this with?

@admiralmaggie
Copy link
Author

@jerryneedell I did but I haven't had a chance to run a test. It's on my list for this weekend.

@jerryneedell
Copy link
Contributor

@jerryneedell I did but I haven't had a chance to run a test. It's on my list for this weekend.

No rush -- I see it was merged - I suggest we hold off releasing the new version until you have tested it.

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Mar 10, 2021
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@6f44843)
d4ff12f 
Updating https://github.com/adafruit/Adafruit_CircuitPython_Fingerprint to 2.2.0 from 2.1.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_Fingerprint#28 from admiralmaggie/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_GPS to 3.8.0 from 3.7.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_GPS#57 from lesamouraipourpre/bad-sentences

Updating https://github.com/adafruit/Adafruit_CircuitPython_HT16K33 to 4.1.5 from 4.1.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_HT16K33#86 from SAK917/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_IS31FL3731 to 3.0.1 from 3.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_IS31FL3731#39 from dglaude/patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_MONSTERM4SK to 0.1.2 from 0.1.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_MONSTERM4SK#6 from FoamyGuy/pylintrc_update
  > Removed pylint process from github workflow

Updating https://github.com/adafruit/Adafruit_CircuitPython_AirLift to 1.0.1 from 1.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_AirLift#2 from FoamyGuy/pylintrc
  > Set correct black and reuse versions
  > Removed pylint process from github workflow
  > Hardcoded black version

Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Text to 2.15.4 from 2.15.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#132 from jposada202020/tab_replacement
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#130 from kmatch98/master
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#126 from FoamyGuy/label_base_class

Updating https://github.com/adafruit/Adafruit_CircuitPython_PIOASM to 0.2.2 from 0.2.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_PIOASM#9 from tannewt/audiobusio_examples
  > Merge pull request adafruit/Adafruit_CircuitPython_PIOASM#13 from FoamyGuy/pylintrc

Updating https://github.com/adafruit/Adafruit_CircuitPython_Pixel_Framebuf to 1.1.2 from 1.1.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_Pixel_Framebuf#4 from FoamyGuy/main
  > Hardcoded black version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jerryneedell jerryneedell jerryneedell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@admiralmaggie @jerryneedell @giacomoverdi @jepler