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

Hitag #2658

Open
wants to merge 25 commits into
base: dev
Choose a base branch
from
Open

Hitag #2658

wants to merge 25 commits into from

Conversation

blackvault88
Copy link

@blackvault88 blackvault88 commented May 10, 2023

What's new

  • Included support for RTF (Reader Talks First) mode in RFID app, both for read & emulate modes.
  • Included the Hitag1 protocol as first RTF protocol (more to come later, like Hitag2, HitagS) supporting read & emulate of public non encrypted pages
  • Updated the RFID scenes & views accordingly
  • Updated the RFID key file save & load functions, by adding 'Hitag1 specific data' after the general section (fully backwards compatible with existing keys)

Verification

  • Run RFID app and read/emulate/save/view/edit any of the already supported protocols
  • Run RFID app and read/emulate/save a new Hitag1 card
  • Run RFID app and emulate/view/edit a saved Hitag1 card (sample key files for verification are added below)
    sampleHitag1.zip

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

@hedger hedger added RFID 125kHz 125, 134 kHz RFID UI Affects UI New Feature Contains an IMPLEMENTATION of a new feature labels May 11, 2023
@blackvault88
Copy link
Author

hey @hedger , @DrZlo13 , @nminaylov , @skotopes ,
any action required from my side?
forgive my ignorance, this is my first contribution to an open source project

@skotopes
Copy link
Member

Scheduled on next release

@skotopes
Copy link
Member

skotopes commented Jun 9, 2023

Waiting for hardware to verity, moving to next release

@skotopes
Copy link
Member

Sorry for delay. This PR requires some extra spaces that we planning to free in 0.87 release.

@blackvault88
Copy link
Author

not sure if I understand what you mean with extra spaces (extra memory space?)

but no worries, just let me know when you plan to pick this up, since I've seen that meanwhile furi_hal_rfid has undergone some changes with the FuriHal bus abstraction. and I might need to rewrite some code to make it compatible again

no point in continuously keeping it updated with all intermediate commits, I'd rather do a final update when it's relevant...

@skotopes
Copy link
Member

I'll post update as soon as we free enough space

@skotopes
Copy link
Member

We are ready to merge this PR, please merge dev and update to match new API. There were couple changes that may require change in furi_hal_rfid, please ping @DrZlo13 to get more details

@blackvault88
Copy link
Author

alright, on it.
reviewing the furi_hal_rfid changes, though I suspect there's some thread handling changes as well I might need to catch up with
any advice on where to search for this/whom to contact?

@skotopes
Copy link
Member

@blackvault88 feel free to ping @DrZlo13

@blackvault88
Copy link
Author

