1AdityaX / mfrc522-python

The mfrc522-python library is used to interact with RFID readers that use the MFRC522 chip interfaced with a Raspberry Pi.
https://pypi.org/project/mfrc522-python/
GNU General Public License v3.0
13 stars 4 forks source link

Fix incorrect use of ~ (bitwise complement) operator #3

Closed nickd4 closed 5 months ago

nickd4 commented 6 months ago

Thankyou for putting this repo up, it worked for me and saved me a heap of time. With some modifications I could quickly verify that my MFRC522 prototype board works. I am using a standard PC with an FTDI FT232H to access the SPI interface (not the Raspberry Pi GPIO as in the original). If you want me to contribute the code for the FTDI chip I am happy to do so, but it's a bit of a mess so you'd have to tidy it up.

However, I had to fix a minor bug in the logic where it waits for an interrupt bit to become set. When you use the ~ operator on a bitfield, it will not just complement that bitfield, it will complement the entire number, usually making it always become "true".

Example: In line 245 of MFRC522.py the original code is:

        if (~(temp & 0x03)):

but what you meant was probably one of

        if ((temp & 0x03) ^ 0x03):

or

        if (temp & 0x03) != 0x03:

and both alternatives would correctly check if the 2-bit field temp[1:0] is not equal to 3. (The code within the body of the if then goes to set the 2-bit field to 3).

To see why the original code fails, imagine that temp is a 32-bit number, and it is one of 0x00000000, 0x00000001, 0x00000002, 0x00000003. Then complementing that number will give you one of 0xFFFFFFFF, 0xFFFFFFFE, 0xFFFFFFFD, 0xFFFFFFFC and all of these are "true". So the use of the ~ operator is not a suitable test here. In Python the situation is slightly more complex because Python "int" has effectively infinite width, so the result will be one of -1, -2, -3, -4 which are considered to have the nonzero "sign" bit effectively repeated forever, but the principle is the same.

The case that actually tripped me up was when it was waiting for an IRq bit to become set, you can see the diff for the code that worked for me.

Thanks again for putting this online!