miguelbalboa / rfid

Arduino RFID Library for MFRC522
The Unlicense
2.77k stars 1.44k forks source link

Extension Class for MIFARE DESFire Support #274

Closed JPG-Consulting closed 3 years ago

JPG-Consulting commented 7 years ago

Not really an issue... more like a discussion/testing.

I've seen some issues regarding MIFARE DESFire support and it makes me wonder if MIFARE DESFire code is welcomed inside the library. So instead of making a pull request I decided to extend the library. The code is not clean, plenty of patches have been applied and data length are not optimal (to speed up work i did some copy-and-paste). Cleanup is needed and further enhancements (Adding missing methods would be great). I should also mention that authentication/encryption is currentlly unsupported (if someone wants to make a contribution it is welcomed).

Here is the current Library extension:

https://github.com/JPG-Consulting/rfid-desfire

The example will try to output as much information about the card as possible without using authentication.

Other reasons why I did not submit a pull request are:

  1. Not sure the block will work 100%. I only have one MIFARE DESFire card and it seems to work on it but I would need confirmation it is working on other MIFARE DESFire cards.
  2. Some methods suchs as RATS and PPS should be pulled into the main repository as it is ISO 14443 compatible and not a MIFARE DESFire command.
  3. Not sure MIFARE DESFire support is welcomed in the library itself (Even if kept as an additional extension).
Rotzbua commented 7 years ago
  1. Some methods suchs as RATS and PPS should be pulled into the main repository as it is ISO 14443 compatible and not a MIFARE DESFire command.

I didnt got time to review yet, maybe someone will do it? Then I will merge #271.

  1. Not sure MIFARE DESFire support is welcomed in the library itself (Even if kept as an additional extension).

Every PR which make sense is welcomed :). The usage of an additional class seems to be a good idea. I'm not sure about adding the desfire class in this library.

JPG-Consulting commented 7 years ago

I thought it would be usefull to have RATS and PPS in the main library as they are defined in ISO/IEC 14443 and I think the standard commands should be on MFRC522 library.

In case of a pull request for the MIFARE DESFire methods I would send them as separate files (Like they are now). The main reason to do it that way (now I know they would be welcomed) is that not everybody will be using MIFARE DESFire cards and the methods for this type of card will take more memory on the Arduino device (In fact it needs two additional libraries for DES and AES). And I guess I'm not the only one who had troubles with memory on the Arduino :) So this way anyone can decide to get the standard RFID methods through MFRC522 or including and calling DESFire.

DrLou commented 7 years ago

also might be good to consider a more robust processor for the DES ( floating point? ) operations. We're looking at ARM Cortex...

herrmanthegerman commented 6 years ago

@DrLou: I don't think that DES or AES use floating point operations. These algorithms are rather based on bitwise operations.