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

refactor getWinUsbMxId() to fix luxonis/XLink#57 #60

Merged
merged 3 commits into from
May 16, 2023

Conversation

diablodale
Copy link
Contributor

@diablodale diablodale commented May 13, 2023

complete refactor of getWinUsbMxId() to fix #57

Ready for review. Some things to keep in mind

  • I've temporarily increased the log level to MVLOG_FATAL in this function for debug+test. I will decrease it before ready for merge.
  • Movidius MyriadX usb devices often change their usb path when they load their bootloader/firmware
  • Since USB is dynamic, it is technically possible for a device to change its path at any time
  • I learned that USB host controllers each have exactly one root usb hub. It is a 1:1 relationship. I use that to find usb root hubs faster using a GUID specific to host controllers.
  • libusb creates its usb path "x.x.x.x" during libusb_get_device_list() using a single snapshot of the entire PCI+USB tree using the same SetupDiGetClassDevsA() Win32 api. That libusb list and libusb path is refreshed on every call to libusb_get_device_list() and the path to any specific device might change between calls.
  • It is possible for windows usb paths to change in the nanoseconds between the two calls to SetupDiGetClassDevsA() in this function. If it is critical to handle that scenario, then this PR can be changed to be similar to libusb and get a single snapshot of the complete PCI+USB tree. That will make this function somewhat slower since the entire tree has to be walked to find hubs and devices. I don't recommend a single snapshot at this time.

@diablodale
Copy link
Contributor Author

This needs testing on various Windows hardware/setups to ensure the new Win32 path -> libusb path code works and the assumptions it makes are valid. I'll test on three machines: Win11 usb3, Win10 usb3, Win10 usb2 👴

@diablodale
Copy link
Contributor Author

diablodale commented May 14, 2023

Good results on my 3 windows test machines. One was additionally interesting...

I used my updated multiple_devices_test from my depthai-core PR + additional debug output so I can watch for behaviors.
The

cpu windows usb ports/hubs notes
intel 10th gen i7 (2020) Win 11 USB 3, no hubs tests worked as expected
intel 3rd gen i7 (2012) Win 10 USB 3, USB 2, USB hubs tests worked as expected
intel core 2 duo (2007) Win 10 USB 2, no hubs tests work better than expected 🤩

That last computer, 16 years old, still works. Yes the frames come in slowly from 3 devices while some share the same controller's usb2 bandwidth. Yet, it works with 3 OAKs attached. And works...better. Better meaning that when the Device() for the 3 OAKs is destructed, and it waits for reboot of the device, it FINDS the rebooted devices almost always. I would say it finds all three devices more than 80% of the time. In comparison, when the same test is run on the 1st and 2nd computers, it usually finds the 1st rebooted device but usually does NOT FIND the 2rd and 3rd devices...a rough estimate is a 40% success rate of reboots.

I continue to think the following:

  • Top guess: reboot failures are a hardware/firmware side issue combined with the time needed to reboot and the USB subsystems on the device and host to detect the UNBOOTED VidPid and that device to be able to run the "small program" to get its mxid. I suspect the device and USB subsystem(s) often needs more than 5s. On the faster hosts, it timesout. But on my 16 year old host, I suspect everything is slower and maybe it leads to the device getting more than the 5s which allows for more often finding the device.
  • Alternate: This is a bug or latency in libusb code itself. Something which causes a lag or failure to detect a USB device transitioning VidPids aka boot states.
  • Least guess: is that there is a difference in USB3 vs USB2 behavior...and that USB2 is somehow faster which allows for USB2-only hosts like my old computer to more successfully find them on reboot.

@themarpe
Copy link
Collaborator

Thanks a bunch for this exploration!
I'll create corresponding PRs on DepthAI to go through some of our Windows testing infra as well

@themarpe
Copy link
Collaborator

FATAL logs would likely have to be removed for that, if you have another commit ready with that, will then reference that

@diablodale
Copy link
Contributor Author

Can you/your team do some dev unit testing on Windows machines with this PR? I'd like to have one or two other people review/test this code before removing the debugging code.
If your dev's don't have the time, then -yes- I will remove the debug and also rebase the commits together

@themarpe
Copy link
Collaborator

We will - if you have a commit with that removed I can reference your repo /w changes (can be separate from this PR/branch), in DepthAI and get the testing in

@diablodale
Copy link
Contributor Author

diablodale commented May 15, 2023

We will - if you have a commit with that removed I can reference your repo /w changes (can be separate from this PR/branch), in DepthAI and get the testing in

Done. This PR's head has CI ready code.
For dev unit testing, I recommend using one commit backwards 4ac4914

@themarpe
Copy link
Collaborator

Changes went through our Windows testing as well - I'll go ahead and merge to develop for further testing in core as well
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants