raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.26k stars 838 forks source link

Deadlock problem when connecting to Bluetooth devices with high frequency output #1455

Open sunnyqeen opened 12 months ago

sunnyqeen commented 12 months ago

My test setup is using single core, reading data from Bluetooth and write to USB When connecting to Bluetooth devices with high frequency output, the HCI layer always has data to read. The Pico driver has a while loop to read data, it causes the loop will never end. So that other data processing functions will never be called. My solution is, killing the while loop, it may generate delay for reading but it works OK. Is there any other possible solution?

Here are my changes btstack_hci_transport_cyw43.c

static void hci_transport_cyw43_process(void) {
    CYW43_THREAD_LOCK_CHECK
    uint32_t len = 0;
    //bool has_work;
    //do {
        int err = cyw43_bluetooth_hci_read(hci_packet_with_pre_buffer, sizeof(hci_packet_with_pre_buffer), &len);
        BT_DEBUG("bt in len=%lu err=%d\n", len, err);
        if (err == 0 && len > 0) {
            hci_transport_cyw43_packet_handler(hci_packet_with_pre_buffer[3], hci_packet_with_pre_buffer + 4, len - 4);
            //has_work = true;
        } else {
            //has_work = false;
        }
    //} while (has_work);
}
mringwal commented 11 months ago

How much processing do you do with the data? If that loop isn't exit, then the Bluetooth receives data over the air faster than it can be processed. BR/EDR maxes out at 250 kB/s.

I also think that the CYW43439 is configured with flow control, which means that it can only send a few HCI packets before the stack needs to report these packets as processed.

sunnyqeen commented 11 months ago

I try to connect the same time 2 BT classic devices, with 1000hz of 80bytes stream. The data from each device is cached to a separated buffer, and the buffer will be overwritten. I then use a btstack_timer at 4ms interval, that handle the 2 buffers and push to USB. In this case the timer is not called correctly because of the while loop.

mringwal commented 11 months ago

Could you elaborate on "the timer is not called correctly"?

I've checked and Controller to Host Flow Control is used. However, the delivered HCI ACL packet might trigger a HCI Host Num Completed Packets immediately, such that we might stay in this loop.

If you're getting the packets quite fast, you could store the last time the timer was processed and in the packet handler for the new data, check if it's already time to feed USB and just do it.

@peter I'm wondering if we could/should limit the processing to a low number of packets (e.g. 2 or 4) per hci_transport_cyw43_process to both keep the timers alive, but also provide a way to catch up on received HCI packets.

peterharperuk commented 11 months ago

Yes, limiting the number of iterations around the loop is probably a good idea. I had this in the code originally but it got removed at some point. The reporter doesn't make it clear what method they're using, polling or background threadsafe. Neither can delay timer interrupts but other async work might be delayed.

sunnyqeen commented 11 months ago

I mean the timer is delayed a lot, it may happen 10ms. 20ms or even more. I have other timer tasks like control the led, etc. They are all affected.

By the way I used polling, as I have other tinyusb tasks also. my main while loop looks like this

while(1) { bt_polling_tasks(); tusb_polling_tasks(); }

peterharperuk commented 11 months ago

Right, so the "timer" is just another task executed in the polling loop. Yes I can see how that could be delayed.