Skip to content

usbif is not optional, and CDC+MSC as the default costume changes what a board can be debugged with #14

Description

@bdbarnett

Two related problems, found bringing spremote up on a LilyGO T-Embed (ESP32-S3).

1. usbif cannot be excluded from an ESP32 build

The documented way to drop a C module is to delete its cmods/ symlink. Doing that for usbif breaks the build outright:

micropython/ports/esp32/boards/ESP32_GENERIC_S3/mpconfigboard.h:15:41:
    fatal error: usbif_tusb_ext.h: No such file or directory
FAILED: .../espressif__tinyusb/.../cdc_device.c.obj

patches/usbif-02-micropython-esp32-boards-enable-tusb-ext.patch adds

#define MICROPY_HW_USB_EXT_TUSB_CONFIG  "usbif_tusb_ext.h"

to the ESP32 board headers unconditionally, so every ESP32 build requires usbif whether the board wants USB device support or not — and TinyUSB itself stops compiling without it.

The patch should be conditional: applied only when the module is present, or guarded so the define is absent when the header is not.

2. CDC+MSC is the wrong default costume

On the T-Embed the board enumerates as composite CDC + Mass Storage (VID_303A&PID_4003).

  • It silently changes observability per board. Every fprintf(stderr, ...) in spremote — including its success-path [spremote] pairing: HTTP ... line — goes nowhere here, because the composite claims the USB device and the second interface is Mass Storage rather than a console. On the ESP32-P4 the same code is debuggable, because there the second interface is a debug CDC (COM72). Same source, opposite debuggability, and nothing in the application asked for either. That cost real time tonight before the cause was found.
  • A USB drive is an application decision. A board acting as a Spotify Connect speaker has no reason to advertise mass storage to every host it is plugged into.
  • It is inconsistent across boards, which is worse than either choice made consistently.

dev_functions() already exists and works — it was used tonight to bring a USB sound card up and restore the costume afterwards. The mechanism for opting in is there; only the default is wrong.

Suggested: default to CDC alone; MSC/HID/MIDI/AUDIO/VIDEO opt in through dev_functions(). A board wanting a debug console asks for it; a board wanting none of this leaves usbif out entirely once (1) is fixed.

Raised by Brad: "usbif defaulting to cdc+msc may not be a good thing." Agreed — and (1) bites hardest, because it removes the choice altogether.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions