earlephilhower / arduino-pico

Raspberry Pi Pico Arduino core, for all RP2040 boards
GNU Lesser General Public License v2.1
1.87k stars 390 forks source link

Bluetooth hci_power_control() interferes with Adafruit TinyUSB #2022

Open tttapa opened 4 months ago

tttapa commented 4 months ago

Hi,

I'm trying to use Bluetooth LE together with the Adafruit TinyUSB library. When initializing the Bluetooth stack first, this breaks the USB code.

I've narrowed it down to the call to the hci_power_control(HCI_POWER_ON) function. Here's a minimal example:

sketch.ino

void power_on_hci();

#include <Adafruit_TinyUSB.h>

Adafruit_USBD_MIDI usb_midi;

void setup() {
#if 1 // When 1, USB MIDI does not work, when 0, it does work
  power_on_hci();
#endif
  pinMode(LED_BUILTIN, OUTPUT);
  usb_midi.begin();
}

void loop() { // Blink as a liveness check
  static bool state = false;
  digitalWrite(LED_BUILTIN, state = !state);
  delay(500);
}

power_on_hci.cpp (second tab)

// This is a separate file because Adafruit_TinyUSB_Arduino/src/class/hid/hid.h
// conflicts with pico-sdk/lib/btstack/src/btstack_hid.h
#include <btstack.h>

void power_on_hci() { hci_power_control(HCI_POWER_ON); }

You can change the #if 1 to #if 0 to see the difference: when leaving out the call to hci_power_control(HCI_POWER_ON), the Pico W correctly show up as a USB MIDI device; when including the call to hci_power_control(HCI_POWER_ON), it only shows up as a serial port. (See the difference between the USB descriptor at the bottom of this post.)

Version and hardware:

Context: I'm the developer of the https://github.com/tttapa/Control-Surface library, and a user reported this issue: https://github.com/tttapa/Control-Surface/discussions/1014#discussioncomment-8576397.
The full BLE code used there can be found here: https://github.com/tttapa/Control-Surface/blob/d6a5fb1313f8e483b13619dba1fa80d0ec09ca62/src/MIDI_Interfaces/BLEMIDI/BTstack/gatt_midi.cpp (although I don't think this really matters for this issue, simply calling hci_power_control suffices to reproduce the issue).

Difference in USB descriptors between #if 0 and #if 1 (click to open) ```patch --- descr-midi.txt 2024-02-24 15:08:49.003533654 +0100 +++ descr-no-midi.txt 2024-02-24 15:08:00.876148087 +0100 @@ -17,8 +17,8 @@ Configuration Descriptor: bLength 9 bDescriptorType 2 - wTotalLength 0x00a7 - bNumInterfaces 4 + wTotalLength 0x004b + bNumInterfaces 2 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xa0 @@ -95,107 +95,3 @@ Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 0 - Interface Descriptor: - bLength 9 - bDescriptorType 4 - bInterfaceNumber 2 - bAlternateSetting 0 - bNumEndpoints 0 - bInterfaceClass 1 Audio - bInterfaceSubClass 1 Control Device - bInterfaceProtocol 0 - iInterface 0 - AudioControl Interface Descriptor: - bLength 9 - bDescriptorType 36 - bDescriptorSubtype 1 (HEADER) - bcdADC 1.00 - wTotalLength 0x0009 - bInCollection 1 - baInterfaceNr(0) 3 - Interface Descriptor: - bLength 9 - bDescriptorType 4 - bInterfaceNumber 3 - bAlternateSetting 0 - bNumEndpoints 2 - bInterfaceClass 1 Audio - bInterfaceSubClass 3 MIDI Streaming - bInterfaceProtocol 0 - iInterface 0 - MIDIStreaming Interface Descriptor: - bLength 7 - bDescriptorType 36 - bDescriptorSubtype 1 (HEADER) - bcdADC 1.00 - wTotalLength 0x0041 - MIDIStreaming Interface Descriptor: - bLength 6 - bDescriptorType 36 - bDescriptorSubtype 2 (MIDI_IN_JACK) - bJackType 1 Embedded - bJackID 1 - iJack 0 - MIDIStreaming Interface Descriptor: - bLength 6 - bDescriptorType 36 - bDescriptorSubtype 2 (MIDI_IN_JACK) - bJackType 2 External - bJackID 2 - iJack 0 - MIDIStreaming Interface Descriptor: - bLength 9 - bDescriptorType 36 - bDescriptorSubtype 3 (MIDI_OUT_JACK) - bJackType 1 Embedded - bJackID 3 - bNrInputPins 1 - baSourceID( 0) 2 - BaSourcePin( 0) 1 - iJack 0 - MIDIStreaming Interface Descriptor: - bLength 9 - bDescriptorType 36 - bDescriptorSubtype 3 (MIDI_OUT_JACK) - bJackType 2 External - bJackID 4 - bNrInputPins 1 - baSourceID( 0) 1 - BaSourcePin( 0) 1 - iJack 0 - Endpoint Descriptor: - bLength 9 - bDescriptorType 5 - bEndpointAddress 0x02 EP 2 OUT - bmAttributes 2 - Transfer Type Bulk - Synch Type None - Usage Type Data - wMaxPacketSize 0x0040 1x 64 bytes - bInterval 0 - bRefresh 0 - bSynchAddress 0 - MIDIStreaming Endpoint Descriptor: - bLength 5 - bDescriptorType 37 - bDescriptorSubtype 1 (GENERAL) - bNumEmbMIDIJack 1 - baAssocJackID( 0) 1 - Endpoint Descriptor: - bLength 9 - bDescriptorType 5 - bEndpointAddress 0x83 EP 3 IN - bmAttributes 2 - Transfer Type Bulk - Synch Type None - Usage Type Data - wMaxPacketSize 0x0040 1x 64 bytes - bInterval 0 - bRefresh 0 - bSynchAddress 0 - MIDIStreaming Endpoint Descriptor: - bLength 5 - bDescriptorType 37 - bDescriptorSubtype 1 (GENERAL) - bNumEmbMIDIJack 1 - baAssocJackID( 0) 3 ```
earlephilhower commented 4 months ago

