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

RFID: Extra Action - Wipe T5577 #765

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

wooferguy
Copy link
Contributor

@wooferguy wooferguy commented Jun 16, 2024

What's new

  • Add RFID Extra Action - Wipe T5577 (This is a port from RougeMaster FW, and uses some of their methods.)
  • Sets T5577 Block 0 to default, all other blocks to 0.
  • Requires passwords cleared before-hand.

Verification

  • Write data to a T5577 tag
  • Read back to verify data exists
  • Run Wipe T5577
  • Read tag again to verify it no longer contains data

Checklist (For Reviewer)

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

This is a port from RougeMaster FW, and uses some of their methods.
Sets T5577 Block 0 to default, all other blocks to 0. Requires passwords cleared before-hand.
@wooferguy wooferguy requested a review from xMasterX as a code owner June 16, 2024 14:22
@xMasterX xMasterX marked this pull request as draft July 4, 2024 03:25
@xMasterX xMasterX requested a review from Leptopt1los July 4, 2024 03:26
@Leptopt1los
Copy link
Member

@wooferguy Hello! I'm sorry for the delay. about this pull request:

  1. I see that you made a wipe at the stage level. You shouldn't do that. The view should not contain business logic; this is an architecturally flawed decision that can lead to big problems when maintaining the code in the future. The correct way is to implement the new mode in https://github.com/DarkFlippers/unleashed-firmware/blob/dev/lib/lfrfid/lfrfid_worker_modes.c. For reference, you can see how, for example, clearing a password is done. unfortunately we cannot accept this code into our codebase for these reasons
  2. I have big doubts that we should implement this as a basic feature of the lfrfid app. It seems to me that users need this functionality in rather specific situations. The main problem is that we do not have a low-level API for reading t5577, which does not allow us to validate whether we really wiped the tag. This is similar to the fire and forget in T5577 password clearing, but password clearing is, in my opinion, an important enough feature that it's worth sacrificing ux for functionality. so far the wipe tag doesn't look like that to me
  3. however, it seems to me that this functionality has the right to life in a separate plugin. I have plans to implement it in my t5577_multiwriter app after its rework (with the addition of other low-level features for working with t5577). but we will accept any working implementation of this feature in a separate plugin in our -e builds. So, I suggest you either wait for this functionality to be implemented in t5577_multiwriter, or implement it as a separate plugin yourself and tag me or @xMasterX to add it to the all the plugins repository

I will be glad to hear your opinion on this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants