diff --git a/libfprint/drivers/vfs5011.c b/libfprint/drivers/vfs5011.c index c1e684c..dd6c7f6 100644 --- a/libfprint/drivers/vfs5011.c +++ b/libfprint/drivers/vfs5011.c @@ -196,6 +196,7 @@ usb_exchange_async (FpiSsm *ssm, FpiSsm *subsm = fpi_ssm_new_full (FP_DEVICE (data->device), usbexchange_loop, data->stepcount, + data->stepcount, exchange_name); fpi_ssm_set_data (subsm, data, NULL); diff --git a/libfprint/drivers/vfs7552.c b/libfprint/drivers/vfs7552.c index 46589f5..971eb4f 100644 --- a/libfprint/drivers/vfs7552.c +++ b/libfprint/drivers/vfs7552.c @@ -327,6 +327,7 @@ usb_exchange_async (FpiSsm *ssm, FpiSsm *subsm = fpi_ssm_new_full (fpi_ssm_get_device (ssm), usbexchange_loop, data->stepcount, + data->stepcount, exchange_name); fpi_ssm_set_data (subsm, data, NULL); diff --git a/libfprint/fpi-ssm.c b/libfprint/fpi-ssm.c index e283a48..b0bc17e 100644 --- a/libfprint/fpi-ssm.c +++ b/libfprint/fpi-ssm.c @@ -35,9 +35,11 @@ * is often entirely linear. You can however also jump to a specific state * or do an early return from the SSM by completing it. * - * e.g. `S1` ↦ `S2` ↦ `S3` ↦ `S4` + * e.g. `S1` ↦ `S2` ↦ `S3` ↦ `S4` ↦ `C1` ↦ `C2` ↦ `final` * - * Where `S1` is the start state. + * Where `S1` is the start state. The `C1` and later states are cleanup states + * that may be defined. The difference is that these states will never be + * skipped when marking the SSM as completed. * * Use fpi_ssm_new() to create a new state machine with a defined number of * states. Note that the state numbers start at zero, making them match the @@ -76,6 +78,7 @@ struct _FpiSsm gpointer ssm_data; GDestroyNotify ssm_data_destroy; int nr_states; + int start_cleanup; int cur_state; gboolean completed; GSource *timeout; @@ -94,8 +97,9 @@ struct _FpiSsm * * Allocate a new ssm, with @nr_states states. The @handler callback * will be called after each state transition. - * This is a macro that calls fpi_ssm_new_full() using the stringified - * version of @nr_states, so will work better with named parameters. + * This is a macro that calls fpi_ssm_new_full() using @nr_states as the + * cleanup states and using the stringified version of @nr_states. It should + * be used with an enum value. * * Returns: a new #FpiSsm state machine */ @@ -105,6 +109,7 @@ struct _FpiSsm * @dev: a #fp_dev fingerprint device * @handler: the callback function * @nr_states: the number of states + * @start_cleanup: the first cleanup state * @machine_name: the name of the state machine (for debug purposes) * * Allocate a new ssm, with @nr_states states. The @handler callback @@ -116,17 +121,21 @@ FpiSsm * fpi_ssm_new_full (FpDevice *dev, FpiSsmHandlerCallback handler, int nr_states, + int start_cleanup, const char *machine_name) { FpiSsm *machine; BUG_ON (dev == NULL); BUG_ON (nr_states < 1); + BUG_ON (start_cleanup < 1); + BUG_ON (start_cleanup > nr_states); BUG_ON (handler == NULL); machine = g_new0 (FpiSsm, 1); machine->handler = handler; machine->nr_states = nr_states; + machine->start_cleanup = start_cleanup; machine->dev = dev; machine->name = g_strdup (machine_name); machine->completed = TRUE; @@ -370,11 +379,15 @@ fpi_ssm_start_subsm (FpiSsm *parent, FpiSsm *child) * @machine: an #FpiSsm state machine * * Mark a ssm as completed successfully. The callback set when creating - * the state machine with fpi_ssm_new () will be called synchronously. + * the state machine with fpi_ssm_new() will be called synchronously. + * + * Note that any later cleanup state will still be executed. */ void fpi_ssm_mark_completed (FpiSsm *machine) { + int next_state; + g_return_if_fail (machine != NULL); BUG_ON (machine->completed); @@ -382,6 +395,19 @@ fpi_ssm_mark_completed (FpiSsm *machine) fpi_ssm_clear_delayed_action (machine); + /* complete in a cleanup state just moves forward one step */ + if (machine->cur_state < machine->start_cleanup) + next_state = machine->start_cleanup; + else + next_state = machine->cur_state + 1; + + if (next_state < machine->nr_states) + { + machine->cur_state = next_state; + __ssm_call_handler (machine); + return; + } + machine->completed = TRUE; if (machine->error) @@ -451,7 +477,9 @@ fpi_ssm_mark_failed (FpiSsm *machine, GError *error) { g_return_if_fail (machine != NULL); g_assert (error); - if (machine->error) + + /* During cleanup it is OK to call fpi_ssm_mark_failed a second time */ + if (machine->error && machine->cur_state < machine->start_cleanup) { fp_warn ("[%s] SSM %s already has an error set, ignoring new error %s", fp_device_get_driver (machine->dev), machine->name, error->message); @@ -459,10 +487,15 @@ fpi_ssm_mark_failed (FpiSsm *machine, GError *error) return; } - fp_dbg ("[%s] SSM %s failed in state %d with error: %s", + fp_dbg ("[%s] SSM %s failed in state %d%s with error: %s", fp_device_get_driver (machine->dev), machine->name, - machine->cur_state, error->message); - machine->error = g_steal_pointer (&error); + machine->cur_state, + machine->cur_state >= machine->start_cleanup ? " (cleanup)" : "", + error->message); + if (!machine->error) + machine->error = g_steal_pointer (&error); + else + g_error_free (error); fpi_ssm_mark_completed (machine); } @@ -560,13 +593,16 @@ fpi_ssm_jump_to_state (FpiSsm *machine, int state) g_return_if_fail (machine != NULL); BUG_ON (machine->completed); - BUG_ON (state < 0 || state >= machine->nr_states); + BUG_ON (state < 0 || state > machine->nr_states); BUG_ON (machine->timeout != NULL); fpi_ssm_clear_delayed_action (machine); machine->cur_state = state; - __ssm_call_handler (machine); + if (machine->cur_state == machine->nr_states) + fpi_ssm_mark_completed (machine); + else + __ssm_call_handler (machine); } typedef struct @@ -607,7 +643,7 @@ fpi_ssm_jump_to_state_delayed (FpiSsm *machine, g_autofree char *source_name = NULL; g_return_if_fail (machine != NULL); - BUG_ON (state < 0 || state >= machine->nr_states); + BUG_ON (state < 0 || state > machine->nr_states); data = g_new0 (FpiSsmJumpToStateDelayedData, 1); data->machine = machine; diff --git a/libfprint/fpi-ssm.h b/libfprint/fpi-ssm.h index 66871de..eaadfc7 100644 --- a/libfprint/fpi-ssm.h +++ b/libfprint/fpi-ssm.h @@ -60,10 +60,11 @@ typedef void (*FpiSsmHandlerCallback)(FpiSsm *ssm, /* for library and drivers */ #define fpi_ssm_new(dev, handler, nr_states) \ - fpi_ssm_new_full (dev, handler, nr_states, #nr_states) + fpi_ssm_new_full (dev, handler, nr_states, nr_states, #nr_states) FpiSsm *fpi_ssm_new_full (FpDevice *dev, FpiSsmHandlerCallback handler, int nr_states, + int start_cleanup, const char *machine_name); void fpi_ssm_free (FpiSsm *machine); void fpi_ssm_start (FpiSsm *ssm, diff --git a/tests/test-fpi-ssm.c b/tests/test-fpi-ssm.c index 32b3179..7969c68 100644 --- a/tests/test-fpi-ssm.c +++ b/tests/test-fpi-ssm.c @@ -129,12 +129,12 @@ test_ssm_completed_callback (FpiSsm *ssm, } static FpiSsm * -ssm_test_new_full (int nr_states, const char *name) +ssm_test_new_full (int nr_states, int cleanup_state, const char *name) { FpiSsm *ssm; FpiSsmTestData *data; - ssm = fpi_ssm_new_full (fake_device, test_ssm_handler, nr_states, name); + ssm = fpi_ssm_new_full (fake_device, test_ssm_handler, nr_states, cleanup_state, name); data = fpi_ssm_test_data_new (); data->expected_last_state = nr_states; fpi_ssm_set_data (ssm, data, (GDestroyNotify) fpi_ssm_test_data_unref_by_ssm); @@ -145,7 +145,7 @@ ssm_test_new_full (int nr_states, const char *name) static FpiSsm * ssm_test_new (void) { - return ssm_test_new_full (FPI_TEST_SSM_STATE_NUM, "FPI_TEST_SSM"); + return ssm_test_new_full (FPI_TEST_SSM_STATE_NUM, FPI_TEST_SSM_STATE_NUM, "FPI_TEST_SSM"); } static gboolean @@ -188,7 +188,8 @@ test_ssm_new_full (void) FpiSsm *ssm; ssm = fpi_ssm_new_full (fake_device, test_ssm_handler, - FPI_TEST_SSM_STATE_NUM, "Test SSM Name"); + FPI_TEST_SSM_STATE_NUM, FPI_TEST_SSM_STATE_NUM, + "Test SSM Name"); g_assert_null (fpi_ssm_get_data (ssm)); g_assert_no_error (fpi_ssm_get_error (ssm)); @@ -215,6 +216,8 @@ test_ssm_new_wrong_states (void) g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, "*BUG:*nr_states*"); + g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, + "*BUG:*start_cleanup*"); ssm = fpi_ssm_new (fake_device, test_ssm_handler, -1); g_test_assert_expected_messages (); } @@ -271,7 +274,7 @@ test_ssm_start_single (void) g_autoptr(FpiSsmTestData) data = NULL; FpiSsm *ssm; - ssm = ssm_test_new_full (1, "FPI_TEST_SSM_SINGLE_STATE"); + ssm = ssm_test_new_full (1, 1, "FPI_TEST_SSM_SINGLE_STATE"); data = fpi_ssm_test_data_ref (fpi_ssm_get_data (ssm)); fpi_ssm_start (ssm, test_ssm_completed_callback); @@ -1170,7 +1173,7 @@ test_ssm_subssm_start (void) { g_autoptr(FpiSsm) ssm = ssm_test_new (); g_autoptr(FpiSsm) subssm = - ssm_test_new_full (FPI_TEST_SSM_STATE_NUM, "FPI_TEST_SUB_SSM"); + ssm_test_new_full (FPI_TEST_SSM_STATE_NUM, FPI_TEST_SSM_STATE_NUM, "FPI_TEST_SUB_SSM"); FpiSsmTestData *data = fpi_ssm_get_data (ssm); g_autoptr(FpiSsmTestData) subdata = @@ -1222,7 +1225,7 @@ test_ssm_subssm_mark_failed (void) { g_autoptr(FpiSsm) ssm = ssm_test_new (); g_autoptr(FpiSsm) subssm = - ssm_test_new_full (FPI_TEST_SSM_STATE_NUM, "FPI_TEST_SUB_SSM"); + ssm_test_new_full (FPI_TEST_SSM_STATE_NUM, FPI_TEST_SSM_STATE_NUM, "FPI_TEST_SUB_SSM"); g_autoptr(FpiSsmTestData) data = fpi_ssm_test_data_ref (fpi_ssm_get_data (ssm)); g_autoptr(FpiSsmTestData) subdata = @@ -1261,7 +1264,7 @@ test_ssm_subssm_start_with_started (void) { g_autoptr(FpiSsm) ssm = ssm_test_new (); g_autoptr(FpiSsm) subssm = - ssm_test_new_full (FPI_TEST_SSM_STATE_NUM, "FPI_TEST_SUB_SSM"); + ssm_test_new_full (FPI_TEST_SSM_STATE_NUM, FPI_TEST_SSM_STATE_NUM, "FPI_TEST_SUB_SSM"); FpiSsmTestData *data = fpi_ssm_get_data (ssm); g_autoptr(FpiSsmTestData) subdata = @@ -1305,7 +1308,7 @@ test_ssm_subssm_start_with_delayed (void) { g_autoptr(FpiSsm) ssm = ssm_test_new (); g_autoptr(FpiSsm) subssm = - ssm_test_new_full (FPI_TEST_SSM_STATE_NUM, "FPI_TEST_SUB_SSM"); + ssm_test_new_full (FPI_TEST_SSM_STATE_NUM, FPI_TEST_SSM_STATE_NUM, "FPI_TEST_SUB_SSM"); FpiSsmTestData *data = fpi_ssm_get_data (ssm); g_autoptr(FpiSsmTestData) subdata = @@ -1348,6 +1351,76 @@ test_ssm_subssm_start_with_delayed (void) g_assert_no_error (data->error); } +static void +test_ssm_cleanup_complete (void) +{ + FpiSsm *ssm = ssm_test_new_full (4, FPI_TEST_SSM_STATE_2, "FPI_TEST_SSM"); + + g_autoptr(FpiSsmTestData) data = fpi_ssm_test_data_ref (fpi_ssm_get_data (ssm)); + + fpi_ssm_start (ssm, test_ssm_completed_callback); + g_assert_cmpuint (g_slist_length (data->handlers_chain), ==, 1); + + data->expected_last_state = FPI_TEST_SSM_STATE_3; + + /* Completing jumps to the cleanup state */ + fpi_ssm_mark_completed (ssm); + g_assert_cmpint (data->handler_state, ==, FPI_TEST_SSM_STATE_2); + g_assert_cmpuint (g_slist_length (data->handlers_chain), ==, 2); + g_assert_false (data->completed); + g_assert_false (data->ssm_destroyed); + + /* Completing again jumps to the next cleanup state */ + fpi_ssm_mark_completed (ssm); + g_assert_cmpint (data->handler_state, ==, FPI_TEST_SSM_STATE_3); + g_assert_cmpuint (g_slist_length (data->handlers_chain), ==, 3); + g_assert_false (data->completed); + g_assert_false (data->ssm_destroyed); + + /* Completing again finalizes everything */ + fpi_ssm_mark_completed (ssm); + g_assert_cmpint (data->handler_state, ==, FPI_TEST_SSM_STATE_3); + g_assert_cmpuint (g_slist_length (data->handlers_chain), ==, 4); + g_assert_true (data->completed); + g_assert_no_error (data->error); + g_assert_true (data->ssm_destroyed); +} + +static void +test_ssm_cleanup_fail (void) +{ + FpiSsm *ssm = ssm_test_new_full (4, FPI_TEST_SSM_STATE_2, "FPI_TEST_SSM"); + + g_autoptr(FpiSsmTestData) data = fpi_ssm_test_data_ref (fpi_ssm_get_data (ssm)); + + fpi_ssm_start (ssm, test_ssm_completed_callback); + g_assert_cmpuint (g_slist_length (data->handlers_chain), ==, 1); + + data->expected_last_state = FPI_TEST_SSM_STATE_3; + + /* Failing jumps to the cleanup state */ + fpi_ssm_mark_failed (ssm, g_error_new (G_IO_ERROR, G_IO_ERROR_CANCELLED, "non-cleanup")); + g_assert_cmpint (data->handler_state, ==, FPI_TEST_SSM_STATE_2); + g_assert_cmpuint (g_slist_length (data->handlers_chain), ==, 2); + g_assert_false (data->completed); + g_assert_false (data->ssm_destroyed); + + /* Failing again jumps to the next cleanup state */ + fpi_ssm_mark_failed (ssm, g_error_new (G_IO_ERROR, G_IO_ERROR_FAILED, "cleanup 1")); + g_assert_cmpint (data->handler_state, ==, FPI_TEST_SSM_STATE_3); + g_assert_cmpuint (g_slist_length (data->handlers_chain), ==, 3); + g_assert_false (data->completed); + g_assert_false (data->ssm_destroyed); + + /* Failing again finalizes everything */ + fpi_ssm_mark_failed (ssm, g_error_new (G_IO_ERROR, G_IO_ERROR_FAILED, "cleanup 2")); + g_assert_cmpint (data->handler_state, ==, FPI_TEST_SSM_STATE_3); + g_assert_cmpuint (g_slist_length (data->handlers_chain), ==, 4); + g_assert_true (data->completed); + g_assert_error (data->error, G_IO_ERROR, G_IO_ERROR_CANCELLED); + g_assert_true (data->ssm_destroyed); +} + int main (int argc, char *argv[]) { @@ -1402,6 +1475,8 @@ main (int argc, char *argv[]) g_test_add_func ("/ssm/subssm/start/with_started", test_ssm_subssm_start_with_started); g_test_add_func ("/ssm/subssm/start/with_delayed", test_ssm_subssm_start_with_delayed); g_test_add_func ("/ssm/subssm/mark_failed", test_ssm_subssm_mark_failed); + g_test_add_func ("/ssm/cleanup/complete", test_ssm_cleanup_complete); + g_test_add_func ("/ssm/cleanup/fail", test_ssm_cleanup_fail); return g_test_run (); }