Skip to content

Commit d805c04

Browse files
committed
Send bulk packet ZLP based on max packet size
The bulk_loopback device test sends a zero-length packet (ZLP) from host to device when the bulk endpoint packet size is a multiple of the max packet size (MPS). Before this commit, the MPS was hard-coded to 64, the MPS of the full-speed bulk endpoint. This commit updates the test to use the MPS provided, at runtime, by the device. I was having issues testing a high-speed device with the test suite before this change (compiling both firmware and test suite with test-class-high-speed feature). Specifically, the test would fail when trying to read back data from the 65 byte transfer. I think I traced the issue back to this condition, but let me know if you think there might be an issue in my device driver. Tested against i.MX RT firmware built with and without the test-class-high-speed feature. A simple 'cargo test' in this crate properly detected the full-/high-speed device, and used the MPS provided by the device. test_class does not seem to support overlapping bulk transfers (multiple INs and OUTs concurrently in flight), even if the underlying USB bus could support it. Without this commit, a host that sends 64 bytes followed by a ZLP could put a device into the state where INs and OUTs are happening concurrently, leading to a failed test. I took some notes below. Rather than update the test_class to handle this, this commit revises the test to do what test_class expects: send all the data (do all the OUTs), then loopback all the data (do all the INs). --- Notes from my testing on a high-speed device. Suppose the bulk_loopback sends a 64 byte packet, followed by a 65 byte packet. We're build a high-speed device, so our MPS is 512 bytes. Here's what I saw: - Host sends 64 bytes to device. - TestClass successfully reads 64 bytes. 64 < 512 (bulk endpoint MPS in high speed), so it schedules a loopback transfer with 64 bytes. - (When compiled with full speed MPS == 64, TestClass is expected to buffer the 64 bytes until receiving the ZLP. This is the discrepancy we're addressing.) - Host sends a ZLP to the device. This happens before TestClass has sent back the 64 bytes. - TestClass successfully reads 0 bytes. 0 < 512, so it tries to write back a ZLP. Suppose this bus cannot support overlapping transfers (there's 64 bytes pending), so the bus responds with "WouldBlock." - (Note: even if this call were to succeed, TestClass state management still cannot support overlapped transfers.) - TestClass's 64 bytes are sent to the host, and endpoint_in_complete is called. Due to the way it manages its state, TestClass schedules another 64 bytes to write. (Study the operations on test_class 'len' and 'i' members to show this.) We have another transfer pending, but this shouldn't be pending. - Host sends 65 bytes to TestClass. - TestClass successfully reads 65 bytes. It tries to loopback this buffer, but there's already 64 bytes pending, so this is another "WouldBlock" write. - TestClass's (second) 64 bytes are sent to the host, and endpoint_in_complete is called. Due to more state management reasons, device schedules 129 bytes back to the host. - Host receives 64 bytes, but it expected 65 bytes. Test failed.
1 parent a94b2ff commit d805c04

File tree

2 files changed

+21
-1
lines changed

2 files changed

+21
-1
lines changed

tests/test_class_host/device.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,25 @@ pub struct DeviceHandles {
1212
pub en_us: Language,
1313
}
1414

15+
impl DeviceHandles {
16+
/// Returns the max packet size for the `TestClass` bulk endpoint(s).
17+
pub fn bulk_max_packet_size(&self) -> u16 {
18+
self.config_descriptor
19+
.interfaces()
20+
.flat_map(|intf| intf.descriptors())
21+
.flat_map(|desc| {
22+
desc.endpoint_descriptors()
23+
.find(|ep| {
24+
// Assumes that IN and OUT endpoint MPSes are the same.
25+
ep.transfer_type() == rusb::TransferType::Bulk
26+
})
27+
.map(|ep| ep.max_packet_size())
28+
})
29+
.next()
30+
.expect("TestClass has at least one bulk endpoint")
31+
}
32+
}
33+
1534
impl ::std::ops::Deref for DeviceHandles {
1635
type Target = DeviceHandle<Context>;
1736

tests/test_class_host/tests.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ fn interface_descriptor(dev, _out) {
164164
}
165165

166166
fn bulk_loopback(dev, _out) {
167+
let max_packet_size: usize = dev.bulk_max_packet_size().into();
167168
for len in &[0, 1, 2, 32, 63, 64, 65, 127, 128, 129] {
168169
let data = random_data(*len);
169170

@@ -173,7 +174,7 @@ fn bulk_loopback(dev, _out) {
173174
data.len(),
174175
"bulk write len {}", len);
175176

176-
if *len > 0 && *len % 64 == 0 {
177+
if *len > 0 && *len % max_packet_size == 0 {
177178
assert_eq!(
178179
dev.write_bulk(0x01, &[], TIMEOUT)
179180
.expect("bulk write zero-length packet"),

0 commit comments

Comments
 (0)