thx, though not needed anymore, seems as if the thread updates which I was referring to (#2637 ) have no impact on my coding

meanwhile I updated my code to comply with the bus abstraction (#2614 ). it builds and runs nicely on my FD.
reading is working as it should
emulation is running without errors, though haven't had the time yet to test it with a reader, hope to do that over the weekend...

if successful, I'll recommit the updates

@blackvault88
Copy link
Author

blackvault88 commented Jul 17, 2023

@skotopes , @DrZlo13

tested and recommitted, ready for your review/approval

fyi, there seems to be some other changes in the firmware that have a negative on the successrate of the hitag emulation mode.
when built on the dev branch at time of my first developments (May 10th) the hitag emulation yielded results in +/2-5 seconds
when built on current dev branch the same coding for emulation only yields results somewhere after 10-30s (meaning that the reader which I'm testing with needs quite a few tries to successfully read the pages consecutively without issues).

note: to check if it is related to my coding updates or to other changes in the firmware, I also rewrote and built the code to match the dev branch at time of that first deep sleep mode release and that also has the same negative impact.
not 100% sure if it is related to that deep sleep setup, since also running in legacy seems to have the same 10-30s success rate, but it does make me quite sure that there's some 'external' impact on how my code gets executed which impacts the emulation mode.

I would welcome a critical eye that might know what could impact the coding in my emulation routines
for now, it's a working version with 100% successrate on the reading, and some successrate on the emulation

@skotopes
Copy link
Member

skotopes commented Aug 2, 2023

@blackvault88 sorry for delay, we are waiting for hitags for testing. We'll merge as soon as they arrive.

@blackvault88
Copy link
Author

@skotopes , be aware, the Hitag key data is more than 256 bytes, which is why I had to update applications/services/gui/modules/byte_input.c
to use a uint16_t instead of uint8_t variable for the byte selection related functions
(cf one of my first commits: 203f4ad )

I see you've updated that back to a uint8_t which cannot keep track of +256 bytes...

@skotopes
Copy link
Member

@blackvault88 that was merge artifact, I'll fix it.

Couple questions:

  • I have bunch of tags: one of them is forever reading(really small one, cable marking type), 2 other pages read: 50/64. Do your tags behave same?
  • How emulation should be tested? Reading with another flipper barely detects it.

@blackvault88
Copy link
Author

blackvault88 commented Aug 16, 2023

  • I have bunch of tags: one of them is forever reading(really small one, cable marking type), 2 other pages read: 50/64. Do your tags behave same?

50/64 is expected behavior: there's a minimum of 14 non-public (encrypted) pages on a HITAG 1 tag.
current functionality is reading the public (non encrypted) pages only, which could be anything between 34 & 50 pages.

About the one that is reading forever, are you sure it's a HITAG 1 and not a HITAG 2 or HITAG u or HITAG S?
Does it detect an ID and then get's stuck reading the pages, or does it not detect anything at all?
can you check the debug log while reading?
it should give out something like this:

6373214 [D][LFRFIDWorker] Start ASK
6373219 [D][DolphinState] icounter 1017, butthurt 0
6373677 [D][LFRFIDWorker] Read started
6376986 [D][LFRFIDWorker] Read stopped
6377041 [D][LFRFIDWorker] Start PSK
6377503 [D][LFRFIDWorker] Read started
6379522 [D][LFRFIDWorker] Read stopped
6379576 [D][LFRFIDWorker] Start RTF
6379579 [D][DolphinState] icounter 1018, butthurt 0
6379587 [D][LFRFIDWorker] Read started
6380900 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] found
6380902 [D][DolphinState] icounter 1021, butthurt 0
6381562 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] config page read
6381565 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 1 not public/readible
6381567 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 2 not public/readible
6381570 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 3 not public/readible
6382048 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 4 read
6382507 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 5 read
6382917 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 6 read
6383327 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 7 read
6383721 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 8 read
6384164 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 9 read
6384574 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 10 read
6384968 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 11 read
6385378 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 12 read
6385805 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 13 read
6386199 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 14 read
6386624 [D][LFRFIDHitagWorker] Hitag1, [34 80 68 A5] block 15 read
6386671 [D][LFRFIDWorker] Read stopped
  • How emulation should be tested? Reading with another flipper barely detects it.

I've tested it using a real reader, though am also experiencing issues with readers taking up to 30s (sometimes a minute) prior to succesfully reading it since after the FZ firmware updates of May June (cf my post #2658 (comment) )
Though I have no idea what caused the sudden fallback in emulation results :(

@skotopes
Copy link
Member

image

Some PVS warnings that need to be addressed

@skotopes
Copy link
Member

As for emulation issues: difficult to say what exactly happened in June. There were some major changes in July, but not in June. If you can specify working commit in this branch then I can try to fix it.

Couple more things that prevents us from merging this PR:

  • TODO: we are currently cleaning up firmware by fixing all of them, please try to close as many as possible.
  • Worker source code: requires refactoring. Not like 2k lines of code are frightening us, but we clearly we'll be unable to maintain it in its current form. I see there multiple layers mixed it will be nice if they will be split into separate files.

@blackvault88
Copy link
Author

Ok, will look into the PVS warnings & my TODO notes

If you can specify working commit in this branch then I can try to fix it.

for the emulation results, I'll see if I can trace back in time to find latest version where emulation was yielding faster results

  • Worker source code: requires refactoring. Not like 2k lines of code are frightening us, but we clearly we'll be unable to maintain it in its current form. I see there multiple layers mixed it will be nice if they will be split into separate files.

I can easily split up the functional blocks into separate files, though one of the blocks would likely be positioned better into furi_hal_rfid, though that would also increase file size there... Any advice on what would be acceptable?

@skotopes
Copy link
Member

skotopes commented Sep 4, 2023

I can easily split up the functional blocks into separate files, though one of the blocks would likely be positioned better into furi_hal_rfid, though that would also increase file size there... Any advice on what would be acceptable?

Yes, everything related to low level should be in furi_hal_rfid, also it will speedup this code. Also @DrZlo13 may have some ideas.

@blackvault88
Copy link
Author

worker source code refactored and moved all low level code towards furi_hal_rfid

ready for your review and approval to merge @skotopes

@blackvault88
Copy link
Author

blackvault88 commented Oct 26, 2023

and some good news on emulation mode:
emulation mode is back to normal result times (eg reader manages to read the full emulated tag in +/- 5s)

not sure if this is due to moving low level code to furi_rfid_hal or due to other changes elsewhere in firmware, but it's a nice extra :)

@skotopes
Copy link
Member

I'll take a look after we release 0.94

@blackvault88
Copy link
Author

hey @skotopes , how's the review going?

any input/changes required?

@skotopes
Copy link
Member

skotopes commented Dec 7, 2023

@blackvault88 yes, are you on discord? can we chat a little bit?

@blackvault88
Copy link
Author

@blackvault88 yes, are you on discord? can we chat a little bit?

nope, not active there, any alternative? eg the FZ forum?
if not, i can always create a discord account... let me know

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 RFID 125kHz 125, 134 kHz RFID UI Affects UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants