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

Support Ultralight NTAG215 #264

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

FrankWu100
Copy link

@FrankWu100 FrankWu100 commented Jun 18, 2020

Thanks for #253 too. I compare NTAG215 with lots of codes.
And read the Ultralight & NTAG spec from NXP, to make sure those can just be implemented together with the same structure.

//NTAG213 & NTAG216 can also be supported by little add-on codes.

note:
Ultralight NTAG: https://www.nxp.com/docs/en/data-sheet/NTAG213_215_216.pdf
Ultralight EV1: https://www.nxp.com/docs/en/data-sheet/MF0ULX1.pdf

FrankWu100 and others added 5 commits June 16, 2020 23:58
(cherry picked from commit 83a8350)
(cherry picked from commit 4b13ab1)
(cherry picked from commit 928cc10)
(cherry picked from commit 61e455b)
@FrankWu100 FrankWu100 changed the title Added support for NTAG215 #253 Support Ultralight NTAG215 Jun 18, 2020
@fptrs
Copy link
Collaborator

fptrs commented Jun 19, 2020

Hi @FrankWu100,
I just merged #253, what is the difference to your pull request?

@FrankWu100
Copy link
Author

FrankWu100 commented Jun 19, 2020

Hi @FrankWu100,
I just merged #253, what is the difference to your pull request?

Just use the same code from MifareUltralight.c and else. If we need to do other methods's improve or implements, no need to maintained two files. Because of almost the same core functions between Ultralight EV1 and NTAG213/215/216.

@fptrs
Copy link
Collaborator

fptrs commented Jun 19, 2020

Hi @FrankWu100,
I just merged #253, what is the difference to your pull request?

Just use the same code from MifareUltralight.c and else. If we need to do other methods's improve or implements, no need to maintained two files. Because of almost the same core functions between Ultralight EV1 and NTAG213/215/216.

So you suggest to remove the other pull request and use your code?

@FrankWu100
Copy link
Author

Hi @FrankWu100,
I just merged #253, what is the difference to your pull request?

Just use the same code from MifareUltralight.c and else. If we need to do other methods's improve or implements, no need to maintained two files. Because of almost the same core functions between Ultralight EV1 and NTAG213/215/216.

So you suggest to remove the other pull request and use your code?

No, not at all. Because #253 still is good begining. Maybe we can discuss together. Which the is better way for future. Then if need combine into the ultralight, rebase it and pull the new request would more better.

@gcammisa
Copy link
Contributor

I can confirm that #253 is based on the Mifare Ultralight code.
Mifare Ultralight and NTAG21x tags are similar in the basic command and state machine implementation, but are a bit different regarding the lockbits / security mechanism used.

Since I think that having clean and easy to read and understand code should have priority over having "less" code, my idea when I started working on NTAG215 was to have a separate Application, with its respective .c and .h file, that could be extended to then support NTAG213 and NTAG216 with minimal modifications.

Merging the applications for the Mifare Ultralight tag family and the NTAG21x tag family would make the code harder to read and to "compare" to the datasheet for whoever is trying to make any modification to such code, and modification to the Mifare Ultralight code would end up affecting the NTAG21x code and vice-versa, which makes the code harder to maintain.

Of course, the decision of which approach to take depends on what the maintainer prefer and what whoever ends up adding the missing stuff to the NTAG code prefers, I'm just explaining the rationale behind my choice to make the NTAG21x code as a separate application.

@fptrs
Copy link
Collaborator

fptrs commented Jun 26, 2020

We really appreciate both of your contributions. I think for the time being we keep @gcammisa approach since it is easier for beginners to get into the code. If we run out of program memory we can still merge these applications.

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.

4 participants