Merge fce8ca69c5 into d11ca092f7

In the case where an OUT control transfer triggers with wLength==0 (i.e.
all data sent in the SETUP phase, and no additional data phase) the
callbacks were previously implemented to return b"" (i.e. an empty buffer
for the data phase).

However this didn't actually work as intended because b"" can't provide a
RW buffer (needed for OUT transfers with a data phase to write data into),
so actually the endpoint would stall.

The symptom was often that the device process the request (if processing
it in the SETUP phase when all information was already available), but the
host sees the endpoint stall and eventually returns an error.

This commit changes the behaviour so returning True from the SETUP phase of
a control transfer queues a zero length status response.

Signed-off-by: Angus Gratton <angus@redyak.com.au>

docs/library/machine.USBDevice.rst

@@ -130,15 +130,25 @@ Methods
 
       Second argument is a memoryview to read the USB control request
       data for this stage. The memoryview is only valid until the
-      callback function returns.
+      callback function returns. Data in this memoryview will be the same
+      across each of the three stages of a single transfer.
+
+      A successful transfer consists of this callback being called in sequence
+      for the three stages. Generally speaking, if a device wants to do
+      something in response to a control request then it's best to wait until
+      the ACK stage to confirm the host controller completed the transfer as
+      expected.
 
       The callback should return one of the following values:
 
-      - ``False`` to stall the endpoint and reject the transfer.
+      - ``False`` to stall the endpoint and reject the transfer. It won't
+        proceed to any remaining stages.
       - ``True`` to continue the transfer to the next stage.
-      - A buffer object to provide data for this stage of the transfer.
-        This should be a writable buffer for an ``OUT`` direction transfer, or a
-        readable buffer with data for an ``IN`` direction transfer.
+      - A buffer object can be returned at the SETUP stage when the transfer
+        will send or receive additional data. Typically this is the case when
+        the ``wLength`` field in the request has a non-zero value. This should
+        be a writable buffer for an ``OUT`` direction transfer, or a readable
+        buffer with data for an ``IN`` direction transfer.
 
     - ``xfer_cb`` - This callback is called whenever a non-control
       transfer submitted by calling :func:`USBDevice.submit_xfer` completes.

examples/bluetooth/ble_hid_keyboard.py

