adafruit / Adafruit_CircuitPython_Fingerprint

CircuitPython library for talking to UART-based Fingerprint sensors
MIT License
54 stars 52 forks source link

Error in _send_data #27

Closed admiralmaggie closed 3 years ago

admiralmaggie commented 3 years ago

Hello, I think this statement for j in range(len(data[start:end])): needs to be revised to for j in range(start, end): otherwise it is sending the same chunk of data over and over again. Just checking...

https://github.com/adafruit/Adafruit_CircuitPython_Fingerprint/blob/7c12b51d4c7eaae312fa87bbea82f0f2bf98b0cf/adafruit_fingerprint.py#L421-L454

jepler commented 3 years ago

@admiralmaggie I agree there's something about that code that doesn't look quite right. When the code is wrong, does it stop something specific from working? If you were able to fix it, please consider opening a pull request so others can benefit!

(I think the suggested modification may not be quite right -- for instance, if the buffer contains 7 bytes but end is 16, we still want to just work on 7 bytes)

@stitchesnburns any chance you can check it out? It looks like you may have contributed this code.

jerryneedell commented 3 years ago

FYI -- I just wanted to point out the the send_data function is not exercised in any of the examples. I agree that the code in question does look odd but I have never tried to use it to upload a fingerprint image.

admiralmaggie commented 3 years ago

@jepler Yes you are correct, it is theoretically possible that the buffer would be smaller than data_length but in practice it should be equal to either a template or an image which are both greater than 256 (largest data_length). I can do a PR but wanted to check to make sure I'm not missing anything.

@jerryneedell You are also correct. This function it is not used an any of the examples. I was playing around with uploading/downloading the templates and came across the issue.

admiralmaggie commented 3 years ago

@jepler @jerryneedell I got upload/download of templates working. I simplified _send_data as well. I found another bug though: We should not be calling the self.read_sysparam() in _send_data routine since sensor is expecting a data packets and not another command:

https://github.com/adafruit/Adafruit_CircuitPython_Fingerprint/blob/7c12b51d4c7eaae312fa87bbea82f0f2bf98b0cf/adafruit_fingerprint.py#L392-L405

I added a new method to compare two templates. I also created a separate example for downloading/uploading templates and comparing them without storing them on the flash memory. This helps with projects where templates are stored centrally (i.e. a MySQL database).

I can submit a PR if you guys like...

jerryneedell commented 3 years ago

I think PR would be great -- Thank you! Which Finger Print Sensor are you working with?

admiralmaggie commented 3 years ago

@jerryneedell I have been testing with a R503 purchased from Adafruit. I just submitted a PR.

jerryneedell commented 3 years ago

Great! That is the sensor I have. I'll look at and test the PR this evening or tomorrow. Thanks!