Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bluetooth hci_power_control() interferes with Adafruit TinyUSB #2022

Open
tttapa opened this issue Feb 24, 2024 · 3 comments
Open

Bluetooth hci_power_control() interferes with Adafruit TinyUSB #2022

tttapa opened this issue Feb 24, 2024 · 3 comments
Labels
waiting for feedback Requires response from original poster

Comments

@tttapa
Copy link

tttapa commented Feb 24, 2024

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:

  • Board: Raspberry Pi Pico W
  • Core version: earlephilhower/arduino-pico 3.7.2
  • FQBN: rp2040:rp2040:rpipicow:usbstack=tinyusb,ipbtstack=ipv4btcble

Context: I'm the developer of the https://github.com/tttapa/Control-Surface library, and a user reported this issue: tttapa/Control-Surface#1014 (reply in thread).
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)
--- 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
Copy link
Owner

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 earlephilhower added the waiting for feedback Requires response from original poster label Feb 25, 2024
@earlephilhower
Copy link
Owner

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
Copy link
Author

tttapa commented Mar 2, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for feedback Requires response from original poster
Projects
None yet
Development

No branches or pull requests

2 participants