Actually add the changes methoned in the previous commit message, and…

… add more documentation to msc.py

micropython/usbd/device.py

@@ -2,6 +2,7 @@
 # MIT license; Copyright (c) 2022 Angus Gratton
 from micropython import const
 import machine
+import micropython
 import ustruct
 
 from .utils import split_bmRequestType
@@ -72,6 +73,9 @@ def __init__(self):
         self.config_str = None
         self.max_power_ma = 50
 
+        # Workaround
+        self._always_cb = set()
+
         self._strs = self._get_device_strs()
 
         usbd = self._usbd = machine.USBD()
@@ -84,12 +88,14 @@ def __init__(self):
             xfer_cb=self._xfer_cb,
         )
 
-    def add_interface(self, itf):
+    def add_interface(self, itf, always_cb=False):
         # Add an instance of USBInterface to the USBDevice.
         #
         # The next time USB is reenumerated (by calling .reenumerate() or
         # otherwise), this interface will appear to the host.
         self._itfs.append(itf)
+        if always_cb:
+            self._always_cb.add(itf)
 
     def remove_interface(self, itf):
         # Remove an instance of USBInterface from the USBDevice.
@@ -302,10 +308,21 @@ def _submit_xfer(self, ep_addr, data, done_cb=None):
             return True
         return False
 
+    def _retry_xfer_cb(self, args):
+        # Workaround for when _xfer_cb is called before the callback can be set
+        (ep_addr, result, xferred_bytes) = args
+        self._xfer_cb(ep_addr, result, xferred_bytes)
+
     def _xfer_cb(self, ep_addr, result, xferred_bytes):
         # Singleton callback from TinyUSB custom class driver when a transfer completes.
         try:
             itf, cb = self._eps[ep_addr]
+            # Sometimes this part can be reached before the callback has been registered,
+            # if this interface will *always* have callbacks then reschedule this function
+            if cb is None and itf in self._always_cb:
+                micropython.schedule(self._retry_xfer_cb, (ep_addr, result, xferred_bytes))
+                return
+
             self._eps[ep_addr] = (itf, None)
         except KeyError:
             cb = None

micropython/usbd/hid.py

@@ -6,14 +6,16 @@
 from .utils import (
     endpoint_descriptor,
     split_bmRequestType,
-    EP_OUT_FLAG,
     STAGE_SETUP,
     REQ_TYPE_STANDARD,
     REQ_TYPE_CLASS,
 )
 from micropython import const
 import ustruct
 
+EP_IN_FLAG = const(1 << 7)
+EP_OUT_FLAG = const(0x7F)
+
 _DESC_HID_TYPE = const(0x21)
 _DESC_REPORT_TYPE = const(0x22)
 _DESC_PHYSICAL_TYPE = const(0x23)
@@ -45,6 +47,7 @@ def __init__(
         extra_descriptors=[],
         protocol=_INTERFACE_PROTOCOL_NONE,
         interface_str=None,
+        use_out_ep=False,
     ):
         # Construct a new HID interface.
         #
@@ -60,14 +63,23 @@ def __init__(
         # - protocol can be set to a specific value as per HID v1.11 section 4.3 Protocols, p9.
         #
         # - interface_str is an optional string descriptor to associate with the HID USB interface.
+        #
+        # - use_out_ep needs to be set to True if you're using the OUT endpoint, e.g. to get
+        #   keyboard LEDs
         super().__init__(_INTERFACE_CLASS, _INTERFACE_SUBCLASS_NONE, protocol, interface_str)
         self.extra_descriptors = extra_descriptors
         self.report_descriptor = report_descriptor
         self._int_ep = None  # set during enumeration
+        self._out_ep = None
+        self.use_out_ep = use_out_ep
 
     def get_report(self):
         return False
 
+    def set_report(self):
+        # Override this if you are expecting reports from the host
+        return False
+
     def send_report(self, report_data):
         # Helper function to send a HID report in the typical USB interrupt
         # endpoint associated with a HID interface.  return
@@ -80,12 +92,19 @@ def get_endpoint_descriptors(self, ep_addr, str_idx):
         # As per HID v1.11 section 7.1 Standard Requests, return the contents of
         # the standard HID descriptor before the associated endpoint descriptor.
         desc = self.get_hid_descriptor()
-        ep_addr |= EP_OUT_FLAG
-        desc += endpoint_descriptor(ep_addr, "interrupt", 8, 8)
+        self._int_ep = ep_addr | EP_IN_FLAG
+        ep_addrs = [self._int_ep]
+
+        desc += endpoint_descriptor(self._int_ep, "interrupt", 8, 8)
+
+        if self.use_out_ep:
+            self._out_ep = (ep_addr + 1) & EP_OUT_FLAG
+            desc += endpoint_descriptor(self._out_ep, "interrupt", 8, 8)
+            ep_addrs.append(self._out_ep)
+
         self.idle_rate = 0
         self.protocol = 0
-        self._int_ep = ep_addr
-        return (desc, [], [ep_addr])
+        return (desc, [], ep_addrs)
 
     def get_hid_descriptor(self):
         # Generate a full USB HID descriptor from the object's report descriptor
@@ -102,6 +121,7 @@ def get_hid_descriptor(self):
             0x22,  # bDescriptorType, Report
             len(self.report_descriptor),  # wDescriptorLength, Report
         )
