RfidResearchGroup / proxmark3

Iceman Fork - Proxmark3
http://www.icedev.se
GNU General Public License v3.0
3.62k stars 979 forks source link

Improve reliability of `lf em 4x70 writekey` #2316

Closed henrygab closed 3 months ago

henrygab commented 4 months ago

After key is written, authentication is attempted using the new key. This is particularly important for glass modules, where the coupling can be quite weak, to ensure the key is properly written.

Split parameter parsing from client/firmware interaction by adding helper functions. This enables re-use and building more complex commands easily.

Firmware: Partially-tracked at least one of the places where the notice of extra bits was coming from. Add a TODO comment to remind where to look for further review.

No changelog entry needed for this, it's primarily internal, and a continuation of the last PR.

github-actions[bot] commented 4 months ago

You are welcome to add an entry to the CHANGELOG.md as well

iceman1001 commented 4 months ago

I am all for refactoring. We have noticed over the years that some of the contributions did that a lot in their code parts, leaving the overall client diversified and gave a nightmare to maintain. Trying to understand and keep track of those alternative ways. I am thinking of HITAG code and ISO15693 code, so I see similarities here in your refactoring.

Please don't. Try using the code patterns we have and even if its not how you would like it to be, it is what it is. Otherwise your changes will be need to be implemented across the whole client. And I think you are not ready for that commitment.

henrygab commented 4 months ago

Please don't. Try using the code patterns we have and even if its not how you would like it to be, it is what it is.

Can you help me understand which code patterns you are referring to? I'm generally happy to conform to the patterns of any given project.

I do know of one place where I diverged: I had to manually expand the CLIGetHexWithReturn() macro, because the macro was failing to compile with the proper size. I expanded only one level of the macro ... the minimum necessary to avoid the particular issue. Is this the divergence you refer to?