raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.74k stars 927 forks source link

BLE Host is only able to receive one packet per interval #1342

Open sanjay900 opened 1 year ago

sanjay900 commented 1 year ago

I currently have two pico ws, one configured as a HOG Host, and one configured as a HOG Device, using code very similar to the hog keyboard hog host examples.

If i connect the device over bluetooth from a PC, i can get to poll rate around 850hz or so, which suggests that multiple packets are being sent per connection interval. However, if i instead connect to the HOG host running on a pico w, i end up with a poll rate of around 32hz, which lines up with the default 30ms connection interval.

If I then add the following line to the HOG host, I can lower the connection interval to the minimum 7.5ms, and I do end up seeing a poll rate of around 130hz or so, which also lines up with the 7.5ms connection interval. gap_set_connection_parameters(0x0060, 0x0030, 0x06, 0x06, 0, 0x0048, 2, 0x0030);

I also ran BTStack locally on my PC using the libusb port, and found that that did get to around 850hz as well, so it does appear to just be an issue with the pico w port specifically.

My eventual goal is to use this to allow a user to create a usb adapter for a bluetooth controller i am working on that can then be plugged into a console, hence the need for a very low latency.

mringwal commented 1 year ago

@sanjay900 No, there aren't. I'd need to get an air trace to confirm that it's still the central side that slows down, then the question is why. I was busy with other stuff though.

kilograham commented 1 year ago

@peterharperuk should this be in 1.5.1 milestone?

sanjay900 commented 1 year ago

@mringwal have you had a chance to grab an air trace? Sorry that i can't help there i don't have the tools for that myself

peterharperuk commented 1 year ago

@kilograham No - it shouldn't be on 1.5.1

mringwal commented 1 year ago

@sanjay900 Sorry, nope, was busy otherwise.

How urgent is this for your project? Could you use a custom solution, e.g. sending multiple reports in a single notification to get full throughput if the connection is between two Pico Ws for the time being.

It's unclear what exactly is causing this (stack, interface to controller, controller itself, radio link). The air trace might narrow this a bit down.

sanjay900 commented 1 year ago

Yeah for the moment I'll do something custom, was just curious if you'd had any time to look into it or not

On Tue, 13 Jun 2023, 9:19 pm Matthias Ringwald, @.***> wrote:

@sanjay900 https://github.com/sanjay900 Sorry, nope, was busy otherwise.

How urgent is this for your project? Could you use a custom solution, e.g. sending multiple reports in a single notification to get full throughput if the connection is between two Pico Ws for the time being.

It's unclear what exactly is causing this (stack, interface to controller, controller itself, radio link). The air trace might narrow this a bit down.

— Reply to this email directly, view it on GitHub https://github.com/raspberrypi/pico-sdk/issues/1342#issuecomment-1588891041, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOGYQCSEX2C2XZ4GST4RYTXLAWCBANCNFSM6AAAAAAW3JVOAU . You are receiving this because you were mentioned.Message ID: @.***>

mringwal commented 1 year ago

@sanjay900 I'm glad to hear. The nice folks at Raspberry will get to this as well, but there's the 1.5.1 release upcoming now - I've heard :)

sanjay900 commented 1 year ago

@sanjay900 I'm glad to hear. The nice folks at Raspberry will get to this as well, but there's the 1.5.1 release upcoming now - I've heard :)

Nice, good to hear they will get to it at some point

rppicomidi commented 1 year ago

@mringwal Back on April 12, you said

"...which would indicate a single packet with 200 bytes (if DLE is supported and I think it is)."

Have you resolved whether DLE is supported? The documentation for the BlueKitchen stack says "Don't Know" for DLE support for the CYW43439. The file pico-examples/pico_w/bt/config/btstack_config.h does not contain #define ENABLE_LE_DATA_LENGTH_EXTENSION but the BlueKitchen btstack/port/raspi/btstack_config.h directory does. Is there some work to do to enable DLE that requires calling the HCI commands that ENABLE_LE_DATA_LENGTH_EXTENSION enables?

mringwal commented 1 year ago

@rppicomidi The 43439 supports DLE. I have a HCI trace which contains a LE Data Length Change to 251 bytes, which indicates that DLE has been activated for that connection.

You are correct that pico-examples/pico_w/bt/config/btstack_config.h does not defines ENABLE_LE_DATA_LENGTH_EXTENSION, which is needed for DLE. To enable DLE, you can add it to btstack_config.h yourself.

@peterharperuk could you add ENABLE_LE_DATA_LENGTH_EXTENSION to the default configuration in the pico-examples repo?

peterharperuk commented 1 year ago

Yes - I have raised this so I don't forget https://github.com/raspberrypi/pico-examples/issues/406

rppicomidi commented 1 year ago

@mringwal @peterharperuk Thank you!

sanjay900 commented 1 year ago

@sanjay900 Sorry, nope, was busy otherwise.

How urgent is this for your project? Could you use a custom solution, e.g. sending multiple reports in a single notification to get full throughput if the connection is between two Pico Ws for the time being.

It's unclear what exactly is causing this (stack, interface to controller, controller itself, radio link). The air trace might narrow this a bit down.

@mringwal how exactly would one send more than one report in a single notification?

mringwal commented 1 year ago

I've setup a test environment with a CSR8510 as Peripheral connected to my Mac and the Pico W with the CYW43439 in Central role. The CSR8510 is an older Bluetooth v4.0 USB Bluetooth dongle, which does not support DLE or 2M-PHY, but is able to send the max amount of PDUs per connection interval (hence, it's trustworthy from my side).

The Peripheral is sending GATT notifications of 20 bytes, similar to the HID reports in the original issue, the Central only receives the Notifications.

With the new firmware, I got 500-900 packets per second, depending on the connection interval (30ms, 15ms, 7.5ms). With 15 m I got 850 and with 7.5 ms even 900 packets. This is a reasonable number of packets per interval, and clearly more than one packet per interval.

@sanjay900 Could you try to repeat this with the firmware @peterharperuk posted above https://github.com/raspberrypi/pico-sdk/issues/1342#issuecomment-1541775040 and use the same settings for GATT Streamer as well as LE Streamer client?

peterharperuk commented 1 year ago

Thanks @mringwal. Is the new firmware behaving better than the current release firmware?

mringwal commented 1 year ago

@peterharperuk To be honest, I didn't go back. As mentioned in the support case, more buffers doesn't hurt. Would it help if repeat the same test with the firmware from master?

aallan commented 1 year ago

Thanks @mringwal. Is the new firmware behaving better than the current release firmware?

Should I replace the UF2 on the currently hosted on the datasheets site with an updated one?

peterharperuk commented 1 year ago

@aallan The firmware was for debug / test purposes only. We would need an official release from Infineon @mringwal I would like to know how the current firmware is behaving before we spend time getting a new official version of the firmware.

mringwal commented 1 year ago

@peterharperuk I see. I'll re-run the same tests with the other firmware then.

sanjay900 commented 1 year ago

I've setup a test environment with a CSR8510 as Peripheral connected to my Mac and the Pico W with the CYW43439 in Central role. The CSR8510 is an older Bluetooth v4.0 USB Bluetooth dongle, which does not support DLE or 2M-PHY, but is able to send the max amount of PDUs per connection interval (hence, it's trustworthy from my side).

The Peripheral is sending GATT notifications of 20 bytes, similar to the HID reports in the original issue, the Central only receives the Notifications.

With the new firmware, I got 500-900 packets per second, depending on the connection interval (30ms, 15ms, 7.5ms). With 15 m I got 850 and with 7.5 ms even 900 packets. This is a reasonable number of packets per interval, and clearly more than one packet per interval.

@sanjay900 Could you try to repeat this with the firmware @peterharperuk posted above #1342 (comment) and use the same settings for GATT Streamer as well as LE Streamer client?

* le_streamer_client: receive only

  * #define TEST_MODE TEST_MODE_ENABLE_NOTIFICATIONS

* gatt_streamer_server: 20 byte PDUs

  * limit packet size by changing the size of the test_data arary
  * char test_data[20];

Just so i can be sure, what are you using for your btstack_config.h?

mringwal commented 1 year ago

@sanjay900 btstack_config.h is was not modified.

mringwal commented 1 year ago

@peterharperuk As expected/feared, I get pretty much the same throughput values with the firmware on master/v1.0.1

mringwal commented 1 year ago

@sanjay900 btw. I'm using the 'poll' version of the Pico run loop variants. Please use that as well to minimize the differences.