+
         # Fill in any additional descriptor type/length pairs
         #
         # TODO: unclear if this functionality is ever used, may be easier to not
@@ -115,7 +135,7 @@ def get_hid_descriptor(self):
 
     def handle_interface_control_xfer(self, stage, request):
         # Handle standard and class-specific interface control transfers for HID devices.
-        bmRequestType, bRequest, wValue, _, _ = request
+        bmRequestType, bRequest, wValue, wIndex, wLength = request
 
         recipient, req_type, _ = split_bmRequestType(bmRequestType)
 
@@ -144,6 +164,9 @@ def handle_interface_control_xfer(self, stage, request):
             if bRequest == _REQ_CONTROL_SET_PROTOCOL:
                 self.protocol = wValue
                 return b""
+            if bRequest == _REQ_CONTROL_SET_REPORT:
+                return self.set_report()
+
         return False  # Unsupported
 
 

micropython/usbd/hidkeypad.py

@@ -2,36 +2,91 @@
 
 from .hid import HIDInterface
 from .keycodes import KEYPAD_KEYS_TO_KEYCODES
+from .utils import STAGE_SETUP, split_bmRequestType
 from micropython import const
+import micropython
+
 _INTERFACE_PROTOCOL_KEYBOARD = const(0x01)
+_REQ_CONTROL_SET_REPORT = const(0x09)
+_REQ_CONTROL_SET_IDLE = const(0x0A)
 
+# fmt: off
 _KEYPAD_REPORT_DESC = bytes(
     [
         0x05, 0x01,  # Usage Page (Generic Desktop)
             0x09, 0x07,  # Usage (Keypad)
             0xA1, 0x01,  # Collection (Application)
                 0x05, 0x07,  # Usage Page (Keypad)
                 0x19, 0x00,  # Usage Minimum (00),
-                0x29, 0xff,  # Usage Maximum (ff),
+                0x29, 0xFF,  # Usage Maximum (ff),
                 0x15, 0x00,  # Logical Minimum (0),
-                0x25, 0xff,  # Logical Maximum (ff),
+                0x25, 0xFF,  # Logical Maximum (ff),
                 0x95, 0x01,  # Report Count (1),
                 0x75, 0x08,  # Report Size (8),
                 0x81, 0x00,  # Input (Data, Array, Absolute)
+                0x05, 0x08,  # Usage page (LEDs)
+                0x19, 0x01,  # Usage minimum (1)
+                0x29, 0x05,  # Usage Maximum (5),
+                0x95, 0x05,  # Report Count (5),
+                0x75, 0x01,  # Report Size (1),
+                0x91, 0x02,  # Output (Data, Variable, Absolute)
+                0x95, 0x01,  # Report Count (1),
+                0x75, 0x03,  # Report Size (3),
+                0x91, 0x01,  # Output (Constant)
             0xC0,  # End Collection
     ]
 )
+# fmt: on
 
 
 class KeypadInterface(HIDInterface):
     # Very basic synchronous USB keypad HID interface
 
     def __init__(self):
+        self.numlock = None
+        self.capslock = None
+        self.scrolllock = None
+        self.compose = None
+        self.kana = None
+        self.set_report_initialised = False
         super().__init__(
             _KEYPAD_REPORT_DESC,
             protocol=_INTERFACE_PROTOCOL_KEYBOARD,
             interface_str="MicroPython Keypad!",
+            use_out_ep=True,
         )
 
-    def send_report(self, key):
-        super().send_report(KEYPAD_KEYS_TO_KEYCODES[key].to_bytes(1, "big"))
+    def handle_interface_control_xfer(self, stage, request):
+        if request[1] == _REQ_CONTROL_SET_IDLE and not self.set_report_initialised:
+            # Hacky initialisation goes here
+            self.set_report()
+            self.set_report_initialised = True
+
+        if stage == STAGE_SETUP:
+            return super().handle_interface_control_xfer(stage, request)
+
+        bmRequestType, bRequest, wValue, _, _ = request
+        recipient, req_type, _ = split_bmRequestType(bmRequestType)
+
+        return True
+
+    def set_report(self, args=None):
+        self.out_buffer = bytearray(1)
+        self.submit_xfer(self._out_ep, self.out_buffer, self.set_report_cb)
+        return True
+
+    def set_report_cb(self, ep_addr, result, xferred_bytes):
+        buf_result = int(self.out_buffer[0])
+        self.numlock = buf_result & 1
+        self.capslock = (buf_result >> 1) & 1
+        self.scrolllock = (buf_result >> 2) & 1
+        self.compose = (buf_result >> 3) & 1
+        self.kana = (buf_result >> 4) & 1
+
+        micropython.schedule(self.set_report, None)
+
+    def send_report(self, key=None):
+        if key is None:
+            super().send_report(bytes(1))
+        else:
+            super().send_report(KEYPAD_KEYS_TO_KEYCODES[key].to_bytes(1, "big"))

