NordicSemiconductor / Android-BLE-Library

A library that makes working with Bluetooth LE on Android a pleasure. Seriously.
BSD 3-Clause "New" or "Revised" License
1.98k stars 413 forks source link

Library DefaultMtuSplitter is splitting incorrectly for android OS 14 #541

Closed uberchilly closed 6 months ago

uberchilly commented 8 months ago

According to this change in Android OS 14 that we can read here: https://developer.android.com/about/versions/14/behavior-changes-all#mtu-set-to-517

Max message size should be A reminder that you should reduce 5 bytes from the supported size for the headers For example: arrayMaxLength = min(SUPPORTED_MTU, GATT_MAX_ATTR_LEN(517)) - 5

And the library is calculating max message size = 514 as max from mtu 517 - 3 here: final int maxLength = writeType != BluetoothGattCharacteristic.WRITE_TYPE_SIGNED ? mtu - 3 : mtu - 12;

This affects the app I am working on in a way that I am reading around 5000 bytes chunks and trying to upload to the hw device, but on the first message, it just waits forever. When I change the buffer size to 512 for example it works like normal it is just slower. This issue is not observed on android os <= 13 only on 14

philips77 commented 8 months ago

OMG! What?! Why?! 5?! Five? Are they out of their mind? Why 5? It was always 3, one for OpCode, 2 for Handle number. What the hell! I need to find out the root cause of this change in their source code. Give me some time. Btw, thank you for bringing it to us. I wasn't aware of the change.

uberchilly commented 8 months ago

I've also tested via nRF connect app, with mtu 517 I cannot send 513 bytes message on android 14 it fails, but 512 works correctly

philips77 commented 8 months ago

But if I have a device which doesn't support higher MTU, that is defaults to 23, I'm getting value 23 in onMtuChanged, from which I have to deduct 3, not 5. Perhaps it's MTU-3, where max is 512? For me this looks like a bug either in Android doc or some internal logic. And I don't know what the additional 2 bytes would mean and where are they coming from.

uberchilly commented 8 months ago

It is weird. What is definitely the case is that you cannot send 513,514 or up on 517 mtu on os14. If it is a bug it is in os 14 cause it is reproducible on pixel and samsung running os 14.

philips77 commented 8 months ago

I think it's still 3, but perhaps your device supports max 512-long packets. So the MTU value you should be getting from onMtuChanged is 515.

uberchilly commented 8 months ago

Not sure how valid this test is but I've tried to exclude our HW from the story, by creating a BLE server with one GATT characteristic that you can write on, running on the Samsung s21 phone with Android os 14, via nRF connect app, and I connected to it via Xiaomi phone with android 13. Then I created MTU request to 517 successfully from the BLE client. After that, I've sent 514 bytes from Xiaomi BLE client to Samsung BLE server and the interesting bit is that Samsung received 512 bytes, instead of 514.

When I reversed the roles so that os13 Xiaomi is BLE server and os14 Samsung is BLE client, set MTU again to 517 and tried sending 514 bytes it completely failed just showing loading indicator forever, and worked just fine with 512 bytes.

I think the best test would be between 2, os 14 devices but I don't have such a setup, hopefully, these results can help.

weliem commented 8 months ago

According to the BLE standard, the maximum length of a characteristic is 512. What is new in Android 14, is that Android now enforces this. Previously it would allow up to 600 even though this is not allowed by the BLE standard.....

So 512 it is....not more

weliem commented 8 months ago

The 517 values is because of long operations that need an additional 2 bytes for the offset