goodix533c: add native driver (FpDeviceClass + SIGFM), verified on real hardware - #40
Open
daemonhorn wants to merge 12 commits into
Open
goodix533c: add native driver (FpDeviceClass + SIGFM), verified on real hardware#40daemonhorn wants to merge 12 commits into
daemonhorn wants to merge 12 commits into
Conversation
27c6:533c's mcu_get_image request is 4 bytes (flags, 0x06, gain, 0x00) rather than this family's usual 1-byte payload, with flags distinguishing a no-finger calibration frame (0x01) from a live capture (0x41) -- confirmed against driver_53xc.py, the proven Python reference for this device. Added goodix_send_mcu_get_image_gain()/goodix_tls_read_image_gain() alongside the existing 1-byte-payload functions (unchanged), and a new opt-in use_gain_image_request/image_gain pair on FpiDeviceGoodixTls5xxClass, defaulting to FALSE/0 so goodix511 and every other driver sharing this base class keeps its exact current behavior. goodix5xx.c's calibrate_run/scan_get_img now route through this opt-in at both image-request call sites. This does not make a working 533c driver on its own -- see findings/native-driver-architecture.md in the parent project for why goodix5xx.c's FDT mode-switching (a separate, structural gap -- static get_mcu_cfg() vs. this device's per-session dynamic FDT template) still needs a real fork of the shared SCAN state machine, not a patch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y9XXWG7y2kfBEXJj3MS5yv
New FpDevice-rooted driver (not FpImageDevice, not FpiDeviceGoodixTls)
for 27c6:533c, living entirely under libfprint/drivers/goodix533c/.
goodix5xx.c's shared FDT state machine assumes a static per-device
config blob, but 533c's finger-detect baseline is read fresh from the
device every session -- a structural mismatch documented in
findings/native-driver-architecture.md -- so this driver bypasses
goodix5xx.c and goodix.c entirely and talks to the device directly,
reusing only the device-agnostic wire codec (goodix_proto.c/.h) and
embedded TLS-PSK server (goodixtls.c/.h) from the sibling goodixtls/
driver, unmodified.
The command layer, TLS handshake pump, and FDT/capture sequence are
ported from goodix.c's logic and cross-checked stage for stage against
vendor/goodix-fp-dump-nikicat/driver_53xc.py, which is authoritative
for this exact silicon. Two real protocol shapes in goodix.c disagree
with the Python driver and were NOT carried over: preset_psk_read's
payload/reply shape (goodix.c's GoodixPresetPsk struct omits the
offset field and reply parsing differs), and mcu_switch_to_fdt_down/up
prepending an undocumented control byte that driver_53xc.py's wire
format does not have. mcu_get_image's TLS_DATA reply (pack flags
0xb2) also isn't a case goodix.c's receive dispatcher recognizes at
all; this driver adds it.
Scope is deliberately narrow: open() (USB claim, read loop, nop,
firmware regex check, PSK-hash verification) plus a single no-finger
reference frame capture, decoded and squashed to 8-bit, exposed only
through a test-only entry point (fpi_device_goodix533c_capture_test)
since no enroll/verify/identify vfuncs exist yet. No flat-fielding
against a second frame -- the goal here is proving the USB/TLS/protocol
chain end-to-end, not ridge visibility, which needs a real finger.
Verified against real 27c6:533c hardware via the new
goodix533c-capture-test harness (libfprint/drivers/goodix533c/capture_test.c,
built only when 'goodix533c' is in the enabled driver list): open()
succeeds, TLS-PSK handshake completes, config uploads, FDT baseline
reads 12 samples, and the captured frame decodes to a sane, non-degenerate
108x88 raw pixel range of roughly [0, 4060] out of a 12-bit sensor range,
written out as a standard (non-transposed) binary PGM.
Two bugs were found and fixed against real hardware during this pass:
- preset_psk_read's reply carries the PSK's SHA-256 hash directly at
offset 9; re-hashing it before comparing (as an initial draft did)
can never match.
- write_sensor_register and nop are ACK-only on this device (no
second data reply); requesting one hangs until the command timeout.
goodix.c's 10ms timeout on tls_successfully_established, which its
own comment already flags as suspect ("always times out for some
reason"), was replaced with the same generic timeout used elsewhere.
Build wiring mirrors goodixtls511 exactly: a driver_sources entry,
a default_drivers entry, and a driver_helper_mapping entry pointing at
the existing 'goodixtls' helper bundle, which already pulls in
goodix_proto.c/goodixtls.c (plus goodix.c/goodix5xx.c, compiled but
unused by this driver) and the openssl/threads dependencies.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y9XXWG7y2kfBEXJj3MS5yv
Adds the second half of driver_53xc.py's run_driver() to the existing CAPTURE_STAGE_* SSM (one continuous state machine, not a parallel one): mcu_switch_to_sleep_mode/query_mcu_state after the reference frame, then arm finger detection (mcu_switch_to_fdt_down), wait for the device's asynchronous "touched" push (await_fdt_down_push, a bounded single read standing in for driver_53xc.py's PyUSB-timeout-driven polling loop), re-arm via mcu_switch_to_fdt_mode, capture a live (finger-present) frame, and mcu_switch_to_fdt_up. The reference frame is now kept in the private struct (self->reference_pixels) instead of only handed to a callback and discarded, so the live frame can be flat-fielded against it (ported least-squares scale+offset subtraction from flat_field() in driver_53xc.py, double precision) and min-max stretched to a spec-correct PGM. Live-capture gain is 0xc2, not driver_53xc.py's default 0x86 -- a deliberate, hardware-verified deviation documented in GOODIX533C_LIVE_IMAGE_GAIN's comment (0x86 clips ~47% of pixels on this unit; see NOTES.md's "Ridge visibility resolved" section). Verified against real 27c6:533c hardware without touching the sensor: all four new commands (sleep_mode 0x60, query_mcu_state 0xae, fdt_down 0x32 arm, fdt_up 0x34) ACK cleanly, the reference frame is still captured and written even though the sequence fails later, and the finger-wait stage times out cleanly at 30s with a specific "No finger detected within 30 seconds" error -- no hang, no crash, no generic error. The finger-detected/live-capture/flat-field success path is unverified (requires a physical touch during a follow-up run). capture_test.c is extended to drive the whole sequence, print a "Touch the sensor now" prompt right as finger-detection arms, and write both the reference PGM (always, if captured) and a flat-fielded "-live" PGM (only if a touch was detected). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y9XXWG7y2kfBEXJj3MS5yv
Vendored verbatim from AndyHazz's goodix53x5-libfprint fork (sibling 27c6:5335/5385/5395 driver, same 108x88 sensor resolution, same SIGFM approach, different/incompatible transport). This is a self-contained C++ library operating on raw pixel buffers with a plain extern "C" API (sigfm_extract/sigfm_free_info/sigfm_match_score/sigfm_serialize_binary/ sigfm_deserialize_binary/sigfm_keypoints_count/sigfm_copy_info) with no device/GObject coupling, so it can be reused as-is for goodix533c. Placed at the repo root's sigfm/ (not libfprint/sigfm/) per the task's literal instruction; verified separately that libfprint/meson.build's root_inc already puts the repo root on every driver's include path, so #include "sigfm/sigfm.hpp" resolves the same way regardless of which of the two locations is chosen.
Add a 'sigfm' driver_helper (goodix533c -> ['goodixtls', 'sigfm']) using the existing driver_helper_mapping/optional_deps convention (same pattern already used for openssl/threads via the 'goodixtls' helper), so OpenCV is resolved and libsigfm only gets declared/linked when -Ddrivers includes goodix533c -- never unconditionally for the whole libfprint build. Tries opencv5 first, falls back to opencv4, and links only the specific opencv_core/opencv_features2d(-or-opencv_features on OpenCV 5)/opencv_flann/opencv_imgproc modules libsigfm actually needs, per AndyHazz's proven meson-integration-goodix533c.patch snippet (adapted here to be conditional rather than unconditional, and to declare libsigfm from the top-level meson.build so its 'sigfm/sigfm.cpp' source path resolves against the repo root where sigfm/ actually lives). libsigfm is threaded into libfprint_drivers, the final libfprint shared library, and libfprint_private_dep (which several test/tool executables consume via `dependencies:` rather than `link_with:`), all guarded by the same have_sigfm flag, so goodix533c-capture-test and fprint-list-udev-hwdb keep linking correctly now that goodix533c-match.c pulls in sigfm_* symbols.
Implements FpDeviceClass's enroll, verify, identify, and cancel vfuncs,
architected after AndyHazz's goodix53x5 driver (same SIGFM approach,
same sensor resolution) but built entirely on this driver's own
already-proven transport/crypto/capture code -- no goodix53x5 protocol
code is reused, only the shape of its enroll/auth/match/scan modules.
New files:
- goodix533c-private.h: shared device struct (moved out of goodix533c.c)
and sub-SSM entry-point declarations, so match/enroll/auth stay
decoupled from the wire protocol.
- goodix533c-match.{c,h}: driver-side wrapper around sigfm/sigfm.hpp
(extract/serialize/deserialize/score), with this driver's own
"G533"/v1 template magic (distinct from goodix53x5's "G53S" --
templates are never interchangeable, the preprocessing pipelines
differ).
- goodix533c-enroll.c/.h: enroll FpiSsm -- capture-reference ->
wait-finger -> capture -> process (quality gates: reject low
keypoint count via FP_DEVICE_RETRY_REMOVE_FINGER, reject excessive
clipped/non-contact fraction via FP_DEVICE_RETRY_CENTER_FINGER, both
without advancing the stage) -> wait-finger-up -> next-stage-or-done,
repeated GOODIX533C_ENROLL_SAMPLES (8) times. Stores one GVariant
"aay" of serialized SIGFM templates as the print's fpi-data.
- goodix533c-auth.c/.h: shared verify/identify FpiSsm (dispatches on
fpi_device_get_current_action()), same capture shape plus a match
step; verify checks fpi_device_get_verify_data()'s stored samples,
identify checks every sample of every fpi_device_get_identify_data()
gallery print, tracking the best score. Accepts at
GOODIX533C_SIGFM_BEST_MIN (150). Ports the queued-report pattern
(goodix533c_queue_*_report / goodix533c_flush_pending_result_report)
so a match verdict isn't flushed to libfprint until finger-up
cleanup has run.
Refactor of goodix533c.c: the single-shot capture_run() SSM used by
fpi_device_goodix533c_capture_test() is split along a session-scoped
vs. attempt-scoped line --
- Session-scoped (USB reset, TLS-PSK handshake, config upload, FDT
threshold-template baseline) moves into open_run(), extended with
OPEN_STAGE_RESET..OPEN_STAGE_FDT_BASELINE. This must happen once per
fp_device_open() session, not once per enroll stage, or an 8-sample
enrollment would mean 8 full USB re-handshakes instead of 8 fast
touches.
- Attempt-scoped (reference capture, finger wait, live capture, finger
up) becomes four reusable sub-SSM starters declared in
goodix533c-private.h: goodix533c_start_ref_capture_subsm(),
_start_finger_wait_subsm(), _start_live_capture_subsm(),
_start_finger_up_subsm(). fpi_device_goodix533c_capture_test() now
just chains all four (regression-safe, same wire sequence as
before); enroll/auth chain them their own way.
- The live-capture sub-SSM gained a PROCESS state doing the
flat-field-against-reference + clipped-fraction quality-metric work
that capture_done() used to do only after the whole sequence
finished -- enroll/verify need self->captured_image and
self->captured_clipped_fraction ready immediately, before waiting
for finger-up, not deferred to a final callback.
- "Finger up" reuses mcu_switch_to_fdt_up as-is (hardware-verified
this session to block until lift-off) rather than adding new
detection logic, but its reply handler now tolerates a bare timeout
as "assume lifted" instead of failing the whole action -- a single
slow lift-off should not fail an 8-stage enrollment.
- dev_class->features now comes from fpi_device_class_auto_initialize_features()
(matching goodixtls511's convention) instead of a hand-set
FP_DEVICE_FEATURE_CAPTURE, which was never backed by a real
dev_class->capture vfunc to begin with.
See the session report for the full list of deliberate deviations from
the goodix53x5 reference shape (no REINIT/suspend story, no separate
"deactivate" cleanup, GOODIX533C_RAW12_CLIP gate likely inert at this
driver's headroom-safe gain) and what remains unverified against real
hardware (the enroll-then-verify success path, which needs a human).
Host-side test with no hardware/sensor dependency: extract SIFT features from a synthetic 108x88 structured frame (a grid of Gaussian blobs -- not flat grey, which yields zero SIFT keypoints and would make a self-match pass vacuously), serialize, deserialize, and score the round trip against the original, plus a few auxiliary checks (sigfm_copy_info(), a second independent extraction, a cross-match against a different synthetic frame). Confirms the OpenCV/SIGFM integration itself links and behaves correctly, independent of the driver build. Not wired into the meson build, matching the upstream goodix53x5-libfprint repo's own sigfm/tests convention (manually invoked via g++, not a `meson test` target). Build/run command is in the file's header comment.
…ags)
Copies the finger-absent device/capture.pcapng fixture from the parent
project's tests/goodix533c/ (unmodified there) into this submodule as
tests/goodix533c/{device,custom.pcapng}, matching the custom.pcapng/
custom.py naming convention used by sibling drivers (goodixmoc, fpcmoc,
elanmoc), and wires 'goodix533c' into tests/meson.build's drivers_tests
list following the existing goodixmoc pattern.
custom.py currently only exercises device discovery and feature-flag
assertions (VERIFY/IDENTIFY present, CAPTURE/STORAGE* absent, matching
the driver's current auto-derived features). It deliberately does not
call open_sync() or beyond: replaying custom.pcapng against the real
goodix533c-capture-test binary and via meson test showed the fixture's
bulk-IN response endpoint (0x83) carries zero captured reply payload
throughout the entire file (confirmed with tshark, cross-checked against
tests/goodixmoc/custom.pcapng as a control), so it cannot actually
replay the driver's open() sequence past its second command. See
tests/goodix533c/README.md's "Verified replay result" section for the
full transcript and diagnosis, and its "Follow-up needed" section for
what a corrected capture and, later, enroll/verify/identify coverage
would require -- both need a new capture that only a human can safely
make, per the finger-present PSK-decryptability constraint documented
there. No new USB capture was made as part of this change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y9XXWG7y2kfBEXJj3MS5yv
Tried two fresh finger-absent recapture attempts (tshark -s 0, then -s 65535) specifically to test the snaplen hypothesis for why custom.pcapng carries zero bulk-IN reply payload bytes. Both reproduced the identical result -- every completion event capped at exactly 64 bytes, entirely consumed by usbmon's own capture header, no truncated- but-present payload. Rules out the most obvious fix and narrows what a follow-up pass needs to check next. Both recapture attempts (finger- absent, so not sensitive, but no reason to keep) were discarded. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y9XXWG7y2kfBEXJj3MS5yv
- Pull goodix533c out of default_drivers: it's the only driver needing OpenCV/SIGFM, so a plain `meson setup` was forcing that dependency on everyone, including unrelated in-flight driver work. Opt in via -Ddrivers=goodix533c or -Ddrivers=all. - Fix the GLib version guard around g_log_writer_default_would_drop() (2.68+) to check GLIB_VERSION_MAX_ALLOWED, the project's declared 2.56 floor, instead of GLIB_CHECK_VERSION, which just reflects the build machine's installed headers and left the deprecation warning firing on any machine with newer glib. - Replace unaligned guint16*/guint32* pointer casts in the PSK-read, sensor-register read/write command builders and the PSK-length reply parse with memcpy -- the original casts were UB and a SIGBUS risk on strict-alignment architectures. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y9XXWG7y2kfBEXJj3MS5yv
Root cause: host kernel lockdown mode (confidentiality) redacts usbmon's
captured USB payload data system-wide, including for root -- confirmed
via the text interface returning EPERM and the binary interface (used by
both tshark and dumpcap, ruling out tool choice) reporting correct
urb_len but always data_len=0. Not a driver, harness, or capture-tool bug.
Fix: recapture from inside a VM whose guest kernel has no lockdown
enabled, passing the physical sensor through via QEMU usb-host. The new
custom.pcapng is byte-verified complete: every bulk-IN completion's
data_len matches its urb_len, 14835/14835 bytes total including the full
14338-byte image-capture frame.
custom.py stays scoped to discovery/feature-flag assertions rather than
re-enabling open_sync()/close_sync(): replaying the corrected capture
surfaces a second, separate umockdev replay desync
("Reaping discard URB... without corresponding submit") not present with
the old zero-payload capture. Documented in the README's "Replay status"
section for whoever picks this up next -- not resolved here.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y9XXWG7y2kfBEXJj3MS5yv
The fixture's `device` file still declared busnum=3/devnum=6 from the original host capture; the VM-recaptured custom.pcapng (previous commit) recorded the sensor at bus=1/device=2, its own USB topology. umockdev's pcap replay needs these to match or its submit/complete bookkeeping desyncs silently -- surfacing as the same "Reaping discard URB... without corresponding submit" message regardless of cause, which is what made this take so long to isolate (ruled out first: write ordering, urb_id reuse, root-hub interleaving, and reply payload content itself, none of which were it). Relabeling every packet's busnum/devnum fields to 3/6 (a mechanical, structure-preserving rewrite -- payload data reconfirmed byte-identical afterward) fixes replay completely through the non-TLS portion of open(): nop, firmware_version, preset_psk_read, reset, read_sensor_register, read_otp, and request_tls_connection all now replay and decode correctly. Replay still can't get past the TLS handshake itself, but this is now a confirmed structural limitation rather than an open question: traced through goodix533c.c, on_request_tls_connection_reply() feeds the replayed ClientHello into the driver's own embedded TLS server, whose SSL_accept() generates a genuinely fresh ServerHello (new randomness, new ECDHE keys) every run -- output that can never byte-match a previously recorded session. No pcap fix can address this. Documented in the README along with a concrete follow-up: a scoped test stopping before TLS, which this fixture can now actually support. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y9XXWG7y2kfBEXJj3MS5yv
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
1 participant
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
Adds a native, from-scratch
libfprintdriver for the Goodix27c6:533cfingerprint sensor (GF5288 silicon, GTLS/TLS-PSK transport, match-on-host).
This device family (
530c/533c/538c) had no open-source driver beforethis work — the only existing support was Canonical's closed TOD blob.
Builds on this repo's
goodixtlstransport (goodixtls.c's embeddedTLS-PSK server) combined with
FpDeviceClass+SIGFM matching (ported fromAndyHazz/goodix53x5-libfprint), rather than either existing base alone —see the "Why not X" section below for why neither fit as-is.
Verified against real hardware, end to end:
open(), live capture withfinger-detect, and a full
enroll_sync()(all 8 stages) →verify_sync()cycle returning a genuine SIGFM match against a second physical touch —
not a self-comparison or synthetic test.
What's included
libfprint/drivers/goodix533c/— the driver itself.FpDevice-rooted(not
FpImageDevice, see below), reusinggoodix_proto.c/goodixtls.cverbatim for the wire protocol and TLS transport, with a new
transport-adapted command layer, dynamic per-session FDT baseline
(this chip's FDT flow is per-session, unlike
goodix5xx.c's static-configassumption), and real
enroll/verify/identify/cancelvfuncs backedby SIGFM.
sigfm/— vendored SIFT+CLAHE matcher (fromAndyHazz/goodix53x5-libfprint),plus a standalone host-side round-trip test
(
sigfm/tests/test_sigfm_roundtrip.cpp) that needs no hardware.tests/goodix533c/— a real, finger-absentumockdevfixture capturedfrom physical hardware (
device/custom.pcapng/custom.py). See itsREADME for exactly what it does and doesn't cover, and why (short
version: full protocol replay through
request_tls_connectionisverified; TLS handshake replay is a confirmed structural limitation,
not a gap in the capture — the driver's embedded TLS server generates
fresh, non-deterministic
ServerHellomaterial every run by design).goodix.c/goodix5xx.cgain an opt-in4-byte gain-aware
mcu_get_imagerequest variant this driver needs;meson.buildgates the new OpenCV/SIGFM dependency behind-Ddrivers=goodix533c/allspecifically so it isn't pulled into aplain default build.
Why not the two existing candidate bases
goodix53x5-libfprint(same 108×88 sensor family, already has SIGFMwired in): its crypto layer assumes a proprietary non-standard "GTLS"
handshake.
533cspeaks real, standards-compliant TLS-PSK — proven bysuccessfully proxying it through genuine
openssl s_server -psk.Discovered only after getting all the way through OTP/chip-ID/PSK
verification on real hardware, into the handshake itself.
goodixtls.c(real embedded TLS-PSK server, packflags matching this device exactly): its shared FDT state machine
assumes a static config blob, but
533c's FDT needs a dynamic,per-session baseline read from the device at the start of each session
— confirmed by reading
goodix.cdirectly, not inferred. Also,FpImageDeviceinheritance forecloses SIGFM as that base stands (SIGFMneeds
FpDeviceClass-level enroll/verify control).Net: a working driver needed this transport + new FDT/capture code written
directly against
goodix.c's primitives (bypassinggoodix5xx.c) +goodix53x5-libfprint'sFpDeviceClass/SIGFM shape ported onto thattransport. That's what this PR is.
Verification
meson test goodix533cpasses (umockdev-driven device discovery andfeature-flag assertions).
session's own report — rebuilt from a clean tree and re-run):
open()+one no-finger reference-frame capture, decoding to a non-degenerate
12-bit image.
flat-fielding, visible ridge structure (gain-calibration-sensitive —
see
tests/goodix533c/README.md's neighbor findings doc references fordetail if useful).
enroll_sync()through all 8 stages (realtouch per stage), then
verify_sync()against a fresh touch, returninga genuine SIGFM match.
Known limitations / out of scope
533cis hardware-tested.530c/538cshare the PID grouping andlikely the protocol, but are untested (no hardware).
"G533"magic) is driver-own, not interoperablewith
goodix53x5's templates — different preprocessing pipeline.0xc2) appears unit-specific for headroom safety (seetests/goodix533c/README.md's cross-references); a production concernworth a follow-up, not blocking here.
whole family) — see
tests/goodix533c/README.md's "Deliberatelyfinger-absent" section for why no finger-present capture is included or
should ever be added without human review.
Related
goodix-fp-dumpissue #31("538c compatibility?", closed
wontfix) — this PR is the first workingopen-source support for this PID grouping, in
libfprintC form.goodix-fp-dumpPR #75(nikicat's independent Python reference implementation for the same
device family) on a second physical
533cunit — see that PR's commentthread for what was cross-checked (all-zero PSK, 108×88 dimensions,
DEVICE_CONFIGportability across units).🤖 Generated with Claude Code
https://claude.ai/code/session_01Y9XXWG7y2kfBEXJj3MS5yv