raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.24k stars 837 forks source link

bluetooth/cyw43439: LE Data Length Extensions (DLE) not working properly #1465

Open mringwal opened 11 months ago

mringwal commented 11 months ago

I'm filing an issue against the CYW43439 reported by a user on the BTstack mailing list.

When ENABLE_LE_DATA_LENGTH_EXTENSION is added to the btstack_config.h, BTstack will enable DLE by sending a LE Write Suggested Default Data Length with 251 bytes and 2120 us.

In the test, the user connects to the Pico W from an iPhone. After connection setup, the data length is exchanged and we receive this event:

Jan 01 18:00:14.214 HCI Event LE - Data Length Change RECV
Parameter Length: 11 (0x0B) Connection Handle: 0x0040 Max Tx Octets: 251 (0xFB) Max Tx Time: 328 (0x148) Max Rx Octets: 251 (0xFB) Max Rx Time: 2120 (0x848)

Weirdly, while Tx octets is set to the expected 251, the Tx time is only 328 us which does only allow for the minimal 27 bytes packets.

The air trace from a Nordic sniffer shows that the 43439 sends a LL_LENGTH_RSP with the values shown above and later that only small packets are sent.

If the CYW43449 respects the 251 octets/328 us, it's clear that it cannot send larger packets, but we're wondering why it sets Tx timer to 328 us instead of the expected 2120 us

The HCI log as well as the air traces are available here: https://drive.google.com/file/d/1ETqv2eZ10VQFjd5k935EAb6Hg-f1E4D4/view?usp=sharing

Sending large 251 bytes packet instead of the smaller 27 bytes packets roughly doubles throughput, or not.

@peterharperuk could you ask Infineon about this?

jchatt0 commented 11 months ago

Thanks for opening this for me Matthias; I'm new to Github/opening issues. I also uploaded the Ellisys trace here

Thanks for everyone's help.

peterharperuk commented 10 months ago

@mringwal Is this related to https://github.com/raspberrypi/pico-sdk/issues/1342 "BLE Host is only able to receive one packet per interval". I think you'd done some analysis of that and found that there was a limit of 27 bytes?

mringwal commented 10 months ago

@peterharperuk At the moment, I don't think that these two issues are related. Event with 27 byte PDUs, there should be multiple PDUs per connection interval in #1342

peterharperuk commented 9 months ago

We have some new firmware that may fix this problem. The firmware is stored in pic-sdk/lib/cyw43-driver. After updating it should show the version...

BT FW download, version = CYW4343A2_001.003.016.0065.0000_Generic_SDIO_37MHz_wlbga_BU_RPI_dl_signed

See https://github.com/peterharperuk/cyw43-driver/tree/firmware_dle_fix or use the attached patch file.

0001-Update-firmware-for-Data-Length-Extension-fix.patch.zip

cd pico-sdk/lib/cyw43-driver git am 0001-Update-firmware-for-Data-Length-Extension-fix.patch

It would be great if someone could test this, otherwise I'm trying to get hold of a sniffer, but it'll take a few days before I can test it.

peterharperuk commented 9 months ago

I gt my sniffer working and my interpretation of the log is that the firmware fix seemed to work? before-after-firmware-change.zip

mringwal commented 9 months ago

Hi @peterharperuk Yes, after applying this patch the DLE setup looks correct and I also get a faster throughput to my iPhone. That's a clear improvement.

peterharperuk commented 9 months ago

Thanks!