c++ wrapper for libusb, add try/catch to prevent app crashes #63
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- coordinate usb_host.cpp mutex and usb_mx_id_cache init - make usb_mx_id_cache apis noexcept
- adds exception handling, RAII for device list
- add claim/release interfaces so to always release interfaces before closing handle. Fixes several error handling scenarios in XLink
- test was failing due to stucts not being init, they would contain random data in fields expecting null
- better libusb call parameter checking - adds control of loglevel and throw/not
- update refLibusbDeviceByName() to have c++ signature and use RAII ref counting
- add function-wide try/catch in getUSBDevices() and nested try to catch usb-specific errors - removed/simplified code that is now in the RAII wrappers - adjusted several function signatures to pass const usb_device& instead of raw pointers that cannot be safely ref counted
…_ascii() - libusb_get_device_list() is not thread-safe, workaround with mutex in device_list class - fix get_port_numbers() - optimize device_handle::set_configuration()
diablodale
force-pushed
the
fix59-wrap-libusb-trycatch
branch
from
af9f215
to
d3c0cfe
Compare
- Fixes luxonis#61 major issues - get bcdUSB spec version from device description during usb_boot() and use with send_file() during boot - add MVLOG_DEBUG log for usb spec during usb_boot()
- add NOMINMAX to cmake for win32; well known need on modern code; was already in depthai-core
- begin refactoring of usb_host.cpp file to eliminate redundant code and collapse code into simplier functions using the libusb wrappers - add simple dai::details::span that works on C++11 and when c++20 is available will use that instead
- RAII partially implemented due to non-RAII capable fd lookup container
- fixes luxonis#67 - fixes mingw compile since errant code followed Windows macro logic
- fixes luxonis#69 - workaround libusb/libusb#1287
- behavior is undefined/crash using resource methods when `get() == nullptr`
- note: it does not have all std::span features
- reduces risk that callers will use a raw libusb_device* without managing its ownership and lifetime - simplifies 2-step (get iter, create usb_device) to 1-step (get usb_device)
diablodale
force-pushed
the
fix59-wrap-libusb-trycatch
branch
from
be19c74
to
f78945d
Compare
|
I've pushed the last commit to this branch. It includes markdown and mermaid documentation for usage and object model for the libusb wrapper. Its really quite simple. 🔢 This wrapper replaces much of the old Xlink C-style code for its usb transport. Moving forward, fixes from this PR and more will usually be in my fork at https://github.com/diablodale/XLink/ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds c++ wrappers for the minimum set of features needed without introducing major Xlink code refactoring. fixes #59
I can't move much further until the questions below are answered.
dai::libusbnamespaceusb_errorfor exceptionsusb_context,usb_device,device_list,device_handle,config_descriptordevice_handle::claim_interface(),device_handle::set_configuration(), etc. to manage interfaces and fix resource leak bugs I foundlibusb_get_device_list()Questions
what code should makeThe wrapper classes do the logging as it matched the majority of use cases.mvLogentries. Should that be in the wrapper? Or should the Caller do it withcatch() { mvLog("had an error: " + myexception.what()); }. Pros and Cons for both. This is more a style and feature choice.Can I removeI removed it.extern CfromrefLibusbDeviceByName()? I don't see a need for it. Does the device firmware code need it?refLibusbDeviceByName()say it filters by Myraid devices. It actually doesn't. This can be added if needed.I would like to pass between a few functions wrapper classes instead of raw pointers. Does your team have any concerns?Already changed some func signatures to use the new classes; usually as a reference.