micropython/usbd/msc.py

@@ -9,7 +9,6 @@
 from micropython import const
 import micropython
 import ustruct
-import time
 from machine import Timer
 
 _INTERFACE_CLASS_MSC = const(0x08)
@@ -26,7 +25,7 @@
 
 
 class CBW:
-    """Command Block Wrapper"""
+    """Command Block Wrapper - handles the incoming data from the host to the device"""
 
     DIR_OUT = const(0)
     DIR_IN = const(1)
@@ -86,7 +85,7 @@ def from_binary(self, binary):
 
 
 class CSW:
-    """Command Status Wrapper"""
+    """Command Status Wrapper - handles status messages from the device to the host"""
 
     STATUS_PASSED = const(0)
     STATUS_FAILED = const(1)
@@ -111,7 +110,13 @@ def __bytes__(self):
 
 
 class MSCInterface(USBInterface):
-    """Mass storage interface - contains the USB parts"""
+    """Mass storage interface - contains the USB parts
+
+    Properties:
+    storage_device -- A StorageDevice object used by this instance, which handles all SCSI/filesystem-related operations
+    cbw -- A CBW object to keep track of requests from the host to the device
+    csw -- A CSW object to send status responses to the host
+    lun -- The LUN of this device (currently only 0)"""
 
     MSC_STAGE_CMD = const(0)
     MSC_STAGE_DATA = const(1)
@@ -130,6 +135,16 @@ def __init__(
         uart=None,
         print_logs=False,
     ):
+        """Create a new MSCInterface object
+
+        Properties are all optional:
+        subclass -- should always be _INTERFACE_SUBCLASS_SCSI
+        protocol -- should likely always be _PROTOCOL_BBB
+        filesystem -- can be left as None to have no currently mounted filesystem, or can be a bytes-like object containing a filesystem to use
+        lcd -- an optional LCD object with a "putstr" method, used for logging
+        uart -- an optional UART for serial logging
+        print_logs -- set to True to log via print statements, useful if you have put the REPL on a UART
+        """
         super().__init__(_INTERFACE_CLASS_MSC, subclass, protocol)
         self.lcd = lcd
         self.uart = uart
@@ -173,6 +188,11 @@ def get_endpoint_descriptors(self, ep_addr, str_idx):
         return (desc, [], (self.ep_out, self.ep_in))
 
     def try_to_prepare_cbw(self, args=None):
+        """Attempt to prepare a CBW, and if it fails, reschedule this.
+
+        This is mostly needed due to a bug where control callbacks aren't being received for interfaces other than the first
+        that have been added. Otherwise calling prepare_cbw after the max LUN request has been received works fine.
+        """
         try:
             self.prepare_cbw()
         except KeyError:
@@ -204,7 +224,7 @@ def handle_interface_control_xfer(self, stage, request):
         return False
 
     def reset(self):
-        """Theoretically reset, in reality just break things a bit"""
+        """Theoretically reset, in reality just break things a bit at the moment"""
         self.log("reset()")
         # This doesn't work properly at the moment, needs additional
         # functionality in the C side
@@ -437,7 +457,13 @@ def send_csw_callback(self, ep_addr, result, xferred_bytes):
 
 
 class StorageDevice:
-    """Storage Device - holds the SCSI parts"""
+    """Storage Device - holds the SCSI parts
+
+    Properties:
+    filesystem -- a bytes-like thing representing the data this device is handling. If set to None, then the
+                  object will behave as if there is no medium inserted. This can be changed at runtime.
+    block_size -- what size the blocks are for SCSI commands. This should probably be left as-is, at 512.
+    """
 
     class StorageError(OSError):
         def __init__(self, message, status):
@@ -449,6 +475,10 @@ def __init__(self, message, status):
     INVALID_COMMAND = const(0x02)
 
     def __init__(self, filesystem):
+        """Create a StorageDevice object
+
+        filesystem -- either None or a bytes-like object to represent the filesystem being presented
+        """
         self.filesystem = filesystem
         self.block_size = 512
         self.sense = None