egis_etu905: Fix identify cancel to report cancelled error correctly

task_ssm_done cleared task_ssm before calling maybe_cancel, so identify_cancel_ssm_done couldn't tell whether the identify already completed or was cancelled. Use fpi_device_action_is_cancelled() and skip the duplicate identify_complete call in IDENTIFY_COMPLETE state when cancelled, letting the cancel flow handle completion.
2026-09-09 04:40:06 +00:00 · 2026-07-25 21:21:50 +00:00
parent a6240828c1
commit 695663f30d
4 changed files with 104 additions and 141 deletions
@@ -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,
@@ -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
@@ -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