aes3k: Re-send some commands for each capture

The driver seems to have relied on the device to be fully
deactivated and activated again for each capture. This is not the case
during enroll, and as such in can cause issues.

Try fixing this by splitting out the last few commands send to the
device and assuming that this starts the capture.

Fixes: #306
This commit is contained in:
Benjamin Berg 2020-10-01 18:38:07 +02:00
parent c7cab77fc1
commit 81e0f4dfe5
4 changed files with 68 additions and 19 deletions

View file

@ -106,7 +106,9 @@ static struct aes_regwrite init_reqs[] = {
{ 0x9e, 0x53 }, /* clear challenge word bits */ { 0x9e, 0x53 }, /* clear challenge word bits */
{ 0x9f, 0x6b }, /* set some challenge word bits */ { 0x9f, 0x6b }, /* set some challenge word bits */
{ 0, 0 }, { 0, 0 },
};
static struct aes_regwrite capture_reqs[] = {
{ 0x80, 0x00 }, { 0x80, 0x00 },
{ 0x81, 0x00 }, { 0x81, 0x00 },
{ 0, 0 }, { 0, 0 },
@ -155,4 +157,6 @@ fpi_device_aes3500_class_init (FpiDeviceAes3500Class *klass)
aes_class->enlarge_factor = ENLARGE_FACTOR; aes_class->enlarge_factor = ENLARGE_FACTOR;
aes_class->init_reqs = init_reqs; aes_class->init_reqs = init_reqs;
aes_class->init_reqs_len = G_N_ELEMENTS (init_reqs); aes_class->init_reqs_len = G_N_ELEMENTS (init_reqs);
aes_class->capture_reqs = capture_reqs;
aes_class->capture_reqs_len = G_N_ELEMENTS (capture_reqs);
} }

View file

