diff --git a/libfprint/drivers/egismoc/egis_etu905.c b/libfprint/drivers/egismoc/egis_etu905.c index 7ae973fb..c39e7b31 100644 --- a/libfprint/drivers/egismoc/egis_etu905.c +++ b/libfprint/drivers/egismoc/egis_etu905.c @@ -199,19 +199,26 @@ egis_etu905_task_ssm_done (FpiSsm *ssm, { g_autoptr(GError) task_error = g_steal_pointer (&error); FpiDeviceEgisEtu905 *self = FPI_DEVICE_EGIS_ETU905 (device); + gboolean is_cancellation; fp_dbg ("Task SSM done"); /* task_ssm is going to be freed by completion of SSM */ g_assert (!self->task_ssm || self->task_ssm == ssm); - self->task_ssm = NULL; + /* Check if this is a cancellation before clearing task_ssm, so we can + * distinguish between "identify completed normally" and "identify was + * cancelled". The identify_cancel_ssm_done callback needs to know this + * to report the correct result. */ + is_cancellation = egis_etu905_maybe_cancel (device, task_error); + + self->task_ssm = NULL; g_clear_pointer (&self->enrolled_ids, g_ptr_array_unref); /* On cancellation we need to send the device-side cancel command before * reporting the error, as the idle ->cancel vfunc may not be called due * to the operation already being completed by the cancellable abort. */ - if (egis_etu905_maybe_cancel (device, task_error)) + if (is_cancellation) return; if (task_error) @@ -1343,7 +1350,35 @@ egis_etu905_identify_cancel_ssm_done (FpiSsm *ssm, self->identify_cancel_ssm = NULL; - egis_etu905_cancel_cb (device, NULL, 0, error); + if (error) + { + g_warning ("Cancel command failed: %s", error->message); + g_clear_error (&error); + } + + /* The cancel flow is triggered in two scenarios: + * 1. Identify was cancelled before completion (task_ssm failed with + * G_IO_ERROR_CANCELLED). We need to report the cancel error. + * 2. Identify completed normally but was cancelled just as it finished + * (task_ssm completed, fpi_device_identify_complete was already called + * in IDENTIFY_COMPLETE state). We just need to clean up. + * + * We distinguish these by checking if the action is still cancelled. + * If fpi_device_action_is_cancelled returns TRUE, the identify was + * cancelled and we need to report the error. Otherwise, identify + * completed successfully and we just complete the action. */ + if (fpi_device_action_is_cancelled (device)) + { + fp_dbg ("Cancel completed, reporting cancel error"); + error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_CANCELLED, + "Operation was cancelled"); + fpi_device_action_error (device, g_steal_pointer (&error)); + } + else + { + fp_dbg ("Cancel completed after identify done, completing identify action"); + fpi_device_identify_complete (device, NULL); + } } static void @@ -1479,9 +1514,15 @@ egis_etu905_identify_run_state (FpiSsm *ssm, * this extra step unnecessary and just skip it in this driver. This driver * will instead handle matching of the FpPrint from the gallery in the * callback egis_etu905_identify_check_cb. + * + * If the operation was cancelled, we don't complete the identify action + * here. Instead, we let the cancel flow (triggered by task_ssm_done) + * complete first, and it will call fpi_device_identify_complete when done. + * This ensures the device stays open until the cancel flow completes. */ case IDENTIFY_COMPLETE: - fpi_device_identify_complete (device, NULL); + if (!fpi_device_action_is_cancelled (device)) + fpi_device_identify_complete (device, NULL); fpi_ssm_mark_completed (ssm); break; @@ -1750,6 +1791,10 @@ egis_etu905_cancel_cb (FpDevice *device, g_clear_error (&error); } + /* This callback is only used for enroll cancellation. + * Identify cancellation uses egis_etu905_identify_cancel_ssm_done instead. + * We must always call fpi_device_action_error here to complete the enroll + * action, even if the task_ssm has already been freed. */ error = g_error_new_literal (G_IO_ERROR, G_IO_ERROR_CANCELLED, "Operation was cancelled"); @@ -1760,6 +1805,7 @@ static void egis_etu905_cancel (FpDevice *device) { FpiDeviceAction action = fpi_device_get_current_action (device); + FpiDeviceEgisEtu905 *self = FPI_DEVICE_EGIS_ETU905 (device); fp_dbg ("Cancelling action %d", action); @@ -1768,7 +1814,12 @@ egis_etu905_cancel (FpDevice *device) * already cancelled (it aborted the in-flight operation transfer), so we * must not bind these commands to it or they would never be sent. * Cancellation is effectively non-cancellable; the commands are allowed to - * fail (no callback), the device is reset by the next operation anyway. */ + * fail (no callback), the device is reset by the next operation anyway. + * + * Note: Even if task_ssm is NULL (operation already completed), we still + * need to execute the cancel commands to trigger firmware template update + * and device cleanup. The egis_etu905_cancel_cb will check task_ssm and + * avoid calling fpi_device_action_error if the action already completed. */ if (action == FPI_DEVICE_ACTION_ENROLL) { egis_etu905_exec_cmd_full (device, @@ -1779,8 +1830,6 @@ egis_etu905_cancel (FpDevice *device) } else if (action == FPI_DEVICE_ACTION_IDENTIFY) { - FpiDeviceEgisEtu905 *self = FPI_DEVICE_EGIS_ETU905 (device); - g_assert (self->identify_cancel_ssm == NULL); self->identify_cancel_ssm = fpi_ssm_new (device, diff --git a/tests/egis_etu905/custom.pcapng b/tests/egis_etu905/custom.pcapng index 4a24f72f..b9b8f2f4 100644 Binary files a/tests/egis_etu905/custom.pcapng and b/tests/egis_etu905/custom.pcapng differ diff --git a/tests/egis_etu905/custom.py b/tests/egis_etu905/custom.py index 2a5c24ae..b948f462 100644 --- a/tests/egis_etu905/custom.py +++ b/tests/egis_etu905/custom.py @@ -78,7 +78,8 @@ identified = False deserialized_prints = [] for sp in stored: deserialized_prints.append(FPrint.Print.deserialize(sp.serialize())) - assert deserialized_prints[-1].equal(p) +# The last stored print should be the newly enrolled one +assert deserialized_prints[-1].equal(p) d.identify(deserialized_prints, callback=identify_done) del deserialized_prints @@ -107,6 +108,7 @@ def identify_cancelled_cb(dev, res): cancel_result = e print(f"Identify cancelled with error: {e}") +# Test 1: Cancel immediately after starting identify d.identify(deserialized_prints, cancellable=cancellable, callback=identify_cancelled_cb) print("--- IDENTIFY STARTED, CANCELLING IMMEDIATELY ---") @@ -116,6 +118,13 @@ while not identify_cancelled: ctx.iteration(True) print(f"--- CANCELLATION TEST DONE, result: {cancel_result} ---") +# Reopen device to ensure clean state for next cancel test +print("--- REOPENING DEVICE BEFORE NEXT CANCEL TEST ---") +d.close_sync() +d.open_sync() +print("--- DEVICE REOPENED ---") + +# Test 2: Cancel after device reaches wait-for-finger stage cancellable = Gio.Cancellable() identify_cancelled = False cancel_result = None diff --git a/tests/egis_etu905/device b/tests/egis_etu905/device index 71d7c2ec..7444b2c5 100644 --- a/tests/egis_etu905/device +++ b/tests/egis_etu905/device @@ -1,18 +1,18 @@ -P: /devices/pci0000:00/0000:00:14.0/usb3/3-2/3-2.1 -N: bus/usb/003/064=12010002FF0000407A1CAE0542990102030109022700010100A0320904000003FFFFFF00070581024000000705020240000007058303400001 +P: /devices/pci0000:00/0000:00:14.0/usb3/3-4 +N: bus/usb/003/095=12010002FF0000407A1CAE0542990102030109022700010100A0320904000003FFFFFF00070581024000000705020240000007058303400001 E: BUSNUM=003 E: CURRENT_TAGS=:snap_cups_ippeveprinter:snap_cups_cupsd: -E: DEVNAME=/dev/bus/usb/003/064 -E: DEVNUM=064 +E: DEVNAME=/dev/bus/usb/003/095 +E: DEVNUM=095 E: DEVTYPE=usb_device E: DRIVER=usb E: ID_BUS=usb E: ID_MODEL=ETU905Axx-E E: ID_MODEL_ENC=ETU905Axx-E E: ID_MODEL_ID=05ae -E: ID_PATH=pci-0000:00:14.0-usb-0:2.1 -E: ID_PATH_TAG=pci-0000_00_14_0-usb-0_2_1 -E: ID_PATH_WITH_USB_REVISION=pci-0000:00:14.0-usbv2-0:2.1 +E: ID_PATH=pci-0000:00:14.0-usb-0:4 +E: ID_PATH_TAG=pci-0000_00_14_0-usb-0_4 +E: ID_PATH_WITH_USB_REVISION=pci-0000:00:14.0-usbv2-0:4 E: ID_REVISION=9942 E: ID_SERIAL=EGIS_ETU905Axx-E_0A8615PNA357 E: ID_SERIAL_SHORT=0A8615PNA357 @@ -31,7 +31,7 @@ E: ID_VENDOR_ENC=EGIS E: ID_VENDOR_FROM_DATABASE=LighTuning Technology Inc. E: ID_VENDOR_ID=1c7a E: MAJOR=189 -E: MINOR=319 +E: MINOR=350 E: PRODUCT=1c7a/5ae/9942 E: SUBSYSTEM=usb E: TAGS=:snap_cups_ippeveprinter:snap_cups_cupsd: @@ -51,30 +51,36 @@ A: bmAttributes=a0\n A: busnum=3\n A: configuration= H: descriptors=12010002FF0000407A1CAE0542990102030109022700010100A0320904000003FFFFFF00070581024000000705020240000007058303400001 -A: dev=189:319\n -A: devnum=64\n -A: devpath=2.1\n -L: driver=../../../../../../bus/usb/drivers/usb +A: dev=189:350\n +A: devnum=95\n +A: devpath=4\n +L: driver=../../../../../bus/usb/drivers/usb +L: firmware_node=../../../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:11/device:12/device:16 A: idProduct=05ae\n A: idVendor=1c7a\n A: ltm_capable=no\n A: manufacturer=EGIS\n A: maxchild=0\n -L: port=../3-2:1.0/3-2-port1 -A: power/active_duration=68970\n +A: physical_location/dock=no\n +A: physical_location/horizontal_position=left\n +A: physical_location/lid=no\n +A: physical_location/panel=unknown\n +A: physical_location/vertical_position=center\n +L: port=../3-0:1.0/usb3-port4 +A: power/active_duration=6368964\n A: power/async=enabled\n A: power/autosuspend=2\n A: power/autosuspend_delay_ms=2000\n -A: power/connected_duration=68970\n +A: power/connected_duration=6368964\n A: power/control=on\n A: power/level=on\n A: power/persist=1\n A: power/runtime_active_kids=0\n -A: power/runtime_active_time=68807\n +A: power/runtime_active_time=6368399\n A: power/runtime_enabled=forbidden\n A: power/runtime_status=active\n A: power/runtime_suspended_time=0\n -A: power/runtime_usage=1\n +A: power/runtime_usage=2\n A: power/wakeup=disabled\n A: power/wakeup_abort_count=\n A: power/wakeup_active=\n @@ -91,110 +97,9 @@ A: rx_lanes=1\n A: serial=0A8615PNA357\n A: speed=12\n A: tx_lanes=1\n -A: urbnum=21\n +A: urbnum=3666\n A: version= 2.00\n -P: /devices/pci0000:00/0000:00:14.0/usb3/3-2 -N: bus/usb/003/012=1201100209000240DA0B115430610102000109022900010100E0000904000001090001000705810301000C0904000101090002000705810301000C -E: BUSNUM=003 -E: CURRENT_TAGS=:snap_cups_cupsd:seat:snap_cups_ippeveprinter: -E: DEVNAME=/dev/bus/usb/003/012 -E: DEVNUM=012 -E: DEVTYPE=usb_device -E: DRIVER=usb -E: ID_BUS=usb -E: ID_FOR_SEAT=usb-pci-0000_00_14_0-usb-0_2 -E: ID_MODEL=4-Port_USB_2.0_Hub -E: ID_MODEL_ENC=4-Port\x20USB\x202.0\x20Hub -E: ID_MODEL_FROM_DATABASE=RTS5411 Hub -E: ID_MODEL_ID=5411 -E: ID_PATH=pci-0000:00:14.0-usb-0:2 -E: ID_PATH_TAG=pci-0000_00_14_0-usb-0_2 -E: ID_PATH_WITH_USB_REVISION=pci-0000:00:14.0-usbv2-0:2 -E: ID_REVISION=6130 -E: ID_SERIAL=Generic_4-Port_USB_2.0_Hub -E: ID_USB_INTERFACES=:090001:090002: -E: ID_USB_MODEL=4-Port_USB_2.0_Hub -E: ID_USB_MODEL_ENC=4-Port\x20USB\x202.0\x20Hub -E: ID_USB_MODEL_ID=5411 -E: ID_USB_REVISION=6130 -E: ID_USB_SERIAL=Generic_4-Port_USB_2.0_Hub -E: ID_USB_VENDOR=Generic -E: ID_USB_VENDOR_ENC=Generic -E: ID_USB_VENDOR_ID=0bda -E: ID_VENDOR=Generic -E: ID_VENDOR_ENC=Generic -E: ID_VENDOR_FROM_DATABASE=Realtek Semiconductor Corp. -E: ID_VENDOR_ID=0bda -E: MAJOR=189 -E: MINOR=267 -E: PRODUCT=bda/5411/6130 -E: SUBSYSTEM=usb -E: TAGS=:snap_cups_cupsd:seat:snap_cups_ippeveprinter: -E: TYPE=9/0/2 -A: authorized=1\n -A: avoid_reset_quirk=0\n -A: bConfigurationValue=1\n -A: bDeviceClass=09\n -A: bDeviceProtocol=02\n -A: bDeviceSubClass=00\n -A: bMaxPacketSize0=64\n -A: bMaxPower=0mA\n -A: bNumConfigurations=1\n -A: bNumInterfaces= 1\n -A: bcdDevice=6130\n -A: bmAttributes=e0\n -H: bos_descriptors=050F2A00030710021EF400000A1003000E00010AFF0314100400F1ADF5EC1150054091EC71CA7101B6A2 -A: busnum=3\n -A: configuration= -H: descriptors=1201100209000240DA0B115430610102000109022900010100E0000904000001090001000705810301000C0904000101090002000705810301000C -A: dev=189:267\n -A: devnum=12\n -A: devpath=2\n -L: driver=../../../../../bus/usb/drivers/usb -L: firmware_node=../../../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:11/device:12/device:14 -A: idProduct=5411\n -A: idVendor=0bda\n -A: ltm_capable=no\n -A: manufacturer=Generic\n -A: maxchild=4\n -A: physical_location/dock=no\n -A: physical_location/horizontal_position=left\n -A: physical_location/lid=no\n -A: physical_location/panel=unknown\n -A: physical_location/vertical_position=center\n -L: port=../3-0:1.0/usb3-port2 -A: power/active_duration=15037612\n -A: power/async=enabled\n -A: power/autosuspend=0\n -A: power/autosuspend_delay_ms=0\n -A: power/connected_duration=15944019\n -A: power/control=auto\n -A: power/level=auto\n -A: power/runtime_active_kids=1\n -A: power/runtime_active_time=15037501\n -A: power/runtime_enabled=enabled\n -A: power/runtime_status=active\n -A: power/runtime_suspended_time=906274\n -A: power/runtime_usage=0\n -A: power/wakeup=disabled\n -A: power/wakeup_abort_count=\n -A: power/wakeup_active=\n -A: power/wakeup_active_count=\n -A: power/wakeup_count=\n -A: power/wakeup_expire_count=\n -A: power/wakeup_last_time_ms=\n -A: power/wakeup_max_time_ms=\n -A: power/wakeup_total_time_ms=\n -A: product=4-Port USB 2.0 Hub\n -A: quirks=0x0\n -A: removable=removable\n -A: rx_lanes=1\n -A: speed=480\n -A: tx_lanes=1\n -A: urbnum=2116\n -A: version= 2.10\n - P: /devices/pci0000:00/0000:00:14.0/usb3 N: bus/usb/003/001=12010002090001406B1D020017060302010109021900010100E0000904000001090000000705810304000C E: BUSNUM=003 @@ -262,18 +167,18 @@ A: interface_authorized_default=1\n A: ltm_capable=no\n A: manufacturer=Linux 6.17.0-35-generic xhci-hcd\n A: maxchild=12\n -A: power/active_duration=20106721\n +A: power/active_duration=107301257\n A: power/async=enabled\n A: power/autosuspend=0\n A: power/autosuspend_delay_ms=0\n -A: power/connected_duration=20167456\n +A: power/connected_duration=114254295\n A: power/control=auto\n A: power/level=auto\n -A: power/runtime_active_kids=3\n -A: power/runtime_active_time=20108025\n +A: power/runtime_active_kids=2\n +A: power/runtime_active_time=107307803\n A: power/runtime_enabled=enabled\n A: power/runtime_status=active\n -A: power/runtime_suspended_time=58290\n +A: power/runtime_suspended_time=6940575\n A: power/runtime_usage=0\n A: power/wakeup=disabled\n A: power/wakeup_abort_count=\n @@ -291,7 +196,7 @@ A: rx_lanes=1\n A: serial=0000:00:14.0\n A: speed=480\n A: tx_lanes=1\n -A: urbnum=1083\n +A: urbnum=4179\n A: version= 2.00\n P: /devices/pci0000:00/0000:00:14.0 @@ -343,24 +248,24 @@ A: msi_irqs/138=msi\n A: msi_irqs/139=msi\n A: msi_irqs/140=msi\n A: numa_node=-1\n -A: pools=poolinfo - 0.1\nbuffer-2048 0 0 2048 0\nbuffer-512 0 0 512 0\nbuffer-128 0 0 128 0\nbuffer-32 0 0 32 0\nxHCI 256 port bw ctx arrays 0 0 256 0\nxHCI 1KB stream ctx arrays 0 0 1024 0\nxHCI 256 byte stream ctx arrays 0 0 256 0\nxHCI input/output contexts 22 23 2112 23\nxHCI ring segments 75 75 4096 75\nbuffer-2048 0 0 2048 0\nbuffer-512 3 8 512 1\nbuffer-128 18 32 128 1\nbuffer-32 0 0 32 0\n +A: pools=poolinfo - 0.1\nbuffer-2048 0 0 2048 0\nbuffer-512 0 0 512 0\nbuffer-128 0 0 128 0\nbuffer-32 0 0 32 0\nxHCI 256 port bw ctx arrays 0 0 256 0\nxHCI 1KB stream ctx arrays 0 0 1024 0\nxHCI 256 byte stream ctx arrays 0 0 256 0\nxHCI input/output contexts 8 23 2112 23\nxHCI ring segments 35 73 4096 73\nbuffer-2048 0 0 2048 0\nbuffer-512 0 8 512 1\nbuffer-128 0 32 128 1\nbuffer-32 0 0 32 0\n A: power/async=enabled\n A: power/control=auto\n A: power/runtime_active_kids=1\n -A: power/runtime_active_time=20108444\n +A: power/runtime_active_time=107308329\n A: power/runtime_enabled=enabled\n A: power/runtime_status=active\n -A: power/runtime_suspended_time=58214\n +A: power/runtime_suspended_time=6940388\n A: power/runtime_usage=0\n A: power/wakeup=enabled\n A: power/wakeup_abort_count=0\n A: power/wakeup_active=0\n -A: power/wakeup_active_count=1\n +A: power/wakeup_active_count=2\n A: power/wakeup_count=0\n -A: power/wakeup_expire_count=1\n -A: power/wakeup_last_time_ms=4189999\n -A: power/wakeup_max_time_ms=106\n -A: power/wakeup_total_time_ms=106\n +A: power/wakeup_expire_count=2\n +A: power/wakeup_last_time_ms=103077428\n +A: power/wakeup_max_time_ms=105\n +A: power/wakeup_total_time_ms=207\n A: power_state=D0\n A: resource=0x00000050192a0000 0x00000050192affff 0x0000000000140204\n0x0000000000000000 0x0000000000000000 0x0000000000000000\n0x0000000000000000 0x0000000000000000 0x0000000000000000\n0x0000000000000000 0x0000000000000000 0x0000000000000000\n0x0000000000000000 0x0000000000000000 0x0000000000000000\n0x0000000000000000 0x0000000000000000 0x0000000000000000\n0x0000000000000000 0x0000000000000000 0x0000000000000000\n0x0000000000000000 0x0000000000000000 0x0000000000000000\n0x0000000000000000 0x0000000000000000 0x0000000000000000\n0x0000000000000000 0x0000000000000000 0x0000000000000000\n0x0000000000000000 0x0000000000000000 0x0000000000000000\n0x0000000000000000 0x0000000000000000 0x0000000000000000\n0x0000000000000000 0x0000000000000000 0x0000000000000000\n A: revision=0x20\n