Tests: USB: Fix 'endpoint halt' test

Abort all endpoint transfers before running the test again.
Use an updated vendor request to explicitly restart device reads.

Author: Filip Jagodzinski

TESTS/host_tests/pyusb_basic.py

@@ -56,7 +56,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_RW_RESTART = 11
 VENDOR_TEST_ABORT_BUFF_CHECK = 12
 VENDOR_TEST_UNSUPPORTED_REQUEST = 32
 
@@ -984,7 +984,7 @@ def random_size_loopback_ep_test(ep_out, ep_in, failure, error, seconds, log, mi
         time.sleep(0.01)
 
 
-def halt_ep_test(dev, ep_out, ep_in, ep_to_halt, log):
+def halt_ep_test(dev, ep_out, ep_in, log):
     """OUT/IN endpoint halt test.
 
     Verify that halting an endpoint at a random point of OUT or IN transfer
@@ -1003,6 +1003,8 @@ def halt_ep_test(dev, ep_out, ep_in, ep_to_halt, log):
         except usb.core.USBError as err:
             raise_unconditionally(lineno(), 'Unable to get endpoint status ({!r}).'.format(err))
 
+    ep_to_halt = random.choice([ep_out, ep_in])
+
     def timer_handler():
         """Halt an endpoint using a USB control request."""
         try:
@@ -1044,17 +1046,19 @@ def timer_handler():
     finally:
         # Always wait for the Timer thread created above.
         delayed_halt.join()
+        ep_out.clear_halt()
+        ep_in.clear_halt()
     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):
+def request_endpoint_loops_restart(dev):
     ctrl_kwargs = {
-        'bmRequestType': build_request_type(CTRL_OUT, CTRL_TYPE_VENDOR, CTRL_RECIPIENT_ENDPOINT),
-        'bRequest': VENDOR_TEST_READ_START,
+        'bmRequestType': build_request_type(CTRL_OUT, CTRL_TYPE_VENDOR, CTRL_RECIPIENT_DEVICE),
+        'bRequest': VENDOR_TEST_RW_RESTART,
         'wValue': 0,
-        'wIndex': ep.bEndpointAddress}
+        'wIndex': 0}
     dev.ctrl_transfer(**ctrl_kwargs)
 
 
@@ -1149,35 +1153,26 @@ def ep_test_halt(dev, log, verbose=False):
 
         bulk_out, bulk_in = find_ep_pair(intf, usb.ENDPOINT_TYPE_BULK)
         interrupt_out, interrupt_in = find_ep_pair(intf, usb.ENDPOINT_TYPE_INTERRUPT)
-        iso_out, iso_in = find_ep_pair(intf, usb.ENDPOINT_TYPE_ISOCHRONOUS)
 
         if verbose:
             log('\tbulk_out      {0.bEndpointAddress:#04x}, {0.wMaxPacketSize:02} B'.format(bulk_out))
             log('\tbulk_in       {0.bEndpointAddress:#04x}, {0.wMaxPacketSize:02} B'.format(bulk_in))
             log('\tinterrupt_out {0.bEndpointAddress:#04x}, {0.wMaxPacketSize:02} B'.format(interrupt_out))
             log('\tinterrupt_in  {0.bEndpointAddress:#04x}, {0.wMaxPacketSize:02} B'.format(interrupt_in))
-            log('\tiso_out       {0.bEndpointAddress:#04x}, {0.wMaxPacketSize:02} B'.format(iso_out))
-            log('\tiso_in        {0.bEndpointAddress:#04x}, {0.wMaxPacketSize:02} B'.format(iso_in))
 
         if verbose:
             log('Testing endpoint halt at a random point of bulk transmission.')
         end_ts = time.time() + 1.0
         while time.time() < end_ts:
-            halt_ep_test(dev, bulk_out, bulk_in, bulk_out, log)
-            bulk_out.clear_halt()
-            request_endpoint_read_start(dev, bulk_out)
-            halt_ep_test(dev, bulk_out, bulk_in, bulk_in, log)
-            bulk_in.clear_halt()
+            halt_ep_test(dev, bulk_out, bulk_in, log)
+            request_endpoint_loops_restart(dev)
 
         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()
-            request_endpoint_read_start(dev, interrupt_out)
-            halt_ep_test(dev, interrupt_out, interrupt_in, interrupt_in, log)
-            interrupt_in.clear_halt()
+            halt_ep_test(dev, interrupt_out, interrupt_in, log)
+            request_endpoint_loops_restart(dev)
 
 
 def ep_test_parallel_transfers(dev, log, verbose=False):
@@ -1466,7 +1461,10 @@ def ep_test_data_toggle(dev, log, verbose=False):
         # ClearFeature(ENDPOINT_HALT) request always results in the data toggle being reinitialized to DATA0.
         # "
         bulk_out.clear_halt()
-#         request_endpoint_read_start(dev, bulk_out)
+        # The ClearFeature(ENDPOINT_HALT) terminates a pending read operation on the device end.
+        # Use a custom vendor request to restart reading on the OUT endpoint.
+        # This does not impact the state of the data toggle bit.
+        request_endpoint_loops_restart(dev)
 
         # 2.4 verify that host and USB device are still in sync with respect to data toggle
         try:

TESTS/usb_device/basic/USBEndpointTester.cpp

@@ -36,7 +36,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 VENDOR_TEST_RW_RESTART          11
 #define VENDOR_TEST_ABORT_BUFF_CHECK    12
 
 #define EVENT_READY (1 << 0)
@@ -241,8 +241,8 @@ 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;
+            case VENDOR_TEST_RW_RESTART:
+                result = (_request_rw_restart(setup)) ? Success : Failure;
                 break;
             case VENDOR_TEST_ABORT_BUFF_CHECK:
                 result = Send;
@@ -254,54 +254,23 @@ void USBEndpointTester::callback_request(const setup_packet_t *setup)
                 result = PassThrough;
                 break;
         }
-    } else if ((setup->bmRequestType.Type == STANDARD_TYPE) && (setup->bmRequestType.Recipient == ENDPOINT_RECIPIENT)) {
-        if (setup->bRequest == CLEAR_FEATURE) {
-            usb_ep_t ep = setup->wIndex;
-            bool valid = false;
-            uint32_t ep_index = 0;
-            if (ep == _endpoints[EP_BULK_OUT]) {
-                valid = true;
-                ep_index = EP_BULK_OUT;
-            } else if (ep == _endpoints[EP_INT_OUT]) {
-                valid = true;
-                ep_index = EP_INT_OUT;
-            } else if (ep == _endpoints[EP_ISO_OUT]) {
-                valid = true;
-                ep_index = EP_ISO_OUT;
-            }
-
-            if (valid) {
-                // Restart reads when an OUT endpoint is unstalled
-                result = Success;
-                endpoint_unstall(ep);
-                read_start(_endpoints[ep_index], _endpoint_buffs[ep_index], (*_endpoint_configs)[ep_index].max_packet);
-            }
-        }
     }
     complete_request(result, data, size);
 }
 
-bool USBEndpointTester::_request_read_start(const setup_packet_t *setup)
+bool USBEndpointTester::_request_rw_restart(const setup_packet_t *setup)
 {
     assert_locked();
-    if (setup->bmRequestType.Recipient != ENDPOINT_RECIPIENT) {
-        return false;
-    }
-    size_t ep_index = NUM_ENDPOINTS;
+    ep_config_t *epc = NULL;
     for (size_t i = 0; i < NUM_ENDPOINTS; i++) {
-        if (_endpoints[i] == setup->wIndex) {
-            ep_index = i;
-            break;
+        epc = &((*_endpoint_configs)[i]);
+        endpoint_abort(_endpoints[i]);
+        if (epc->dir_in == false) {
+            // Wait for data on every OUT endpoint
+            read_start(_endpoints[i], _endpoint_buffs[i], epc->max_packet);
         }
     }
-    if (ep_index == NUM_ENDPOINTS) {
-        return false;
-    }
-    if (_endpoint_buffs[ep_index] == NULL) {
-        return false;
-    }
-    endpoint_abort(_endpoints[ep_index]);
-    return read_start(_endpoints[ep_index], _endpoint_buffs[ep_index], (*_endpoint_configs)[ep_index].max_packet);
+    return true;
 }
 
 bool USBEndpointTester::_request_abort_buff_check(const setup_packet_t *setup)
@@ -424,9 +393,8 @@ void USBEndpointTester::_setup_non_zero_endpoints()
         if (epc->callback == NULL) {
             continue;
         }
-        if (epc->dir_in == true) {
-//            write_start(_endpoints[i], _endpoint_buffs[i], epc->max_packet);
-        } else {
+        if (epc->dir_in == false) {
+            // Wait for data on every OUT endpoint
             read_start(_endpoints[i], _endpoint_buffs[i], epc->max_packet);
         }
     }

TESTS/usb_device/basic/USBEndpointTester.h

@@ -103,7 +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);
+    bool _request_rw_restart(const setup_packet_t *setup);
     bool _request_abort_buff_check(const setup_packet_t *setup);
 };