Android part 1: scanning #10
Conversation
Dirbaio
force-pushed
the
android
branch
3 times, most recently
from
4fda0e2 to
6b76b11
Compare
src/android/adapter.rs
Outdated
| pub unsafe fn new(vm: *mut jni_glue::sys::JavaVM, manager: jni_glue::sys::jobject) -> Result<Self> { | ||
| let vm = VM::from_raw(vm); | ||
| let manager: Global<BluetoothManager> = Global::from_raw(vm, manager); |
There was a problem hiding this comment.
Could this just take a VM and a Global<BluetoothManager> instead of the raw pointers? I think that would push the unsafe bits onto the invariants of VM::from_raw and Global::from_raw and this could become safe?
There was a problem hiding this comment.
I didn't do it because it'd require making crate::android::bindings public, or at least BluetoothManager. The bindings stuff feels a bit like "implementation details" to me: we decided to abstract JNI this way, but it's not necessarily the way the user would want to. Or if they're just getting the pointer from java and passing it to bluest they might not even need any abstractions, just jni-sys.
keeping the bindings private allows upgrading jni-glue/jni-bindgen to new versions that break absolutely everything, without breaking bluest's public api.
I don't like using raw pointers and unsafe either, but given the "generate ad-hoc bindings and embed them in the crate" it makes more sense I think. If we were using jni-andtoid-sys then it'd make nmore sense to take a Global<BluetoothManager> indeed.
Cargo.toml
Outdated
| [target.'cfg(target_os = "android")'.dependencies] | ||
| jni-glue = { git = "https://github.com/akiles/jni-bindgen", rev = "45ddf3129bbe5987ab19f9c9be091de3bf3c5c25" } |
There was a problem hiding this comment.
I'd like to keep main releasable on crates.io, so perhaps we should merge into an android branch until you're ready to publish jni-glue?
There was a problem hiding this comment.
I can publish the fork right now before merging. I think it's mostly ready (or at least ready to the extent bluest needs to work...). I just need to come up with a name :D
There was a problem hiding this comment.
i've released 🍝 java-spaghetti v0.1.0, there's no git deps anymore.
Part 1 of #9
This PR lays down basic infrastructure to use Android APIs from Rust, and implements Bluetooth scanning.
Some interesting thing:
jnicrate: works, requires no unsafe which is nice. However it's incredibly annoying, each call ends up being 10+ lines and there is A TON of calls you have to do (ScanRecordhas a TON of getters for example 🥲).jni-bindgen+jni-android-sys: it's abandoned,jni-android-sysneeds updating to newer Android API levels (for L2CAP CoC support). The approach of generating one "mega-crate" has some problems: it requires thousands of Cargo features to avoid compile time bloat, which is not accepted anymore in crates.io. Its approach is great though, and allows calling Java APIs with reasonably non-ugly code.jni-bindgen, bringing it up to date and improving/simplifyingjni-glue. It generates a standalonebindings.rsinstead of a crate, so we can include it directly inbluest. It supports filtering so we only generateandroid.bluetooth.*and other utils. I plan on renaming the fork and publishing to crates.io under a new name, so we can remove the git dep inbluestCargo.toml.src/android/java/which users have to copypaste for now. I'll study providing a.jaror prebuilt gradle/maven deps in the future.CallbackRouterthing to receive callbacks from our java listeners and route them back to the right channel. Routing is done by ani32ID, to avoid having to pass Rust pointers through Java which is scary.Adapter::default(). Using bluetooth requires access to theBluetoothManager, which you have to obtain from an AndroidContextwhich you can't get out of thin air. I've added an Android-specificAdapter::new(vm, manager)for this.TODO:
unwraps.