search

monkeyboard / Wiegand-Protocol-Library-for-Arduino

Wiegand 26 and Wiegand 34 Protocol Library for Arduino
323 stars 131 forks source link

High bit on _cardTemp always zero for W32/W34 #45

Closed JimGrim82 closed 3 years ago

JimGrim82 commented 4 years ago

As the left shift in ::ReadD0 and ::ReadD1 is done after setting the low bit of_cardTemp , the necessary right shift during ::DoWiegandConversion does not restore the high bit on _cardTemp if the wiegand type is 32bits or higher, meaning the high bit of _cardTemp is always zero.

The easiest solution is to do the left shift in ::ReadD0 and ::ReadD1 before setting the low bit so no right shift is required during ::DoWiegandConversion.

jpliew commented 4 years ago

Thanks @JimGrim82 I am so sorry for the late reply. Stuck at home for the moment. Let me get back to this asap.

jpliew commented 4 years ago

@JimGrim82 I had a look at this and I thank you for a bug you found for W32.

The issue is not right shifting the _cardTemp.

This library handles 32 and over with two 32bit long variable, _cardTempHigh and _cardTemp and the library also drop out the parity bits, the most significant bit and least significant bit.

For W32 wiegand, only 30 bit will be used for data.

For W34 wiegand, only 32 bit will be used for data.

Therefore you formed an impression that after left shifting, the right shifting did not restore the high bit. The high bit is meant to be removed, because that is the parity bit.

However, based on your pull request, I did a code review and found out that for W32, at GetCardId, there should be this

if (bitlength==32) {
        return (*codelow & 0x7FFFFFFE ) >>1;

I will fix this. Thanks for your help.

Cheers

JimGrim82 commented 3 years ago

Ahh I had not appreciated you were excluding the parity bits, I had been assuming the whole string would be available and for the whole wiegand string to be processed outside of your library.

But hey, it still located a bug with the low parity bit, so not a complete waste of time!

I was using this library to read a 32bit Mifare Card Serial Number which has no parity bits. I was getting the wrong number hence me flagging this up. But I adjusted the library to suit my need at the time (which was the pull request).

Thanks for your time.

jpliew commented 3 years ago

@JimGrim82 I should have another function to return a raw unprocessed data. Thanks for your suggestion.

Cheers