[fprint] VFS0050 driver

[Vasily Khoruzhick]

Hi Konstantin,

Could you please squash your driver into a single commit (use git rebase
-i) for easier review? Please leave core changes as a separate commit.

Thanks!

Regards,
Vasily

On Thu, Aug 6, 2015 at 11:34 AM, Vasily Khoruzhick <anarsoul at gmail.com> wrote:
> 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