diff --git a/libfprint/drivers/elan.c b/libfprint/drivers/elan.c index f88ec3e9..b29a2046 100644 --- a/libfprint/drivers/elan.c +++ b/libfprint/drivers/elan.c @@ -983,11 +983,14 @@ elan_change_state (FpImageDevice *idev) elan_calibrate (dev); break; + case FPI_IMAGE_DEVICE_STATE_IDLE: + case FPI_IMAGE_DEVICE_STATE_ACTIVATING: + case FPI_IMAGE_DEVICE_STATE_INACTIVE: case FPI_IMAGE_DEVICE_STATE_CAPTURE: /* not used */ break; - case FPI_IMAGE_DEVICE_STATE_INACTIVE: + case FPI_IMAGE_DEVICE_STATE_DEACTIVATING: case FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF: elan_stop_capture (dev); break; @@ -1012,6 +1015,11 @@ dev_change_state (FpImageDevice *dev, FpiImageDeviceState state) /* Inactive and await finger off are equivalent for the elan driver. */ if (state == FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF) + state = FPI_IMAGE_DEVICE_STATE_DEACTIVATING; + + /* The internal state may already be inactive, ignore deactivation then. */ + if (self->dev_state_next == FPI_IMAGE_DEVICE_STATE_INACTIVE && + state == FPI_IMAGE_DEVICE_STATE_DEACTIVATING) state = FPI_IMAGE_DEVICE_STATE_INACTIVE; if (self->dev_state_next == state) @@ -1022,7 +1030,7 @@ dev_change_state (FpImageDevice *dev, FpiImageDeviceState state) switch (state) { - case FPI_IMAGE_DEVICE_STATE_INACTIVE: + case FPI_IMAGE_DEVICE_STATE_DEACTIVATING: case FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON: case FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF: { char *name; @@ -1041,14 +1049,14 @@ dev_change_state (FpImageDevice *dev, FpiImageDeviceState state) break; } + case FPI_IMAGE_DEVICE_STATE_IDLE: + case FPI_IMAGE_DEVICE_STATE_ACTIVATING: + case FPI_IMAGE_DEVICE_STATE_INACTIVE: case FPI_IMAGE_DEVICE_STATE_CAPTURE: /* TODO MAYBE: split capture ssm into smaller ssms and use this state */ self->dev_state = state; self->dev_state_next = state; break; - - default: - g_assert_not_reached (); } } @@ -1070,7 +1078,7 @@ dev_deactivate (FpImageDevice *dev) * need to signal back deactivation) and then ensure we will change * to the inactive state eventually. */ self->deactivating = TRUE; - dev_change_state (dev, FPI_IMAGE_DEVICE_STATE_INACTIVE); + dev_change_state (dev, FPI_IMAGE_DEVICE_STATE_DEACTIVATING); } } diff --git a/libfprint/drivers/uru4000.c b/libfprint/drivers/uru4000.c index 2ba4d396..0e67b2dc 100644 --- a/libfprint/drivers/uru4000.c +++ b/libfprint/drivers/uru4000.c @@ -412,18 +412,6 @@ dev_change_state (FpImageDevice *dev, FpiImageDeviceState state) { FpiDeviceUru4000 *self = FPI_DEVICE_URU4000 (dev); - switch (state) - { - case FPI_IMAGE_DEVICE_STATE_INACTIVE: - case FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON: - case FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF: - case FPI_IMAGE_DEVICE_STATE_CAPTURE: - break; - - default: - g_assert_not_reached (); - } - self->activate_state = state; if (self->img_transfer != NULL) return; @@ -1185,7 +1173,10 @@ deactivate_write_reg_cb (FpiUsbTransfer *transfer, FpDevice *dev, static void dev_deactivate (FpImageDevice *dev) { - dev_change_state (dev, FPI_IMAGE_DEVICE_STATE_INACTIVE); + /* This is started/handled by execute_state_change in order to delay the + * action until after the image transfer has completed. + * We just need to override the function so that the complete handler is + * not called automatically. */ } static void @@ -1196,7 +1187,7 @@ execute_state_change (FpImageDevice *dev) switch (self->activate_state) { - case FPI_IMAGE_DEVICE_STATE_INACTIVE: + case FPI_IMAGE_DEVICE_STATE_DEACTIVATING: fp_dbg ("deactivating"); self->irq_cb = NULL; self->irq_cb_data = NULL; @@ -1251,6 +1242,12 @@ execute_state_change (FpImageDevice *dev) write_reg (dev, REG_MODE, MODE_AWAIT_FINGER_OFF, change_state_write_reg_cb, NULL); break; + + /* Ignored states */ + case FPI_IMAGE_DEVICE_STATE_IDLE: + case FPI_IMAGE_DEVICE_STATE_ACTIVATING: + case FPI_IMAGE_DEVICE_STATE_INACTIVE: + break; } } diff --git a/libfprint/drivers/virtual-image.c b/libfprint/drivers/virtual-image.c index 612fbcff..aa06080c 100644 --- a/libfprint/drivers/virtual-image.c +++ b/libfprint/drivers/virtual-image.c @@ -195,7 +195,7 @@ recv_image (FpDeviceVirtualImage *self, GInputStream *stream) g_debug ("Starting image receive (if active), state is: %i", state); /* Only register if the state is active. */ - if (state != FPI_IMAGE_DEVICE_STATE_INACTIVE) + if (state >= FPI_IMAGE_DEVICE_STATE_IDLE) { g_input_stream_read_all_async (stream, self->recv_img_hdr, @@ -338,10 +338,10 @@ dev_activate (FpImageDevice *dev) { FpDeviceVirtualImage *self = FPI_DEVICE_VIRTUAL_IMAGE (dev); + fpi_image_device_activate_complete (dev, NULL); + if (self->connection) recv_image (self, g_io_stream_get_input_stream (G_IO_STREAM (self->connection))); - - fpi_image_device_activate_complete (dev, NULL); } static void diff --git a/libfprint/fpi-image-device.c b/libfprint/fpi-image-device.c index 0ad06d21..a4eb1ad3 100644 --- a/libfprint/fpi-image-device.c +++ b/libfprint/fpi-image-device.c @@ -41,6 +41,9 @@ fp_image_device_get_instance_private (FpImageDevice *self) g_type_class_get_instance_private_offset (img_class)); } +static void fp_image_device_change_state (FpImageDevice *self, + FpiImageDeviceState state); + /* Private shared functions */ void @@ -52,6 +55,7 @@ fpi_image_device_activate (FpImageDevice *self) g_assert (!priv->active); fp_dbg ("Activating image device"); + fp_image_device_change_state (self, FPI_IMAGE_DEVICE_STATE_ACTIVATING); cls->activate (self); } @@ -62,40 +66,77 @@ fpi_image_device_deactivate (FpImageDevice *self, gboolean cancelling) FpImageDevicePrivate *priv = fp_image_device_get_instance_private (self); FpImageDeviceClass *cls = FP_IMAGE_DEVICE_GET_CLASS (device); - if (!priv->active || priv->state == FPI_IMAGE_DEVICE_STATE_INACTIVE) + if (!priv->active || priv->state == FPI_IMAGE_DEVICE_STATE_DEACTIVATING) { /* XXX: We currently deactivate both from minutiae scan result * and finger off report. */ fp_dbg ("Already deactivated, ignoring request."); return; } - if (!cancelling && priv->state == FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON) - g_warning ("Deactivating image device while waiting for finger, this should not happen."); - - priv->state = FPI_IMAGE_DEVICE_STATE_INACTIVE; - g_object_notify (G_OBJECT (self), "fpi-image-device-state"); + if (!cancelling && priv->state != FPI_IMAGE_DEVICE_STATE_IDLE) + g_warning ("Deactivating image device while it is not idle, this should not happen."); fp_dbg ("Deactivating image device"); + fp_image_device_change_state (self, FPI_IMAGE_DEVICE_STATE_DEACTIVATING); cls->deactivate (self); } /* Static helper functions */ +/* This should not be called directly to activate/deactivate the device! */ static void fp_image_device_change_state (FpImageDevice *self, FpiImageDeviceState state) { FpImageDevicePrivate *priv = fp_image_device_get_instance_private (self); g_autofree char *prev_state_str = NULL; g_autofree char *state_str = NULL; + gboolean transition_is_valid = FALSE; + gint i; - /* Cannot change to inactive using this function. */ - g_assert (state != FPI_IMAGE_DEVICE_STATE_INACTIVE); + struct + { + FpiImageDeviceState from; + FpiImageDeviceState to; + } valid_transitions[] = { + { FPI_IMAGE_DEVICE_STATE_INACTIVE, FPI_IMAGE_DEVICE_STATE_ACTIVATING }, + + { FPI_IMAGE_DEVICE_STATE_ACTIVATING, FPI_IMAGE_DEVICE_STATE_IDLE }, + { FPI_IMAGE_DEVICE_STATE_ACTIVATING, FPI_IMAGE_DEVICE_STATE_INACTIVE }, + + { FPI_IMAGE_DEVICE_STATE_IDLE, FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON }, + { FPI_IMAGE_DEVICE_STATE_IDLE, FPI_IMAGE_DEVICE_STATE_CAPTURE }, /* raw mode -- currently not supported */ + { FPI_IMAGE_DEVICE_STATE_IDLE, FPI_IMAGE_DEVICE_STATE_DEACTIVATING }, + + { FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON, FPI_IMAGE_DEVICE_STATE_CAPTURE }, + { FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON, FPI_IMAGE_DEVICE_STATE_DEACTIVATING }, /* cancellation */ + + { FPI_IMAGE_DEVICE_STATE_CAPTURE, FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF }, + { FPI_IMAGE_DEVICE_STATE_CAPTURE, FPI_IMAGE_DEVICE_STATE_IDLE }, /* raw mode -- currently not supported */ + { FPI_IMAGE_DEVICE_STATE_CAPTURE, FPI_IMAGE_DEVICE_STATE_DEACTIVATING }, /* cancellation */ + + { FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF, FPI_IMAGE_DEVICE_STATE_IDLE }, + { FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF, FPI_IMAGE_DEVICE_STATE_DEACTIVATING }, /* cancellation */ + + { FPI_IMAGE_DEVICE_STATE_DEACTIVATING, FPI_IMAGE_DEVICE_STATE_INACTIVE }, + }; prev_state_str = g_enum_to_string (FPI_TYPE_IMAGE_DEVICE_STATE, priv->state); state_str = g_enum_to_string (FPI_TYPE_IMAGE_DEVICE_STATE, state); fp_dbg ("Image device internal state change from %s to %s", prev_state_str, state_str); + for (i = 0; i < G_N_ELEMENTS (valid_transitions); i++) + { + if (valid_transitions[i].from == priv->state && valid_transitions[i].to == state) + { + transition_is_valid = TRUE; + break; + } + } + if (!transition_is_valid) + g_warning ("Internal state machine issue: transition from %s to %s should not happen!", + prev_state_str, state_str); + priv->state = state; g_object_notify (G_OBJECT (self), "fpi-image-device-state"); g_signal_emit_by_name (self, "fpi-image-device-state-changed", priv->state); @@ -199,7 +240,7 @@ fpi_image_device_minutiae_detected (GObject *source_object, GAsyncResult *res, g if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { fp_image_device_maybe_complete_action (self, g_steal_pointer (&error)); - fpi_image_device_deactivate (self, FALSE); + fpi_image_device_deactivate (self, TRUE); return; } @@ -231,7 +272,7 @@ fpi_image_device_minutiae_detected (GObject *source_object, GAsyncResult *res, g { fp_image_device_maybe_complete_action (self, g_steal_pointer (&error)); /* We might not yet be deactivating, if we are enrolling. */ - fpi_image_device_deactivate (self, FALSE); + fpi_image_device_deactivate (self, TRUE); return; } } @@ -385,6 +426,8 @@ fpi_image_device_report_finger_status (FpImageDevice *self, * In the enroll case, the decision can only be made after minutiae * detection has finished. */ + fp_image_device_change_state (self, FPI_IMAGE_DEVICE_STATE_IDLE); + if (action != FPI_DEVICE_ACTION_ENROLL) fpi_image_device_deactivate (self, FALSE); else @@ -432,6 +475,7 @@ fpi_image_device_image_captured (FpImageDevice *self, FpImage *image) fpi_image_device_minutiae_detected, self); + /* XXX: This is wrong if we add support for raw capture mode. */ fp_image_device_change_state (self, FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF); } @@ -567,6 +611,7 @@ fpi_image_device_activate_complete (FpImageDevice *self, GError *error) action = fpi_device_get_current_action (FP_DEVICE (self)); g_return_if_fail (priv->active == FALSE); + g_return_if_fail (priv->state == FPI_IMAGE_DEVICE_STATE_ACTIVATING); g_return_if_fail (action == FPI_DEVICE_ACTION_ENROLL || action == FPI_DEVICE_ACTION_VERIFY || action == FPI_DEVICE_ACTION_IDENTIFY || @@ -585,6 +630,7 @@ fpi_image_device_activate_complete (FpImageDevice *self, GError *error) /* We always want to capture at this point, move to AWAIT_FINGER * state. */ + fp_image_device_change_state (self, FPI_IMAGE_DEVICE_STATE_IDLE); fp_image_device_change_state (self, FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON); } @@ -601,7 +647,7 @@ fpi_image_device_deactivate_complete (FpImageDevice *self, GError *error) FpImageDevicePrivate *priv = fp_image_device_get_instance_private (self); g_return_if_fail (priv->active == TRUE); - g_return_if_fail (priv->state == FPI_IMAGE_DEVICE_STATE_INACTIVE); + g_return_if_fail (priv->state == FPI_IMAGE_DEVICE_STATE_DEACTIVATING); g_debug ("Image device deactivation completed"); @@ -610,6 +656,8 @@ fpi_image_device_deactivate_complete (FpImageDevice *self, GError *error) /* Assume finger was removed. */ priv->finger_present = FALSE; + fp_image_device_change_state (self, FPI_IMAGE_DEVICE_STATE_INACTIVE); + fp_image_device_maybe_complete_action (self, error); } diff --git a/libfprint/fpi-image-device.h b/libfprint/fpi-image-device.h index 43492d8c..71472dbb 100644 --- a/libfprint/fpi-image-device.h +++ b/libfprint/fpi-image-device.h @@ -25,6 +25,9 @@ /** * FpiImageDeviceState: * @FPI_IMAGE_DEVICE_STATE_INACTIVE: inactive + * @FPI_IMAGE_DEVICE_STATE_ACTIVATING: State during activate callback + * @FPI_IMAGE_DEVICE_STATE_IDLE: Activated but idle + * @FPI_IMAGE_DEVICE_STATE_DEACTIVATING: State during deactivate callback * @FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON: waiting for the finger to be pressed or swiped * @FPI_IMAGE_DEVICE_STATE_CAPTURE: capturing an image * @FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF: waiting for the finger to be removed @@ -35,9 +38,33 @@ * The driver needs to call fpi_image_device_report_finger_status() to move * between the different states. Note that the capture state might be entered * unconditionally if the device supports raw capturing. + * + * A usual run would look like: + * - inactive -> activating: activate vfunc is called + * - activating -> idle: fpi_image_device_activate_complete() + * - idle -> await-finger-on + * - await-finger-on -> capture: fpi_image_device_report_finger_status() + * - capture -> await-finger-off: fpi_image_device_image_captured() + * - await-finger-off -> idle: fpi_image_device_report_finger_status() + * - idle -> deactivating: deactivate vfunc is called + * - deactivating -> inactive: fpi_image_device_deactivate_complete() + * + * Raw mode is currently not supported (not waiting for finger), but in that + * case the following transitions are valid: + * - idle -> capture + * - capture -> idle + * + * Also valid are these transitions in case of errors or cancellations: + * - activating -> inactive: fpi_image_device_activate_complete() + * - await-finger-on -> deactivating: deactivate vfunc is called + * - capture -> deactivating: deactivate vfunc is called + * - await-finger-off -> deactivating: deactivate vfunc is called */ typedef enum { FPI_IMAGE_DEVICE_STATE_INACTIVE, + FPI_IMAGE_DEVICE_STATE_ACTIVATING, + FPI_IMAGE_DEVICE_STATE_DEACTIVATING, + FPI_IMAGE_DEVICE_STATE_IDLE, FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_ON, FPI_IMAGE_DEVICE_STATE_CAPTURE, FPI_IMAGE_DEVICE_STATE_AWAIT_FINGER_OFF, @@ -58,11 +85,6 @@ typedef enum { * finger or image capture). Implementing this is optional, it can e.g. be * used to flash an LED when waiting for a finger. * - * These are the main entry points for image based drivers. For all but the - * change_state vfunc, implementations *must* eventually call the corresponding - * function to finish the operation. It is also acceptable to call the generic - * - * * These are the main entry points for drivers to implement. Drivers may not * implement all of these entry points if they do not support the operation * (or a default implementation is sufficient).