elanspi: add ELAN7001 support for HID PID 04F3:3212 and use SIGFM matching - #41
Open
EleazarGamezD wants to merge 42 commits into
Open
EleazarGamezD wants to merge 42 commits into
EleazarGamezD wants to merge 42 commits into
Conversation
now it uses a proper binary serialization, but this will break any stored enrolls with the old format
Co-authored-by: Dinolek <18579460+Dinolek@users.noreply.github.com>
Integrate sigfm algorithm
Remove .vscode, adjust .gitignore
Remove .vscode, adjust .gitignore
Add workflow config
Signed-off-by: Alexander Meiler <rootd@users.noreply.github.com>
Signed-off-by: Alexander Meiler <rootd@users.noreply.github.com>
Update package generation
…o freedesktop-master
Signed-off-by: Alexander Meiler <rootd@users.noreply.github.com>
Signed-off-by: Alexander Meiler <rootd@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
It introduces multiple build/runtime correctness issues (public header includes a non-installed SIGFM header, incorrect freeing in fp-print serialization, SIGFM extraction null-deref risk, and Meson integration problems) that should be resolved before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds SIGFM-based matching support to libfprint and switches the elanspi image driver to use SIGFM, while also adding a new ELAN7001 HID companion PID (04F3:3212) so the device is claimed.
Changes:
- Add SIGFM matcher implementation + serialization helpers and build integration (OpenCV-based).
- Extend print/image plumbing to support a new
FPI_PRINT_SIGFMprint type and matching path. - Add ELAN7001 HID PID
0x3212toelanspidevice table and setelanspito use SIGFM.
File summaries
| File | Description |
|---|---|
| libfprint/sigfm/tests.cpp | Adds doctest-based SIGFM unit tests. |
| libfprint/sigfm/sigfm.hpp | Introduces SIGFM C API header for extraction/matching/serialization. |
| libfprint/sigfm/sigfm.cpp | Implements SIGFM extraction + matching using OpenCV features. |
| libfprint/sigfm/meson.build | Builds SIGFM static lib and test executable (currently unconditional). |
| libfprint/sigfm/img-info.hpp | Defines SigfmImgInfo backing struct (C++). |
| libfprint/sigfm/binary.hpp | Adds binary serialization helpers for OpenCV types/SigfmImgInfo. |
| libfprint/meson.build | Integrates SIGFM subdir and links SIGFM into libfprint-private. |
| libfprint/fpi-print.h | Extends internal print types and declares SIGFM match entrypoint. |
| libfprint/fpi-print.c | Implements SIGFM print storage, add-from-image, and SIGFM matching path. |
| libfprint/fpi-image.h | Extends internal image struct with sigfm_info storage. |
| libfprint/fpi-image-device.h | Adds algorithm selection field to image device class. |
| libfprint/fpi-image-device.c | Selects SIGFM vs NBIS matching based on device algorithm. |
| libfprint/fp-print.c | Extends public print serialization/deserialization to SIGFM. |
| libfprint/fp-image.h | Adds SIGFM extraction/accessors to public image API surface. |
| libfprint/fp-image.c | Implements async SIGFM extraction and stores result on FpImage. |
| libfprint/fp-image-device.c | Plumbs algorithm from class into instance behavior (enroll/verify/capture). |
| libfprint/fp-image-device-private.h | Stores selected algorithm on the device private struct. |
| libfprint/drivers/elanspi.h | Adds the new ELAN7001 HID companion PID 0x3212 mapping. |
| libfprint/drivers/elanspi.c | Switches elanspi to use SIGFM matching algorithm. |
| .gitignore | Ignores .vscode/. |
| .github/workflows/main.yml | Adds a GitHub Actions workflow for building packages. |
| .github/FUNDING.yml | Adds funding metadata. |
Review details
Suppressed comments (3)
libfprint/fp-image.h:30
- Public header fp-image.h includes "sigfm/sigfm.hpp", but that header is not installed with the public headers set, so downstream consumers will fail to compile. Since only an opaque SigfmImgInfo* is used here, forward-declare the type instead of including a non-public header.
#include "sigfm/sigfm.hpp"
#include <gio/gio.h>
G_BEGIN_DECLS
#define FP_TYPE_IMAGE (fp_image_get_type ())
typedef struct fp_minutia FpMinutia;
libfprint/fp-image.c:319
- fp_image_sigfm_extract_thread_func calls sigfm_keypoints_count(data->sigfm_info) without checking whether sigfm_extract returned NULL, which can crash. Also the error string "No enough keypoints found" is ungrammatical.
data->sigfm_info = sigfm_extract (data->image, data->width, data->height);
g_timer_stop (timer);
fp_dbg ("sigfm extract completed in %f secs", g_timer_elapsed (timer, NULL));
g_timer_destroy (timer);
if (sigfm_keypoints_count (data->sigfm_info) < 25)
libfprint/fp-print.c:768
- g_clear_object(&to_free) is incorrect for a GPtrArray (not a GObject) and can cause invalid unref at runtime. Free the GPtrArray with g_ptr_array_unref instead (and ensure it has an element free func so serialized buffers are freed).
g_clear_object (&to_free);
- Files reviewed: 21/23 changed files
- Comments generated: 8
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+289
to
+297
| if (template->type != FPI_PRINT_SIGFM) | ||
| { | ||
| *error = fpi_device_error_new_msg ( | ||
| FP_DEVICE_ERROR_NOT_SUPPORTED, | ||
| "Cannot call sigfm match with non-sigfm print data, type was %d", | ||
| template->type); | ||
| return FPI_MATCH_ERROR; | ||
| } | ||
| SigfmImgInfo * against = g_ptr_array_index (print->prints, 0); |
Comment on lines
+52
to
+53
| FpiMatchResult fpi_print_sigfm_match (FpPrint * template, FpPrint * print, | ||
| gint bz3_threshold, GError * *error); |
Comment on lines
+251
to
260
| priv_deps = deps + libsigfm | ||
|
|
||
| libfprint_private = static_library('fprint-private', | ||
| sources: [ | ||
| fpi_enums, | ||
| libfprint_private_sources, | ||
| ], | ||
| dependencies: deps, | ||
| link_with: libnbis, | ||
| link_with: [libnbis, libsigfm], | ||
| install: false) |
Comment on lines
+212
to
+215
| g_clear_pointer (&image->data, g_free); | ||
| image->data = g_steal_pointer (&data->image); | ||
| image->sigfm_info = g_steal_pointer (&data->sigfm_info); | ||
| } |
| g_variant_builder_open (&builder, G_VARIANT_TYPE_VARDICT); | ||
| g_variant_builder_close (&builder); | ||
|
|
||
| GPtrArray * to_free = g_ptr_array_new (); |
Comment on lines
+4
to
+11
| opencv = dependency('opencv4', required: true, version: '>=4.5.0') | ||
| doctest = dependency('doctest', required: true, version: ['>=2.0.0', '<3.0.0']) | ||
|
|
||
| libsigfm = static_library('sigfm', | ||
| sigfm_sources, | ||
| dependencies: [opencv], | ||
| ) | ||
| sigfm_tests = executable('sigfm-tests', ['./tests.cpp'], dependencies: [doctest, opencv], link_with: [libsigfm]) |
Comment on lines
+76
to
+80
| bool operator<(const match& right) const | ||
| { | ||
| return (this->p1.y < right.p1.y) || | ||
| ((this->p1.y < right.p1.y) && this->p1.x < right.p1.x); | ||
| } |
Comment on lines
23
to
26
| #include "fp-image.h" | ||
| #include "sigfm/sigfm.hpp" | ||
| #include <config.h> | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds support for an ELAN7001 SPI fingerprint sensor paired with an ELAN HID device using VID:PID 04F3:3212, and enables the SIGFM matcher for the elanspi driver.
The hardware was found in an ASUS VivoBook X1605ZA and identifies as:
Fingerprint ACPI device: ELAN7001
Transport: SPI (spidev)
HID companion device: 04F3:3212
Detected fingerprint sensor: eFSA80SC
Sensor resolution: 80x80
Problem
The ELAN7001 fingerprint controller was correctly exposed through SPI, but libfprint did not claim the device because the corresponding HID PID 0x3212 was missing from elanspi_id_table.
After adding the device ID, image acquisition, calibration, swipe capture, image assembly, and enrollment all worked correctly.
However, verification with the default NBIS/Bozorth3 matcher consistently failed even when using the same enrolled finger.
Typical results with NBIS were:
successful sensor detection
successful image capture
successful enrollment
successful minutiae extraction
verification score: 0/24
result: Finger not matched
The detected sensor is an eFSA80SC with an 80x80 imaging area, for which SIGFM produced successful matching in this test setup.
Changes
Add the new ELAN7001 HID pairing
This PR adds support for:
{.udev_types = ELANSPI_UDEV_TYPES,
.spi_acpi_id = "ELAN7001",
.hid_id = {.vid = ELANSPI_TP_VID, .pid = 0x3212},
.driver_data = ELANSPI_90LEFT_ROTATE},
The HID device is also successfully used by the existing ELAN SPI hardware reset path.
Enable SIGFM for elanspi
The image device class is configured with:
img_class->algorithm = FPI_DEVICE_ALGO_SIGFM;
This switches fingerprint matching from NBIS to the SIGFM implementation available in the sigfm branch.
Testing
Tested on:
ASUS VivoBook X1605ZA
Linux Mint 22.3 / Ubuntu 24.04 base
Linux kernel 7.0.0-30-generic
ELAN7001 SPI fingerprint controller
ELAN HID 04F3:3212
eFSA80SC 80x80 sensor
SPI userspace buffer was configured as:
options spidev bufsiz=32768
Enrollment
The sensor is correctly detected:
Selected device 0 (ElanTech Embedded Fingerprint Sensor) claimed by elanspi driver
raw height = 80
raw width = 80
<init/detect> found sensor ID 14 => [eFSA80SC] (80 x 80)
Enrollment completes successfully for the right index finger.
Direct libfprint verification
Using the SIGFM build:
sigfm score 72/24
Match report: device ElanTech Embedded Fingerprint Sensor matched finger right index successfully
MATCH!
fprintd verification
After installing the patched library and enrolling the fingerprint through fprintd:
Using device /net/reactivated/Fprint/Device/0
Verify started!
Verifying: right-index-finger
Verify result: verify-match (done)
Fingerprint authentication is also working through PAM/sudo.
Notes
The 04F3:3212 HID device is the ELAN companion HID device used together with the ELAN7001 SPI fingerprint controller. The fingerprint sensor itself is not exposed as a USB fingerprint device.
The SIGFM matcher was necessary in this setup because capture and enrollment worked with NBIS, but repeated verification of the enrolled finger did not.
If enabling SIGFM globally for all elanspi devices is considered too broad, I am happy to adjust the implementation so SIGFM selection is limited to specific ELAN7001/eFSA80SC device combinations.
Result
With these changes, the previously unsupported integrated fingerprint reader can be used end-to-end with libfprint/fprintd, including desktop enrollment, verification, and PAM authentication.