-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Add API to enforce ISO15693 mode #3885
base: dev
Are you sure you want to change the base?
Conversation
A bit of background, picopass only ever uses 1of4 modulation not 1of256. As readers will never use 1of256 if noise ever makes it appear like a 1of256 SOF was sent it will hang there indefinitely as the reader will never send a 1of256 EOF causing #3343 It is probably worth also having a timeout (we know the maximum frame size, if we don't see the rest of the frame in this time we should reset the emulation state ready for the next frame) and fix what looks like a buffer overrun in the 1of256 code (flipperdevices/flipperzero-good-faps#105), but even with those two things fixed I still think it makes sense allowing emulation code to opt out of 1of256 when it's known it should never happen as accidentally interpreting something as the 1of256 SOF would still likely interrupt emulation until the timeout occurs which would almost certainly cause the reader to time out. A quick look at the patch seems fine to me, but I'm not an expert on the HAL design. |
This should be an easier one to review since it is just adding a new API and not changing how anything functions |
* Add API to enforce ISO15693 mode See flipperdevices/flipperzero-firmware#3885 * Revert symbols version * Format * Update changelog --------- Co-authored-by: Willy-JL <[email protected]>
@@ -26,6 +26,7 @@ typedef enum { | |||
struct Iso15693Parser { | |||
Iso15693ParserState state; | |||
Iso15693ParserMode mode; | |||
bool detect_mode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this detect_mode be moved to the Iso15693ParserMode enum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, so you're proposing to add two new modes like Iso15693ParserModeForced1OutOf4
and Iso15693ParserModeForced1OutOf256
in place of the boolean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but there is a more important thing, which I've missed previously. Putting protocol specific functions into nfc.c / nfc.h layer it's not very good idea, because nfc layer was designed to be a generic protocol free. I had a quick conversation with original nfc stack designer and he agreed on that.
Also, we came up to the idea that we need to investigate this problem deeper and try to fix this in iso15 parser. Currently I'm investigating this issue and will be in touch with you on the results.
What's new
Verification
nfc_iso15693_force_1outof4
method and emulate a card. Scanning the card on the reader no longer causes the Flipper to crash when the reader has LF enabled.Checklist (For Reviewer)