Tests: USB: Update endpoint tests

Fix the host script issues present on Windows machines.
Add 0 B payload size to bulk endpoints test.
Update halt and abort tests according to PR comments.
Add an explicit request to start reading on OUT endpoints.

Author: fkjagodzinski

TESTS/host_tests/pyusb_basic.py

@@ -55,6 +55,7 @@ def get_interface(dev, interface, alternate=0):
 VENDOR_TEST_CTRL_OUT_STATUS_DELAY = 8
 VENDOR_TEST_CTRL_IN_SIZES = 9
 VENDOR_TEST_CTRL_OUT_SIZES = 10
+VENDOR_TEST_READ_START = 11
 VENDOR_TEST_UNSUPPORTED_REQUEST = 32
 
 REQUEST_GET_STATUS = 0
@@ -982,38 +983,63 @@ def halt_ep_test(dev, ep_out, ep_in, ep_to_halt, log):
     """
     MIN_HALT_DELAY = 0.01
     MAX_HALT_DELAY = 0.1
-    delay = random.uniform(MIN_HALT_DELAY, MAX_HALT_DELAY)
-    ctrl_kwargs = {
-        'bmRequestType': build_request_type(CTRL_OUT, CTRL_TYPE_STANDARD, CTRL_RECIPIENT_ENDPOINT),
-        'bRequest': REQUEST_SET_FEATURE,
-        'wValue': FEATURE_ENDPOINT_HALT,
-        'wIndex': ep_to_halt.bEndpointAddress}
+    POST_HALT_DELAY = 0.1
     ctrl_error = Event()
 
+    for ep in (ep_out, ep_in):
+        try:
+            if (usb.control.get_status(dev, ep) == 1):
+                raise_unconditionally(lineno(), 'Endpoints must NOT be halted at the start of this test')
+        except usb.core.USBError as err:
+            raise_unconditionally(lineno(), 'Unable to get endpoint status ({!r}).'.format(err))
+
     def timer_handler():
         """Halt an endpoint using a USB control request."""
         try:
-            dev.ctrl_transfer(**ctrl_kwargs)
+            usb.control.set_feature(dev, FEATURE_ENDPOINT_HALT, ep_to_halt)
+            if (usb.control.get_status(dev, ep_to_halt) != 1):
+                raise RuntimeError('Invalid endpoint status after halt operation')
         except Exception as err:
-            log('Endpoint {:#04x} halt failed ({}).'.format(ctrl_kwargs['wIndex'], err))
             ctrl_error.set()
+            log('Endpoint {:#04x} halt failed ({}).'.format(ep_to_halt.bEndpointAddress, err))
+        # Whether the halt operation was successful or not,
+        # wait a bit so the main thread has a chance to run into a USBError
+        # or report the failure of halt operation.
+        time.sleep(POST_HALT_DELAY)
 
+    delay = random.uniform(MIN_HALT_DELAY, MAX_HALT_DELAY)
     delayed_halt = Timer(delay, timer_handler)
     delayed_halt.start()
-    end_ts = time.time() + 1.5 * delay
-    try:
-        while time.time() < end_ts and not ctrl_error.is_set():
-            loopback_ep_test(ep_out, ep_in, ep_out.wMaxPacketSize)
-    except usb.core.USBError as err:
-        if err.errno not in (32, 110):
-            raise_unconditionally(lineno(), 'Unexpected error ({!r}).'.format(err))
+    # Keep transferring data to and from the device until one of the endpoints
+    # is halted.
+    while delayed_halt.is_alive():
         if ctrl_error.is_set():
             raise_unconditionally(lineno(), 'Halting endpoint {0.bEndpointAddress:#04x} failed'
                                   .format(ep_to_halt))
-    else:
-        raise_unconditionally(lineno(), 'Halting endpoint {0.bEndpointAddress:#04x}'
-                              ' during transmission did not raise USBError.'
-                              .format(ep_to_halt))
+        try:
+            loopback_ep_test(ep_out, ep_in, ep_out.wMaxPacketSize)
+        except usb.core.USBError as err:
+            try:
+                ep_status = usb.control.get_status(dev, ep_to_halt)
+            except usb.core.USBError as err:
+                raise_unconditionally(lineno(), 'Unable to get endpoint status ({!r}).'.format(err))
+            if ep_status == 1:
+                # OK, got USBError because of endpoint halt
+                return
+            else:
+                raise_unconditionally(lineno(), 'Unexpected error ({!r}).'.format(err))
+    raise_unconditionally(lineno(), 'Halting endpoint {0.bEndpointAddress:#04x}'
+                          ' during transmission did not raise USBError.'
+                          .format(ep_to_halt))
+
+
+def request_endpoint_read_start(dev, ep):
+    ctrl_kwargs = {
+        'bmRequestType': build_request_type(CTRL_OUT, CTRL_TYPE_VENDOR, CTRL_RECIPIENT_ENDPOINT),
+        'bRequest': VENDOR_TEST_READ_START,
+        'wValue': 0,
+        'wIndex': ep.bEndpointAddress}
+    dev.ctrl_transfer(**ctrl_kwargs)
 
 
 USB_ERROR_FMT = str('Got {0!r} while testing endpoints '
@@ -1054,7 +1080,7 @@ def ep_test_data_correctness(dev, log, verbose=False):
 
         if verbose:
             log('Testing OUT/IN data correctness for bulk endpoint pair.')
-        for payload_size in range(1, bulk_out.wMaxPacketSize + 1):
+        for payload_size in range(bulk_out.wMaxPacketSize + 1):
             try:
                 loopback_ep_test(bulk_out, bulk_in, payload_size)
             except usb.USBError as err:
@@ -1113,21 +1139,19 @@ def ep_test_halt(dev, log, verbose=False):
         while time.time() < end_ts:
             halt_ep_test(dev, bulk_out, bulk_in, bulk_out, log)
             bulk_out.clear_halt()
-            intf.set_altsetting()  # Force the device to start reading data from all OUT endpoints again.
+            request_endpoint_read_start(dev, bulk_out)
             halt_ep_test(dev, bulk_out, bulk_in, bulk_in, log)
             bulk_in.clear_halt()
-            intf.set_altsetting()  # Force the device to start reading data from all OUT endpoints again.
 
         if verbose:
             log('Testing endpoint halt at a random point of interrupt transmission.')
         end_ts = time.time() + 1.0
         while time.time() < end_ts:
             halt_ep_test(dev, interrupt_out, interrupt_in, interrupt_out, log)
             interrupt_out.clear_halt()
-            intf.set_altsetting()  # Force the device to start reading data from all OUT endpoints again.
+            request_endpoint_read_start(dev, interrupt_out)
             halt_ep_test(dev, interrupt_out, interrupt_in, interrupt_in, log)
-            interrupt_out.clear_halt()
-            intf.set_altsetting()  # Force the device to start reading data from all OUT endpoints again.
+            interrupt_in.clear_halt()
 
 
 def ep_test_parallel_transfers(dev, log, verbose=False):
@@ -1285,7 +1309,13 @@ def ep_test_abort(dev, log, verbose=False):
             log('Testing aborting an in progress transfer for IN endpoints.')
         for ep_in in (bulk_in, interrupt_in):
             payload_size = (NUM_PACKETS_UNTIL_ABORT + NUM_PACKETS_AFTER_ABORT) * ep_in.wMaxPacketSize
-            payload_in = ep_in.read(payload_size)
+            payload_in = array.array('B')
+            while len(payload_in) < payload_size:
+                try:
+                    packet = ep_in.read(ep_in.wMaxPacketSize)
+                    payload_in.extend(packet)
+                except usb.core.USBError as err:
+                    break
             if verbose:
                 log('The size of data successfully received from endpoint {0.bEndpointAddress:#04x}: {1} B.'
                     .format(ep_in, len(payload_in)))
@@ -1303,8 +1333,13 @@ def ep_test_abort(dev, log, verbose=False):
             log('Testing aborting an in progress transfer for OUT endpoints.')
         for ep_out in (bulk_out, interrupt_out):
             payload_size = (NUM_PACKETS_UNTIL_ABORT + NUM_PACKETS_AFTER_ABORT) * ep_out.wMaxPacketSize
-            payload_out = array.array('B', (0x01 for _ in range(payload_size)))
-            num_bytes_written = ep_out.write(payload_out)
+            payload_out = array.array('B', (0x01 for _ in range(ep_out.wMaxPacketSize)))
+            num_bytes_written = 0
+            while num_bytes_written < payload_size:
+                try:
+                    num_bytes_written += ep_out.write(payload_out)
+                except usb.core.USBError:
+                    break
             if verbose:
                 log('The size of data successfully sent to endpoint {0.bEndpointAddress:#04x}: {1} B.'
                     .format(ep_out, num_bytes_written))

TESTS/usb_device/basic/USBEndpointTester.cpp

@@ -29,6 +29,7 @@
 #define VENDOR_TEST_CTRL_OUT        2
 #define VENDOR_TEST_CTRL_IN_SIZES   9
 #define VENDOR_TEST_CTRL_OUT_SIZES  10
+#define VENDOR_TEST_READ_START      11
 
 #define EVENT_READY (1 << 0)
 
@@ -130,6 +131,10 @@ USBEndpointTester::USBEndpointTester(USBPhy *phy, uint16_t vendor_id, uint16_t p
     _cnt_cb_int_in = 0;
     _cnt_cb_iso_out = 0;
     _cnt_cb_iso_in = 0;
+    _num_packets_bulk_out_abort = 0;
+    _num_packets_bulk_in_abort = 0;
+    _num_packets_int_out_abort = 0;
+    _num_packets_int_in_abort = 0;
 
     EndpointResolver resolver(endpoint_table());
     resolver.endpoint_ctrl(64);
@@ -147,7 +152,6 @@ USBEndpointTester::USBEndpointTester(USBPhy *phy, uint16_t vendor_id, uint16_t p
     init();
     USBDevice::connect();
     flags.wait_any(EVENT_READY, osWaitForever, false);
-
 }
 
 USBEndpointTester::~USBEndpointTester()
@@ -217,6 +221,9 @@ void USBEndpointTester::callback_request(const setup_packet_t *setup)
                 data = ctrl_buf;
                 size = setup->wValue;
                 break;
+            case VENDOR_TEST_READ_START:
+                result = (_request_read_start(setup)) ? Success : Failure;
+                break;
             default:
                 result = PassThrough;
                 break;
@@ -225,6 +232,26 @@ void USBEndpointTester::callback_request(const setup_packet_t *setup)
     complete_request(result, data, size);
 }
 
+bool USBEndpointTester::_request_read_start(const setup_packet_t *setup)
+{
+    assert_locked();
+    if (setup->bmRequestType.Recipient != ENDPOINT_RECIPIENT) {
+        return false;
+    }
+    size_t ep_index = NUM_ENDPOINTS + 1;
+    for (size_t i = 0; i < NUM_ENDPOINTS; i++) {
+        if (_endpoints[i] == setup->wIndex) {
+            ep_index = i;
+            break;
+        }
+    }
+    if (ep_index > NUM_ENDPOINTS) {
+        return false;
+    }
+    endpoint_abort(_endpoints[ep_index]);
+    return read_start(_endpoints[ep_index], _endpoint_buffs[ep_index], (*_endpoint_configs)[ep_index].max_packet);
+}
+
 void USBEndpointTester::callback_request_xfer_done(const setup_packet_t *setup, bool aborted)
 {
     if (aborted) {
@@ -297,10 +324,10 @@ bool USBEndpointTester::_setup_interface(uint16_t interface, uint8_t alternate)
     _setup_non_zero_endpoints();
 
     if (_abort_transfer_test && alternate >= 1) {
-        _cnt_cb_bulk_out_abort = _cnt_cb_bulk_out;
-        _cnt_cb_bulk_in_abort = _cnt_cb_bulk_in;
-        _cnt_cb_int_out_abort = _cnt_cb_int_out;
-        _cnt_cb_int_in_abort = _cnt_cb_int_in;
+        _num_packets_bulk_out_abort = 0;
+        _num_packets_bulk_in_abort = 0;
+        _num_packets_int_out_abort = 0;
+        _num_packets_int_in_abort = 0;
         start_ep_in_abort_test();
     }
     return true;
@@ -669,9 +696,9 @@ void USBEndpointTester::_cb_bulk_out(usb_ep_t endpoint)
         write_start(_endpoints[EP_BULK_IN], _endpoint_buffs[EP_BULK_IN], rx_size);
     } else {
         // Abort the transfer if enough data was received.
-        uint32_t num_packets_received = _cnt_cb_bulk_out - _cnt_cb_bulk_out_abort;
+        _num_packets_bulk_out_abort++;
         read_start(_endpoints[EP_BULK_OUT], _endpoint_buffs[EP_BULK_OUT], (*_endpoint_configs)[EP_BULK_OUT].max_packet);
-        if (num_packets_received >= NUM_PACKETS_UNTIL_ABORT) {
+        if (_num_packets_bulk_out_abort == NUM_PACKETS_UNTIL_ABORT) {
             endpoint_abort(endpoint);
         }
     }
@@ -686,14 +713,14 @@ void USBEndpointTester::_cb_bulk_in(usb_ep_t endpoint)
         // Receive more data from the host using the OUT endpoint.
         read_start(_endpoints[EP_BULK_OUT], _endpoint_buffs[EP_BULK_OUT], (*_endpoint_configs)[EP_BULK_OUT].max_packet);
     } else {
-        uint32_t num_packets_sent = _cnt_cb_bulk_in - _cnt_cb_bulk_in_abort;
-        if (num_packets_sent >= NUM_PACKETS_UNTIL_ABORT + NUM_PACKETS_AFTER_ABORT) {
+        _num_packets_bulk_in_abort++;
+        if (_num_packets_bulk_in_abort >= NUM_PACKETS_UNTIL_ABORT + NUM_PACKETS_AFTER_ABORT) {
             return;
         }
         // Abort the transfer if enough data was sent.
-        memset(_endpoint_buffs[EP_BULK_IN], num_packets_sent, (*_endpoint_configs)[EP_BULK_IN].max_packet);
+        memset(_endpoint_buffs[EP_BULK_IN], _num_packets_bulk_in_abort, (*_endpoint_configs)[EP_BULK_IN].max_packet);
         write_start(_endpoints[EP_BULK_IN], _endpoint_buffs[EP_BULK_IN], (*_endpoint_configs)[EP_BULK_IN].max_packet);
-        if (num_packets_sent >= NUM_PACKETS_UNTIL_ABORT) {
+        if (_num_packets_bulk_in_abort == NUM_PACKETS_UNTIL_ABORT) {
             endpoint_abort(endpoint);
         }
     }
@@ -710,9 +737,9 @@ void USBEndpointTester::_cb_int_out(usb_ep_t endpoint)
         write_start(_endpoints[EP_INT_IN], _endpoint_buffs[EP_INT_IN], rx_size);
     } else {
         // Abort the transfer if enough data was received.
-        uint32_t num_packets_received = _cnt_cb_int_out - _cnt_cb_int_out_abort;
+        _num_packets_int_out_abort++;
         read_start(_endpoints[EP_INT_OUT], _endpoint_buffs[EP_INT_OUT], (*_endpoint_configs)[EP_INT_OUT].max_packet);
-        if (num_packets_received >= NUM_PACKETS_UNTIL_ABORT) {
+        if (_num_packets_int_out_abort == NUM_PACKETS_UNTIL_ABORT) {
             endpoint_abort(endpoint);
         }
     }
@@ -726,14 +753,14 @@ void USBEndpointTester::_cb_int_in(usb_ep_t endpoint)
         // Receive more data from the host using the OUT endpoint.
         read_start(_endpoints[EP_INT_OUT], _endpoint_buffs[EP_INT_OUT], (*_endpoint_configs)[EP_INT_OUT].max_packet);
     } else {
-        uint32_t num_packets_sent = _cnt_cb_int_in - _cnt_cb_int_in_abort;
-        if (num_packets_sent >= NUM_PACKETS_UNTIL_ABORT + NUM_PACKETS_AFTER_ABORT) {
+        _num_packets_int_in_abort++;
+        if (_num_packets_int_in_abort >= NUM_PACKETS_UNTIL_ABORT + NUM_PACKETS_AFTER_ABORT) {
             return;
         }
         // Abort the transfer if enough data was sent.
-        memset(_endpoint_buffs[EP_INT_IN], num_packets_sent, (*_endpoint_configs)[EP_INT_IN].max_packet);
+        memset(_endpoint_buffs[EP_INT_IN], _num_packets_int_in_abort, (*_endpoint_configs)[EP_INT_IN].max_packet);
         write_start(_endpoints[EP_INT_IN], _endpoint_buffs[EP_INT_IN], (*_endpoint_configs)[EP_INT_IN].max_packet);
-        if (num_packets_sent >= NUM_PACKETS_UNTIL_ABORT) {
+        if (_num_packets_int_in_abort == NUM_PACKETS_UNTIL_ABORT) {
             endpoint_abort(endpoint);
         }
     }

TESTS/usb_device/basic/USBEndpointTester.h

@@ -79,10 +79,10 @@ class USBEndpointTester: public USBDevice {
     volatile uint32_t _cnt_cb_iso_out;
     volatile uint32_t _cnt_cb_iso_in;
 
-    volatile uint32_t _cnt_cb_bulk_out_abort;
-    volatile uint32_t _cnt_cb_bulk_in_abort;
-    volatile uint32_t _cnt_cb_int_out_abort;
-    volatile uint32_t _cnt_cb_int_in_abort;
+    volatile uint32_t _num_packets_bulk_out_abort;
+    volatile uint32_t _num_packets_bulk_in_abort;
+    volatile uint32_t _num_packets_int_out_abort;
+    volatile uint32_t _num_packets_int_in_abort;
 
     virtual const uint8_t *configuration_desc(uint8_t index);
     virtual void callback_state_change(DeviceState new_state);
@@ -103,6 +103,7 @@ class USBEndpointTester: public USBDevice {
 
 private:
     const char *get_desc_string(const uint8_t *desc);
+    bool _request_read_start(const setup_packet_t *setup);
 };
 
 #endif