@@ -0,0 +1,125 @@
+# Implements a BLE HID keyboard
+
+from micropython import const
+import struct
+import bluetooth
+
+
+def ble_irq(event, data):
+    global conn_handle
+    if event == 1:
+        print("connect")
+        conn_handle = data[0]
+    else:
+        print("event:", event, data)
+
+
+ble = bluetooth.BLE()
+ble.active(1)
+ble.irq(ble_irq)
+
+UUID = bluetooth.UUID
+
+F_READ = bluetooth.FLAG_READ
+F_WRITE = bluetooth.FLAG_WRITE
+F_READ_WRITE = bluetooth.FLAG_READ | bluetooth.FLAG_WRITE
+F_READ_NOTIFY = bluetooth.FLAG_READ | bluetooth.FLAG_NOTIFY
+
+ATT_F_READ = 0x01
+ATT_F_WRITE = 0x02
+
+hid_service = (
+    UUID(0x1812),  # Human Interface Device
+    (
+        (UUID(0x2A4A), F_READ),  # HID information
+        (UUID(0x2A4B), F_READ),  # HID report map
+        (UUID(0x2A4C), F_WRITE),  # HID control point
+        (UUID(0x2A4D), F_READ_NOTIFY, ((UUID(0x2908), ATT_F_READ),)),  # HID report / reference
+        (UUID(0x2A4D), F_READ_WRITE, ((UUID(0x2908), ATT_F_READ),)),  # HID report / reference
+        (UUID(0x2A4E), F_READ_WRITE),  # HID protocol mode
+    ),
+)
+
+# fmt: off
+HID_REPORT_MAP = bytes([
+    0x05, 0x01,     # Usage Page (Generic Desktop)
+    0x09, 0x06,     # Usage (Keyboard)
+    0xA1, 0x01,     # Collection (Application)
+    0x85, 0x01,     #     Report ID (1)
+    0x75, 0x01,     #     Report Size (1)
+    0x95, 0x08,     #     Report Count (8)
+    0x05, 0x07,     #     Usage Page (Key Codes)
+    0x19, 0xE0,     #     Usage Minimum (224)
+    0x29, 0xE7,     #     Usage Maximum (231)
+    0x15, 0x00,     #     Logical Minimum (0)
+    0x25, 0x01,     #     Logical Maximum (1)
+    0x81, 0x02,     #     Input (Data, Variable, Absolute); Modifier byte
+    0x95, 0x01,     #     Report Count (1)
+    0x75, 0x08,     #     Report Size (8)
+    0x81, 0x01,     #     Input (Constant); Reserved byte
+    0x95, 0x05,     #     Report Count (5)
+    0x75, 0x01,     #     Report Size (1)
+    0x05, 0x08,     #     Usage Page (LEDs)
+    0x19, 0x01,     #     Usage Minimum (1)
+    0x29, 0x05,     #     Usage Maximum (5)
+    0x91, 0x02,     #     Output (Data, Variable, Absolute); LED report
+    0x95, 0x01,     #     Report Count (1)
+    0x75, 0x03,     #     Report Size (3)
+    0x91, 0x01,     #     Output (Constant); LED report padding
+    0x95, 0x06,     #     Report Count (6)
+    0x75, 0x08,     #     Report Size (8)
+    0x15, 0x00,     #     Logical Minimum (0)
+    0x25, 0x65,     #     Logical Maximum (101)
+    0x05, 0x07,     #     Usage Page (Key Codes)
+    0x19, 0x00,     #     Usage Minimum (0)
+    0x29, 0x65,     #     Usage Maximum (101)
+    0x81, 0x00,     #     Input (Data, Array); Key array (6 bytes)
+    0xC0,           # End Collection
+])
+# fmt: on
+
+# register services
+ble.config(gap_name="MP-keyboard")
+handles = ble.gatts_register_services((hid_service,))
+print(handles)
+h_info, h_hid, _, h_rep, h_d1, _, h_d2, h_proto = handles[0]
+
+# set initial data
+ble.gatts_write(h_info, b"\x01\x01\x00\x02")  # HID info: ver=1.1, country=0, flags=normal
+ble.gatts_write(h_hid, HID_REPORT_MAP)  # HID report map
+ble.gatts_write(h_d1, struct.pack("<BB", 1, 1))  # report: id=1, type=input
+ble.gatts_write(h_d2, struct.pack("<BB", 1, 2))  # report: id=1, type=output
+ble.gatts_write(h_proto, b"\x01")  # protocol mode: report
+
+# advertise
+adv = (
+    b"\x02\x01\x06"
+    b"\x03\x03\x12\x18"  # complete list of 16-bit service UUIDs: 0x1812
+    b"\x03\x19\xc1\x03"  # appearance: keyboard
+    b"\x0c\x09MP-keyboard"  # complete local name
+)
+conn_handle = None
+ble.gap_advertise(100_000, adv)
+
+# once connected use the following to send reports
+
+
+def send_char(char):
+    if char == " ":
+        mod = 0
+        code = 0x2C
+    elif ord("a") <= ord(char) <= ord("z"):
+        mod = 0
+        code = 0x04 + ord(char) - ord("a")
+    elif ord("A") <= ord(char) <= ord("Z"):
+        mod = 2
+        code = 0x04 + ord(char) - ord("A")
+    else:
+        assert 0
+    ble.gatts_notify(conn_handle, h_rep, struct.pack("8B", mod, 0, code, 0, 0, 0, 0, 0))
+    ble.gatts_notify(conn_handle, h_rep, b"\x00\x00\x00\x00\x00\x00\x00\x00")
+
+
+def send_str(st):
+    for c in st:
+        send_char(c)

examples/bluetooth/ble_hid_mouse.py

