[fprint] VFS0050 driver

[Vasily Khoruzhick]

Hi,

I'm quite busy atm and I don't know when I get some time to take a look at it.

Regards,
Vasily

On Fri, Jul 31, 2015 at 5:52 AM, Константин Семенов <i at zem7.ru> wrote:
> Hello,
>
> I tested my driver for two weeks and I am completely satisfied with its
> work. I guess that it doesn't need more image and protocol improvements. I
> formatted the source and removed debug functions and waiting for future
> review.
>
> Thanks
>
> 2015-07-16 1:21 GMT+04:00 Константин Семенов <i at zem7.ru>:
>>
>> Hello Vasily,
>>
>> The first and the second issues are easy to fix, but I will do it when
>> everything else is ok.
>>
>> About the third point: fpi_im_resize can only increase the size of image,
>> so I wrote similar function fpi_img_downscale in pixman.c.
>>
>> Fourth part was the hardest one: I actually don't understand, how does
>> verifying process work in libfprint. In fprint_demo and examples
>> dev_deactivate is called after every fingerprint. But in fprintd one loop is
>> used for all scans. However, after entering
>> IMG_ACQUIRE_STATE_AWAIT_FINGER_OFF, state could be changed to
>> IMG_ACQUIRE_STATE_AWAIT_FINGER_ON only if action is enroll! I wrote a small
>> stub to fix it, but this thing needs more attention because of identify and
>> capture actions.
>>
>> About transfer cancellation: I checked it plenty of times when was playing
>> with fprint_demo, it works fine.
>>
>> I found a corner case which appears very rarely(e.g. when finger is on the
>> sensor for about a minute). It seems that driver doesn't work properly in
>> this case. Moreover, restarting fprintd and fprint_demo doesn't help, only
>> rebooting. I suggest to use libusb_reset_device in this case.
>>
>> Thanks
>>
>> 2015-07-14 21:25 GMT+04:00 Vasily Khoruzhick <anarsoul at gmail.com>:
>>>
>>> Hi Konstantin,
>>>
>>> Thank you for your work!
>>>
>>> I've taken only a brief look, will do a proper review on weekend,
>>> here're few issues (however I suppose not all):
>>>
>>> 1) You use formatting that differs from other libfprint. Please note
>>> that libfprint uses linux kernel coding style, see [1] for details,
>>> you can use script/Lindent script from Linux kernel sources to
>>> reformat your code.
>>> 2) Be ready to drop all debugging stuff (like one that wrapped into
>>> #ifdef VFS_DEBUG_IMAGE) before preparing a patch to submit
>>> 3) Drop your simple scaling, use libfprint functions, see fpi_im_resize()
>>> 4) I'm not very happy with [2]. Could you try to fix it without ugly
>>> hacks? Btw, how does it pass enroll which does 5 scans in row?
>>>
>>> Please also make sure that cancellation works well, i.e. try to press
>>> "cancel" button in fprint_demo while it waits for finger and ensure
>>> that it cancels scan properly
>>> and further scans for verification/enrollment work well.
>>>
>>> [1] https://www.kernel.org/doc/Documentation/CodingStyle
>>> [2]
>>> https://github.com/zemen/libfprint/blob/master/libfprint/drivers/vfs0050.c#L440
>>>
>>> Regards,
>>> Vasily
>>>
>>> On Mon, Jul 13, 2015 at 12:30 PM, Константин Семенов <i at zem7.ru> wrote:
>>> > Hello Vasily,
>>> >
>>> > I have updated my driver and now it satisfies all 4 requirements. Could
>>> > you
>>> > please check it again?
>>> >
>>> > The repo is still here: https://github.com/zemen/libfprint
>>> >
>>> > I will test it for several days and close some of TODO's.
>>> >
>>> > 2015-07-10 7:29 GMT+04:00 Vasily Khoruzhick <anarsoul at gmail.com>:
>>> >>
>>> >> Hi Konstantin,
>>> >>
>>> >> On Thu, Jul 9, 2015 at 5:31 PM, Константин Семенов <i at zem7.ru> wrote:
>>> >> > Hello,
>>> >> >
>>> >> > Recently I have managed to write working Validity VFS0050 driver
>>> >> > with
>>> >> > some
>>> >> > help of payden's source on github
>>> >> > (https://github.com/payden/libfprint).
>>> >> > I
>>> >> > made it from scratch, sniffing USB packets from windows' driver via
>>> >> > USBLyzer. Now it works very well for me without any crashes and with
>>> >> > low
>>> >> > false negatives.
>>> >> >
>>> >> > In a few days I will add USB errors checkings and cut out all debug
>>> >> > parts.
>>> >> >
>>> >> > Could you please take a look on it and if everything is OK add the
>>> >> > driver to
>>> >> > the library?
>>> >>
>>> >> 1) You should use asynchronous libusb API. Using synchronous API is
>>> >> not acceptable.
>>> >> 2) Please try to avoid using libusb_device_reset(). Find out how
>>> >> Windows driver deinitializes the device
>>> >> 3) Move functions from header into *c file
>>> >>
>>> >> > My repo is here: https://github.com/zemen/libfprint
>>> >>
>>> >> 4) Last but not least - you have to prepare proper git-formatted
>>> >> patch. Clone libfprint git repo instead of creating new one, add your
>>> >> changes, commit, push to github.
>>> >>
>>> >> Regards,
>>> >> Vasily
>>> >>
>>> >> P.S. Please keep ML in CC.
>>> >>
>>> >> > --
>>> >> > Kind regards,
>>> >> > Konstantin Semenov
>>> >> >
>>> >> > _______________________________________________
>>> >> > fprint mailing list
>>> >> > fprint at lists.freedesktop.org
>>> >> > http://lists.freedesktop.org/mailman/listinfo/fprint
>>> >> >
>>> >> _______________________________________________
>>> >> fprint mailing list
>>> >> fprint at lists.freedesktop.org
>>> >> http://lists.freedesktop.org/mailman/listinfo/fprint
>>> >
>>> >
>>> >
>>> >
>>> > --
>>> > Kind regards,
>>> > Konstantin Semenov
>>
>>
>>
>>
>> --
>> Kind regards,
>> Konstantin Semenov
>
>
>
>
> --
> Kind regards,
> Konstantin Semenov