Introduce GATT functionality for Android

[wuwbobo2021]

Note: this is still experimental, and behaviors on services changed event is untested; pairing with user confirmation needs testing on newest Android versions.

[wuwbobo2021]

In CharacteristicImpl::max_write_len, there is an "XXX: call BluetoothGatt.requetMtu(int) on connection and get the value in onMtuChanged.". See https://developer.android.com/about/versions/14/behavior-changes-all#mtu-set-to-517. I guess there could be an boolean option in the adapter config for determining whether or not to call requestMtu and wait for result on connection, because it might fail with problematic firmware of some remote devices.

I just found the private final static long CONNECTION_TIMEOUT_THRESHOLD = 20000; in the Android-BLE-Library; I guess that GATT operations might not require such forceful timeout setting.

[abezukor]

I have been also been working on this is an as yet unpublished crate (still working out the bugs). I also have a branch that uses that crate in bluest (See the basic commit MaticianInc@76e6457). We are using it in our (still in beta) Android app at Matic. I have been waiting to publish it until we have something stable. I can either help review this or publish my version when is its ready.

[wuwbobo2021]

@abezukor I'm glad to see yet another solution of Android support; however, where is the rust_android_integration/rust/bluedroid? Could it be made public?

[abezukor]

@abezukor I'm glad to see yet another solution of Android support; however, where is the rust_android_integration/rust/bluedroid? Could it be made public?

Will do. I wasnt planning on publishing it until I had fixed all the bugs, but I can if you want to collaborate.

[wuwbobo2021]

@abezukor Would you please describe about those most significant bugs? I think It's difficult to determine which implementation is more viable if you don't make it public.

[alexmoon]

In CharacteristicImpl::max_write_len, there is an "XXX: call BluetoothGatt.requetMtu(int) on connection and get the value in onMtuChanged.". See https://developer.android.com/about/versions/14/behavior-changes-all#mtu-set-to-517. I guess there could be an boolean option in the adapter config for determining whether or not to call requestMtu and wait for result on connection, because it might fail with problematic firmware of some remote devices.

I think requestMtu should be called inside max_write_len. You may need to cache the MTU value for the connection after the first call. Note that the cached value must be cleared on disconnect.

An adapter config parameter to enable requesting the MTU on connection would also be good.

[abezukor]

@abezukor Would you please describe about those most significant bugs? I think It's difficult to determine which implementation is more viable if you don't make it public.

I hope to open up my code later this week. It has been an internal project until now, and I need to spend some time writing documentation etc.

[wuwbobo2021]

@abezukor I am willing to dismiss my current implementation if the rust_android_integration also uses some binding generator for JNI calls. Hopefully, RFCOMM connections can be supported in bluedroid as well.

On the other hand, something like blurdroid (with C code) might not be acceptable. I guess you are not making something similar to that crate.

[wuwbobo2021]

@abezukor I can't wait for knowing if your implementation has structural advantages over this attempt. Could you make it public earlier, even with minor bugs? Otherwise I may continue to improve my own implementation.

[abezukor]

@abezukor I can't wait for knowing if your implementation has structural advantages over this attempt. Could you make it public earlier, even with minor bugs? Otherwise I may continue to improve my own implementation.

I have released a version https://github.com/abezukor/android_rust. I had to re-organize a bunch of it to make it compatible with NativeActivity.

Quickly looking at your implementation, the main structural difference is I decided to put more logic on the java side, because I found it more ergonomic to write that way. I also have created a bunch of components that are reused in an NSD (mDNS) crate.

[wuwbobo2021]

@abezukor Some notes for bluedroid are provided in #9 (comment). Thank you.

[wuwbobo2021]

I am still confused about the schedule of "planned support for Android". Maybe it's time to make the decision: to close #40 as unmerged and accept the implementation from @abezukor, or to check this PR seriously.

A few changes could be made here if #40 should be kept:

