Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix maximal supported MTU on Android 13 #808

Merged
merged 2 commits into from Oct 3, 2022

Conversation

marsounjan
Copy link
Contributor

@marsounjan marsounjan commented Aug 26, 2022

Hello @dariuszseweryn,

since Android 13 the real maximal supported MTU by Android is 515.

Even though in Android source code the GATT_MAX_MTU_SIZE is still set to 517, the data buffer size GAT_MAX_ATTR_LEN is now capped at 512 bytes.

It leads to the situation when Android allows you to negotiate MTU 517, but it drops all the packets with data larger then 512 bytes

Here is the commit where google engineers did this amazing change

@CLAassistant
Copy link

CLAassistant commented Aug 26, 2022

CLA assistant check
All committers have signed the CLA.

@dariuszseweryn
Copy link
Owner

Hi,
Thats a good finding, thank you! I will sleep with it to think if it would be possible to make a backwards-compatible change.

@weliem
Copy link

weliem commented Aug 26, 2022

@weliem
Copy link

weliem commented Aug 26, 2022

oops...i meant 1 year ago....so might indeed be new to Android 13

@marsounjan
Copy link
Contributor Author

Hi,
Thats a good finding, thank you! I will sleep with it to think if it would be possible to make a backwards-compatible change.

@dariuszseweryn what do you mean by backwards compatible? this change won't make it incompatible with previous Android versions.

If you have that 2 byte difference on you mind and you'd like to keep the value GATT_MTU_MAXIMUM set to 517 prior Android 13 and use 515 only since then, then we can easily introduce GATT_MTU_MAXIMUM_PRE_API_33 = 517or something like that.

Sadly Java interface don't know computed properties like Kotlin does so you we can't use it for inlining this IF statement :(

@dariuszseweryn
Copy link
Owner

I'm thinking of deprecating the static value and introduce a function instead.
I'm off for holidays now and I didn't manage to make the change before. I should be back in a week.
@weliem Happy Birthday!

@marsounjan
Copy link
Contributor Author

@dariuszseweryn great, thanks!

Copy link
Owner

@dariuszseweryn dariuszseweryn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, I revisited your PR and added some small suggestions to Javadoc and after that I will happily merge your change :)

…ection.java

Co-authored-by: Dariusz Seweryn <dariusz.seweryn@gmail.com>
@marsounjan
Copy link
Contributor Author

@dariuszseweryn great, thanks!

@dariuszseweryn dariuszseweryn merged commit 90d729e into dariuszseweryn:master Oct 3, 2022
@dariuszseweryn
Copy link
Owner

Released in version 1.17.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants