Comment on attachment 75783 Patch (against latest git version) with driver implementation
Review of attachment 75783: -----------------------------------------------------------------
::: libfprint/drivers/vfs5011.c @@ +25,5 @@ > +} > + > +static void debugprint(const char *line) > +{ > +#ifdef DEBUG
There's fp_dbg/fp_err/fp_whatever already. Please don't reinvent a wheel
@@ +67,5 @@ > +static int usb_send(libusb_device_handle *handle, unsigned char *data, unsigned size) > +{ > + int transferred = 0; > + int r = libusb_bulk_transfer(handle, VFS5011_OUT_ENDPOINT, data, size, > + &transferred, VFS5011_DEFAULT_WAIT_TIMEOUT);
No synchronous transfers are allowed in driver
@@ +85,5 @@ > +static int usb_recv(libusb_device_handle *handle, unsigned endpoint, unsigned char *buf, > + unsigned max_bytes, int *transferred) > +{ > + int r = libusb_bulk_transfer(handle, endpoint, buf, max_bytes, transferred, > + VFS5011_DEFAULT_WAIT_TIMEOUT);
Same
@@ +404,5 @@ > + > +// Rescale image to account for variable swiping speed > +int vfs5011_rescale_image(unsigned char *image, int input_lines, > + unsigned char *output, int max_output_lines) > +{
Could you use existing scale functions?
@@ +435,5 @@ > +// if (! on_good_offsets && (i >= GOOD_OFFSETS_CRITERION)) { > +// if (get_deviation_int(offsets + i - GOOD_OFFSETS_CRITERION, GOOD_OFFSETS_CRITERION) < > +// GOOD_OFFSETS_THRESHOLD) > +// on_good_offsets = 1; > +// }
Please drop commented out blocks of code
@@ +876,5 @@ > +struct fp_img_driver vfs5011_driver = > +{ > + .driver = > + { > + .id = 12,
Please don't use hardcoded ID anymore, define your own in driver_ids.h and use that one
@@ +886,5 @@ > + > + .flags = 0, > + .img_width = VFS5011_IMAGE_WIDTH, > + .img_height = -1, > + .bz3_threshold = 20,
Why threshold is so low?
Comment on attachment 75783
Patch (against latest git version) with driver implementation
Review of attachment 75783: ------- ------- ------- ------- ------- ------- ------- ------- --
-------
::: libfprint/ drivers/ vfs5011. c
@@ +25,5 @@
> +}
> +
> +static void debugprint(const char *line)
> +{
> +#ifdef DEBUG
There's fp_dbg/ fp_err/ fp_whatever already. Please don't reinvent a wheel
@@ +67,5 @@ libusb_ device_ handle *handle, unsigned char *data, unsigned size) bulk_transfer( handle, VFS5011_ OUT_ENDPOINT, data, size, DEFAULT_ WAIT_TIMEOUT) ;
> +static int usb_send(
> +{
> + int transferred = 0;
> + int r = libusb_
> + &transferred, VFS5011_
No synchronous transfers are allowed in driver
@@ +85,5 @@ libusb_ device_ handle *handle, unsigned endpoint, unsigned char *buf, bulk_transfer( handle, endpoint, buf, max_bytes, transferred, DEFAULT_ WAIT_TIMEOUT) ;
> +static int usb_recv(
> + unsigned max_bytes, int *transferred)
> +{
> + int r = libusb_
> + VFS5011_
Same
@@ +404,5 @@ rescale_ image(unsigned char *image, int input_lines,
> +
> +// Rescale image to account for variable swiping speed
> +int vfs5011_
> + unsigned char *output, int max_output_lines)
> +{
Could you use existing scale functions?
@@ +435,5 @@ CRITERION) ) { int(offsets + i - GOOD_OFFSETS_ CRITERION, GOOD_OFFSETS_ CRITERION) < THRESHOLD)
> +// if (! on_good_offsets && (i >= GOOD_OFFSETS_
> +// if (get_deviation_
> +// GOOD_OFFSETS_
> +// on_good_offsets = 1;
> +// }
Please drop commented out blocks of code
@@ +876,5 @@
> +struct fp_img_driver vfs5011_driver =
> +{
> + .driver =
> + {
> + .id = 12,
Please don't use hardcoded ID anymore, define your own in driver_ids.h and use that one
@@ +886,5 @@ IMAGE_WIDTH,
> +
> + .flags = 0,
> + .img_width = VFS5011_
> + .img_height = -1,
> + .bz3_threshold = 20,
Why threshold is so low?