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

Minor "bug" (doesn't exhibit) and CRC question #312

Open
conoror opened this issue May 7, 2017 · 2 comments
Open

Minor "bug" (doesn't exhibit) and CRC question #312

conoror opened this issue May 7, 2017 · 2 comments
Labels
bug 🐛 a not intended feature need_confirmation 🧷 bug report must be confirmed by a second person, write if you have the same issue

Comments

@conoror
Copy link

conoror commented May 7, 2017

This isn't a bug per se but it might become one (it's by code inspection)

On line 1711 on MFRC522.cpp there is a set of validBits to 8. It should be 0:

validBits = 8;
status = PCD_TransceiveData(&cmd, (byte)1, response, &received, &validBits, (byte)0, false); // 43

if you do this, the code in PCD_CommunicateWithPICC (line 436) which reads:

byte bitFraming = (rxAlign << 4) + txLastBits;		// RxAlign = BitFramingReg[6..4]. TxLastBits = BitFramingReg[2..0]

will attempt to set BitFramingReg[3] to 1. Now that bit is reserved so nothing bad actually happens!

Question about the same routine (PCD_CommunicateWithPICC). On line 501 there is a check to see if you have at least 2 bytes before calling CRC. Which, if you have exactly 2 bytes, will result in calling the CRC with zero data. I've no idea if this a bad idea... perhaps it is just fine and will return the CRC initialisation value. Just wondering :-)

@Rotzbua Rotzbua added bug 🐛 a not intended feature need_confirmation 🧷 bug report must be confirmed by a second person, write if you have the same issue labels May 7, 2017
@me21
Copy link

me21 commented May 29, 2018

Now there is less than 1700 lines in MFRC522.cpp and I cannot find such code snippet.
So the only relevant part of this issue is the question about CRC.

@Rotzbua
Copy link
Collaborator

Rotzbua commented Jun 1, 2018

@me21 you have to look at the release branch https://github.com/miguelbalboa/rfid/blob/release_1_4_X/src/MFRC522.cpp
At the master I separated the file into smaller peaces to increase maintainability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 a not intended feature need_confirmation 🧷 bug report must be confirmed by a second person, write if you have the same issue
Projects
None yet
Development

No branches or pull requests

3 participants