DarkFlippers / unleashed-firmware

Flipper Zero Unleashed Firmware
https://flipperunleashed.com
GNU General Public License v3.0
17.66k stars 1.46k forks source link

RFID: Extra Action - Wipe T5577 #765

Open wooferguy opened 5 months ago

wooferguy commented 5 months ago

What's new

Verification

Checklist (For Reviewer)

Leptopt1los commented 4 months ago

@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!