jiegec
Add basic implementation of FtdiDeviceHandler
Adds a non-functional implementation of UsbDeviceHandler for FTDI devices. An FTDI device now enumerates with FtdiDeviceHandler using the following code:
use usbip::ftdi::FtdiDeviceHandler;
...
let device_handler = Arc::new(Mutex::new(Box::new(FtdiDeviceHandler::new())
as Box<dyn usbip::UsbDeviceHandler + Send>));
...
usbip::UsbDevice::new(0).with_interface(...).with_device_handler(device_handler);
Closes #7
@jiegec - sure! I also just want to say that having no filter on new_from_host completely messed up USB on my machine. I would suggest it be removed and only allow new_from_host with a filter (user provided). There could be other wrapper functions that implement default filters: e.g. new_from_host_with_vid_pid
@jiegec I'm not sure what you meant with your comment. I've added an example that spins up a fake 4 port FTDI device and echoes "hello" once a second. If you would like me to move the entirety of the FTDI code into examples, then I'd suggest that would be part of a bigger refactor. I mirrored the structure of the cdc/hid keyboard examples.
@jiegec I'm not sure what you meant with your comment. I've added an example that spins up a fake 4 port FTDI device and echoes "hello" once a second.
Good job!
If you would like me to move the entirety of the FTDI code into examples, then I'd suggest that would be part of a bigger refactor. I mirrored the structure of the cdc/hid keyboard examples.
I put cdc/acm in src because it is a standard usb interface. But ftdi is not.
Pull Request Test Coverage Report for Build 4105479928
- 17 of 164 (10.37%) changed or added relevant lines in 4 files are covered.
- 3 unchanged lines in 3 files lost coverage.
- Overall coverage decreased (-1.7%) to 40.03%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| src/host.rs | 0 | 1 | 0.0% |
| src/device.rs | 17 | 32 | 53.13% |
| src/ftdi.rs | 0 | 42 | 0.0% |
| src/lib.rs | 0 | 89 | 0.0% |
| <!-- | Total: | 17 | 164 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| ../../../.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.25.0/src/io/util/write_int.rs | 1 | 72.22% |
| src/host.rs | 1 | 0% |
| src/lib.rs | 1 | 52.7% |
| <!-- | Total: | 3 |
| Totals | |
|---|---|
| Change from base Build 4061119800: | -1.7% |
| Covered Lines: | 542 |
| Relevant Lines: | 1354 |
💛 - Coveralls
I put cdc/acm in src because it is a standard usb interface. But ftdi is not.
I think that's a rather arbitrary distinction. A driver is a driver. Putting ftdi code in examples means it has less reuse for others. It does lessen the maintenance burden which might be your concern. If you provide me a concrete suggestion for a folder structure you'd like me to observe I will implement it.
I'm also going to break this PR up into smaller PRs since it has taken a while to get this one merged in and there are now other improvements and bugfixes contained in the commits.
I put cdc/acm in src because it is a standard usb interface. But ftdi is not.
I think that's a rather arbitrary distinction. A driver is a driver. Putting ftdi code in examples means it has less reuse for others. It does lessen the maintenance burden which might be your concern. If you provide me a concrete suggestion for a folder structure you'd like me to observe I will implement it.
You are right, maybe we can take driver code to a separate crate e.g. usbip-xxx?
I'm also going to break this PR up into smaller PRs since it has taken a while to get this one merged in and there are now other improvements and bugfixes contained in the commits.
Happy to see that.
Pull Request Test Coverage Report for Build 4103377666
Warning: This coverage report may be inaccurate.
This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
- For more information on this, see Tracking coverage changes with pull request builds.
- To avoid this issue with future PRs, see these Recommended CI Configurations.
- For a quick fix, rebase this PR at GitHub. Your next report should be accurate.
Details
- 22 of 157 (14.01%) changed or added relevant lines in 4 files are covered.
- 75 unchanged lines in 3 files lost coverage.
- Overall coverage decreased (-1.0%) to 40.694%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| src/host.rs | 0 | 1 | 0.0% |
| src/device.rs | 22 | 29 | 75.86% |
| src/ftdi.rs | 0 | 42 | 0.0% |
| src/lib.rs | 0 | 85 | 0.0% |
| <!-- | Total: | 22 | 157 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| src/host.rs | 1 | 0.0% |
| src/lib.rs | 11 | 53.36% |
| src/device.rs | 63 | 50.41% |
| <!-- | Total: | 75 |
| Totals | |
|---|---|
| Change from base Build 4061119800: | -1.0% |
| Covered Lines: | 516 |
| Relevant Lines: | 1268 |