daynix / UsbDk

Usb Drivers Development Kit for Windows
Apache License 2.0
535 stars 142 forks source link

Issues with Android's accessory mode #34

Closed codido closed 7 years ago

codido commented 7 years ago

Following the initial AOA (https://source.android.com/devices/accessories/aoa.html) handshake, Android re-enumerates as a different device (VID/PID/bcdDevice) but obviously still connected to the same port. Trying to close the old device results in a stall, and the port is never cycled even after the host app exits.

Unplugging the device does not seem to have any effect.

Please attach trace logs for 1.0.15_x64. trace.zip

dmitryfleytman commented 7 years ago

Parsed log is here.

dmitryfleytman commented 7 years ago

From the log I see that UsbDk posted port cycle request to the bus driver (IOCTL_INTERNAL_USB_CYCLE_PORT) but port reset did not happen.

Do you have any idea what might be a problem?

codido commented 7 years ago

Is it possible that the port cycling happens while Android re-enumerates any way, so something gets lost in the process?

codido commented 7 years ago

By the way, deferring the port cycling (after the device re-enumerates) might actually break the protocol, as it would cause Android to re-enumerate back to its original device ID. At least that's how it behaves on linux/libusb when libusb_reset_device is called.

codido commented 7 years ago

Would it help to be able to reproduce this locally? In terms of HW, you just need an Android device running Honeycomb and above (most support AOA it seems).

There are quite a few examples on github on how to start AOA (e.g. https://github.com/chris-blay/android-open-accessory-bridge) but I can also hack some minimal example based on go/gousb or just plain C/libusb if you'd like.

dmitryfleytman commented 7 years ago

@codido, thanks, but let's first see what can be understood from logs and code before loading you with additional work :)

Regarding deferred port cycle - UsbDk initiates port cycle via device "interface", this means that it will not reset device after its re-enumeration, so there is no issue with that.

What might happen, is a race condition between device reset initiated by UsbDk and device reset initiated by device itself. In this case UsbDk might miss device detachment callback, keep waiting and then fail to remote device redirection.

Basically code is written with this scenario in mind, but I suspect there may be a bug that leaves some corner case not covered. I will check if there is a bug indeed, and then will see how to proceed.

I will take a timeout for a few days due to FOSDEM, and after that I'm going to continue with investigation.

For a meanwhile, you are welcome to look into the source code, if there is an issue, it is probably because of incorrect setting/clearing of m_RedirectionRemoved event.

dmitryfleytman commented 7 years ago

Hi @codido ,

I did a change in UsbDk that should fix the problem you observe and prepared a test build. Could you please give it a try and collect a new traces in case it does not work?

You will need to enable test signing mode on your system in order to load this driver, see instructions here Also, please uninstall original UsbDk version before installing this one.

codido commented 7 years ago

@dmitryfleytman Thanks! I'm afraid I'm still seeing a stall when trying to close the switched device. Please find attached a trace of this. trace.zip

dmitryfleytman commented 7 years ago

@codido

According to the log it helped, but there is (at least) one more issue ;) Please try this build and see if it works for you.

codido commented 7 years ago

This definitely looks better than before, but still breaks following the switch - When trying to re-open the new device, the port is being reset. This causes two issues:

  1. Android turns off AOA and re-enumerates back as its default device. This basically makes it impossible to use the AOA device.
  2. 2 minutes stall.

Please find attached a trace of the above. trace.zip

Semi-related to this, do you think it would be possible to make libusb+usbdk behave similarly to libusb on Linux wrt resetting the port? On Linux closing a device doesn't cycle the port, while resetting a device does. As far as I can tell, on usbdk closing a device cycles the port and resetting behaves a bit like winusb in that regard. I realize this might be tricky due to the hiding logic.

dmitryfleytman commented 7 years ago

Thanks for testing, I will push corresponding patches upstream. They fix a real problem. Reason for 2 minutes stall on open is pretty clear - UsbDk is waiting for device with same IDs to re-appear after reset. I can think of a heuristics that may be implemented to make waiting time shorter, but it does not look like it will bring any added value at this stage.

As to reset on open/close - unfortunately this is the only reliable way to force Windows recreate driver stack for a device.

One of most powerful UsbDk features - dynamic device ownership acquisition - is implemented by running some logic during driver stack recreation, so there is no other option but to cycle the port.

A can think of a special "permanent" acquisition rules that will make UsbDK take ownership of the device on it's first appearance and do not release it on handle close. In this case it will be possible to work with the device without cycling the port.

I put this feature into the future roadmap, however I cannot guarantee it will be completed soon because it is not in current development focus.

Are there any volunteers willing to contribute?

Dmitry

codido commented 7 years ago

Thanks again for fixing these issues!

I believe the reset upon open is more problematic than the close one (at least in this particular case), and even having a permanent acquisition wont really work around that. However, for such devices which do need a permanent acquisition, winusb can be used instead of usbdk. The only issue with that is that winusb never cycles the port, and using usbdk for just cycling the port would result in a 2 minutes stall.

There might be other ways around this in user space as well (e.g. switching more than once), but that depends on the exact use case.

dmitryfleytman commented 7 years ago

AFAIU, permanent acquisition will fix the problem because there will be no port cycling at all. Not on open and not on close.

W.r.t. port cycling - you don't need drivers for that. Port cycling may be relatively easily implemented in user mode via Win32 API.

codido commented 7 years ago

Regarding the permanent acquisition - I guess you'll need at least one cycle when acquiring the device for the very first time, no? If that's not required, can we save the reset when opening (non-permanent) devices which have no drivers driving them?

Re port cycling - I was under the impression that this is only possible (at least in a documented manner) from kernel space, but I guess I missed a bit.

dmitryfleytman commented 7 years ago

Permanent acquisition - no reset will be needed if permanent acquisition rule was set prior to the device appearance, i.e. before device plug, switch to that special mode etc.

Devices without drivers - unfortunately not, port cycle is a trigger for driver stack creation.

Device reset - see https://github.com/daynix/UsbDk/blob/master/UsbDkHelper/DeviceMgr.cpp#L52 for some clues.

codido commented 7 years ago

Thanks for the clue! :)

It seems like installing such a permanent acquisition would be equivalent to just installing winusb (or any other static solution), so this is probably only a nice-to-have.

dmitryfleytman commented 7 years ago

You're welcome :)

Not exactly, such a rule may be more dynamic than WinUsb INF installation. For example application may set this rule before switching the device into a special mode and clear it afterwards. Also such a rule may be cleared automatically on reboot (like UsbDk non-persistent hide rules), so device will not stick without driver in case of application malfunction.

Also WinUsb is limited on some platforms, for example it does not support isochronous devices prior to Windows 8.1.

From the other point of view, for devices (device modes) that do not have standard driver, driver binding lifetime is less important.

codido commented 7 years ago

That sounds like a useful API to have :)

dmitryfleytman commented 7 years ago

Issues mentioned in original report fixed by 5288bfcbcbba10dd822b097656fddafba9bd6d2d and 848bb79ac2379c1ce0681088320f4ae504f2bce3.

Full AOA support pending "permanent" acquisition rules feature (put into backlog).

Closing the issue for now.