Requesting MTU #407
Requesting MTU #407
Conversation
src/winrtble/peripheral.rs
Outdated
| @@ -93,6 +94,7 @@ impl Peripheral { | |||
| adapter, | |||
| device: tokio::sync::Mutex::new(None), | |||
| address, | |||
| mtu: AtomicU16::new(23), | |||
There was a problem hiding this comment.
Where does this 23 come from?
There was a problem hiding this comment.
It's the default size of the MTU prior to negotiation. It includes the L2CAP header (4 bytes), so usable size would be 19.
There was a problem hiding this comment.
I think I'll add an initial assignment from MaxPduSize prior to the event handler registration.
There was a problem hiding this comment.
Please add a constant for it, with a comment explaining this.
|
@robbym Hi, have you finish it on bluez? |
|
@laxian I just pushed a working bluez implementation. I tested it on one of my BLE devices and it works, but let me know if there is any issues. @qwandor I added the constant and now moved it to the api namespace so that each implementation can use it. Not really happy with where it currently lives. Maybe move it to common? |
| let services = self.services.lock().unwrap(); | ||
| for (_, service) in services.iter() { | ||
| for (_, characteristic) in service.characteristics.iter() { | ||
| return characteristic.info.mtu.unwrap(); |
There was a problem hiding this comment.
Can this be different for different characteristics?
There was a problem hiding this comment.
In Bluetooth 5.2 and forward, yes. The reason they implemented it as a per-characteristic MTU is because of the enhanced ATT (EATT) bearer feature.
They mention it here: bluez/bluez#199
I mention it here: #246 (comment)
The majority of devices on the market are going to be using the unenhanced version (single MTU per connection), so I don't understand why bluez didn't add support for that first.
In any case, the implementation was based on how SimpleBle did it: https://github.com/OpenBluetoothToolbox/SimpleBLE/blob/614a5af35cdd22ba08318cb776795ecd2da8c389/simpleble/src/backends/linux/PeripheralBase.cpp#L57
There was a problem hiding this comment.
Okay, seems reasonable given the API limitations.
qwandor
added
enhancement
|
Windows parts of this are looking good so far. Would like to get all platforms in before we land it (otherwise I will have people on unsupported platforms yelling) but I'm fine to also do a point release whenever we get all the parts in. |
|
Converted this to a draft until we get all the platforms in. Just makes my life easier for the little amount of time I actually get to do triage on this project. |
|
One more quick update here: I brought us up to bluez-async 0.8.0 in v0.11.7, so mtu() is available now and we can drop that change here (it'll probably just skip the commit on rebase). |
This PR will include a method to retrieve the negotiated MTU size on each platform.
Info grabbed from: #246
Windows - GattSession.MaxPduSize
Negotiation happens on user's behalf, and a GattSession.MaxPduSizeChanged event is emitted when MTU is updated.
macOS / iOS - CBPeripheral.maximumWriteValueLength
Negotiation happens on user's behalf, have used peripheral.maximumWriteValueLength(for: .withoutResponse) reliably in the past. .withResponse has some issues. See here.
Linux
Instead of a dedicated MTU property, BlueZ has decided to adopt the newer Bluetooth 5.2 standard. Example here: Link
Android - BluetoothGatt.requestMtu
The way I understand it and have used it in the past, you call requestMtu with the largest you can support (e.g. 512), and it will negotiate the min(host.maxMtu, periph.maxMtu) and call onMtuChanged with the new mtu size.
TODO: