search

edrosten / libblepp

Modern clean C++ Bluetooth Low Energy on Linux without the Bluez DBUS API
Other
239 stars 62 forks source link

support ATT MTU response #39

Closed dylanetaft closed 5 years ago

dylanetaft commented 5 years ago

Got this pull-ready. This OK?

Current behavior: Library will crash. See issue #31

Change justification: By spec, BT devices may request an MTU negotiation. They may refuse anything but MTU responses after that.

Validation: hcidump shows MTU response packets being sent

ACL data: handle 74 flags 0x02 dlen 7 ATT: MTU req (0x02) client rx mtu 247 < HCI Command: LE Read Remote Used Features (0x08|0x0016) plen 2 0000: 4a 00 J. HCI Event: Command Status (0x0f) plen 4 LE Read Remote Used Features (0x08|0x0016) status 0x00 ncmd 1 HCI Event: LE Meta Event (0x3e) plen 12 LE Read Remote Used Features Complete status 0x00 handle 74 Features: 0x01 0x00 0x00 0x00 0x00 0x00 0x00 0x00 < ACL data: handle 74 flags 0x00 dlen 7 ATT: MTU resp (0x03) server rx mtu 23

Unrelated: Did you intend for PDUResponse to be used for outbound packet generation too? I see PDUResponse uses const pointers. That can be dangerous, if source buffers that fed it go out of scope. Working with sockets in the past, send() can also return < len if the send buffers are full.

Should PDUResponse's constructor be converted to a copy constructor, into an internal std::vector?

edrosten commented 5 years ago

Thanks for the pull request, reading it now. In answers to your question:

PDUResponse was only meant to be for the inbound packet generation, in that that's all I've ever used it for. I C code taken from the internals of libbluetooth to generate the packets. The idea is that PDU response wasn't meant to own it's data, it was meant to be a view (like string_view) on top of an existing data buffer. It was also meant to be trivial so it didn't need virtual destructors.

That design made sense to me at the time but in hindsight, I'm not really sure it was worth doing.

edrosten commented 5 years ago

Also, out of interest what does your device want to resize the MTU to? I've never encountered one of these personally?

dylanetaft commented 5 years ago

It’s a BGM111 module from SILabs, plugged into their dev kit - to 247. I’ll make those changes this evening, thanks!

dylanetaft commented 5 years ago

This got larger. Added MTU response PDU support as well.
Here is a trace to show it is working Device A sends request saying it's largest MTU is 247. Device B(us) sends a request that MTU is 247. We set ours to 247. B then sends a response that MTU is 247. A sends back a response that MTU is 247.

I think this handshake COULD go another way A sends a request that it's MTU is 247. B sends one back that it is 150. B keeps 150. A sends a response that it can do 247. A and B resize to 150 as it was the lowest common denominator.

Apparently this complete handshake is necessary - see figure 3.1 in 3.4.2.2 in BT core spec Vol 3 part f It seems odd, but the device is responding to the implementation of the illustration.
I'd be interested in other people testing it.

It could go one step further - a send_att_mtu_request could be added, which should trigger this whole chain with any device, and any BT compliant device should respond.
If we do that though - this exchange should only happen once, it would need to get tracked somehow.

But really though, there's no reason to do this, BTLE isn't really for high IO throughput...

HCI Event: LE Meta Event (0x3e) plen 19 LE Connection Complete status 0x00 handle 72, role master bdaddr 00:0B:57:7F:72:BF (Public) ACL data: handle 72 flags 0x02 dlen 7 ATT: MTU req (0x02) client rx mtu 247 < HCI Command: LE Read Remote Used Features (0x08|0x0016) plen 2 0000: 48 00 H. HCI Event: Command Status (0x0f) plen 4 LE Read Remote Used Features (0x08|0x0016) status 0x00 ncmd 1 HCI Event: LE Meta Event (0x3e) plen 12 LE Read Remote Used Features Complete status 0x00 handle 72 Features: 0x01 0x00 0x00 0x00 0x00 0x00 0x00 0x00 < ACL data: handle 72 flags 0x00 dlen 7 ATT: MTU req (0x02) client rx mtu 247 < ACL data: handle 72 flags 0x00 dlen 7 ATT: MTU resp (0x03) server rx mtu 247 < ACL data: handle 72 flags 0x00 dlen 11 ATT: Read By Group req (0x10) start 0x0001, end 0xffff type-uuid 0x2800 ACL data: handle 72 flags 0x02 dlen 7 ATT: MTU resp (0x03) server rx mtu 247

edrosten commented 5 years ago

Thanks for your great work on this!