Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NFC refactoring #3050

Merged
merged 217 commits into from
Oct 24, 2023
Merged

NFC refactoring #3050

merged 217 commits into from
Oct 24, 2023

Conversation

gornekich
Copy link
Member

@gornekich gornekich commented Sep 7, 2023

What's new

  • New NFC stack
  • Remove RFAL
  • Rework furi hal nfc
  • Introduce NFC library
  • Rework supported protocols
  • Rework NFC application

Verification

  • Test NFC application.

Checklist (For Reviewer)

  • PR has description of feature/bug or link to Confluence/Jira task
  • Description contains actions to verify feature/bugfix
  • I've built this code, uploaded it to the device and verified feature/bugfix

Copy link
Contributor

@wosk wosk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is quite large to be reviewed in full, so I stopped on lib\nfc\protocols\iso14443_3b\iso14443_3b.c =)
Overall this is very good code! Thanks!

firmware/targets/f7/furi_hal/furi_hal_nfc.c Show resolved Hide resolved
firmware/targets/f7/furi_hal/furi_hal_nfc.c Outdated Show resolved Hide resolved
lib/nfc/protocols/iso14443_3a/iso14443_3a.c Outdated Show resolved Hide resolved
@gornekich gornekich marked this pull request as ready for review October 22, 2023 20:20
@noproto
Copy link
Contributor

noproto commented Oct 22, 2023

@noproto do you have proxmark or other device that can sniff data exchange between flipper and your card? The sniffer log would be so helpful to debug the issue.

Here are the traces @gornekich : traces.zip

The files are named desfire_<card ID>_sniff_broken_nfc_refactor_attempt_<attempt number>(_<quality of trace>).txt. The ones which capture all of the communication are marked "excellent". I noticed when the tag sent this at the end of the trace, the Flipper Zero froze:

   15735252 |   15765268 | Tag |02  00  00  83  14  32  10  00  00  07  1f  52  1f  62  1f  72  1f  82   |     | 
            |            |     |1f  92  1f  a2  1f  b2  44  0c                                           |     | 

@gsurkov
Copy link
Member

gsurkov commented Oct 23, 2023

@noproto Thanks a lot for the effort! The sniffer data you have provided did clarify things quite a bit. It turns out that at least in one instance, a DESFire card responds with an unexpected amount of data to the GetFileSettings command. This discrepancy was ignored in the pre-refactoring version, but it's unclear whether that data was valid or just garbage. It may be some new addition to the proprietary DESFire protocol.

In any case, I would like to have some more logs from you :) Please fetch the last commit on gsurkov/3626_mf_desfire_regression_bis, flash it and try to read both cards again and post the Flipper logs here. There will be quite a lot, as it now dumps all exchange happening at ISO14443-4 level and above to the serial output.


instance->signal = iso15693_signal_alloc(&gpio_spi_r_mosi);
instance->parser =
iso15693_parser_alloc(&gpio_spi_r_miso, FURI_HAL_NFC_ISO15693_MAX_FRAME_SIZE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of MISO here is an issue. It breaks emulation against readers that use ASK modulation instead of OOK modulation. For example legacy iClass readers flipperdevices/flipperzero-good-faps#62 (comment)

The previous code uses the IRQ line instead as per

pulse_reader_alloc(&gpio_nfc_irq_rfid_pull, NFCV_PULSE_BUFFER);
which allows this to work.

Note that due to the diode on the IRQ line, using that pin requires enabling a PullDown which the old code achieved here:

pulse_reader_set_pull(nfcv_data->emu_air.reader_signal, GpioPullDown);

@pcunning

@skotopes skotopes merged commit d92b0a8 into dev Oct 24, 2023
9 checks passed
@skotopes
Copy link
Member

@nvx @pcunning @bettse @noproto we'll fix mentioned issues in bugfix PR which will be merged in second RC

@gsurkov
Copy link
Member

gsurkov commented Oct 27, 2023

@noproto I have created a separate issue for your problem.
If you will have any further info please post it there.

@G7369
Copy link

G7369 commented Oct 29, 2023

Hi there. Is there anyone who can guide me a bit to install qflipper on Linux Ubuntu? The image doesn't do anything. Or is there an online terminal with q flipper as the firmware cannot be updated through the phone app and its always crashing bus fault'. Its been stuck on 0.92. All the best. Blessing everyone 1.0.x

@skotopes
Copy link
Member

Hi there. Is there anyone who can guide me a bit to install qflipper on Linux Ubuntu? The image doesn't do anything. Or is there an online terminal with q flipper as the firmware cannot be updated through the phone app and its always crashing bus fault'. Its been stuck on 0.92. All the best. Blessing everyone 1.0.x

https://docs.flipper.net/basics/firmware-update

@G7369
Copy link

G7369 commented Oct 29, 2023

I tried mostly everything. I guess I should try easier on windows. Thanks a lot 🙏

@gornekich gornekich mentioned this pull request Oct 31, 2023
@gornekich gornekich deleted the gornek/nfc_refactoring_v1 branch April 15, 2024 12:08
kbembedded added a commit to kbembedded/flipperzero-firmware that referenced this pull request Oct 18, 2024
Per the comment at the top of the file, defining DIGITAL_SIGNAL_DEBUG_OUTPUT_PIN
to be a GpioPin variable name should allow additional debug output on
that pin. However, this would not work without modifying the file as
well to add the furi_hal.h header. Wrap including that header in the
same macro define to automatically include it when used.

Fixes: d92b0a8 ("NFC refactoring (flipperdevices#3050)")

Signed-off-by: Kris Bahnsen <[email protected]>
skotopes pushed a commit that referenced this pull request Oct 20, 2024
…fs (#3964)

Per the comment at the top of the file, defining DIGITAL_SIGNAL_DEBUG_OUTPUT_PIN
to be a GpioPin variable name should allow additional debug output on
that pin. However, this would not work without modifying the file as
well to add the furi_hal.h header. Wrap including that header in the
same macro define to automatically include it when used.

Fixes: d92b0a8 ("NFC refactoring (#3050)")

Signed-off-by: Kris Bahnsen <[email protected]>
Co-authored-by: hedger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Feature Contains an IMPLEMENTATION of a new feature NFC NFC-related
Projects
None yet
Development

Successfully merging this pull request may close these issues.