sanjay900 commented 1 year ago

I wonder if it's dle related, as I was able to replicate your results, so I went back to hid, and it seems that with a report size of 20 things work, but with a report size of 28 I get limited to the interval again. I'll play around with this more tomorrow, and I'm happy to just shorten my report size and call it a day but it's an interesting result nonetheless.

That also explains why I didn't see this before when I tested previously - I did not shorten test data to 20 bytes when I tried this last time. @mringwal if you use a test_data size of 28, do you see a reduced rate?

mringwal commented 1 year ago

@sanjay900 Thanks for the feedback. When you play around a bit more, I'd be interested what's the smallest size where it slows down. Just enabling DLE doesn't cause an issue, right?

If we can find a clear test case, we can report it back.

I haven't tried 28, but that would requires DLE which is not supported by the CSR8510. I'll need/can use a different dongle then.

sanjay900 commented 1 year ago

@sanjay900 Thanks for the feedback. When you play around a bit more, I'd be interested what's the smallest size where it slows down. Just enabling DLE doesn't cause an issue, right?

If we can find a clear test case, we can report it back.

I haven't tried 28, but that would requires DLE which is not supported by the CSR8510. I'll need/can use a different dongle then.

20 bytes appears to be the magic number, anything higher and i get limited to the interval

sanjay900 commented 1 year ago
  • le_streamer_client: receive only
  • define TEST_MODE TEST_MODE_ENABLE_NOTIFICATIONS

    • gatt_streamer_server: 20 byte PDUs
  • limit packet size by changing the size of the test_data arary
    • char test_data[20];

As for a test case, this works perfectly. settting test_data to 20 results in a rate of around 15 kB/s, while with it set to 21, the rate drops to 1.5kB/s. I used two pico ws, and I kept the server running, all i did was change test_data on the client, everything else stayed the same.

sanjay900 commented 1 year ago

@mringwal Im just glad you somehow picked 20 bytes and that just happend to be the fix haha, don't think i would have figured this out as that wasn't even a consideration on my mind! Weird though that its 20 bytes and not 27, that means something else is at play here i guess.

Also, i can confirm that the run loop doesn't matter, i was replicating these results on the threadsafe-background variant as well.

sanjay900 commented 1 year ago

I just redesigned all my HID reports to be within 20 bytes and now im getting >850hz between the two picos haha, awesome.

mringwal commented 1 year ago

@sanjay900 Glad we found this workaround for you. The limit is 20 bytes as there's a 3 byte ATT header for notifications and a 4 byte header for L2CAP, so the ACL packet will be 27 bytes, which is the maximum packet size without DLE.

I'll probably re-run the tests with 21 bytes payload tomorrow. If that becomes slow, as in your tests, we might have something to show Infineon.

sanjay900 commented 1 year ago

@mringwal that makes sense, i figured there were headers in play. It was kinda good cause it gave me an excuse to do some things more properly, so now im sending 7 byte reports instead of 28 byte ones lol.

mringwal commented 1 year ago

I've tested this again without DLE. The issues shows there as well. Throughput drops from 11 kB/s to 0.7 kB/s when increasing from 20 to 21 bytes. I would have tolerated up to half the throughput (as two packets have to be sent over the air), but the air trace shows that only one 28-byte packet is sends as a 27 bytes and a 1 byte packet is sent per connection interval.

sanjay900 commented 1 year ago

Good to have it replicated,.and in a way that's easy to demonstrate

On Fri, 21 Jul 2023, 1:59 am Matthias Ringwald, @.***> wrote:

I've tested this again without DLE. The issues shows there as well. Throughput drops from 11 kB/s to 0.7 kB/s when increasing from 20 to 21 bytes. I would have tolerated up to half the throughput (as two packets have to be sent over the air), but the air trace shows that only one 28-byte packet is sends as a 27 bytes and a 1 byte packet is sent per connection interval.

— Reply to this email directly, view it on GitHub https://github.com/raspberrypi/pico-sdk/issues/1342#issuecomment-1643981652, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOGYQFE2QAXYQIKV3EBSOLXRE2VZANCNFSM6AAAAAAW3JVOAU . You are receiving this because you were mentioned.Message ID: @.***>

Invictaz commented 8 months ago

Any updates?

peterharperuk commented 8 months ago

No. The issue is still open against the wifi vendor.