• Avoid frequent DetachCurrentThread calls by pushing and popping a JNI local reference frame in non-nested with_env calls; however, such operations cannot be done in nested with_env calls (lying in an outer with_env), otherwise it would be unsound.
• Add an option in AdapterConfig for allowing connections with devices already connected outside the current bluest library instance. This should be okay on well-implemented Android API implementations.
• Add an option in AdapterConfig to enable requesting the MTU on connection.

[wuwbobo2021]

I have just fixed some problems noted above, and solved the problems of no responding in case of scanning without permission and reading/writing data while the device is disconnected.

Test case to be built by cargo-apk or cargo-apk2

[workspace]

[package]
name = "bluest-test"
version = "0.1.0"
edition = "2024"
publish = false

[dependencies]
bluest = { path = "..", features = ["unstable", "l2cap"] }
tracing = "0.1.36"
tracing-subscriber = "0.3.15"
# android-activity uses `log`
log = "0.4"
tracing-log = "0.2.0"
ndk-context = "0.1.1"
android-activity = { version = "0.6", features = ["native-activity"] }
# jni-min-helper = { version = "0.3", features = ["futures"] }
futures-lite = "2.6"
async-channel = "2.2.0"
futures-timer = "3.0.3"

[lib]
crate-type = ["cdylib"]

[package.metadata.android]
package = "com.example.bluest_test"

build_targets = ["aarch64-linux-android"]

# "Arm64V8a" is for `cargo-apk2` which is required for putting `PermActivity` in the app
# build_targets = [ "Arm64V8a" ]
# put <https://docs.rs/crate/jni-min-helper/0.3.2/source/java/PermActivity.java> in this folder
# java_sources = "java"

# Android 12 or above may require runtime permission request.
# <https://developer.android.com/develop/connectivity/bluetooth/bt-permissions>
# <https://docs.rs/jni-min-helper/0.3.2/jni_min_helper/struct.PermissionRequest.html>
# Use `cargo-apk2` or check <https://github.com/rust-mobile/cargo-apk/pull/72>
[package.metadata.android.sdk]
min_sdk_version = 23
target_sdk_version = 33

[[package.metadata.android.uses_feature]]
name = "android.hardware.bluetooth_le"
required = true

[[package.metadata.android.uses_permission]]
name = "android.permission.BLUETOOTH_SCAN"
min_sdk_version = 31

[[package.metadata.android.uses_permission]]
name = "android.permission.BLUETOOTH_CONNECT"
min_sdk_version = 31

[[package.metadata.android.uses_permission]]
name = "android.permission.ACCESS_FINE_LOCATION"
# TODO: uncomment this line when `usesPermissionFlags` becomes supported in `cargo-apk2`.
# max_sdk_version = 30

[[package.metadata.android.uses_permission]]
name = "android.permission.BLUETOOTH"
max_sdk_version = 30

[[package.metadata.android.uses_permission]]
name = "android.permission.BLUETOOTH_ADMIN"
max_sdk_version = 30

# these are for `cargo-apk2`

# [[package.metadata.android.application.activity]]
# name = "android.app.NativeActivity"

# [[package.metadata.android.application.activity.intent_filter]]
# actions = ["android.intent.action.VIEW", "android.intent.action.MAIN"]
# categories = ["android.intent.category.LAUNCHER"]

# [[package.metadata.android.application.activity.meta_data]]
# name = "android.app.lib_name"
# value = "bluest_test"

# [[package.metadata.android.application.activity]]
# name = "rust.jniminhelper.PermActivity"

#![allow(unused)]

use bluest::btuuid::bluetooth_uuid_from_u16;
use bluest::Uuid;
use futures_timer::Delay;
use std::time::Duration;

use android_activity::{AndroidApp, MainEvent, PollEvent};
use futures_lite::{FutureExt, StreamExt};
use tracing::{error, info};

#[unsafe(no_mangle)]
fn android_main(app: AndroidApp) {
    // android_logger::init_once(
    //     android_logger::Config::default()
    //         .with_max_level(log::LevelFilter::Info)
    //         .with_tag("bluest_test".as_bytes()),
    // );

    // NOTE: View tracing log on the host with `adb logcat RustStdoutStderr:D '*:S'`.
    let subscriber = tracing_subscriber::FmtSubscriber::builder().without_time().finish();
    tracing::subscriber::set_global_default(subscriber).expect("setting tracing default failed");
    tracing_log::LogTracer::init().expect("setting log tracer failed");

    // calling `block_on` with bluetooth operations in `android_main` thread may block forever...
    let (tx, rx) = async_channel::unbounded();
    std::thread::spawn(move || {
        let res = futures_lite::future::block_on(async_main().or(async {
            let _ = rx.recv().await;
            info!("async thread received stop signal.....");
            Ok(())
        }));
        if let Err(e) = res {
            info!("async thread's `block_on` received error: {e}");
        } else {
            info!("async thread terminates itself after it received stop signal.");
        }
    });

    let mut on_destroy = false;
    loop {
        app.poll_events(None, |event| match event {
            PollEvent::Main(MainEvent::Stop) => {
                info!("Main Stop Event.");
                let _ = tx.send(());
            }
            PollEvent::Main(MainEvent::Destroy) => {
                on_destroy = true;
            }
            _ => (),
        });
        if on_destroy {
            return;
        }
    }
}

async fn async_main() -> Result<(), Box<dyn std::error::Error>> {
    // Currently this requires `cargo-apk2` instead of `cargo-apk` to work.
    // But this is required if the user chooses to confirm permission on every startup.
    /*
    let req = jni_min_helper::PermissionRequest::request(
        "BLE Test",
        [
            "android.permission.BLUETOOTH_SCAN",
            "android.permission.BLUETOOTH_CONNECT",
            "android.permission.ACCESS_FINE_LOCATION",
        ],
    )?;
    if let Some(req) = req {
        info!("requesting permissions...");
        let result = req.await;
        for (perm_name, granted) in result.unwrap_or_default() {
            if !granted {
                eprintln!("{perm_name} is denied by the user.");
                return Ok(());
            }
        }
    };
    */

    let adapter = bluest::Adapter::with_config(bluest::AdapterConfig::default()).await?;
    adapter.wait_available().await?;

    info!("adapter is now available.");

    // Please put your test case here.

    info!("async task terminates itself.");
    Ok(())
}

[wuwbobo2021]

I did a test that adds log messages for ACTION_ACL_CONNECTED and ACTION_ACL_DISCONNECTED again and found that I cannot receive the event when a BLE device is connected or disconnected inside/outside bluest.

I am afraid that the problem marked by FIXME: currently this monitors only devices connected/disconnected by this crate, even if 'allow_multiple_connections' is true. comment for AdapterImpl::device_connection_events cannot be actually fixed; https://github.com/abezukor/android_rust has not managed to do this, either.

I will not make further changes without your suggestions. Thank you.

[wuwbobo2021]

I realized a structural problem in my current implementation: while it seems possible to reconnect a device with private random resolvable address, Device::id will still return the old address, and AdapterImpl::is_actually_connected (which is used internally for some extra checking) is inconsistent with this possible awkward mechanism because it returns the new address of the device. The core problem is that I relied on the bluetooth device address too heavily. Maybe this can be corrected, and the device should be identified based on the Java device object instead of the address when the GATT connection to that device doesn't exist; this means DeviceImpl::id should return a cached address only when a valid GattConnection exists, otherwise it should make a Java call to get an address; many corresponding changes are required elsewhere.

Another problem found in my async_util::ExcluderLock: it probably lacks a method for setting a flag which tells it to wait for the callback from Java on dropping (with a timeout limit), so that the callback won't be treated like a callback of a possible second operation in case of the Future of the first operation is interrupted and dropped. There might be a better way to solve this potential problem.

I think the implementation from @abezukor is (currently) less problematic at these points.