Very odd indeed! I was able to repro the failure with the combined sketch below:

#define HID_REPORT_TYPE_INPUT HID_REPORT_TYPE_INPUT_bt
#define HID_REPORT_TYPE_OUTPUT HID_REPORT_TYPE_OUTPUT_bt
#define HID_REPORT_TYPE_FEATURE HID_REPORT_TYPE_FEATURE_bt
#define hid_report_type_t hid_report_type_t_bt
#include <btstack.h>
#undef HID_REPORT_TYPE_INPUT
#undef HID_REPORT_TYPE_OUTPUT
#undef HID_REPORT_TYPE_FEATURE
#undef hid_report_type_t

void power_on_hci() { hci_power_control(HCI_POWER_ON); }
#include <Adafruit_TinyUSB.h>

Adafruit_USBD_MIDI usb_midi;

void setup() {
#if 1 // When 1, USB MIDI does not work, when 0, it does work
  power_on_hci();
#endif
  pinMode(LED_BUILTIN, OUTPUT);
  usb_midi.begin();
}

void loop() { // Blink as a liveness check
  static bool state = false;
  digitalWrite(LED_BUILTIN, state = !state);
  Serial.printf("%s\n", state ? "on" : "off");
  delay(500);
}

My first guess was there was a linking order issue and the core's USB setup was overriding the Adafruit one, but I see the Serial.write is correctly calling Adafruit (using GDB)

Adafruit_USBD_CDC::write (this=this@entry=0x200017f4 <Serial>, ch=ch@entry=97 'a') at /home/earle/Arduino/hardware/pico/rp2040/libraries/Adafruit_TinyUSB_Arduino/src/arduino/Adafruit_USBD_CDC.cpp:215
215 size_t Adafruit_USBD_CDC::write(uint8_t ch) { return write(&ch, 1); }

Moving the BT startup afterwards also works, as you reported.

I also instrumented things and usbmidi.begin() is returning true even though it's obviously failing. So, the Adafruit lib thinks it's good to go.

At this point there's actually no code from the core here that's being invoked in these paths. I think you'll need to move this to their repo since they understand what's going on to reset the USB interface on the add call. My guess is that they're doing something that interferes/doesn't work when BT is working (because there will be other clocks and blocks running to handle the BT virtual serial HCI port), but it's not inside the core here.

earlephilhower commented 4 months ago

Yup, the Adafruit library of TinyUSB is not signaling the PC to re-read the descriptor when this happens and hence the PC will keep the old one. If you cause the USB bus to rescan, the device does show up properly:

earle@amd:~$ aconnect -i
client 0: 'System' [type=kernel]
    0 'Timer           '
    1 'Announce        '
client 14: 'Midi Through' [type=kernel]
    0 'Midi Through Port-0'
client 24: 'SM8150-MTP _SN:825A8D33' [type=kernel,card=2]
    0 'SM8150-MTP _SN:825A8D33 MIDI 1'

earle@amd:~$ sudo ~/usbreset.sh 
Resetting devices from /sys/bus/pci/drivers/uhci_hcd...
/home/earle/usbreset.sh: line 18: echo: write error: No such device
/home/earle/usbreset.sh: line 19: echo: write error: No such device
Resetting devices from /sys/bus/pci/drivers/xhci_hcd...

earle@amd:~$ aconnect -i
client 0: 'System' [type=kernel]
    0 'Timer           '
    1 'Announce        '
client 14: 'Midi Through' [type=kernel]
    0 'Midi Through Port-0'
client 24: 'Pico W' [type=kernel,card=2]
    0 'Pico W MIDI 1   '
client 28: 'SM8150-MTP _SN:825A8D33' [type=kernel,card=3]
    0 'SM8150-MTP _SN:825A8D33 MIDI 1'

(my usbreset for Linux is

#!/bin/bash

if [[ $EUID != 0 ]] ; then
  echo This must be run as root!
  exit 1
fi

for xhci in /sys/bus/pci/drivers/?hci_hcd ; do

  if ! cd $xhci ; then
    echo Weird error. Failed to change directory to $xhci
    exit 1
  fi

  echo Resetting devices from $xhci...

  for i in ????:??:??.? ; do
    echo -n "$i" > unbind
    echo -n "$i" > bind
  done
done

So this is a TinyUSB issue most likely. Adafruit is making a new descriptor and informing TinyUSB about it, but for some reason TinyUSB can't tell the PC about it. Might want to ping them with this info, it may be a simple fix...

tttapa commented 4 months ago

Thanks a lot for looking into this! I'll open an issue with the Adafruit library.