Deadlock problem when connecting to Bluetooth devices with high frequency output

[sunnyqeen]

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]

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]

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]

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]

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]

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]

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

[peterharperuk]

The linked PR adds a define that can be used to limit hci processing loop iterations. However, my testing using spp_streamer didn't seem to show it having much effect. So I've left it undefined so this change will have no effect. It can be defined to a sensible value (10?) if needed.