Created attachment 79017 [details] [review] git formatted diff against commit 7eafca7 AES3500 08ff:5731 ================= The AES3500 possesses a *press-typed* sensor of dimension in 128x128. Which is similiar to AES4000, except the later is in 96x96. The driver is a deriative work of Daniel Drake's AES4000 driver, with a few parameters tuned for AES3500. While there seems no problem scanning fingerprint images with it, the verification rate is pretty low at the moment.
Created attachment 79018 [details] lsusb output
There're a few differences between aes4000 and aes3500, could you extend aes4000 to support aes3500 devices? Btw, please use git format-patch next time
(In reply to comment #2) > There're a few differences between aes4000 and aes3500, could you extend > aes4000 to support aes3500 devices? A driver to support both devices? I'm not sure, but I may not be experienced enough to do it. Will look into it, anyway. > > Btw, please use git format-patch next time Will do so, thanks.
Take a look at aesx660.c, aes1660.c and aes2660.c aesx660.c is common routines for aes1660 and aes2660 devices, and device-specific code is in aes1660.c and aes2660.c
(In reply to comment #4) > Take a look at aesx660.c, aes1660.c and aes2660.c > > aesx660.c is common routines for aes1660 and aes2660 devices, and > device-specific code is in aes1660.c and aes2660.c Thanks Vasily, for pointing me out the way. I'll figure out some time to do it.
Created attachment 79392 [details] [review] Add standalone aes3500
Created attachment 79393 [details] [review] Extract common routines AES3K from AES3500 and AES4000
(In reply to comment #4) > Take a look at aesx660.c, aes1660.c and aes2660.c > > aesx660.c is common routines for aes1660 and aes2660 devices, and > device-specific code is in aes1660.c and aes2660.c Hi Vasily, Following your approach, I've extracted out common routines as aes3k, and with differences left in aes3500 and aes4000, respctively. Please have a look, and tell me if anything's not good yet. Thanks!
(In reply to comment #8) > Hi Vasily, > > Following your approach, I've extracted out common routines as aes3k, and > with differences left in aes3500 and aes4000, respctively. > > Please have a look, and tell me if anything's not good yet. Thanks! Hi Juvenn, please create 2 patches: 1st to extract common routines from aes4k, 2nd to add aes3k device support I know that it's additional work, but it does not look clean to add code in 1st patch that will be removed in 2nd, it's redundant. Thanks!
Created attachment 79398 [details] [review] Extract common routines from aes4000
Created attachment 79399 [details] [review] Add aes3500 support
Hi Vasily, Just prepared new patches as you've outlined. Your feedback are welcome. Thanks!
Comment on attachment 79398 [details] [review] Extract common routines from aes4000 Review of attachment 79398 [details] [review]: ----------------------------------------------------------------- ::: libfprint/drivers/aes4000.c @@ +42,2 @@ > > +// FRAME_WIDTH * AES3K_FRAME_HEIGHT / 2 Substitute it as a macro value, or remove this comment. And don't use c99 comments, use multiline
Comment on attachment 79399 [details] [review] Add aes3500 support Review of attachment 79399 [details] [review]: ----------------------------------------------------------------- Looks good
Created attachment 79452 [details] [review] Cleanup macros and comments for aes3k drivers Hi Vasily, Added this new patch to cleanup the macros and comments. Please review. Thanks!
Hi Vasily, Were you notified the new patch to cleanup the macros and comments? Let me know what you think. Thanks!
Comment on attachment 79452 [details] [review] Cleanup macros and comments for aes3k drivers Review of attachment 79452 [details] [review]: ----------------------------------------------------------------- Please fixup 1st and 2nd patches, don't create another one. You can use git interactive rebase (git rebase -i) to apply fixup against earlier commits. See also some comments: ::: libfprint/drivers/aes3500.c @@ +46,2 @@ > #define FRAME_WIDTH 128 > +#define FRAME_SIZE FRAME_WIDTH * AES3K_FRAME_HEIGHT / 2 Please use brackets around macro value ::: libfprint/drivers/aes4000.c @@ +41,4 @@ > > +/* image size = FRAME_WIDTH x FRAME_WIDTH */ > +#define FRAME_WIDTH 96 > +#define FRAME_SIZE FRAME_WIDTH * AES3K_FRAME_HEIGHT / 2 Same as above @@ +41,5 @@ > > +/* image size = FRAME_WIDTH x FRAME_WIDTH */ > +#define FRAME_WIDTH 96 > +#define FRAME_SIZE FRAME_WIDTH * AES3K_FRAME_HEIGHT / 2 > +#define FRAME_NUMBER FRAME_WIDTH / AES3K_FRAME_HEIGHT Same as above
Created attachment 79604 [details] [review] 0001 aes3k: extract common routines aes3k from aes4000
Created attachment 79605 [details] [review] 0002 aes3k: add aes3500 driver support
Hi Vasily, Newly built 2 patches attached, please have a review. And thanks for guiding me through to produce good code, this is my first C project. Anything that's not good enough, please let me konw. I'd be glad to fix them. Best,
Comment on attachment 79604 [details] [review] 0001 aes3k: extract common routines aes3k from aes4000 Review of attachment 79604 [details] [review]: ----------------------------------------------------------------- Looks OK
Comment on attachment 79605 [details] [review] 0002 aes3k: add aes3500 driver support Review of attachment 79605 [details] [review]: ----------------------------------------------------------------- Looks OK
Merged into libfprint master
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.