search

victordiaz / PHONK

PHONK is a coding playground for new and old Android devices
https://phonk.app
GNU General Public License v3.0
463 stars 25 forks source link

Fix and minor changes regarding MTU in `network.bluetoothLE` #136

Open notEvil opened 1 year ago

notEvil commented 1 year ago

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

victordiaz commented 1 year ago

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

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

notEvil commented 1 year ago

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 commented 1 year ago

Unrelated to this PR:

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

victordiaz commented 1 year ago

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 commented 1 year ago

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

notEvil commented 1 year ago

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 commented 1 year ago

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

victordiaz commented 1 year ago

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 commented 1 year ago

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