@ -42,7 +42,10 @@
typedef struct typedef struct
{ {
GCancellable *img_trf_cancel; /* This is used both as a flag that we are in a capture operation
* and for cancellation.
*/
GCancellable *img_capture_cancel;
} FpiDeviceAes3kPrivate; } FpiDeviceAes3kPrivate;
#define CTRL_TIMEOUT 1000 #define CTRL_TIMEOUT 1000
@ -83,6 +86,9 @@ img_cb (FpiUsbTransfer *transfer, FpDevice *device,
FpImage *img; FpImage *img;
int i; int i;
/* Image capture operation is finished (error/completed) */
g_clear_object (&priv->img_capture_cancel);
if (error) if (error)
{ {
if (g_error_matches (error, if (g_error_matches (error,
@ -91,13 +97,11 @@ img_cb (FpiUsbTransfer *transfer, FpDevice *device,
{ {
/* Cancellation implies we are deactivating. */ /* Cancellation implies we are deactivating. */
g_error_free (error); g_error_free (error);
g_clear_object (&priv->img_trf_cancel);
fpi_image_device_deactivate_complete (dev, NULL); fpi_image_device_deactivate_complete (dev, NULL);
return; return;
} }
fpi_image_device_session_error (dev, error); fpi_image_device_session_error (dev, error);
g_clear_object (&priv->img_trf_cancel);
return; return;
} }
@ -122,11 +126,10 @@ img_cb (FpiUsbTransfer *transfer, FpDevice *device,
fpi_image_device_image_captured (dev, img); fpi_image_device_image_captured (dev, img);
/* FIXME: rather than assuming finger has gone, we should poll regs until /* FIXME: rather than assuming finger has gone, we should poll regs until
* it really has, then restart the capture */ * it really has. */
fpi_image_device_report_finger_status (dev, FALSE); fpi_image_device_report_finger_status (dev, FALSE);
/* Note: We always restart the transfer, it may already be cancelled though. */ /* Note: The transfer is re-started when we switch to the AWAIT_FINGER_ON state. */
do_capture (dev);
} }
static void static void
@ -141,31 +144,51 @@ do_capture (FpImageDevice *dev)
fpi_usb_transfer_fill_bulk (img_trf, EP_IN, cls->data_buflen); fpi_usb_transfer_fill_bulk (img_trf, EP_IN, cls->data_buflen);
img_trf->short_is_error = TRUE; img_trf->short_is_error = TRUE;
fpi_usb_transfer_submit (g_steal_pointer (&img_trf), 0, fpi_usb_transfer_submit (g_steal_pointer (&img_trf), 0,
priv->img_trf_cancel, priv->img_capture_cancel,
img_cb, NULL); img_cb, NULL);
} }
static void
capture_reqs_cb (FpImageDevice *dev, GError *result, void *user_data)
{
FpiDeviceAes3k *self = FPI_DEVICE_AES3K (dev);
FpiDeviceAes3kPrivate *priv = fpi_device_aes3k_get_instance_private (self);
if (result)
{
g_clear_object (&priv->img_capture_cancel);
fpi_image_device_session_error (dev, result);
return;
}
/* FIXME: we never cancel a pending capture. So we are likely leaving the
* hardware in a bad state should we abort the capture operation and the
* user does not touch the device.
* But, we don't know how we might cancel, so just leave it as is. */
do_capture (dev);
}
static void
do_capture_start (FpImageDevice *dev)
{
FpiDeviceAes3k *self = FPI_DEVICE_AES3K (dev);
FpiDeviceAes3kClass *cls = FPI_DEVICE_AES3K_GET_CLASS (self);
aes_write_regv (dev, cls->capture_reqs, cls->capture_reqs_len, capture_reqs_cb, NULL);
}
static void static void
init_reqs_cb (FpImageDevice *dev, GError *result, void *user_data) init_reqs_cb (FpImageDevice *dev, GError *result, void *user_data)
{ {
FpiDeviceAes3kPrivate *priv = fpi_device_aes3k_get_instance_private (FPI_DEVICE_AES3K (dev));
fpi_image_device_activate_complete (dev, result); fpi_image_device_activate_complete (dev, result);
if (!result)
{
priv->img_trf_cancel = g_cancellable_new ();
do_capture (dev);
}
} }
static void static void
aes3k_dev_activate (FpImageDevice *dev) aes3k_dev_activate (FpImageDevice *dev)
{ {
FpiDeviceAes3k *self = FPI_DEVICE_AES3K (dev); FpiDeviceAes3k *self = FPI_DEVICE_AES3K (dev);
FpiDeviceAes3kPrivate *priv = fpi_device_aes3k_get_instance_private (self);
FpiDeviceAes3kClass *cls = FPI_DEVICE_AES3K_GET_CLASS (self); FpiDeviceAes3kClass *cls = FPI_DEVICE_AES3K_GET_CLASS (self);
g_assert (!priv->img_trf_cancel);
aes_write_regv (dev, cls->init_reqs, cls->init_reqs_len, init_reqs_cb, NULL); aes_write_regv (dev, cls->init_reqs, cls->init_reqs_len, init_reqs_cb, NULL);
} }
@ -175,13 +198,28 @@ aes3k_dev_deactivate (FpImageDevice *dev)
FpiDeviceAes3k *self = FPI_DEVICE_AES3K (dev); FpiDeviceAes3k *self = FPI_DEVICE_AES3K (dev);
FpiDeviceAes3kPrivate *priv = fpi_device_aes3k_get_instance_private (self); FpiDeviceAes3kPrivate *priv = fpi_device_aes3k_get_instance_private (self);
/* Deactivation finishes from the cancellation handler */ /* If a capture is running, then deactivation finishes from the cancellation handler */
if (priv->img_trf_cancel) if (priv->img_capture_cancel)
g_cancellable_cancel (priv->img_trf_cancel); g_cancellable_cancel (priv->img_capture_cancel);
else else
fpi_image_device_deactivate_complete (dev, NULL); fpi_image_device_deactivate_complete (dev, NULL);
} }
static void
aes3k_dev_change_state (FpImageDevice *dev, FpiImageDeviceState state)
{
FpiDeviceAes3k *self = FPI_DEVICE_AES3K (dev);
FpiDeviceAes3kPrivate *priv = fpi_device_aes3k_get_instance_private (self);
if (state == FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON)
{
g_assert (!priv->img_capture_cancel);
priv->img_capture_cancel = g_cancellable_new ();
do_capture_start (dev);
}
}
static void static void
fpi_device_aes3k_init (FpiDeviceAes3k *self) fpi_device_aes3k_init (FpiDeviceAes3k *self)
{ {
@ -224,6 +262,7 @@ fpi_device_aes3k_class_init (FpiDeviceAes3kClass *klass)
img_class->img_open = aes3k_dev_init; img_class->img_open = aes3k_dev_init;
img_class->img_close = aes3k_dev_deinit; img_class->img_close = aes3k_dev_deinit;
img_class->activate = aes3k_dev_activate; img_class->activate = aes3k_dev_activate;
img_class->change_state = aes3k_dev_change_state;
img_class->deactivate = aes3k_dev_deactivate; img_class->deactivate = aes3k_dev_deactivate;
/* Extremely low due to low image quality. */ /* Extremely low due to low image quality. */

View file

@ -57,4 +57,6 @@ struct _FpiDeviceAes3kClass
gsize data_buflen; /* buffer length of usb bulk transfer */ gsize data_buflen; /* buffer length of usb bulk transfer */
struct aes_regwrite *init_reqs; /* initial values sent to device */ struct aes_regwrite *init_reqs; /* initial values sent to device */
gsize init_reqs_len; gsize init_reqs_len;
struct aes_regwrite *capture_reqs; /* capture values sent to device */
gsize capture_reqs_len;
}; };

View file

@ -103,7 +103,9 @@ static struct aes_regwrite init_reqs[] = {
{ 0x9e, 0x53 }, /* clear challenge word bits */ { 0x9e, 0x53 }, /* clear challenge word bits */
{ 0x9f, 0x6b }, /* set some challenge word bits */ { 0x9f, 0x6b }, /* set some challenge word bits */
{ 0, 0 }, { 0, 0 },
};
static struct aes_regwrite capture_reqs[] = {
{ 0x80, 0x00 }, { 0x80, 0x00 },
{ 0x81, 0x00 }, { 0x81, 0x00 },
{ 0, 0 }, { 0, 0 },
@ -152,4 +154,6 @@ fpi_device_aes4000_class_init (FpiDeviceAes4000Class *klass)
aes_class->enlarge_factor = ENLARGE_FACTOR; aes_class->enlarge_factor = ENLARGE_FACTOR;
aes_class->init_reqs = init_reqs; aes_class->init_reqs = init_reqs;
aes_class->init_reqs_len = G_N_ELEMENTS (init_reqs); aes_class->init_reqs_len = G_N_ELEMENTS (init_reqs);
aes_class->capture_reqs = capture_reqs;
aes_class->capture_reqs_len = G_N_ELEMENTS (capture_reqs);
} }