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 and minor changes regarding MTU in network.bluetoothLE #136

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

notEvil
Copy link

@notEvil notEvil commented Feb 16, 2023

Hi,

peripheral.requestMtu doesn't do anything if not connected, see https://github.com/weliem/blessed-android/blob/ee8d966692a233457ed8ac967ba37a5ef9d69630/blessed/src/main/java/com/welie/blessed/BluetoothPeripheral.java#L1541
Therefore, mtuSize of connectDevice is essentially ignored.

This PR fixes this issue and additionally exposes two events: disconnect and MTU changed

Feel free to adjust code style if not appropriate

…ctDevice is not used

- network.bluetoothLE: changed to trigger event new device status on disconnect
- network.bluetoothLE: added method onMtuChanged
@notEvil notEvil changed the title - network.bluetoothLE: fixed bug related to argument mtuSize of conne… Fix and minor changes regarding MTU in network.bluetoothLE Feb 16, 2023
@victordiaz
Copy link
Owner

Thanks!
I'll try to do some test during the weekend.

Just out of curiosity, what type of problem did you have?

@notEvil
Copy link
Author

notEvil commented Feb 16, 2023

I connect to Bangle.js which has a max MTU of 53, and for now I require this message size (custom protocol). So the issue was that I called connectDevice with 53, but 500 was used instead and refused, so it fell back to 23.

@notEvil
Copy link
Author

notEvil commented Feb 18, 2023

Unrelated to this PR:

Fyi, I released the first version of my project at https://gitlab.com/notEvil/cyclometer :)

@victordiaz
Copy link
Owner

Wow, such a cool project!
Do you mind if we link it somewhere? I plan to do a section with projects made with PHONK.

Also the Discord has low / near-non traffic but it would be perfect to share it there.

@notEvil
Copy link
Author

notEvil commented Feb 21, 2023

Thanks! You can link/mention it when/wherever you want. A section about projects using PHONK would be really nice!

@notEvil
Copy link
Author

notEvil commented Feb 23, 2023

I've added another commit because String.getBytes in write adds unicode bytes that I wasn't aware of. This is not intended to be merged as is! You probably want to change the way this works (either JS native interface using typed arrays, or a Java native interface combined with a utility similar to util.parseBytes?)

@notEvil
Copy link
Author

notEvil commented Feb 24, 2023

And another which fixes the calling behavior of sensors.location.onSatellitesChange. Please let me know if you want these changes in separate PRs!

@victordiaz
Copy link
Owner

It perfect here! Thanks so much for the fixes.

The bluetooth one totally makes sense. IIRC when that was implemented I made it for a specific use case I had where I was just using strings so I took that shortcut 😅

@victordiaz
Copy link
Owner

Sorry is taking a bit longer than expected to review this.
I don't have with me a BT4 device to check the changes, as soon as I get one I will review it

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

2 participants