On Fri, Feb 16, 2018 at 12:14 PM, Marco Trevisan (Treviño)
<m...@3v1n0.net> wrote:
> Vasily,
>
> See https://github.com/3v1n0/libfprint/commit/8cb5e6945cc26 (and
> related code) in this device after every enroll stage (and after every
> image save) we need to perform the device reactivation...
That sounds like an issue with the driver, not with the library.
> Now, during enroll it's libfprint itself that keeps track of the
> number of stages that have been performed, and after every stage has
> been completed the driver isn't informed about reactivating the device
> (the only thing that changes is that the state is set to
> AWAITING_FINGER). Thus nothing would happen in this sensor for
> example.
>
> Reactivation is something that should be triggered at higher level
> than the driver itself, imho. As it's something that is supposed to
> happen all the other cases when using all the libfprint based tools or
> examples (fprintd is a different thing here).
No, it shouldn't. Deactivation may put scanner into low power mode,
and I don't see serious reasons to call "deactivate" between
enrollment steps. If driver needs to do something before capturing
next image - move it to the end of image capturing SSM.
> 2018-02-16 20:07 GMT+01:00 Vasily Khoruzhick <anars...@gmail.com>:
>> On Thu, Feb 15, 2018 at 5:56 PM, Marco Trevisan (Treviño)
>> <m...@3v1n0.net> wrote:
>>> When a device has this flag set, it has to be re-activated at
>>> each enroll step, without performing any other cleanup.
>>>
>>> This allows to solve this such case in a cleaner way inside
>>> drivers.
>>
>> What's the purpose of this change?
>>
>>> ---
>>> libfprint/fp_internal.h | 1 +
>>> libfprint/imgdev.c | 30 +++++++++++++++++++++++++++---
>>> 2 files changed, 28 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libfprint/fp_internal.h b/libfprint/fp_internal.h
>>> index 74c5f7f..ea46b1e 100644
>>> --- a/libfprint/fp_internal.h
>>> +++ b/libfprint/fp_internal.h
>>> @@ -232,6 +232,7 @@ enum fp_print_data_type
>>> fpi_driver_get_data_type(struct fp_driver *drv);
>>> /* flags for fp_img_driver.flags */
>>> #define FP_IMGDRV_SUPPORTS_UNCONDITIONAL_CAPTURE (1 << 0)
>>> +#define FP_IMGDRV_NEEDS_REACTIVATION_BETWEEN_ENROLLS (1 << 1)
>>> struct fp_img_driver {
>>> struct fp_driver driver;
>>> diff --git a/libfprint/imgdev.c b/libfprint/imgdev.c
>>> index 4408c23..20a1559 100644
>>> --- a/libfprint/imgdev.c
>>> +++ b/libfprint/imgdev.c
>>> @@ -27,6 +27,9 @@
>>> #define BOZORTH3_DEFAULT_THRESHOLD 40
>>> #define IMG_ENROLL_STAGES 5
>>> +static int dev_activate(struct fp_img_dev *imgdev, enum
>>> fp_imgdev_state state);
>>> +static void dev_deactivate(struct fp_img_dev *imgdev);
>>> +
>>> static int img_dev_open(struct fp_dev *dev, unsigned long driver_data)
>>> {
>>> struct fp_img_dev *imgdev = g_malloc0(sizeof(*imgdev));
>>> @@ -124,6 +127,7 @@ void fpi_imgdev_report_finger_status(struct
>>> fp_img_dev *imgdev,
>>> int r = imgdev->action_result;
>>> struct fp_print_data *data = imgdev->acquire_data;
>>> struct fp_img *img = imgdev->acquire_img;
>>> + struct fp_img_driver *imgdrv =
>>> fpi_driver_to_img_driver(imgdev->dev->drv);
>>> fp_dbg(present ? "finger on sensor" : "finger removed");
>>> @@ -158,8 +162,14 @@ void fpi_imgdev_report_finger_status(struct
>>> fp_img_dev *imgdev,
>>> if (imgdev->action == IMG_ACTION_ENROLL &&
>>> r > 0 && r != FP_ENROLL_COMPLETE && r !=
>>> FP_ENROLL_FAIL) {
>>> imgdev->action_result = 0;
>>> - imgdev->action_state =
>>> IMG_ACQUIRE_STATE_AWAIT_FINGER_ON;
>>> - dev_change_state(imgdev,
>>> IMGDEV_STATE_AWAIT_FINGER_ON);
>>> +
>>> + if (imgdrv->flags &
>>> FP_IMGDRV_NEEDS_REACTIVATION_BETWEEN_ENROLLS) {
>>> + imgdev->action_state =
>>> IMG_ACQUIRE_STATE_DEACTIVATING;
>>> + dev_deactivate(imgdev);
>>> + } else {
>>> + imgdev->action_state =
>>> IMG_ACQUIRE_STATE_AWAIT_FINGER_ON;
>>> + dev_change_state(imgdev,
>>> IMGDEV_STATE_AWAIT_FINGER_ON);
>>> + }
>>> }
>>> break;
>>> case IMG_ACTION_VERIFY:
>>> @@ -332,11 +342,16 @@ void fpi_imgdev_session_error(struct fp_img_dev
>>> *imgdev, int error)
>>> void fpi_imgdev_activate_complete(struct fp_img_dev *imgdev, int status)
>>> {
>>> + struct fp_img_driver *imgdrv =
>>> fpi_driver_to_img_driver(imgdev->dev->drv);
>>> +
>>> fp_dbg("status %d", status);
>>> switch (imgdev->action) {
>>> case IMG_ACTION_ENROLL:
>>> - fpi_drvcb_enroll_started(imgdev->dev, status);
>>> + if (!(imgdrv->flags &
>>> FP_IMGDRV_NEEDS_REACTIVATION_BETWEEN_ENROLLS) ||
>>> + imgdev->dev->state != DEV_STATE_ENROLLING) {
>>> + fpi_drvcb_enroll_started(imgdev->dev, status);
>>> + }
>>> break;
>>> case IMG_ACTION_VERIFY:
>>> fpi_drvcb_verify_started(imgdev->dev, status);
>>> @@ -360,10 +375,19 @@ void fpi_imgdev_activate_complete(struct
>>> fp_img_dev *imgdev, int status)
>>> void fpi_imgdev_deactivate_complete(struct fp_img_dev *imgdev)
>>> {
>>> + struct fp_img_driver *imgdrv =
>>> fpi_driver_to_img_driver(imgdev->dev->drv);
>>> +
>>> fp_dbg("");
>>> switch (imgdev->action) {
>>> case IMG_ACTION_ENROLL:
>>> + if ((imgdrv->flags &
>>> FP_IMGDRV_NEEDS_REACTIVATION_BETWEEN_ENROLLS) &&
>>> + imgdev->dev->state == DEV_STATE_ENROLLING) {
>>> + imgdev->action_state = IMG_ACQUIRE_STATE_ACTIVATING;
>>> + dev_activate(imgdev, IMGDEV_STATE_AWAIT_FINGER_ON);
>>> + return;
>>> + }
>>> +
>>> fpi_drvcb_enroll_stopped(imgdev->dev);
>>> break;
>>> case IMG_ACTION_VERIFY:
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> fprint mailing list
>>> fprint@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/fprint
>
>
>
> --
> Treviño's World - Life and Linux
> http://www.3v1n0.net
_______________________________________________
fprint mailing list
fprint@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/fprint
