Skip to content

[NFC] Trim MIFARE Plus generator & admin-key mapping (in-place cleanup)#1035

Merged
mishamyte merged 3 commits into
devfrom
refactor/mf-plus-generator-cleanup
Jul 7, 2026
Merged

[NFC] Trim MIFARE Plus generator & admin-key mapping (in-place cleanup)#1035
mishamyte merged 3 commits into
devfrom
refactor/mf-plus-generator-cleanup

Conversation

@mishamyte

Copy link
Copy Markdown
Member

What

Two behavior-preserving, in-place cleanups of the MIFARE Plus code. No functional change, no public SDK API change (targets/f7/api_symbols.csv untouched).

  1. Data-drive the MIFARE Plus manual-add generators (lib/nfc/helpers/nfc_data_generator.c)

    • The 18 near-identical MIFARE Plus variants were 18 one-line thunk functions + 18 handler-table entries. They differ only by parameters (UID length, product type, memory size, ATS historical bytes), so they now come from one data-driven mf_plus_generator_configs[] table, dispatched via a small range predicate. The Ultralight/NTAG and Classic types keep their bespoke handlers.
    • Dropped mf_plus_ats_hist_ev, which was byte-identical to mf_plus_ats_hist_s (EV1/EV2 are identified by GetVersion, not the ATS).
    • A _Static_assert pins the config table to the enum range so a future type can't desync it.
  2. Single source of truth for the admin-key addresses (lib/nfc/protocols/mf_plus/*)

    • The admin-key address↔type mapping (0x9000 CardMaster, 0x9001 CardConfig, 0x9003 L3Switch, 0x9004 SL1CardAuth) was copied in 3 places — the poller's mf_plus_admin_key_id[] array and two identical switch statements in the listener. Folded into one mf_plus_admin_key_addresses[] table with a forward helper (mf_plus_get_admin_key_address) and a reverse helper (mf_plus_admin_key_type_from_address). Internal helpers only.

Why

Follow-up to #1033. The original plan there was to relocate the whole generator out of firmware into the NFC app to reclaim multiple KiB of internal flash. That turned out not to be feasible: nfc_data_generator is a public SDK API consumed by 5 external FAPs (nfc_magic, nfc_maker, flipcrypt, metroflip, seader), so moving it out of lib/ / the API would break their builds and force a major API bump. See the finding on #1033.

What's left is the safe, non-breaking part: trimming redundancy in place. Also removes 3 copies of the admin-key mapping that had to be kept in sync by hand.

Flash

Measured COMPACT=1 DEBUG=0, this branch vs dev:

Section dev this PR Δ
.text 702520 702104 −416 B
.rodata 172452 172556 +104 B
.free_flash 172572 172884 +312 B free

~0.30 KiB reclaimed — modest, but free, plus the maintainability win of one source of truth.

Testing

  • fbt firmware_all COMPACT=1 DEBUG=0 — links clean.
  • fbt fap_nfc COMPACT=1 DEBUG=0 — APPCHK passes (NFC FAP still resolves the unchanged generator API).
  • clang-format conformant.
  • Reviewed as behavior-preserving: every one of the 18 generator rows verified against the original thunks (name/uid_len/type/size/ATS, in enum order); the listener's default fall-through semantics preserved exactly; poller authenticates the same addresses.

Refs #1033

mishamyte and others added 2 commits July 7, 2026 22:53
Replace the 18 near-identical MIFARE Plus generator thunks and their
handler-table entries with a data-driven mf_plus_generator_configs[]
table (uid_len / type / size / ATS per variant), dispatched via a small
range predicate. The Ultralight/NTAG and Classic types keep their
bespoke handlers, so the handler table now stops before the MF Plus
range.

Also drop mf_plus_ats_hist_ev, which was byte-identical to
mf_plus_ats_hist_s (EV1/EV2 are identified by GetVersion, not the ATS).

Pure refactor: the Add-Manually menu entries, the generated cards and the
public SDK API are all unchanged. Trims firmware flash by removing the
per-variant functions and the duplicate blob; a _Static_assert pins the
config table to the enum range so a future type can't desync it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fold the three copies of the admin-key address<->type mapping
(0x9000 CardMaster, 0x9001 CardConfig, 0x9003 L3Switch, 0x9004
SL1CardAuth) into one mf_plus_admin_key_addresses[] table with a forward
helper (mf_plus_get_admin_key_address, replaces the poller's local id
array) and a reverse helper (mf_plus_admin_key_type_from_address,
replaces two identical switch statements in the listener's key-resolve
and write-store paths).

Behavior-preserving: both listener default fall-throughs are kept (resolve
returns false; store falls through to data/config block handling), the
poller authenticates the same addresses, and a _Static_assert pins the
table to MfPlusAdminKeyNum. Internal helpers only; no SDK API change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mishamyte
mishamyte requested a review from xMasterX as a code owner July 7, 2026 20:00
@github-actions github-actions Bot added the area/nfc Related to the nfc subsystem label Jul 7, 2026
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

🤖 Build & analyze — refactor-mf-plus-generator-cleanup @ 75c15c74

Firmware built — artifact unleashed-fw-refactor-mf-plus-generator-cleanup-75c15c74 (run)

💾 Flash & RAM — f7 (ble_light)

Metric Size
Firmware (flash) 855.18 KiB
DFU image 855.48 KiB
Reserved — radio stack + FUS 164.00 KiB
Free flash (usable) 4.82 KiB ⚠️
RAM (.data + .bss) 7.94 KiB

1 MiB flash = firmware + usable free + radio/FUS. The BLE coprocessor stack sits at the top of flash (load addr 0x080D7000); the linker's .free_flash (168.82 KiB) counts that region as free, so usable free = .free_flash − reserved (164.00 KiB).

📋 Public API changes

✅ No public API changes in this PR.

@mishamyte mishamyte added this to the unlshd-090 milestone Jul 7, 2026
@mishamyte
mishamyte merged commit e4e8a8e into dev Jul 7, 2026
2 checks passed
@mishamyte
mishamyte deleted the refactor/mf-plus-generator-cleanup branch July 7, 2026 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/nfc Related to the nfc subsystem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant