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

Added check for USB alternate settings #561

Open
wants to merge 3 commits into
base: master
from

Conversation

@jpwidera
Copy link

commented Sep 5, 2019

Quick and dirty fix #535

I'm not sure but I think, that the 072f:2200-device does not support alernative usb modes:

Bus 001 Device 013: ID 072f:2200 Advanced Card Systems, Ltd ACR122U:

# lsusb -v -s 001:013 | grep bAlter
     bAlternateSetting       0

Disabling allows to read and write:

./nfc-mfclassic r u a test.mfd
./nfc-mfclassic w u a test.mfd

I'm not sure. But does any of these acr122-devices has an alternative usb mode?

jpwidera
Quick and dirty fix for #535
@jpwidera

This comment has been minimized.

Copy link
Author

commented Sep 5, 2019

This is already recognized by @melchips in #542 (comment)

jpwidera added 2 commits Sep 5, 2019
jpwidera
If there are alternative interfaces, claim interface 0. Otherwise skip this step.
jpwidera
@jpwidera jpwidera changed the title Removed USB alternate setting. Added check for USB alternate settings Sep 5, 2019
@thekix

This comment has been minimized.

Copy link

commented Sep 26, 2019

I provided another patch about the same problem (#563). IMO there are two different ACR122 models, the old, with alternate support (at least, reply the usb call) and the new without support (timeout in the usb call).

This patch is nice, because checks the compatibility with alternate settings. My patch is less conservative, and removes the call to set alternate settings.

The question is: do we need with this device (old and new) the usage of set alternate settings? If the answer is "no", we can remove all these lines. If we want to be more conservative, then this patch is preferred.

I tested both devices with my patch, and they works fine. IMO, I prefer delete all these lines, less code is better, and then if in the future there are some trouble, we can undo the patch and apply this, but is only my opinion.

@jpwidera

This comment has been minimized.

Copy link
Author

commented Sep 27, 2019

To be honest: It is not completly clear to me, what the side-effects of the alternate settings is.
What happens, if a device supporting alternate settings is in mode x and the libnfc requires mode 0?

If there is no side-effect, I'm completely on your side - less code less problems (:

@thekix

This comment has been minimized.

Copy link

commented Sep 30, 2019

I tried to find more info about alternate devices, and I didn't find info about it. For this reason I did this patch.

After testing (see #564) reading, writing and Writing (sector 0) with the patch provided (#563) and with new and the old devices, it works fine. IMO, the patch removing the alternate calls is better, because removes code not needed by these devices.

@fxcoudert

This comment has been minimized.

Copy link

commented Oct 9, 2019

I can confirm that this patch fixes issues on my reader.

1 NFC device(s) found:
- ACS / ACR122U PICC Interface:
    acr122_usb:020:002
        ACR122U PICC Interface:

          Product ID: 0x2200
          Vendor ID: 0x072f
          Version: 2.14
          Speed: Up to 12 Mb/sec
          Manufacturer: ACS
          Location ID: 0x14400000 / 2
          Current Available (mA): 500
          Current Required (mA): 200
          Extra Operating Current (mA): 0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.