@@ -0,0 +1,110 @@
+# Implements a BLE HID mouse
+
+from micropython import const
+import struct
+import bluetooth
+
+
+def ble_irq(event, data):
+    global conn_handle
+    if event == 1:
+        print("connect")
+        conn_handle = data[0]
+    else:
+        print("event:", event, data)
+
+
+ble = bluetooth.BLE()
+ble.active(1)
+ble.irq(ble_irq)
+
+UUID = bluetooth.UUID
+
+F_READ = bluetooth.FLAG_READ
+F_WRITE = bluetooth.FLAG_WRITE
+F_READ_WRITE = bluetooth.FLAG_READ | bluetooth.FLAG_WRITE
+F_READ_NOTIFY = bluetooth.FLAG_READ | bluetooth.FLAG_NOTIFY
+
+ATT_F_READ = 0x01
+ATT_F_WRITE = 0x02
+
+hid_service = (
+    UUID(0x1812),  # Human Interface Device
+    (
+        (UUID(0x2A4A), F_READ),  # HID information
+        (UUID(0x2A4B), F_READ),  # HID report map
+        (UUID(0x2A4C), F_WRITE),  # HID control point
+        (UUID(0x2A4D), F_READ_NOTIFY, ((UUID(0x2908), ATT_F_READ),)),  # HID report / reference
+        (UUID(0x2A4E), F_READ_WRITE),  # HID protocol mode
+    ),
+)
+
+# fmt: off
+HID_REPORT_MAP = bytes([
+    0x05, 0x01,     # Usage Page (Generic Desktop)
+    0x09, 0x02,     # Usage (Mouse)
+    0xA1, 0x01,     # Collection (Application)
+    0x09, 0x01,     #     Usage (Pointer)
+    0xA1, 0x00,     #     Collection (Physical)
+    0x85, 0x01,     #         Report ID (1)
+    0x95, 0x03,     #         Report Count (3)
+    0x75, 0x01,     #         Report Size (1)
+    0x05, 0x09,     #         Usage Page (Buttons)
+    0x19, 0x01,     #         Usage Minimum (1)
+    0x29, 0x03,     #         Usage Maximum (3)
+    0x15, 0x00,     #         Logical Minimum (0)
+    0x25, 0x01,     #         Logical Maximum (1)
+    0x81, 0x02,     #         Input(Data, Variable, Absolute); 3 button bits
+    0x95, 0x01,     #         Report Count(1)
+    0x75, 0x05,     #         Report Size(5)
+    0x81, 0x01,     #         Input(Constant);                 5 bit padding
+    0x75, 0x08,     #         Report Size (8)
+    0x95, 0x02,     #         Report Count (3)
+    0x05, 0x01,     #         Usage Page (Generic Desktop)
+    0x09, 0x30,     #         Usage (X)
+    0x09, 0x31,     #         Usage (Y)
+    0x09, 0x38,     #         Usage (Wheel)
+    0x15, 0x81,     #         Logical Minimum (-127)
+    0x25, 0x7F,     #         Logical Maximum (127)
+    0x81, 0x06,     #         Input(Data, Variable, Relative); 3 position bytes (X,Y,Wheel)
+    0xC0,           #     End Collection
+    0xC0,           # End Collection
+])
+# fmt: on
+
+# register services
+ble.config(gap_name="MP-mouse")
+handles = ble.gatts_register_services((hid_service,))
+print(handles)
+h_info, h_hid, _, h_rep, h_d1, h_proto = handles[0]
+
+# set initial data
+ble.gatts_write(h_info, b"\x01\x01\x00\x02")  # HID info: ver=1.1, country=0, flags=normal
+ble.gatts_write(h_hid, HID_REPORT_MAP)  # HID report map
+ble.gatts_write(h_d1, struct.pack("<BB", 1, 1))  # report: id=1, type=input
+ble.gatts_write(h_proto, b"\x01")  # protocol mode: report
+
+# advertise
+adv = (
+    b"\x02\x01\x06"
+    b"\x03\x03\x12\x18"  # complete list of 16-bit service UUIDs: 0x1812
+    b"\x03\x19\xc2\x03"  # appearance: mouse
+    b"\x09\x09MP-mouse"  # complete local name
+)
+conn_handle = None
+ble.gap_advertise(100_000, adv)
+
+# once connected use the following to send reports
+
+
+def send_mouse(button_mask, x, y, wheel):
+    ble.gatts_notify(conn_handle, h_rep, struct.pack("4B", button_mask, x, y, wheel))
+
+
+def send_click(button):
+    send_mouse(1 << button, 0, 0, 0)
+    send_mouse(0, 0, 0, 0)
+
+
+def send_motion(x, y):
+    send_mouse(0, x, y, 0)

shared/tinyusb/mp_usbd_runtime.c

@@ -295,6 +295,7 @@ static bool runtime_dev_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_cont
     mp_obj_usb_device_t *usbd = MP_OBJ_TO_PTR(MP_STATE_VM(usbd));
     tusb_dir_t dir = request->bmRequestType_bit.direction;
     mp_buffer_info_t buf_info;
+    bool result;
 
     if (!usbd) {
         return false;
@@ -319,7 +320,7 @@ static bool runtime_dev_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_cont
 
     // Check if callback returned any data to submit
     if (mp_get_buffer(cb_res, &buf_info, dir == TUSB_DIR_IN ? MP_BUFFER_READ : MP_BUFFER_RW)) {
-        bool result = tud_control_xfer(USBD_RHPORT,
+        result = tud_control_xfer(USBD_RHPORT,
             request,
             buf_info.buf,
             buf_info.len);
@@ -328,17 +329,21 @@ static bool runtime_dev_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_cont
             // Keep buffer object alive until the transfer completes
             usbd->xfer_data[0][dir] = cb_res;
         }
-
-        return result;
     } else {
         // Expect True or False to stall or continue
+        result = mp_obj_is_true(cb_res);
 
-        if (stage == CONTROL_STAGE_ACK) {
+        if (stage == CONTROL_STAGE_SETUP && result) {
+            // If no additional data but callback says to continue transfer then
+            // queue a status response.
+            tud_control_status(rhport, request);
+        } else if (stage == CONTROL_STAGE_ACK) {
             // Allow data to be GCed once it's no longer in use
             usbd->xfer_data[0][dir] = mp_const_none;
         }
-        return mp_obj_is_true(cb_res);
     }
+
+    return result;
 }
 
 static bool runtime_dev_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_t xferred_bytes) {